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

[Andrea Canciani]

On Fri, Jan 28, 2011 at 9:53 AM, Albert Astals Cid <aacid at kde.org> wrote:
> A Dijous, 27 de gener de 2011, Andrea Canciani va escriure:
>> 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 ;)
>
> So what should i regtest, the last radialsh.patch sent by Thomas (yesterday at
> 09:45:56 Irish Time) or you want to send me a new patchset?

Yes, that patch should be ok (and hopefully it should pass an automated
regtest).

(I haven't found the problem in splash (yet))

Andrea