Bug 15367

Summary:	Improve filtering handling in cairo-pattern.c
Product:	cairo	Reporter:	Owen Taylor <otaylor>
Component:	general	Assignee:	Carl Worth <cworth>
Status:	RESOLVED FIXED	QA Contact:	cairo-bugs mailing list <cairo-bugs>
Severity:	normal
Priority:	medium
Version:	1.5.17
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	The patch

Description Owen Taylor 2008-04-05 08:46:57 UTC

This is an evolution of the patch attached in bug 15439 to special
case pixel-exact transformations when adding the bounds; it also
actually applies the => NEAREST optimization in _cairo_pattern_acquire_surface().

Analyzing that for safety (usage in non-pixel-based backens) turned
up some odd code in cairo-svg-surface.c and cairo-win32-printing-surface.c:

- They called _cairo_pattern_acquire_surface() only for surface patterns
- A hack had been added so they could pass -1,-1 for width/height 
  and get the entire source surface
- The ignored the attributes result from the function

At that point they could just use pattern->surface directly instead,
there was no point in calling cairo_pattern_acquire_surface() at all.

The patch also removes the hack, since it doesn't seem consistent
with other internal API's in cairo and I can't find any other usages.

Suggest this for after 1.6.

commit 40847798271f9b8986cf90560dd1438fd8772847
Author: Owen W. Taylor <otaylor@fishsoup.net>
Date:   Sat Apr 5 11:34:27 2008 -0400

    Clean up handling of filter radius; optimize filter when possible
    cairo-pattern.c:
     - Factor out filter analysis code from _cairo_pattern_get_extents()
       and use it in _cairo_pattern_acquire_surface_for_surface() as well.
     - Optimize the filter to NEAREST when the matrix takes source
       pixels exactly onto destination pixels
    cairo-matrix.c cairoint.h: Add _cairo_matrix_is_pixel_exact()
    cairo-svg-surface.c cairo-win32-printing-surface.c: Remove
     instances of calling _cairo_pattern_acquire_surface() on
     a known-surface-pattern with a hack to say "give me the
     entire surface". If you know you just want the entire surface
     as an image surface, that can be done more simply.

Comment 1 Owen Taylor 2008-04-05 08:48:14 UTC

Created attachment 15703 [details] [review]
The patch

Comment 2 Carl Worth 2008-04-05 10:45:35 UTC

(In reply to comment #0)
> Suggest this for after 1.6.

Sounds good. This looks quite useful.

Your comment suggests that should be at a minimum two separate commits. Two obviously different things you described are:

> actually applies the => NEAREST optimization in
> _cairo_pattern_acquire_surface().

and

> The patch also removes the hack, since it doesn't seem consistent
> with other internal API's in cairo and I can't find any other usages.

It's funny how that word "also" describing a patch almost always means the patch deserves to be split.

And of course, that might be as simple as "git add -p", (or maybe even easier since there are separate files involved here).

Great to have you back hacking on cairo, Owen!

-Carl

Comment 3 Owen Taylor 2008-04-05 11:27:58 UTC

Not going to redo the patch at the moment, but for reference, the "also" hunk is:

@@ -1677,10 +1735,7 @@ _cairo_pattern_acquire_surface_for_surface (cairo_surface_pattern_t   *pattern,
 	    return status;
 
 	/* If we're repeating, we just play it safe and clone the entire surface. */
-	/* If requested width and height are -1, clone the entire surface.
-	 * This is relied on in the svg backend. */
-	if (attr->extend == CAIRO_EXTEND_REPEAT ||
-	    (width == (unsigned int) -1 && height == (unsigned int) -1)) {
+	if (attr->extend == CAIRO_EXTEND_REPEAT) {
 	    x = extents.x;
 	    y = extents.y;
 	    width = extents.width;

Which has to be applied *after* the rest of the patch.

The cairo-svg-surface anbd cairo-win32-printing-surface changes
are logically tied to the rest of the patch, since it's not valid to
make the => NEAREST optimization if the image is going to be used on
a SVG surface or printer.

> Great to have you back hacking on cairo, Owen!

Don't expect anything long term, just trying to unbreak my desktop
at the moment, and cairo is a great tool for writing RENDER test
cases. :-)

Comment 4 Chris Wilson 2008-09-29 02:21:55 UTC

I've integrated this patch with the series to fix bug 15349 as the refactoring centralizes the assumptions made about the filter - in particular its radius and efficacy. Commit 5eec3e378afd6ff9991cea8e42b8478eb3e79773.

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.