[poppler] Followup Bug 32349 & Poppler: More shading fun ;-)

[Andrea Canciani]

On Thu, Jan 27, 2011 at 10:45 AM, Thomas Freitag
<Thomas.Freitag at kabelmail.de> wrote:
> Am 27.01.2011 00:44, schrieb Albert Astals Cid:
>>
>> A Dimecres, 26 de gener de 2011, Albert Astals Cid va escriure:
>>>
>>> The altona file shows a vertical line that wasn't present before.
>>
>> Speaking with Andrea on IRC he blames it on a clip issue that seems to be
>> present on axial shadings too.
>>
>> He says he'll try to find and fix the issue in the comming days.
>>
>> My suggestion is wait to see if he (or maybe you Thomas?) can find that
>> clipping issue and then commit all the patches so we end up with no
>> regression
>> and only improvements.
>
> I fear that this is not such a trivial issue. I think we have here at least
> three different problems, I try to explain what I mean respectively already
> encountered ( and what were the reasons for my compromises):
> a) The way how antialiasing is implemented it splash works well if there is
> a high contrast, i.e. drawing a dark object on light background. But it gets
> worser if background colors and object colors are nearly the same in
> brightness. That probably produces the glitches in Andrea's test PDFs and in
> the altona PDF.
> b) If two PDF objects are close together (without any distance) but not on
> pixel boundaries, we can see sometimes two different effects: Either we have
> glitches like in a) or we get white (or light) lines as everyone probably
> encountered in several PDFs. Also the Acrobat Reader has sometimes this
> problem, but solves it in a better way (I don't know, how).
> c) If no antialiasing is used, but the objects are used in softmasks or in
> transparency groups, we get also problems if the same resulting pixel is
> drawn more than once. This was the effect in wine glass of ducks & roses:
> because of the alpha channel implementation the pixel becomes darker and
> darker the more often it is painted. That was the reason I started
> implementation of radial shading in splash.
> So this probably leads to not changing only a few statements but to
> completely rewrite antialiasing in splash, and on the other hand any changes
> will not only effect radial shading but all objects, so it will be hard to
> test it. So in my company we use in such case often the dictum: It's not a
> bug, it's a feature.
> But let us see what Andrea will discover...
>
>> Of course if we see that poppler 0.17 release date nears and the clip
>> issue is
>> not found-fixed yet we'll have to rethink since this patch adds better
>> radial
>> shadings.
>
> Regarding that time schedule I I suggest once again the following:
> a) we move the switch on of antialiasing from univariateShadedFill back to
> axialShadedFill.

Although I admit I don't like axial shadings being left broken just because
nobody had previously noticed, I guess this is ok. This will still improve
radial gradients and we can enable antialiasing again when the glitches
are fixed.

> b) that means that axial shading behaves like it has done since I
> implemented it last year and also with my patch here, but has the optimize
> of speed from Andrea
> c) that means that radial shading behaves like Albert already regtest it
> with my patch, so antialiasing is not used in radial shading, but has the
> optimize of speed from Andrea. Of course it then still have the glitches in
> Andrea's PDF, but that's not a problem of radial shading. We still get the
> improvement of that implementation.
> d) the regtest should be quite easy and can be done automatically, comparing
> the new results with results from Albert's last regtesting of my patch.
> (This assumes that the cache that Andrea implements is an exact cache,
> otherwise we could have some very small differences, which I suppose would
> be acceptable but would break the automatization of the regtest)

The cache is not exact, there will be differences. The sampling
only guarantees that there will be at least enough samples to have a correct
rasterization. An exact (and efficient) caching is only possible for piecewise
linear color functions (and I hope to get it right soonish, the main
obstacle right
now is that I have to be careful and clip everything to the domain and range).

> e) if Andrea (or someone else) is able to solve the clipping problem, we can
> just move back the switch on of antialiasing to univariateShadedFill
>
> On the assumption that everyone (or at least Albert :-) ) agrees, I attach a
> complete patch which should fulfil that (hopefully not missing something) .
> I'm not able (or at least don't know) how to produce the small patch
> segments how Andrea did it without having an own git repository, otherwise I
> would have attached only the changes to Andrea's proposal.

Yes, I used git to create the patches.

>
> One last point: Andrea removed my code changes from Splash::shadedFill which
> allows the use of it with antialiasing switched off. Of course it is not
> needed if antialiasing would be always switched on, so in my suggestion we
> definitely need it again. But on the other hand it would also be needed if
> someone uses SplashOutputDev without any antialiasing, i.e. switch off
> antialiasing in GlobalParams. If we remove it, radial shading (and also
> axial shading, but in this case it doesn't matter) would fall back to the
> Gfx routines and that would produce not only uglier but wrong results. I use
> this sometimes to speed up rendering, and I suggest we should use this code
> changes in either case, Splash does it also in all other routines.

IIRC you mentioned that antialiasing is disabled for axial gradients when
they have a bbox (although I could only see that usesShape is disabled).
Would this be sufficient for radial gradients? (It seems to be sufficient to
work around the problem in altona, but I don't know if there are other
difficult documents)

>
> Another (German?) dictum says: The whole life is a compromise :-)

And this compromise doesn't even look that bad, radial gradients
improve and other things don't regress ;)

Andrea

>
> Thomas
>>
>> Anyone disagrees?
>>
>> Albert
>>
>>> Albert
>>
>> _______________________________________________
>> poppler mailing list
>> poppler at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/poppler
>>
>> .
>>
>
>
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/poppler
>
>