On Tue, Jan 25, 2011 at 11:36 AM, Thomas Freitag <[email protected]> wrote: > Am 24.01.2011 21:44, schrieb Andrea Canciani: > > On Sun, Jan 23, 2011 at 9:36 PM, Albert Astals Cid <[email protected]> wrote: > > A Diumenge, 23 de gener de 2011, Thomas Freitag va escriure: > > Am 23.01.2011 18:12, schrieb Albert Astals Cid: > > A Dissabte, 15 de gener de 2011, Thomas Freitag va escriure > > The memory problems were coming still from the wrong calculation of > shading extension which I took from Gfx.cc and tried to correct, but it > still leaded to huge circles. Andrea gave me a hint of a correct > calculation of radial shading extension in cairo, and I adapated that > code piece now successfully to Splash. It seems, as if with this the > memory problems are now gone, at least I could render every PDF You sent > me in the past, and I couldn't find any regressions in the rendering > results. > > I got a new regression, will send you privately the pdf file. > > The question now is, you want me to try the other newest patch? Or you > want to fix this "old" one? > > Please test the newest patch. It has a completely other algorithm and as > I already tested, it has not this regression and hopefully no other. I > 'll answer the other mails in a few seconds. > > Ok, i'll run the regtest on that. > > If that passes the regtest, this patchset should pass it as well. > > 0001, 0002 and 0003 are fixes and cleanups for poppler Functions. > > 0004 abstracts a common interface from axial and radial shadings > > 0005 is not completely clear to me. I think it forces antialiasing to be > applied on shadings. > > In 0006 I cleaned up the radial implementation by Thomas. In particular > I simplified the code which chooses one of the two solutions. > > 0007 const-ifies Matrix methods and adds norm() and determinant(). > > 0008 adds simple caching (with uniform sampling) with a number of > samples which depends on the shading extents. It guarantees correct > rendering and provides much better performance than an hardcoded > constant (for example 65536 caused performance regressions with > respect to no caching at all in the ducks&roses pdf). > I left in some debugging code which is helpful while testing the > performance, because it prints the cache size extimate and if the cache > was activated or not. > > 0009 improves the parameter range estimate (in 0007 it is assumed to be > the full range). This might provide a speedup in some very particular > pdf files, but so far I haven't found any in which it actually matters. > > Fantastic!!!!!!!!!! > I just applied the patches because I was full of curiosity. Two examples > a) ducks and roses is now rendered in less than half of time in comparison > without any radial shading patches, and in less than a quarter of the time I > needed with my patch.
This is consistent with about 75% of the time being spent in color function evaluation prior to the color caching, assuming that the color function caching is very effective. > b) rendering the non reduced PDF where acrobat gives up is done now in a few > seconds, and in 2/3 I need with the radial patch. > So the speed optimization is really great! For this case we can even get some additional speed boost (with non-uniform sampling). > > I had only a small problem compiling it with visual studio under windows: > the c-compiler has a problem with using std::min or std::max, s. i.e. > http://heifner.blogspot.com/2008/02/stdmin-and-stdmax.html, so I defined > > #undef MIN > #define MIN(a, b) ((a) < (b) ? (a) : (b)) > #undef MAX > #define MAX(a, b) ((a) > (b) ? (a) : (b)) > > and used MIN and MAX instead of std::min and std::max. And You should remove > the fprintf's in GfxState.cc .... Sorry, I didn't know about the problem with std::min/max. It's quite easy to fix, as suggested in the blog you linked to (and by Pino Toscano as well). The fprintf are in the code only for debug purposes. I will obviously remove that when the patchset will be ready to be committed. > > But as You already seen, Albert will probably commit my patch to master > soon, so it would be very nice, if You can create a new patch after he will > commit it. Because the basic method (calculation the color given the > x,y-position) is the same, a regtest with that patch then will propably not > have such a lot differences and Albert has less work to compare the results. The computation is essentially the same. I just used a mathematical property of the solutions of the equation to clean up the code which chooses which one is the valid solution. I added a comment about it in the code. > And on the other hand I would like to have my patch now in and don't like to > stop that having a additional ping pong game with patches. Although I agree I want radial gradients to be fixed ASAP, I'd like your patch to be cleaned up a little before going into master, especially if we plan to drop the Bresenham code anyway. For example I'm not sure if I want something like 0009 in master, because it adds a huge amount of nontrivial code and it only provides speedups in very particular cases. Andrea > > Thomas > > Commit messages and authorship are to be fixed. I haven't implemented > the non-uniform sampling because I had some problem with clipping > to the domain and range, but it can be solved and I hope to do it soon. > > I'd like to get a confirmation that the cache is working (i.e. it speeds up > the documents which the radial gradient patch slows down and causes > no rendering regressions). > Please remember that this patchset also affects axial shadings, which > should become faster as well. > > Andrea > > _______________________________________________ > poppler mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/poppler > > > _______________________________________________ > poppler mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/poppler > > _______________________________________________ poppler mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/poppler
