[poppler] Fwd: splashModeDeviceN8 in two switch

Sun Apr 28 03:29:13 UTC 2019

> 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 don't have enough files for a good test, and poppler without SPLASH_CMYK produces incorrect results for many of my files.

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
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/poppler/attachments/20190428/89a4a37b/attachment-0001.html>