[cairo] Optimize horizontal/vertical gradients

David Reveman davidr at novell.com
Sun Mar 6 11:41:30 PST 2005


On Sat, 2005-03-05 at 14:42 -0500, Owen Taylor wrote: 
> Owen Taylor wrote:
> > Quick 30 second thought that makes a big difference for pure
> > vertical/horizontal gradients. It's possible that there should
> > be some epsilon in the tests, to handle round-off errors in
> > 90 degree rotations and the like, maybe something like
> > 
> >  fabs (p2.x - p1.x) < EPSILON * fabs (p2.y - p1.y)
> 
> Turns out the patch was pretty essentially broken because it
> was ignoring the pattern matrix ... p1/p2 are in pattern space,
> not device space. Rather than inverse transforming the points
> to device space and trying to detect vertical/horizontal gradients
> with some epsilon, I took the alternate approach of computing
> the gradient position at all four corners of the drawn area.
> If the two positions on the top are the identical, it's a vertical
> pattern. If the two positions on the left are identical, it's
> a horizontal pattern.
> 
> Since the gradient position is handled as a fixed point number
> when computing the gradient image, a simple equality test
> works OK.
> 
> You could imagine more complicated algorithms that did things
> like noticing that the area being drawn was completely outside
> the [0,1] range and could be drawn with a constant color,
> but I think this patch should catch the common cases.
> 
> There also is some cleanup of the linear gradient computation
> in the patch. I'm sure the way it was written before would make
> sense to me if there was a SVG diagram embedded in a comment ;-)
> but it seemed to me it was introducing trig functions for no
> good reason, so I rewrote it to remove them and added a short
> comment.

Looks good. I probably got some more stupid math in there, so don't
hesitate to suggest improvements.

> 
> It probably would make sense to rename various variables to match
> the comment and the current logic, but I haven't yet done so
> so as to keep the patch more reviewable.

Yes, do that before committing it.

> 
> I couldn't figure out a good way to share the gradient computation
> logic between _cairo_linear_pattern_classify _and 
> cairo_image_data_set_linear so it's just duplicated.

That's OK for now. We probably like to restructure some of these
gradient functions a bit when we change so that the pattern filter is no longer
used for interpolation between color stops. We might be able to make these
gradient computations shareable at the same time but I think we should
wait until after the upcoming snapshot release before we do any of
this. 

+       if (is_horizontal) {
+           height = 1;
+           repeat = TRUE;
+       }
+       if (is_vertical) {
+           width = 1;
+           repeat = TRUE;
+       }
+    }
+    
+#undef THRESHOLD

'#undef THRESHOLD', that's just something you accidentally forgot to
remove, right?

The patch looks good and it seems to be working fine. Unless someone
else objects, I think it's OK to commit it.

-David




More information about the cairo mailing list