[cairo] OVER / SOURCE optimization for cairo_paint
Antoine Azar
cairo at antoineazar.com
Sat Feb 16 15:57:30 PST 2008
Hi Carl,
>Could you please attach a patch instead? That way we can more easily
>review just what the changes are, and we can much more easily apply
>the patch, and run your code.
I had actually purposely not attached a patch because the code is in
"draft form". I'm more hoping for feedback on the logic for computing
the source extents and such. Which also explains the comment format
and the personal notes in the code :-) But I'm attaching the patch
here (both inlined and as attachment). You can mostly ignore style
issues for now, I'll clean it up when I'll submit a real patch.
>That's a comment that doesn't really add anything, so please remove
>that. Also we don't use the C++-style one-line comments in cairo[*].
>So please just use the /*...*/ style instead.
Out of curiosity, why not? If I'm not mistaken, most C compilers
accept them anyways? I find it much easier while developing to
comment out blocks of code when the valid comments only use // and
not /* */ (and I'm not a big fan of #if 0)
>Here, you're introducing a very long 'if' block for the
>optimization. I think this would be much readable as a separate
>function. Maybe something
>like cairo_gstate_paint_opaque_with_source or so. Fortunately, the
>current function is accepting only a single parameter, so it's not
>painful to pass a lot of arguments down to a
>new function, (nested functions would be really nice---and support
>for them in gcc is rather tempting).
I don't know the code well enough yet, but it seems many
optimizations have been implemented here and there at various levels.
If it's possible at all, it would be good to move these optimizations
as early in the code path as possible, and engineer our functions
based on that. That was kind of the idea I mentionned to have a layer
between the user and the gstate functions to convert between "what
the user asked for" and "what we really want to do internally". Or is
that already the role of the functions in cairo.c or in gstate?
Looking forward to your feedback!
Thanks,
Antoine
diff --git a/src/cairo-gstate.c b/src/cairo-gstate.c
index 0b775ca..9c764ec 100644
--- a/src/cairo-gstate.c
+++ b/src/cairo-gstate.c
@@ -879,6 +879,16 @@ _cairo_gstate_paint (cairo_gstate_t *gstate)
{
cairo_status_t status;
cairo_pattern_union_t pattern;
+ cairo_operator_t op = gstate->op;
+
+ //optimization related declarations
+ cairo_rectangle_int_t extents;
+ const cairo_pattern_union_t *pattern_union;
+ cairo_fixed_t x_fixed, y_fixed;
+ cairo_fixed_t dx_fixed, dy_fixed;
+ cairo_rectangle_t surface_rect;
+ cairo_path_fixed_t *path;
+ cairo_matrix_t matrix_invert;
if (gstate->source->status)
return gstate->source->status;
@@ -891,8 +901,98 @@ _cairo_gstate_paint (cairo_gstate_t *gstate)
if (status)
return status;
+ //Optimize a very common case of calling paint with an OVER
operator for opaque surfaces.
+ //Replace it with a more efficient SOURCE operator, and
constrain the operation to the source's extents.
+ if ( _cairo_pattern_is_opaque(&pattern.base) && (op ==
CAIRO_OPERATOR_OVER || op == CAIRO_OPERATOR_SOURCE))
+ {
+ pattern_union = (cairo_pattern_union_t *) &pattern;
+ switch (pattern_union->base.type)
+ {
+ case CAIRO_PATTERN_TYPE_SOLID:
+ case CAIRO_PATTERN_TYPE_LINEAR:
+ case CAIRO_PATTERN_TYPE_RADIAL:
+ op = CAIRO_OPERATOR_SOURCE;
+ break;
+ case CAIRO_PATTERN_TYPE_SURFACE:
+ //in all cases set the operator to source
+ op = CAIRO_OPERATOR_SOURCE;
+
+ if (pattern.surface.base.extend != CAIRO_EXTEND_NONE)
+ {
+ //We'll need to fill the whole destination
anyways, so go on with paint
+ break;
+ }
+ else
+ {
+ //create a path around the source's extents and
call fill with that
+ path = _cairo_path_fixed_create();
+
+ //extents of the source
+ status =
_cairo_surface_get_extents(pattern.surface.surface, &extents);
+ if (status) {
+ printf("ERROR\n");
+ return status; //instead continue without optimization?
+ }
+
+ //multiply by the source's inverse matrix and by
the dest context transformation matrix
+ surface_rect.x = (double)extents.x;
+ surface_rect.y = (double)extents.y;
+ surface_rect.width =
surface_rect.x+(double)extents.width; //convert width and height to
point coords for now
+ surface_rect.height =
surface_rect.y+(double)extents.height;
+
+ //FIXME: isn't the inverse already available somewhere?
+ matrix_invert = pattern.surface.base.matrix;
+ cairo_matrix_invert(&matrix_invert);
+ _cairo_matrix_transform_bounding_box (&matrix_invert,
+ &surface_rect.x, &surface_rect.y,
&surface_rect.width, &surface_rect.height, NULL);
+
+ surface_rect.x = floor(surface_rect.x);
+ surface_rect.y = floor(surface_rect.y);
+ surface_rect.width = ceil(surface_rect.width);
+ surface_rect.height = ceil(surface_rect.height);
+
+ if (surface_rect.width <= 0 || surface_rect.height <= 0)
+ return CAIRO_STATUS_SUCCESS;
+
+ cairo_matrix_transform_point
(&gstate->target->device_transform, &surface_rect.x, &surface_rect.y);
+ cairo_matrix_transform_point
(&gstate->target->device_transform, &surface_rect.width, &surface_rect.height);
+
+ //FIXME: why doesn't user_to_backend implement
the floor and ceil as done above (and why do we need them in the
first place anyways?)?
+ // this is apparent in the
filter-nearest-offset test
+// _cairo_gstate_user_to_backend (gstate,
&surface_rect.x, &surface_rect.y);
+// _cairo_gstate_user_to_backend (gstate,
&surface_rect.width, &surface_rect.height);
+
+ //Go back to width and height instead of point coords
+ surface_rect.width -= surface_rect.x;
+ surface_rect.height -= surface_rect.y;
+
+ x_fixed = _cairo_fixed_from_double (surface_rect.x);
+ y_fixed = _cairo_fixed_from_double (surface_rect.y);
+
+ dx_fixed = _cairo_fixed_from_double (surface_rect.width);
+ dy_fixed = _cairo_fixed_from_double (surface_rect.height);
+
+ status = _cairo_path_fixed_move_to (path,
x_fixed, y_fixed);
+ status |= _cairo_path_fixed_rel_line_to (path,
dx_fixed, 0);
+ status |= _cairo_path_fixed_rel_line_to (path,
0, dy_fixed);
+ status |= _cairo_path_fixed_rel_line_to (path,
-dx_fixed, 0);
+ status |= _cairo_path_fixed_rel_line_to (path,
0, -dy_fixed);
+ status |= _cairo_path_fixed_close_path (path);
+
+ if (status) {
+ printf("ERROR\n");
+ return status; //instead continue without optimization?
+ }
+
+ _cairo_gstate_fill(gstate, path);
+ _cairo_path_fixed_destroy(path);
+ return status;
+ }
+ }
+ }
+
status = _cairo_surface_paint (gstate->target,
- gstate->op,
+ op,
&pattern.base);
_cairo_pattern_fini (&pattern.base);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: overSource_optim.patch
Type: application/octet-stream
Size: 5760 bytes
Desc: not available
Url : http://lists.cairographics.org/archives/cairo/attachments/20080216/355bac83/attachment.obj
More information about the cairo
mailing list