[poppler] Fwd: splashModeDeviceN8 in two switch

Wed May 1 17:24:21 UTC 2019

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 <poppler-bounces at lists.freedesktop.org> on behalf of Adam Reichold <adam.reichold at t-online.de>
> Sent: Saturday, April 27, 2019 11:57 AM
> To: poppler at lists.freedesktop.org
> 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 <poppler-bounces at lists.freedesktop.org> on behalf of Adam Reichold <adam.reichold at t-online.de>
> > Sent: Thursday, April 25, 2019 10:08 AM
> > To: poppler at lists.freedesktop.org
> > 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 <thomas.freitag.bbr at gmail.com>
> >> Date: Do., 25. Apr. 2019, 07:42
> >> Subject: Re: [poppler] splashModeDeviceN8 in two switch
> >> To: Albert Astals Cid <aacid at kde.org>
> >>
> >>
> >> 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 <aacid at kde.org> 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
> >>>>> poppler at lists.freedesktop.org
> >>>>> https://lists.freedesktop.org/mailman/listinfo/poppler
> >>>>>
> >>>>
> >>>>
> >>>
> >>>
> >>>
> >>>
> >>> _______________________________________________
> >>> poppler mailing list
> >>> poppler at lists.freedesktop.org
> >>> https://lists.freedesktop.org/mailman/listinfo/poppler
> >>
> >>
> >> _______________________________________________
> >> poppler mailing list
> >> poppler at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/poppler
> >>
> >
> >
> >
> > _______________________________________________
> > poppler mailing list
> > poppler at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/poppler
> >
> 
> 