Bug 10942

Summary: spurious horizontal stripes in color gradients
Product: poppler Reporter: Carlos Garcia Campos <carlosgc>
Component: cairo backendAssignee: poppler-bugs <poppler-bugs>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium CC: alfredo.matos, chris, elvstone, ideasman42, keir.thomas, kostmo, leidola, nils, njs, samtygier, tfc.duke
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: pdf file showing spurious stripes
presentation pdf with stripes on gradients
Rect with gradient from Scribus 1.3.3.11
Implement axialShadedFill() in cairo backend using cairo linear gradient patterns
Implement axial/radial ShadedFill in cairo backend using cairo gradients
Implement axialShadedFill in cairo backend using cairo gradients
Attempt to implement radial patterns
Implement axialShadedFill in cairo backend using cairo gradients
Radial gradients using cairo

Description Carlos Garcia Campos 2007-05-14 10:39:41 UTC
Bug forwarded from Evince: http://bugzilla.gnome.org/show_bug.cgi?id=438386

"Please describe the problem:
While making a scientific poster evince showed annoying stripes in color
gradients. xpdf (version 3.01) and acrobat render the document correctly.

Steps to reproduce:
It should be possible to reproduce the bug by compiling following file with
pdflatex.

\documentclass{article}
\usepackage[T1]{fontenc}
\usepackage[latin1]{inputenc}
\usepackage{color}
\usepackage{tikz}
\usepackage[a0paper,margin=2cm,nohead,nofoot,paperheight=110cm,
paperwidth=95cm]{geometry}

\definecolor{botcol}{rgb}{0.890625, 0.88671875, 0.99609375}
\definecolor{topcol}{rgb}{1.0, 1.0, 1.}

\pagestyle{empty}
\pagecolor{black}



\begin{document}

\begin{tikzpicture}
\draw (0,0) node[shade, color=topcol, bottom color=botcol] {
\parbox{1.\textwidth}{
\Large Test
\vspace{30cm}
}};
\end{tikzpicture}

\end{document}"

It's only reproducible with cairo backend, with splash it's rendered correctly.
Comment 1 Carlos Garcia Campos 2007-05-14 10:40:42 UTC
Created attachment 9961 [details]
pdf file showing spurious stripes
Comment 2 Alfredo Matos 2007-06-28 02:54:52 UTC
Created attachment 10483 [details]
presentation pdf with stripes on gradients

I also see this on two separate setups: 
- using evince 0.8.1 and libpoppler 0.5.4, on Ubuntu Feisty.
- using evince 0.9.1 and libpoppler 0.5.9, on Ubuntu Gutsy.

For me the procedure is:

1. Create a presentation with openoffice
2. Change the background to a gradient
3. Export as PDF
4. Open in evince

Test case file is attached.
Comment 3 Nathaniel Smith 2007-06-29 14:24:53 UTC
This may be related to bug#11165
Comment 4 Behdad Esfahbod 2007-08-04 20:03:42 UTC
I'm seeing the same bug in PDF generated by latest cairo.
Comment 5 Kristian Høgsberg 2007-08-05 07:26:31 UTC
*** Bug 11165 has been marked as a duplicate of this bug. ***
Comment 6 Kristian Høgsberg 2007-08-05 07:29:43 UTC
The problem is that the builtin splash rendering backend doesn't support gradient patterns.  The higher level poppler rendering logic breaks a gradient filled polygon into a series of solid color polygons corresponding to the color bands in the gradient.  When these polygons are rendered in several passes with anti-aliasing, seams occur between the polygons.
Comment 7 David Henry 2007-11-11 10:12:50 UTC
The bug is also present with poppler 0.6 on Ubuntu Gutsy. A bit annoying for Beamer presentations...
Comment 8 Jeff Muizelaar 2008-02-22 18:18:39 UTC
The patch in bug 14408 may help with this.
Comment 9 Elvis Stansvik 2008-08-02 04:54:36 UTC
I can reproduce this by creating a PDF with a gradient that has transparency in it using Scribus 1.3.3.11 running on Kubuntu Hardy and viewing it in either Okular 0.7 or KPDF 0.5.9.

It seems it adds one stripe for each span between gradient points that are transparent or something.

I'm attaching stripes-from-scribus-gradient.pdf which shows the issue.
Comment 10 Elvis Stansvik 2008-08-02 04:57:07 UTC
Created attachment 18078 [details]
Rect with gradient from Scribus 1.3.3.11
Comment 11 David Henry 2009-02-01 03:15:54 UTC
*** Bug 19831 has been marked as a duplicate of this bug. ***
Comment 12 David Henry 2009-02-01 03:16:57 UTC
*** Bug 18025 has been marked as a duplicate of this bug. ***
Comment 13 David Henry 2009-02-01 03:17:40 UTC
*** Bug 16235 has been marked as a duplicate of this bug. ***
Comment 14 David Henry 2009-02-01 03:23:29 UTC
*** Bug 15501 has been marked as a duplicate of this bug. ***
Comment 15 Carlos Garcia Campos 2009-06-03 12:04:34 UTC
Created attachment 26402 [details] [review]
Implement axialShadedFill() in cairo backend using cairo linear gradient patterns

 poppler/CairoOutputDev.cc |  297 +++++++++++++++++++++++++++++++++++++++++++++                                                                              
 poppler/CairoOutputDev.h  |    6 +                                                                                                                          
 2 files changed, 303 insertions(+), 0 deletions(-)

This patch implements linear gradients in cairo backend. I'm not a cairo guru so I'm not sure it's completely right, it fixes the problem with the first attachment.
Comment 16 Adrian Johnson 2009-06-03 14:19:32 UTC
(In reply to comment #15)
> Created an attachment (id=26402) [details]
> Implement axialShadedFill() in cairo backend using cairo linear gradient
> patterns
> 
>  poppler/CairoOutputDev.cc |  297 +++++++++++++++++++++++++++++++++++++++++++++ 
>  poppler/CairoOutputDev.h  |    6 +                                             
>  2 files changed, 303 insertions(+), 0 deletions(-)
> 
> This patch implements linear gradients in cairo backend. I'm not a cairo guru
> so I'm not sure it's completely right, it fixes the problem with the first
> attachment. 
> 

The use of cairo looks fine. Would it be better to avoid duplicating the bisection code from Gfx and instead add a new axial shading OutputDev function that provides the coordinates of each stop?

Comment 17 Albert Astals Cid 2009-06-03 14:29:15 UTC
Yeah man, duplicating the code is BAD

Why the current Gfx code is not enough for the Cairo backend?
Comment 18 Adrian Johnson 2009-06-03 14:32:39 UTC
(In reply to comment #16)
> (In reply to comment #15)
> > Created an attachment (id=26402) [details] [details]
> > Implement axialShadedFill() in cairo backend using cairo linear gradient
> > patterns
> > 
> >  poppler/CairoOutputDev.cc |  297 +++++++++++++++++++++++++++++++++++++++++++++ 
> >  poppler/CairoOutputDev.h  |    6 +                                             
> >  2 files changed, 303 insertions(+), 0 deletions(-)
> > 
> > This patch implements linear gradients in cairo backend. I'm not a cairo guru
> > so I'm not sure it's completely right, it fixes the problem with the first
> > attachment. 
> > 
> 
> The use of cairo looks fine. Would it be better to avoid duplicating the
> bisection code from Gfx and instead add a new axial shading OutputDev function
> that provides the coordinates of each stop?
> 

On closer inspection the use of cairo is less than ideal. It would be better to use the actual stops in the shading in the case of linear interpolation (Type 2 Exponential functions with N=1) and fallback to bisection for other cases. This would allow cairo to do the linear interpolation where possible.
Comment 19 Carlos Garcia Campos 2009-06-04 00:32:14 UTC
(In reply to comment #16)
> 
> The use of cairo looks fine. Would it be better to avoid duplicating the
> bisection code from Gfx and instead add a new axial shading OutputDev function
> that provides the coordinates of each stop?
> 

yes, sure, this patch is just a quick hack to see whether it works. I'll rework the patch and try to implement radial gradients too. 
Comment 20 Carlos Garcia Campos 2009-06-04 00:42:22 UTC
(In reply to comment #17)
> Why the current Gfx code is not enough for the Cairo backend?
> 

I don't know exactly, but the thing is that it doesn't work, see comment #6. Here is a screenshot showing the difference:

http://people.freedesktop.org/~carlosgc/poppler-cairo-patterns.png

The evince window is using poppler without the patch while the glib demo is using the patch. 
Comment 21 Adrian Johnson 2009-06-04 01:52:00 UTC
(In reply to comment #17)
> Yeah man, duplicating the code is BAD
> 
> Why the current Gfx code is not enough for the Cairo backend?
> 

Cairo does a poor job of drawing adjacent polygons due to the antialising causing visible seams. I am not aware of any easy fix.

It is a useful optimization for all backends that can natively draw gradients to be able to get a list of gradient stops to draw the gradient instead of drawing a very large number of solid fills.
Comment 22 Carlos Garcia Campos 2009-06-04 06:08:14 UTC
Created attachment 26432 [details] [review]
Implement axial/radial ShadedFill in cairo backend using cairo gradients

 poppler/CairoOutputDev.cc |   34 ++++++++++++++++++                                                                                                         
 poppler/CairoOutputDev.h  |    8 ++++                                                                                                                       
 poppler/Gfx.cc            |   86 ++++++++++++++++++++++++++++++++-------------                                                                              
 poppler/OutputDev.h       |    1 +                                                                                                                          
 4 files changed, 104 insertions(+), 25 deletions(-)

This patch removes duplicated code and implements radial gradients too.
Comment 23 Carlos Garcia Campos 2009-06-04 06:16:52 UTC
*** Bug 4330 has been marked as a duplicate of this bug. ***
Comment 24 Adrian Johnson 2009-06-04 07:41:26 UTC
A couple of PDFs that don't render correctly with the patch:

http://people.freedesktop.org/~ajohnson/linear.pdf
http://people.freedesktop.org/~ajohnson/radial.pdf
Comment 25 Albert Astals Cid 2009-06-04 10:56:46 UTC
You mean they fail with new patch or failed with old patch work with new one?

Both work on Splash BTW

Also 

@@ -2751,12 +2763,13 @@ void Gfx::doRadialShFill(GfxRadialShading *shading) {
   ya = y0 + sa * (y1 - y0);
   ra = r0 + sa * (r1 - r0);
   if (ta < t0) {
-    shading->getColor(t0, &colorA);
+    tt = t0;
   } else if (ta > t1) {
-    shading->getColor(t1, &colorA);
+    tt = t1;
   } else {
-    shading->getColor(ta, &colorA);
+    tt = ta;
   }
+  shading->getColor(tt, &colorA);
 
   // fill the circles
   while (ia < radialMaxSplits) {

looks like an unneeded change
Comment 26 Carlos Garcia Campos 2009-06-04 11:06:03 UTC
(In reply to comment #25)
> You mean they fail with new patch or failed with old patch work with new one?

yes, they don't work with the patch. 

> Both work on Splash BTW

sure

> Also 
> 
> @@ -2751,12 +2763,13 @@ void Gfx::doRadialShFill(GfxRadialShading *shading) {
>    ya = y0 + sa * (y1 - y0);
>    ra = r0 + sa * (r1 - r0);
>    if (ta < t0) {
> -    shading->getColor(t0, &colorA);
> +    tt = t0;
>    } else if (ta > t1) {
> -    shading->getColor(t1, &colorA);
> +    tt = t1;
>    } else {
> -    shading->getColor(ta, &colorA);
> +    tt = ta;
>    }
> +  shading->getColor(tt, &colorA);
> 
>    // fill the circles
>    while (ia < radialMaxSplits) {
> 
> looks like an unneeded change
> 

tt is used again below, without this change I would have to check again if ta < t1 or ta > t1, and so on. 

I think the problem with the patch is the offset calculation for cairo_pattern_add_color_stop but I don't know how to do it right yet. 
Comment 27 Carlos Garcia Campos 2009-06-09 07:24:14 UTC
Created attachment 26583 [details] [review]
Implement axialShadedFill in cairo backend using cairo gradients

I only managed to fix linear patterns (thanks to adrianj), so this patch contains only the implementation of axialShadedFill. 

Albert, ok to push this one?
Comment 28 Carlos Garcia Campos 2009-06-09 09:10:48 UTC
Created attachment 26589 [details] [review]
Attempt to implement radial patterns

Following the same approach than previous patch I've tried to implement radial patterns, but it still doesn't work with the test case provided by Adrian
Comment 29 Albert Astals Cid 2009-06-09 14:44:15 UTC
I'd say this can break PSOutputDev, PSOutputDev::axialShadedFill also returns false in some cases and i'd bet it needs the calls to updateFillColor, fill etc. in this cases to get a correct output, but that would not happen in your case.

Can you please check?
Comment 30 Carlos Garcia Campos 2009-06-10 07:13:45 UTC
sure, I thought no other backed implemented shaded fills :-( I forgot PSOutputdev. 
Comment 31 Carlos Garcia Campos 2009-07-08 04:06:23 UTC
Created attachment 27495 [details] [review]
Implement axialShadedFill in cairo backend using cairo gradients

I've added a method useFillColorStop() specific to decide whether to use updateFillColorStop or not, instead of using useShadedFills().
Comment 32 Carlos Garcia Campos 2009-07-08 09:58:51 UTC
(In reply to comment #31)
> Created an attachment (id=27495) [details]
> Implement axialShadedFill in cairo backend using cairo gradients

applied to git master :-)

Comment 33 Carlos Garcia Campos 2009-07-29 10:14:25 UTC
Created attachment 28167 [details] [review]
Radial gradients using cairo

This is an updated patch to implement radial gradients using cairo.
Comment 34 Albert Astals Cid 2009-07-30 12:23:10 UTC
Forgot to update radialShadedFill in PSOutputDev 
Comment 35 Carlos Garcia Campos 2009-07-31 09:26:34 UTC
Fixed in git master. 

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.