[cairo] [PATCH] Refactor _cairo_quartz_setup_source

Jeff Muizelaar jeff at infidigm.net
Fri May 14 14:39:53 PDT 2010


commit 0bb308c4408ad045afaf73c299b54a036e7fe9db
Author: Robert O'Callahan <robert at ocallahan.org>
Date:   Thu May 13 15:13:11 2010 -0400

     Refactor _cairo_quartz_setup_source

     This patch doesn't change any behaviour. The per-drawing-operation 
state is
     pulled out of cairo_quartz_surface_t and moved into a new struct
     cairo_quartz_drawing_state_t, because I want to add new state when 
I fix this
     bug and having per-drawing-operation state in the surface is icky.

     The action is also stored in cairo_quartz_drawing_state_t, and I rename
     _cairo_quartz_setup_source to _cairo_quartz_setup_state, because 
we're going to
     add more then just source-related state.

     I move responsibility for saving and restoring the context into
     _cairo_quartz_setup_state/_cairo_quartz_teardown_state, and also 
responsibility
     for calling CGContextSetCompositeOperation. This adds an unnecessary
     Save/RestoreGState pair to _cairo_quartz_surface_paint with a solid 
color or
     pattern (cases which we never use, I think), but removes some 
unnecessary
     duplicate Save/Restore pairs, e.g. when filling or stroking with a 
surface
     pattern. I think it's a net code simplification without affecting 
performance.
     We could easily suppress the save/restore in more cases if we cared.

diff --git a/src/cairo-quartz-private.h b/src/cairo-quartz-private.h
index 38f0d8f..f31a4a8 100644
--- a/src/cairo-quartz-private.h
+++ b/src/cairo-quartz-private.h
@@ -61,20 +61,6 @@ typedef struct cairo_quartz_surface {

      cairo_surface_clipper_t clipper;
      cairo_rectangle_int_t extents;
-
-    /* These are stored while drawing operations are in place, set up
-     * by quartz_setup_source() and quartz_finish_source()
-     */
-    CGAffineTransform sourceTransform;
-
-    CGImageRef sourceImage;
-    cairo_surface_t *sourceImageSurface;
-    CGRect sourceImageRect;
-
-    CGShadingRef sourceShading;
-    CGPatternRef sourcePattern;
-
-    CGInterpolationQuality oldInterpolationQuality;
  } cairo_quartz_surface_t;

  typedef struct cairo_quartz_image_surface {
diff --git a/src/cairo-quartz-surface.c b/src/cairo-quartz-surface.c
index 03fade8..1d539c9 100644
--- a/src/cairo-quartz-surface.c
+++ b/src/cairo-quartz-surface.c
@@ -1324,16 +1324,37 @@ typedef enum {
      DO_SHADING,
      DO_PATTERN,
      DO_IMAGE,
+    DO_TILED_IMAGE,
      DO_UNSUPPORTED,
-    DO_NOTHING,
-    DO_TILED_IMAGE
+    DO_NOTHING
  } cairo_quartz_action_t;

-static cairo_quartz_action_t
+/* State used during a drawing operation. */
+typedef struct {
+    CGContextRef context;
+    cairo_quartz_action_t action;
+
+    // Used with DO_SHADING, DO_IMAGE and DO_TILED_IMAGE
+    CGAffineTransform transform;
+
+    // Used with DO_IMAGE and DO_TILED_IMAGE
+    CGImageRef image;
+    cairo_surface_t *imageSurface;
+    CGRect imageRect;
+
+    // Used with DO_SHADING
+    CGShadingRef shading;
+
+    // Used with DO_PATTERN
+    CGPatternRef pattern;
+} cairo_quartz_drawing_state_t;
+
+static void
  _cairo_quartz_setup_fallback_source (cairo_quartz_surface_t *surface,
-                     const cairo_pattern_t *source)
+                     const cairo_pattern_t *source,
+                     cairo_quartz_drawing_state_t *state)
  {
-    CGRect clipBox = CGContextGetClipBoundingBox (surface->cgContext);
+    CGRect clipBox = CGContextGetClipBoundingBox (state->context);
      double x0, y0, w, h;

      cairo_surface_t *fallback;
@@ -1342,8 +1363,10 @@ _cairo_quartz_setup_fallback_source 
(cairo_quartz_surface_t *surface,
      cairo_status_t status;

      if (clipBox.size.width == 0.0f ||
-    clipBox.size.height == 0.0f)
-    return DO_NOTHING;
+    clipBox.size.height == 0.0f) {
+    state->action = DO_NOTHING;
+    return;
+    }

      x0 = floor(clipBox.origin.x);
      y0 = floor(clipBox.origin.y);
@@ -1387,17 +1410,20 @@ _cairo_quartz_setup_fallback_source 
(cairo_quartz_surface_t *surface,
  #endif

      status = _cairo_surface_to_cgimage (&surface->base, fallback, &img);
-    if (status)
-    return DO_UNSUPPORTED;
-    if (img == NULL)
-    return DO_NOTHING;
-
-    surface->sourceImageRect = CGRectMake (0.0, 0.0, w, h);
-    surface->sourceImage = img;
-    surface->sourceImageSurface = fallback;
-    surface->sourceTransform = CGAffineTransformMakeTranslation (x0, y0);
+    if (status) {
+        state->action = DO_UNSUPPORTED;
+    return;
+    }
+    if (img == NULL) {
+        state->action = DO_NOTHING;
+    return;
+    }

-    return DO_IMAGE;
+    state->imageRect = CGRectMake (0.0, 0.0, w, h);
+    state->image = img;
+    state->imageSurface = fallback;
+    state->transform = CGAffineTransformMakeTranslation (x0, y0);
+    state->action = DO_IMAGE;
  }

  /*
@@ -1413,10 +1439,11 @@ based on the extents of the object (the clip 
region cannot be used here since
  we don't want the rasterization of the entire gradient to depend on the
  clip region).
  */
-static cairo_quartz_action_t
+static void
  _cairo_quartz_setup_linear_source (cairo_quartz_surface_t *surface,
                     const cairo_linear_pattern_t *lpat,
-                   cairo_rectangle_int_t *extents)
+                   cairo_rectangle_int_t *extents,
+                   cairo_quartz_drawing_state_t *state)
  {
      const cairo_pattern_t *abspat = &lpat->base.base;
      cairo_matrix_t mat;
@@ -1426,9 +1453,10 @@ _cairo_quartz_setup_linear_source 
(cairo_quartz_surface_t *surface,
      bool extend = abspat->extend == CAIRO_EXTEND_PAD;

      if (lpat->base.n_stops == 0) {
-    CGContextSetRGBStrokeColor (surface->cgContext, 0., 0., 0., 0.);
-    CGContextSetRGBFillColor (surface->cgContext, 0., 0., 0., 0.);
-    return DO_SOLID;
+    CGContextSetRGBStrokeColor (state->context, 0., 0., 0., 0.);
+    CGContextSetRGBFillColor (state->context, 0., 0., 0., 0.);
+    state->action = DO_SOLID;
+    return;
      }

      if (lpat->p1.x == lpat->p2.x &&
@@ -1438,12 +1466,13 @@ _cairo_quartz_setup_linear_source 
(cairo_quartz_surface_t *surface,
       * Whatever the correct behaviour is, let's at least have only 
pixman's
       * implementation to worry about.
       */
-    return _cairo_quartz_setup_fallback_source (surface, abspat);
+    _cairo_quartz_setup_fallback_source (surface, abspat, state);
+    return;
      }

      mat = abspat->matrix;
      cairo_matrix_invert (&mat);
-    _cairo_quartz_cairo_matrix_to_quartz (&mat, &surface->sourceTransform);
+    _cairo_quartz_cairo_matrix_to_quartz (&mat, &state->transform);

      rgb = CGColorSpaceCreateDeviceRGB();

@@ -1463,21 +1492,22 @@ _cairo_quartz_setup_linear_source 
(cairo_quartz_surface_t *surface,
                                    extents);
      }

-    surface->sourceShading = CGShadingCreateAxial (rgb,
-                           start, end,
-                           gradFunc,
-                           extend, extend);
+    state->shading = CGShadingCreateAxial (rgb,
+                       start, end,
+                       gradFunc,
+                       extend, extend);

      CGColorSpaceRelease(rgb);
      CGFunctionRelease(gradFunc);

-    return DO_SHADING;
+    state->action = DO_SHADING;
  }

-static cairo_quartz_action_t
+static void
  _cairo_quartz_setup_radial_source (cairo_quartz_surface_t *surface,
                     const cairo_radial_pattern_t *rpat,
-                   cairo_rectangle_int_t *extents)
+                   cairo_rectangle_int_t *extents,
+                   cairo_quartz_drawing_state_t *state)
  {
      const cairo_pattern_t *abspat = &rpat->base.base;
      cairo_matrix_t mat;
@@ -1496,9 +1526,10 @@ _cairo_quartz_setup_radial_source 
(cairo_quartz_surface_t *surface,
      double centerDistance = sqrt (dx*dx + dy*dy);

      if (rpat->base.n_stops == 0) {
-    CGContextSetRGBStrokeColor (surface->cgContext, 0., 0., 0., 0.);
-    CGContextSetRGBFillColor (surface->cgContext, 0., 0., 0., 0.);
-    return DO_SOLID;
+    CGContextSetRGBStrokeColor (state->context, 0., 0., 0., 0.);
+    CGContextSetRGBFillColor (state->context, 0., 0., 0., 0.);
+    state->action = DO_SOLID;
+    return;
      }

      if (r2 <= centerDistance + r1 + 1e-6 && /* circle 2 doesn't 
contain circle 1 */
@@ -1509,12 +1540,13 @@ _cairo_quartz_setup_radial_source 
(cairo_quartz_surface_t *surface,
       * implementation to worry about.
       * Note that this also catches the cases where r1 == r2.
       */
-    return _cairo_quartz_setup_fallback_source (surface, abspat);
+    _cairo_quartz_setup_fallback_source (surface, abspat, state);
+    return;
      }

      mat = abspat->matrix;
      cairo_matrix_invert (&mat);
-    _cairo_quartz_cairo_matrix_to_quartz (&mat, &surface->sourceTransform);
+    _cairo_quartz_cairo_matrix_to_quartz (&mat, &state->transform);

      rgb = CGColorSpaceCreateDeviceRGB();

@@ -1533,55 +1565,86 @@ _cairo_quartz_setup_radial_source 
(cairo_quartz_surface_t *surface,
                                    extents);
      }

-    surface->sourceShading = CGShadingCreateRadial (rgb,
-                            start,
-                            r1,
-                            end,
-                            r2,
-                            gradFunc,
-                            extend, extend);
+    state->shading = CGShadingCreateRadial (rgb,
+                        start,
+                        r1,
+                        end,
+                        r2,
+                        gradFunc,
+                        extend, extend);

      CGColorSpaceRelease(rgb);
      CGFunctionRelease(gradFunc);

-    return DO_SHADING;
+    state->action = DO_SHADING;
  }

-static cairo_quartz_action_t
-_cairo_quartz_setup_source (cairo_quartz_surface_t *surface,
-                const cairo_pattern_t *source,
-                cairo_rectangle_int_t *extents)
-{
-    assert (!(surface->sourceImage || surface->sourceShading || 
surface->sourcePattern));
+/**
+ * Sets up internal state to be used to draw the source mask, stored in
+ * cairo_quartz_state_t. Guarantees to call CGContextSaveGState on
+ * surface->cgContext.
+ */
+static cairo_quartz_drawing_state_t
+_cairo_quartz_setup_state (cairo_quartz_surface_t *surface,
+               const cairo_pattern_t *source,
+               cairo_operator_t op,
+               cairo_rectangle_int_t *extents)
+{
+    CGContextRef context = surface->cgContext;
+    cairo_quartz_drawing_state_t state;
+    cairo_status_t status;

-    surface->oldInterpolationQuality = CGContextGetInterpolationQuality 
(surface->cgContext);
-    CGContextSetInterpolationQuality (surface->cgContext, 
_cairo_quartz_filter_to_quartz (source->filter));
+    state.context = context;
+    state.image = NULL;
+    state.imageSurface = NULL;
+    state.shading = NULL;
+    state.pattern = NULL;
+
+    // Save before we change the pattern, colorspace, etc. so that
+    // we can restore and make sure that quartz releases our
+    // pattern (which may be stack allocated)
+    CGContextSaveGState(context);
+
+    CGContextSetInterpolationQuality (context, 
_cairo_quartz_filter_to_quartz (source->filter));
+
+    status = _cairo_quartz_surface_set_cairo_operator (surface, op);
+    if (status == CAIRO_INT_STATUS_NOTHING_TO_DO) {
+        state.action = DO_NOTHING;
+        return state;
+    }
+    if (status) {
+        state.action = DO_UNSUPPORTED;
+        return state;
+    }

      if (source->type == CAIRO_PATTERN_TYPE_SOLID) {
      cairo_solid_pattern_t *solid = (cairo_solid_pattern_t *) source;

-    CGContextSetRGBStrokeColor (surface->cgContext,
+    CGContextSetRGBStrokeColor (context,
                      solid->color.red,
                      solid->color.green,
                      solid->color.blue,
                      solid->color.alpha);
-    CGContextSetRGBFillColor (surface->cgContext,
+    CGContextSetRGBFillColor (context,
                    solid->color.red,
                    solid->color.green,
                    solid->color.blue,
                    solid->color.alpha);

-    return DO_SOLID;
+        state.action = DO_SOLID;
+        return state;
      }

      if (source->type == CAIRO_PATTERN_TYPE_LINEAR) {
      const cairo_linear_pattern_t *lpat = (const cairo_linear_pattern_t 
*)source;
-    return _cairo_quartz_setup_linear_source (surface, lpat, extents);
+    _cairo_quartz_setup_linear_source (surface, lpat, extents, &state);
+    return state;
      }

      if (source->type == CAIRO_PATTERN_TYPE_RADIAL) {
      const cairo_radial_pattern_t *rpat = (const cairo_radial_pattern_t 
*)source;
-    return _cairo_quartz_setup_radial_source (surface, rpat, extents);
+    _cairo_quartz_setup_radial_source (surface, rpat, extents, &state);
+    return state;
      }

      if (source->type == CAIRO_PATTERN_TYPE_SURFACE &&
@@ -1592,31 +1655,35 @@ _cairo_quartz_setup_source 
(cairo_quartz_surface_t *surface,
      CGImageRef img;
      cairo_matrix_t m = spat->base.matrix;
      cairo_rectangle_int_t extents;
-    cairo_status_t status;
      CGAffineTransform xform;
      CGRect srcRect;
      cairo_fixed_t fw, fh;
      cairo_bool_t is_bounded;

      status = _cairo_surface_to_cgimage ((cairo_surface_t *) surface, 
pat_surf, &img);
-    if (status)
-        return DO_UNSUPPORTED;
-    if (img == NULL)
-        return DO_NOTHING;
+        if (status) {
+            state.action = DO_UNSUPPORTED;
+        return state;
+        }
+        if (img == NULL) {
+            state.action = DO_NOTHING;
+        return state;
+        }

      CGContextSetRGBFillColor (surface->cgContext, 0, 0, 0, 1);

-    surface->sourceImage = img;
+    state.image = img;

      cairo_matrix_invert(&m);
-    _cairo_quartz_cairo_matrix_to_quartz (&m, &surface->sourceTransform);
+    _cairo_quartz_cairo_matrix_to_quartz (&m, &state.transform);

      is_bounded = _cairo_surface_get_extents (pat_surf, &extents);
      assert (is_bounded);

      if (source->extend == CAIRO_EXTEND_NONE) {
-        surface->sourceImageRect = CGRectMake (0, 0, extents.width, 
extents.height);
-        return DO_IMAGE;
+        state.imageRect = CGRectMake (0, 0, extents.width, extents.height);
+        state.action = DO_IMAGE;
+        return state;
      }

      /* Quartz seems to tile images at pixel-aligned regions only -- this
@@ -1626,8 +1693,8 @@ _cairo_quartz_setup_source (cairo_quartz_surface_t 
*surface,
       * epsilon), and if not, fall back to the CGPattern type.
       */

-    xform = CGAffineTransformConcat (CGContextGetCTM (surface->cgContext),
-                     surface->sourceTransform);
+    xform = CGAffineTransformConcat (CGContextGetCTM (context),
+                     state.transform);

      srcRect = CGRectMake (0, 0, extents.width, extents.height);
      srcRect = CGRectApplyAffineTransform (srcRect, xform);
@@ -2515,55 +2582,49 @@ _cairo_quartz_surface_show_glyphs_cg (void 
*abstract_surface,
      if (unlikely (rv))
      return rv;

-    rv = _cairo_quartz_surface_set_cairo_operator (surface, op);
-    if (unlikely (rv))
-    return rv == CAIRO_INT_STATUS_NOTHING_TO_DO ? CAIRO_STATUS_SUCCESS 
: rv;
-
-    CGContextSaveGState (surface->cgContext);
-
      if (_cairo_quartz_source_needs_extents (source))
      {
          cairo_rectangle_int_t glyph_extents;
          _cairo_scaled_font_glyph_device_extents (scaled_font, glyphs, 
num_glyphs,
&glyph_extents, NULL);
-        action = _cairo_quartz_setup_source (surface, source, 
&glyph_extents);
+        state = _cairo_quartz_setup_state (surface, source, op, 
&glyph_extents);
      } else {
-        action = _cairo_quartz_setup_source (surface, source, NULL);
+        state = _cairo_quartz_setup_state (surface, source, op, NULL);
      }

-    if (action == DO_SOLID || action == DO_PATTERN) {
-    CGContextSetTextDrawingMode (surface->cgContext, kCGTextFill);
-    } else if (action == DO_IMAGE || action == DO_TILED_IMAGE || action 
== DO_SHADING) {
-    CGContextSetTextDrawingMode (surface->cgContext, kCGTextClip);
+    if (state.action == DO_SOLID || state.action == DO_PATTERN) {
+    CGContextSetTextDrawingMode (state.context, kCGTextFill);
+    } else if (state.action == DO_IMAGE || state.action == 
DO_TILED_IMAGE || state.action == DO_SHADING) {
+    CGContextSetTextDrawingMode (state.context, kCGTextClip);
      isClipping = TRUE;
      } else {
-    if (action != DO_NOTHING)
+    if (state.action != DO_NOTHING)
          rv = CAIRO_INT_STATUS_UNSUPPORTED;
      goto BAIL;
      }

      /* this doesn't addref */
      cgfref = _cairo_quartz_scaled_font_get_cg_font_ref (scaled_font);
-    CGContextSetFont (surface->cgContext, cgfref);
-    CGContextSetFontSize (surface->cgContext, 1.0);
+    CGContextSetFont (state.context, cgfref);
+    CGContextSetFontSize (state.context, 1.0);

      switch (scaled_font->options.antialias) {
      case CAIRO_ANTIALIAS_SUBPIXEL:
-        CGContextSetShouldAntialias (surface->cgContext, TRUE);
-        CGContextSetShouldSmoothFonts (surface->cgContext, TRUE);
+        CGContextSetShouldAntialias (state.context, TRUE);
+        CGContextSetShouldSmoothFonts (state.context, TRUE);
          if (CGContextSetAllowsFontSmoothingPtr &&
-        !CGContextGetAllowsFontSmoothingPtr (surface->cgContext))
+        !CGContextGetAllowsFontSmoothingPtr (state.context))
          {
          didForceFontSmoothing = TRUE;
-        CGContextSetAllowsFontSmoothingPtr (surface->cgContext, TRUE);
+        CGContextSetAllowsFontSmoothingPtr (state.context, TRUE);
          }
          break;
      case CAIRO_ANTIALIAS_NONE:
-        CGContextSetShouldAntialias (surface->cgContext, FALSE);
+        CGContextSetShouldAntialias (state.context, FALSE);
          break;
      case CAIRO_ANTIALIAS_GRAY:
-        CGContextSetShouldAntialias (surface->cgContext, TRUE);
-        CGContextSetShouldSmoothFonts (surface->cgContext, FALSE);
+        CGContextSetShouldAntialias (state.context, TRUE);
+        CGContextSetShouldSmoothFonts (state.context, FALSE);
          break;
      case CAIRO_ANTIALIAS_DEFAULT:
          /* Don't do anything */
@@ -2597,7 +2658,7 @@ _cairo_quartz_surface_show_glyphs_cg (void 
*abstract_surface,
                                     0., 0.),
                           textTransform);

-    CGContextSetTextMatrix (surface->cgContext, textTransform);
+    CGContextSetTextMatrix (state.context, textTransform);

      /* Convert our glyph positions to glyph advances.  We need n-1 
advances,
       * since the advance at index 0 is applied after glyph 0. */
@@ -2634,30 +2695,28 @@ _cairo_quartz_surface_show_glyphs_cg (void 
*abstract_surface,
  #endif

      /* Translate to the first glyph's position before drawing */
-    ctm = CGContextGetCTM (surface->cgContext);
-    CGContextTranslateCTM (surface->cgContext, glyphs[0].x, glyphs[0].y);
+    ctm = CGContextGetCTM (state.context);
+    CGContextTranslateCTM (state.context, glyphs[0].x, glyphs[0].y);

-    CGContextShowGlyphsWithAdvances (surface->cgContext,
+    CGContextShowGlyphsWithAdvances (state.context,
                       cg_glyphs,
                       cg_advances,
                       num_glyphs);

-    CGContextSetCTM (surface->cgContext, ctm);
+    CGContextSetCTM (state.context, ctm);

-    if (action == DO_IMAGE || action == DO_TILED_IMAGE) {
-    _cairo_quartz_draw_image (surface, op, action);
-    } else if (action == DO_SHADING) {
-    CGContextConcatCTM (surface->cgContext, surface->sourceTransform);
-    CGContextDrawShading (surface->cgContext, surface->sourceShading);
+    if (state.action == DO_IMAGE || state.action == DO_TILED_IMAGE) {
+    _cairo_quartz_draw_image (&state, op);
+    } else if (state.action == DO_SHADING) {
+    CGContextConcatCTM (state.context, state.transform);
+    CGContextDrawShading (state.context, state.shading);
      }

  BAIL:
-    _cairo_quartz_teardown_source (surface, source);
-
      if (didForceFontSmoothing)
-    CGContextSetAllowsFontSmoothingPtr (surface->cgContext, FALSE);
+        CGContextSetAllowsFontSmoothingPtr (state.context, FALSE);

-    CGContextRestoreGState (surface->cgContext);
+    _cairo_quartz_teardown_state (&state);

      if (rv == CAIRO_STATUS_SUCCESS &&
      cgfref &&



More information about the cairo mailing list