A Dijous, 14 d'octubre de 2010, Christian Feuersänger va escriure: > Dear Thomas, > > Thanks for taking responsibility for the shader implementation! I am > running a little bit out of time... > I support your suggestions.
Pushed. Albert > > 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
