El diumenge, 28 d’abril de 2019, a les 5:29:13 CEST, William Bader va escriure: > > I expect little slowdown at least on modern desktop/server hardware and > > would thereby argue that we should drop the CMYK preprocessor flag. > > I think that Albert and maybe others have suites of PDF files for regression > tests. Would it be possible to compare the runtime the tests with SPLASH_CMYK > enabled and disabled?
I do have lots of files, but it takes "so much time" to run the regression test (several hours) that i don't think it'd be good to measure this to be honest. My computer doing other things (checking if i have email, etc) will affect the outcome too much. > > I don't have enough files for a good test, and poppler without SPLASH_CMYK > produces incorrect results for many of my files. Using splash? That's news to me. Or you mean using pdftops? Cheers, Albert > > Regards, > William > > ________________________________ > From: poppler <[email protected]> on behalf of Adam > Reichold <[email protected]> > Sent: Saturday, April 27, 2019 11:57 AM > To: [email protected] > Subject: Re: [poppler] Fwd: splashModeDeviceN8 in two switch > > Hello again, > > I had a moment to take a look at the code and from looking at e.g. > SplashBitmap, it seems that we allocate data always in units of bytes > and only as much as need for the actual format in use. SplashColor > itself is only used to store the value of individual pixels when those > are processed. > > And since both unsigned char[4] as well as unsigned char[4+4] fit into > the registers of 64-bit machines, I expect little slowdown at least on > modern desktop/server hardware and would thereby argue that we should > drop the CMYK preprocessor flag. (If someone needs a Poppler for 32-bit > constrained systems, making it possible to reduce SPOT_NCOMPS to zero > seems like it could yield a similar effect.) > > Best regards, > Adam > > Am 25.04.19 um 17:06 schrieb William Bader: > >> As far as i remember i implemented an overprint option in pdftoppm for > >> this purpose. You can see the difference if you run the ghent test suite > >> with and without this switch. > > > > SPLASH_CMYK affects more than overprint. It also matters in pdftops > > -level1sep with some PDFs that have features like transparencies that > > aren't supported in level 1 PS and require rasterizing with splash. > > I can send example PDFs through email or in a poppler issue if anyone wants > > to see what happens. > > > >> But sizeof(SplashColor) == 8 instead of 4, then allocating > >> SplashColor[size] will consume twice the memory even if we only use the > >> first half of entry, will it not? > > > > I forgot about that. I have found bugs due to code that didn't count > > SPOT_NCOMPS like https://bugs.freedesktop.org/show_bug.cgi?id=90570 > > > > If all of the poppler users that need SPLASH_CMYK usually build poppler > > from source, could you compromise by keeping SPLASH_CMYK turned off by > > default but enabling it on CI builds? > > > > Is there any way to handle it at run-time using C++ features? Would > > templates work or would they increase the code size too much? > > > > Regards, > > William > > > > ________________________________ > > From: poppler <[email protected]> on behalf of Adam > > Reichold <[email protected]> > > Sent: Thursday, April 25, 2019 10:08 AM > > To: [email protected] > > Subject: Re: [poppler] Fwd: splashModeDeviceN8 in two switch > > > > Hello again, > > > > Am 25.04.19 um 07:44 schrieb Thomas Freitag: > >> Sorry, i wanted to send it to the list, too > >> > >> ---------- Forwarded message --------- > >> Von:Thomas Freitag <[email protected]> > >> Date: Do., 25. Apr. 2019, 07:42 > >> Subject: Re: [poppler] splashModeDeviceN8 in two switch > >> To: Albert Astals Cid <[email protected]> > >> > >> > >> Your argument is wrong, Albert. > >> Even if we remove the compiler directive, "noone" will use this mode by > >> default, because the default rendering is RGB. So you don't need more > >> memory for rendering and it doesn't become slower. Just the code need more > >> memory. > > > > But sizeof(SplashColor) == 8 instead of 4, then allocating > > SplashColor[size] will consume twice the memory even if we only use the > > first half of entry, will it not? > > > > Best regards, > > Adam > > > >> But the "professionell" user would have the ability to simulate overprint. > >> As far as i remember i implemented an overprint option in pdftoppm for this > >> purpose. You can see the difference if you run the ghent test suite with > >> and without this switch. > >> > >> Cheers, > >> Thomas > >> > >> PS: Not only William uses it, i do it too > >> > >> Albert Astals Cid <[email protected]> schrieb am Mi., 24. Apr. 2019, 19:40: > >> > >>> El dimarts, 23 d’abril de 2019, a les 18:11:04 CEST, Adam Reichold va > >>> escriure: > >>>> Hello, > >>>> > >>>> thinking about this again with a bit of distance, should we maybe remove > >>>> the `SPLASH_CMYK` preprocessor flag entirely? I am assuming the reasons > >>>> for having this upstream is to keep the code from bit rotting as > >>>> Poppler's internals change? If nobody builds this, then this will not > >>>> happen. (CI checks improve things, but it will still be "late" feedback > >>>> instead of being part of the initial edit-compile-test cycle. Also, this > >>>> will not reach e.g. clusterfuzz which thereby will not verify that code.) > >>>> > >>>> Are there any downsides to removing the conditional compilation? Does > >>>> this break code that does not request the CMYK handling explicitly? If > >>>> so, could we fix this via the internal API instead of hard-coded it at > >>>> compile time? > >>> > >>> The main problem i see is that it makes SplashColor for from 4 bytes to 8 > >>> bytes, so may make things slower/use more memory for a feature "noone > >>> uses". > >>> > >>> Cheers, > >>> Albert > >>> > >>> P.S: Of course the "noone uses" is a bit of a self fulfilling prophecy if > >>> we don't enable it > >>> > >>>> > >>>> Best regards, > >>>> Adam > >>>> > >>>> Am 22.04.19 um 13:14 schrieb Albert Astals Cid: > >>>>> This is mostly for William since AFAIK he's the "only" one using > >>> SPLASH_CMYK. > >>>>> > >>>>> I've tried to enable SPLASH_CMYK on the CI and it's loudly complaining > >>> that the splashModeDeviceN8 cases are missing in SplashOutputDev.cc > >>>>> > >>>>> https://gitlab.freedesktop.org/aacid/poppler/-/jobs/253647 > >>>>> > >>>>> Any suggestion of what the code should be? > >>>>> > >>>>> Should they just be the same as case splashModeCMYK8: ? The first one > >>> looks like it may make sense, but not so sure about the second. > >>>>> > >>>>> Cheers, > >>>>> Albert > >>>>> > >>>>> > >>>>> _______________________________________________ > >>>>> poppler mailing list > >>>>> [email protected] > >>>>> https://lists.freedesktop.org/mailman/listinfo/poppler > >>>>> > >>>> > >>>> > >>> > >>> > >>> > >>> > >>> _______________________________________________ > >>> poppler mailing list > >>> [email protected] > >>> https://lists.freedesktop.org/mailman/listinfo/poppler > >> > >> > >> _______________________________________________ > >> poppler mailing list > >> [email protected] > >> https://lists.freedesktop.org/mailman/listinfo/poppler > >> > > > > > > > > _______________________________________________ > > poppler mailing list > > [email protected] > > https://lists.freedesktop.org/mailman/listinfo/poppler > > > > _______________________________________________ poppler mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/poppler
