[poppler] Bugfix proposal for Shading Type 6 and 7
Albert Astals Cid
aacid at kde.org
Thu Oct 14 16:01:41 PDT 2010
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 <Thomas.Freitag at kabelmail.de>
>
> > 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
> >>>>>>>>>>> poppler at lists.freedesktop.org
> >>>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
> >>>>>>>>>>
> >>>>>>>>>> _______________________________________________
> >>>>>>>>
> >>>>>>>> poppler mailing list
> >>>>>>>> poppler at lists.freedesktop.org
> >>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> _______________________________________________
> >>>>>>>> poppler mailing list
> >>>>>>>> poppler at lists.freedesktop.org
> >>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
> >>>>>>>
> >>>>>>> _______________________________________________
> >>>>>>
> >>>>>> poppler mailing list
> >>>>>> poppler at lists.freedesktop.org
> >>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
> >>>>>
> >>>>> _______________________________________________
> >>>>> poppler mailing list
> >>>>> poppler at lists.freedesktop.org
> >>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
> >>>>
> >>>> _______________________________________________
> >>>> poppler mailing list
> >>>> poppler at lists.freedesktop.org
> >>>> http://lists.freedesktop.org/mailman/listinfo/poppler
> >>>
> >>> _______________________________________________
> >>> poppler mailing list
> >>> poppler at lists.freedesktop.org
> >>> http://lists.freedesktop.org/mailman/listinfo/poppler
> >>
> >> _______________________________________________
> >> poppler mailing list
> >> poppler at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/poppler
> >>
> >> .
> >
> > _______________________________________________
> > poppler mailing list
> > poppler at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/poppler
More information about the poppler
mailing list