[poppler] Bugfix proposal for Shading Type 6 and 7
Christian Feuersänger
cfeuersaenger at googlemail.com
Thu Oct 14 05:01:52 PDT 2010
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 <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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/poppler/attachments/20101014/7502d34b/attachment.htm>
More information about the poppler
mailing list