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

[Albert Astals Cid]

A Divendres, 28 de gener de 2011, Andrea Canciani va escriure:
> 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))

FWIW it is in.

And yes i have been a bad guy and commited the whole stuff at once instead by 
proper chunks. I'm sorry.

Congrats to Andrea and Thomas it's a beatiful improvement we got here.

Albert

> 
> Andrea
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/poppler