On Tue, Jan 25, 2011 at 10:54 PM, Albert Astals Cid <[email protected]> wrote: > A Dimarts, 25 de gener de 2011, vàreu escriure: >> On Tue, Jan 25, 2011 at 10:25 PM, Albert Astals Cid <[email protected]> wrote: >> > A Dimarts, 25 de gener de 2011, Andrea Canciani va escriure: >> >> On Tue, Jan 25, 2011 at 9:00 PM, Albert Astals Cid <[email protected]> wrote: >> >> > ... >> >> > >> >> >> Finished successfully, i'll have a look at the code tomorrow and if i >> >> >> don't find anything obviously wrong will commit it to master :-) >> >> > >> >> > Had a look at the code and it is too untidy, please remove all the >> >> > ifdefs of unused code and remove the T_EDGE and T_CORNER defines and >> >> > i'll commit it. >> >> >> >> 0001 and 0002 replace most of Thomas's patch, the exception being >> >> attachment 0003. I don't understand what is the purpose of that change, >> >> maybe Thomas can explain it and suggest a better commit message. >> >> >> >> I think 0001 and 0002 should be quite clean and commit ready. >> >> Please review them, I'll fix any remaining problem as soon as possible. >> > >> > Hmm, sincerely i prefer to commit Thomas patches first. It has taken us >> > lots of regtesting iterations to get to something that gives >> > improvements and no regressions. Once that is in we can start with your >> > patches :-) >> >> These patches replace it, they implement the same change. >> I think it's quite pointless to commit it just to revert it immediately, >> but if you like it better... > > I know they implement the same feature, but can you guarantee 100% that they > will not have any regression that Thomas patches we know do not have?
No, I can't guarantee at 100% (I wouldn't do it even for Thomas's patch, even if it has been tested throughly), but I'm quite confident they are as correct, because they performs the very same computation, which is also the same as the one performed in pixman and in cairo-gl (yes, I've already implemented radial gradients multiple times...) I did not test them as extensively as the current Thomas's patch has been tested, but I think that the same objection would be apply to the cleaned up patch (which would also require additional work). These patch should be small enough to be reviewed easily. Moreover they can render correctly the cairo-generated gradients, which should trigger all the possible paths in the rasterizer. If you don't notice anything weird while reviewing them, I'll give a 99.9% guarantee of correctness ;) Andrea PS: FWIW the backward rasterization technique is simple, so it should be even easier to trust it. Bresenham continued to show some minor glitches even after multiple test&fix rounds, while backward rasterization was immediately correct. PPS: If you want to, I should be able to create pdf files where both Thomas's and this implementation fail at rendering the correct result, because of numerical instability problems. If you care about that, computations need to be performed in arbitrary size floating points, because for intermediate computations might require up to 4 times the input accuracy (i.e. if you use double with 53 bits of mantissa, you might need up to 212 bits), but this can only happen in almost-degenerate and not very interesting cases. _______________________________________________ poppler mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/poppler
