[poppler] Bugfix proposal for Shading Type 6 and 7
Thomas Freitag
Thomas.Freitag at kabelmail.de
Sat Oct 16 10:00:09 PDT 2010
Hi Albert!
I just uploaded the Christian's enhancements together with mine to bug
30436 <https://bugs.freedesktop.org/show_bug.cgi?id=30436>
Please start the regtest.
Thomas
Am 15.10.2010 01:01, schrieb Albert Astals Cid:
> 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
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/poppler
>
> .
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/poppler/attachments/20101016/06b2a0d9/attachment-0001.htm>
More information about the poppler
mailing list