[cairo-commit] 3 commits - src/cairo.c src/cairo-gstate.c src/cairo-gstate-private.h test/group-state.c test/Makefile.sources

Andrea Canciani ranma42 at kemper.freedesktop.org
Sat Jan 22 03:00:37 PST 2011


 src/cairo-gstate-private.h |    5 --
 src/cairo-gstate.c         |   31 +++-----------
 src/cairo.c                |   26 +++++-------
 test/Makefile.sources      |    1 
 test/group-state.c         |   96 +++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 116 insertions(+), 43 deletions(-)

New commits:
commit ff9e962165905f9b3477e125de227c69aebf9510
Author: Andrea Canciani <ranma42 at gmail.com>
Date:   Wed Jan 19 23:22:31 2011 +0100

    gstate: Remove unused code
    
    _cairo_gstate_redirect_target asserts that this surface is NULL
    immediately before destroying it. If the code is compiled with
    assertions disabled and the assert would be false, it is now safer
    because instead of an invalid access it will only memleak.
    
    _cairo_gstate_get_parent_target () is not used anymore in
    cairo_pop_group () and the related code can be removed.
    
    _cairo_gstate_is_redirected () has never been used.
    
    The comment about the clipping is misleading, because the clip is
    translated as expected since fb7f7c2f27f0823d7702f960204d6e638d697624.

diff --git a/src/cairo-gstate-private.h b/src/cairo-gstate-private.h
index 34355be..cd417ec 100644
--- a/src/cairo-gstate-private.h
+++ b/src/cairo-gstate-private.h
@@ -90,9 +90,6 @@ _cairo_gstate_restore (cairo_gstate_t **gstate, cairo_gstate_t **freelist);
 cairo_private cairo_bool_t
 _cairo_gstate_is_group (cairo_gstate_t *gstate);
 
-cairo_private cairo_bool_t
-_cairo_gstate_is_redirected (cairo_gstate_t *gstate);
-
 cairo_private cairo_status_t
 _cairo_gstate_redirect_target (cairo_gstate_t *gstate, cairo_surface_t *child);
 
@@ -100,9 +97,6 @@ cairo_private cairo_surface_t *
 _cairo_gstate_get_target (cairo_gstate_t *gstate);
 
 cairo_private cairo_surface_t *
-_cairo_gstate_get_parent_target (cairo_gstate_t *gstate);
-
-cairo_private cairo_surface_t *
 _cairo_gstate_get_original_target (cairo_gstate_t *gstate);
 
 cairo_private cairo_clip_t *
diff --git a/src/cairo-gstate.c b/src/cairo-gstate.c
index 52d568c..7298a7f 100644
--- a/src/cairo-gstate.c
+++ b/src/cairo-gstate.c
@@ -303,10 +303,6 @@ _cairo_gstate_restore (cairo_gstate_t **gstate, cairo_gstate_t **freelist)
  * Redirect @gstate rendering to a "child" target. The original
  * "parent" target with which the gstate was created will not be
  * affected. See _cairo_gstate_get_target().
- *
- * Unless the redirected target has the same device offsets as the
- * original #cairo_t target, the clip will be INVALID after this call,
- * and the caller should either recreate or reset the clip.
  **/
 cairo_status_t
 _cairo_gstate_redirect_target (cairo_gstate_t *gstate, cairo_surface_t *child)
@@ -320,7 +316,6 @@ _cairo_gstate_redirect_target (cairo_gstate_t *gstate, cairo_surface_t *child)
     /* Set up our new parent_target based on our current target;
      * gstate->parent_target will take the ref that is held by gstate->target
      */
-    cairo_surface_destroy (gstate->parent_target);
     gstate->parent_target = gstate->target;
 
     /* Now set up our new target; we overwrite gstate->target directly,
@@ -358,21 +353,6 @@ _cairo_gstate_is_group (cairo_gstate_t *gstate)
 }
 
 /**
- * _cairo_gstate_is_redirected
- * @gstate: a #cairo_gstate_t
- *
- * This space left intentionally blank.
- *
- * Return value: %TRUE if the gstate is redirected to a target
- * different than the original, %FALSE otherwise.
- **/
-cairo_bool_t
-_cairo_gstate_is_redirected (cairo_gstate_t *gstate)
-{
-    return (gstate->target != gstate->original_target);
-}
-
-/**
  * _cairo_gstate_get_target:
  * @gstate: a #cairo_gstate_t
  *
@@ -388,19 +368,6 @@ _cairo_gstate_get_target (cairo_gstate_t *gstate)
 }
 
 /**
- * _cairo_gstate_get_parent_target:
- * @gstate: a #cairo_gstate_t
- *
- * Return the parent surface of the current drawing target surface;
- * if this particular gstate isn't a redirect gstate, this will return %NULL.
- **/
-cairo_surface_t *
-_cairo_gstate_get_parent_target (cairo_gstate_t *gstate)
-{
-    return gstate->parent_target;
-}
-
-/**
  * _cairo_gstate_get_original_target:
  * @gstate: a #cairo_gstate_t
  *
commit 5d95ae924ed15200a17d240d8f0744c74df7c61b
Author: Andrea Canciani <ranma42 at gmail.com>
Date:   Thu Jan 20 01:44:29 2011 +0100

    gstate: Set an error status when restoring a push_group
    
    cairo_push_group (cr) followed by cairo_restore (cr) should put cr in
    an error status of CAIRO_STATUS_INVALID_RESTORE.
    
    Fixes group-state.

diff --git a/src/cairo-gstate-private.h b/src/cairo-gstate-private.h
index b41c7a2..34355be 100644
--- a/src/cairo-gstate-private.h
+++ b/src/cairo-gstate-private.h
@@ -88,6 +88,9 @@ cairo_private cairo_status_t
 _cairo_gstate_restore (cairo_gstate_t **gstate, cairo_gstate_t **freelist);
 
 cairo_private cairo_bool_t
+_cairo_gstate_is_group (cairo_gstate_t *gstate);
+
+cairo_private cairo_bool_t
 _cairo_gstate_is_redirected (cairo_gstate_t *gstate);
 
 cairo_private cairo_status_t
diff --git a/src/cairo-gstate.c b/src/cairo-gstate.c
index efb8267..52d568c 100644
--- a/src/cairo-gstate.c
+++ b/src/cairo-gstate.c
@@ -342,6 +342,22 @@ _cairo_gstate_redirect_target (cairo_gstate_t *gstate, cairo_surface_t *child)
 }
 
 /**
+ * _cairo_gstate_is_group
+ * @gstate: a #cairo_gstate_t
+ *
+ * Check if _cairo_gstate_redirect_target has been called on the head
+ * of the stack.
+ *
+ * Return value: %TRUE if @gstate is redirected to a target different
+ * than the previous state in the stack, %FALSE otherwise.
+ **/
+cairo_bool_t
+_cairo_gstate_is_group (cairo_gstate_t *gstate)
+{
+    return gstate->parent_target != NULL;
+}
+
+/**
  * _cairo_gstate_is_redirected
  * @gstate: a #cairo_gstate_t
  *
diff --git a/src/cairo.c b/src/cairo.c
index b87274a..73dbaee 100644
--- a/src/cairo.c
+++ b/src/cairo.c
@@ -585,6 +585,11 @@ cairo_restore (cairo_t *cr)
     if (unlikely (cr->status))
 	return;
 
+    if (unlikely (_cairo_gstate_is_group (cr->gstate))) {
+	_cairo_set_error (cr, _cairo_error (CAIRO_STATUS_INVALID_RESTORE));
+	return;
+    }
+
     status = _cairo_gstate_restore (&cr->gstate, &cr->gstate_freelist);
     if (unlikely (status))
 	_cairo_set_error (cr, status);
@@ -754,7 +759,7 @@ slim_hidden_def(cairo_push_group_with_content);
 cairo_pattern_t *
 cairo_pop_group (cairo_t *cr)
 {
-    cairo_surface_t *group_surface, *parent_target;
+    cairo_surface_t *group_surface;
     cairo_pattern_t *group_pattern;
     cairo_matrix_t group_matrix, device_transform_matrix;
     cairo_status_t status;
@@ -762,27 +767,18 @@ cairo_pop_group (cairo_t *cr)
     if (unlikely (cr->status))
 	return _cairo_pattern_create_in_error (cr->status);
 
-    /* Grab the active surfaces */
-    group_surface = _cairo_gstate_get_target (cr->gstate);
-    parent_target = _cairo_gstate_get_parent_target (cr->gstate);
-
     /* Verify that we are at the right nesting level */
-    if (parent_target == NULL) {
+    if (unlikely (! _cairo_gstate_is_group (cr->gstate))) {
 	_cairo_set_error (cr, CAIRO_STATUS_INVALID_POP_GROUP);
 	return _cairo_pattern_create_in_error (CAIRO_STATUS_INVALID_POP_GROUP);
     }
 
-    /* We need to save group_surface before we restore; we don't need
-     * to reference parent_target and original_target, since the
-     * gstate will still hold refs to them once we restore. */
+    /* Get a reference to the active surface before restoring */
+    group_surface = _cairo_gstate_get_target (cr->gstate);
     group_surface = cairo_surface_reference (group_surface);
 
-    cairo_restore (cr);
-
-    if (unlikely (cr->status)) {
-	group_pattern = _cairo_pattern_create_in_error (cr->status);
-	goto done;
-    }
+    status = _cairo_gstate_restore (&cr->gstate, &cr->gstate_freelist);
+    assert (status == CAIRO_STATUS_SUCCESS);
 
     group_pattern = cairo_pattern_create_for_surface (group_surface);
     status = group_pattern->status;
commit e0b741de9006a02acd9b05c8fae52f6b7f775163
Author: Andrea Canciani <ranma42 at gmail.com>
Date:   Tue Jan 18 14:48:15 2011 +0100

    test: Add group-state
    
    The interaction between the group and the state API is currently
    untested and buggy. This test tries to use them incorrectly and check
    that cairo notices the problem and marks the cr object with an error
    status.

diff --git a/test/Makefile.sources b/test/Makefile.sources
index 279b228..3210745 100644
--- a/test/Makefile.sources
+++ b/test/Makefile.sources
@@ -124,6 +124,7 @@ test_sources = \
 	gradient-zero-stops-mask.c			\
 	group-clip.c					\
 	group-paint.c					\
+	group-state.c					\
 	group-unaligned.c				\
 	half-coverage.c					\
 	halo.c						\
diff --git a/test/group-state.c b/test/group-state.c
new file mode 100644
index 0000000..a96b2e3
--- /dev/null
+++ b/test/group-state.c
@@ -0,0 +1,96 @@
+/* -*- Mode: c; c-basic-offset: 4; indent-tabs-mode: t; tab-width: 8; -*- */
+/*
+ * Copyright 2011 Andrea Canciani
+ *
+ * Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy,
+ * modify, merge, publish, distribute, sublicense, and/or sell copies
+ * of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Author: Andrea Canciani <ranma42 at gmail.com>
+ */
+
+#include "cairo-test.h"
+
+#define CHECK_STATUS(status)						\
+    do {								\
+	if (cairo_status (cr) != (status)) {				\
+	    cairo_test_log (ctx, "Expected status: %s\n",		\
+			    cairo_status_to_string (status));		\
+	    cairo_test_log (ctx, "Actual status: %s\n",			\
+			    cairo_status_to_string (cairo_status (cr))); \
+	    result = CAIRO_TEST_FAILURE;				\
+	}								\
+    } while (0)
+
+static void
+reinit_cairo (cairo_t **cr)
+{
+    if (*cr)
+	cairo_destroy (*cr);
+
+    *cr = cairo_create (cairo_image_surface_create (CAIRO_FORMAT_ARGB32, 1, 1));
+    cairo_surface_destroy (cairo_get_target (*cr));
+}
+
+static cairo_test_status_t
+preamble (cairo_test_context_t *ctx)
+{
+    cairo_t *cr;
+    cairo_test_status_t result = CAIRO_TEST_SUCCESS;
+
+    cr = NULL;
+
+    reinit_cairo (&cr);
+
+    /* cairo_restore() must fail with CAIRO_STATUS_INVALID_RESTORE if
+     * no matching cairo_save() call has been performed. */
+    cairo_test_log (ctx, "Checking save(); push(); restore();\n");
+    cairo_save (cr);
+    CHECK_STATUS (CAIRO_STATUS_SUCCESS);
+    cairo_push_group (cr);
+    CHECK_STATUS (CAIRO_STATUS_SUCCESS);
+    cairo_restore (cr);
+    CHECK_STATUS (CAIRO_STATUS_INVALID_RESTORE);
+
+
+    reinit_cairo (&cr);
+
+    /* cairo_restore() must fail with CAIRO_STATUS_INVALID_RESTORE if
+     * no matching cairo_save() call has been performed. */
+    cairo_test_log (ctx, "Checking push(); save(); pop();\n");
+    cairo_push_group (cr);
+    CHECK_STATUS (CAIRO_STATUS_SUCCESS);
+    cairo_save (cr);
+    CHECK_STATUS (CAIRO_STATUS_SUCCESS);
+    cairo_pop_group_to_source (cr);
+    CHECK_STATUS (CAIRO_STATUS_INVALID_POP_GROUP);
+
+
+    cairo_destroy (cr);
+
+    return result;
+}
+
+CAIRO_TEST (group_state,
+	    "Tests the interaction between state (cairo_save, cairo_restore) "
+	    "and group (cairo_push_group/cairo_pop_group) API",
+	    "api", /* keywords */
+	    NULL, /* requirements */
+	    0, 0,
+	    preamble, NULL)


More information about the cairo-commit mailing list