Dear Thomas, Thanks for taking responsibility for the shader implementation! I am running a little bit out of time... I support your suggestions.
Best regards Christian 2010/10/14 Thomas Freitag <[email protected]> > Hi Albert! > > I discussed this issue with Christian, and I have the following suggestion > now: > 1. You should commit the bugfix part of Christian's patch in 0.15.0, so > that it will be part of 0.15.1. To clearify what I mean with the bugfix > part, I attach the bugfix diff I made against 0.15.0 once again. This should > already run over Your regtest, as far as I understand the several mails > here. > 2. I take over the improvement part of the patch, where Christian > implements the gouraud shading in Splash. I made several suggestions to > Christian, i.e. that in the Splash library should be nor references to the > Gfx stuff from the poppler library. I will walk over the code and redesign > it a little bit next weekend. The base should be the same. > 3. After You create the 0.15.1, I will make a new diff including my > improvements of bug 30436 and the improvements of Cjhristian and upload that > patch to bug 30436. > > Is this okay for You (and for Christian, too), or does anyone have other > suggestions? > > Best regards, > Thomas > > Am 13.10.2010 20:56, schrieb Christian Feuersaenger: > >> Am 13.10.2010 20:34, schrieb Albert Astals Cid: >> >>> A Divendres, 8 d'octubre de 2010, Christian Feuersaenger va escriure: >>> >>>> Am 07.10.2010 23:17, schrieb Albert Astals Cid: >>>> >>>>> A Divendres, 1 d'octubre de 2010, Christian Feuersaenger va escriure: >>>>> >>>>>> Am 13.08.2010 23:43, schrieb Albert Astals Cid: >>>>>> >>>>>>> A Divendres, 30 de juliol de 2010, Albert Astals Cid va escriure: >>>>>>> >>>>>>>> A Dimarts, 27 de juliol de 2010, Christian Feuersaenger va escriure: >>>>>>>> >>>>>>>>> Dear Albert, >>>>>>>>> >>>>>>>>> thank you for your time to perform the regression tests! >>>>>>>>> >>>>>>>>> I have fixed the bug; it was a data type problem. >>>>>>>>> >>>>>>>>> Attached you find the fixed version. >>>>>>>>> >>>>>>>>> The file >>>>>>>>> bugfix_shadingtype4567_incremental.patch >>>>>>>>> is relative to the version you used for the regression tests. >>>>>>>>> >>>>>>>>> The file >>>>>>>>> bugfix_shadingtype4567_poppler0.14.patch >>>>>>>>> is relative to poppler-0.14.0-3-gb2427d0 . >>>>>>>>> >>>>>>>>> Thank you for considering my contributions. >>>>>>>>> >>>>>>>> I've ran the regression test with the Splash outputdev and all looks >>>>>>>> ok, will have to run it over the cairo and ps outputdevs before >>>>>>>> committing, though it'll take a while since next week i'm going to >>>>>>>> be >>>>>>>> away on holidays. >>>>>>>> >>>>>>> Bad news, this patch produces a regression in pdftops when running >>>>>>> over >>>>>>> the attached file, that is, the unpatched version creates a ps file >>>>>>> that is valid (gs will open it) and the patched version creates a ps >>>>>>> file that "crashes" gs. >>>>>>> >>>>>>> Do you think you can have a look? >>>>>>> >>>>>>> Albert >>>>>>> >>>>>> Dear Albert, >>>>>> >>>>>> I am having difficulties to reproduce the problem. Here is what I did: >>>>>> >>>>>> 1. try to view bug157704.pdf with the standard gs >>>>>> --> crash (see below) >>>>>> 2. call pdftops (using system's version of libpoppler) and open gs on >>>>>> the result >>>>>> --> works without errors (see below) >>>>>> 3. call pdftops using the patched libpoppler and open gs on the result >>>>>> --> works without errors >>>>>> 4. call pdftops of poppler git version of 0.14 without my patch (I >>>>>> have >>>>>> not yet pulled the new version) and open gs on the result >>>>>> --> works without errors >>>>>> >>>>>> Do you have more detailed information about the crash? I fear I am >>>>>> unable to do anything here... >>>>>> >>>>> Wops, works for me now too, i must have done something weird, sorry :-/ >>>>> >>>>> I'll start the regression test again. >>>>> >>>>> Albert >>>>> >>>> Ok, thanks for the notice! >>>> >>> Dear Albert, >> >> thanks for the thorough testing. >> >> The regression test was successful, though i have some questions i'd like >>> to >>> get answered before commiting the patch. >>> >>> In Gfx::gouraudFillTriangle you removed the check for contentIsHidden, >>> why? >>> >> Hm. It seems as if the test should be there - I must have forgotten to >> include it. Thank you for pointing it out! Could you add it right before the >> fill statement? >> >> Why do this if everything else in the line is intengers, what's the point >>> of >>> having a 2. there? >>> - color01.c[i] = (color0->c[i] + color1->c[i]) / 2; >>> + color01.c[i] = (color0->c[i] + color1->c[i]) / 2.; >>> >>> You are right, that's certainly a waste of time. I should have >> investigated the compiler warnings; sorry about that. Please feel free to >> remove the double suffixes. >> >> Best regards >> >> Christian >> >> >> Here are the detailed outputs for your reference: >>>>>> >>>>>> for (1): >>>>>> [ludewich] tmp>>gs bug157704.pdf >>>>>> GPL Ghostscript 8.71 (2010-02-10) >>>>>> Copyright (C) 2010 Artifex Software, Inc. All rights reserved. >>>>>> This software comes with NO WARRANTY: see the file PUBLIC for details. >>>>>> Processing pages 1 through 1. >>>>>> Page 1 >>>>>> Error: /unknownerror in --run-- >>>>>> >>>>>> Operand stack: >>>>>> --dict:6/15(L)-- >>>>>> >>>>>> Execution stack: >>>>>> %interp_exit .runexec2 --nostringval-- --nostringval-- >>>>>> >>>>>> --nostringval-- 2 %stopped_push --nostringval-- >>>>>> --nostringval-- --nostringval-- false 1 %stopped_push 1878 >>>>>> 1 3 %oparray_pop 1877 1 3 %oparray_pop 1861 1 3 >>>>>> %oparray_pop --nostringval-- --nostringval-- 2 1 1 >>>>>> --nostringval-- %for_pos_int_continue --nostringval-- >>>>>> --nostringval-- false 1 %stopped_push --nostringval-- >>>>>> --nostringval-- --nostringval-- %array_continue --nostringval-- >>>>>> false 1 %stopped_push --nostringval-- %loop_continue >>>>>> --nostringval-- >>>>>> >>>>>> Dictionary stack: >>>>>> --dict:1153/1684(ro)(G)-- --dict:1/20(G)-- --dict:75/200(L)-- >>>>>> >>>>>> --dict:75/200(L)-- --dict:108/127(ro)(G)-- --dict:288/300(ro)(G)-- >>>>>> --dict:22/25(L)-- --dict:6/8(L)-- --dict:21/40(L)-- >>>>>> --dict:3/5(L)-- Current allocation mode is local >>>>>> Last OS error: 11 >>>>>> GPL Ghostscript 8.71: Unrecoverable error, exit code 1 >>>>>> >>>>>> for (2): >>>>>> [ludewich] tmp>>pdftops -? >>>>>> pdftops version 0.12.4 >>>>>> [ludewich] tmp>>time pdftops bug157704.pdf >>>>>> >>>>>> real 0m14.925s >>>>>> user 0m13.900s >>>>>> sys 0m0.140s >>>>>> [ludewich] tmp>>gs bug157704.ps >>>>>> GPL Ghostscript 8.71 (2010-02-10) >>>>>> [ ALL OK ] >>>>>> >>>>>> for (3): >>>>>> [ludewich] poppler>>git describe # (with my patch) >>>>>> poppler-0.14.0-18-g821bc2a >>>>>> [ludewich] tmp>>time ~/code/xpdf/poppler/utils/pdftops bug157704.pdf >>>>>> >>>>>> real 0m2.785s >>>>>> user 0m2.540s >>>>>> sys 0m0.130s >>>>>> [ludewich] tmp>>gs bug157704.ps >>>>>> GPL Ghostscript 8.71 (2010-02-10) >>>>>> Copyright (C) 2010 Artifex Software, Inc. All rights reserved. >>>>>> This software comes with NO WARRANTY: see the file PUBLIC for details. >>>>>> [ ALL OK ] >>>>>> >>>>>> >>>>>> for (4): >>>>>> [ludewich] poppler>>git describe # (without my patch using >>>>>> poppler-0.14 >>>>>> ) poppler-0.14.0-3-gb2427d0 >>>>>> [ludewich] tmp>>time ~/code/xpdf/poppler/utils/pdftops bug157704.pdf >>>>>> >>>>>> real 0m15.844s >>>>>> user 0m13.820s >>>>>> sys 0m0.140s >>>>>> [ludewich] tmp>>gs bug157704.ps >>>>>> GPL Ghostscript 8.71 (2010-02-10) >>>>>> Copyright (C) 2010 Artifex Software, Inc. All rights reserved. >>>>>> This software comes with NO WARRANTY: see the file PUBLIC for details. >>>>>> [ ALL OK ] >>>>>> >>>>>> Albert >>>>>>>> >>>>>>>> Best regards >>>>>>>>> >>>>>>>>> Christian >>>>>>>>> >>>>>>>>> Am 25.07.2010 16:56, schrieb Albert Astals Cid: >>>>>>>>> >>>>>>>>>> A Dissabte, 3 de juliol de 2010, Christian Feuersaenger va >>>>>>>>>> escriure: >>>>>>>>>> >>>>>>>>>>> Hello Albert, >>>>>>>>>>> >>>>>>>>>> Hi >>>>>>>>>> >>>>>>>>>> I've managed to fix a bug in the Shading Type 6/7 (Coons& >>>>>>>>>>> cubic >>>>>>>>>>> tensor patches) implementation. >>>>>>>>>>> >>>>>>>>>>> The bugfix is small and stable (in my eyes); the poppler-0.14 >>>>>>>>>>> branch doesn't implement support for parameterized patch >>>>>>>>>>> shadings. >>>>>>>>>>> I modified the existing implementation accordingly with >>>>>>>>>>> relatively >>>>>>>>>>> few changes. >>>>>>>>>>> >>>>>>>>>>> Attached you find the patch file and the updated test.pdf to see >>>>>>>>>>> the improvement. >>>>>>>>>>> >>>>>>>>>>> The file type4567patch.... also includes the patch of my previous >>>>>>>>>>> mail (they only share the same refinement threshold). >>>>>>>>>>> >>>>>>>>>>> The patch should work relative to poppler-0.14.0-3-gb2427d0 . >>>>>>>>>>> >>>>>>>>>> This patch causes a regression in the attached pdf (the blue area >>>>>>>>>> disappears) >>>>>>>>>> >>>>>>>>>> Albert >>>>>>>>>> >>>>>>>>>> Best regards >>>>>>>>>>> >>>>>>>>>>> Christian >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> _______________________________________________ >>>>>>>>>>> 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 >>>>>>>> >>>>>>> _______________________________________________ >>>>>> 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 >>>> >>> _______________________________________________ >>> 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 > >
_______________________________________________ poppler mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/poppler
