[PATCH] Add cairo_image_surface_stride_for_width

Carl Worth cworth at cworth.org
Sat Jan 26 23:12:14 PST 2008


Document this function as a required call to get the correct
stride value before calling cairo_image_surface_create_for_data.
This means that previously-failing calls with non-multiple-of-4
stride values are now documented as errors. Also, we now have
the possibility of moving to more stringent alignment constraints,
(one can imagine doing 64-bit or 128-bit boundaries for example).
---
 src/cairo-image-surface.c |   60 +++++++++++++++++----
 src/cairo-surface.c       |    3 +
 src/cairo.c               |    4 +-
 src/cairo.h               |    8 +++-
 test/Makefile.am          |    4 --
 test/a8-mask.c            |  132 +++++++++++++++++++++++++++++++++++++++------
 6 files changed, 179 insertions(+), 32 deletions(-)

diff --git a/src/cairo-image-surface.c b/src/cairo-image-surface.c
index 9273f17..d6a3a95 100644
--- a/src/cairo-image-surface.c
+++ b/src/cairo-image-surface.c
@@ -395,16 +395,56 @@ _cairo_image_surface_create_with_content (cairo_content_t	content,
 }

 /**
+ * cairo_image_surface_stride_for_width:
+ * @format: The desired format of an image surface to be created.
+ * @width: The desired width of an image surface to be created.
+ *
+ * Return value: the appropriate stride to use given the desired
+ * format and width.
+ *
+ * This function provides a stride value that will respect all
+ * alignment requirements of the accelerated image-rendering code
+ * within cairo. Typical usage will be of the form:
+ *
+ * <informalexample><programlisting>
+ * int stride;
+ * unsigned char *data;
+ * cairo_surface_t *surface;
+ *
+ * stride = cairo_image_surface_stride_for_width (format, width);
+ * data = malloc (stride * height);
+ * surface = cairo_image_surface_create_for_data (date, format,
+ *						  width, height);
+ * </programlisting></informalexample>
+ *
+ * Since: 1.6
+ */
+int
+cairo_image_surface_stride_for_width (cairo_format_t	format,
+				      int		width)
+{
+    int bpp = _cairo_format_bits_per_pixel (format);
+
+    /* Convert from bits-per-row to bytes-per-row with rounding to the
+     * next 4-byte boundary. This satisifies the current alignment
+     * requirements of pixman. */
+    return (((bpp * width) + 31) >> 5) << 2;
+}
+
+/**
  * cairo_image_surface_create_for_data:
- * @data: a pointer to a buffer supplied by the application
- *    in which to write contents.
+ * @data: a pointer to a buffer supplied by the application in which
+ *     to write contents. This pointer must be suitably aligned for any
+ *     kind of variable, (for example, a pointer returned by malloc).
  * @format: the format of pixels in the buffer
  * @width: the width of the image to be stored in the buffer
  * @height: the height of the image to be stored in the buffer
- * @stride: the number of bytes between the start of rows
- *   in the buffer. Having this be specified separate from @width
- *   allows for padding at the end of rows, or for writing
- *   to a subportion of a larger image.
+ * @stride: the number of bytes between the start of rows in the
+ *    buffer. For performance reasons, not all values are legal stride
+ *    values. Use cairo_image_surface_stride_for_width() to compute a
+ *    legal stride value, (and use that value to allocate data of the
+ *    correct size). An illegal stride value will cause a nil surface
+ *    to be resutrned with a status of CAIRO_STATUS_INVALD_STRIDE.
  *
  * Creates an image surface for the provided pixel data. The output
  * buffer must be kept around until the #cairo_surface_t is destroyed
@@ -433,12 +473,12 @@ cairo_image_surface_create_for_data (unsigned char     *data,
 {
     pixman_format_code_t pixman_format;

-    /* XXX pixman does not support images with arbitrary strides and
-     * attempting to create such surfaces will failure but we will interpret
-     * such failure as CAIRO_STATUS_NO_MEMORY.  */
-    if (! CAIRO_FORMAT_VALID (format) || stride % sizeof (uint32_t) != 0)
+    if (! CAIRO_FORMAT_VALID (format))
 	return _cairo_surface_create_in_error (_cairo_error(CAIRO_STATUS_INVALID_FORMAT));

+    if (stride % sizeof (uint32_t) != 0)
+	return _cairo_surface_create_in_error (_cairo_error (CAIRO_STATUS_INVALID_STRIDE));
+
     pixman_format = _cairo_format_to_pixman_format_code (format);

     return _cairo_image_surface_create_with_pixman_format (data, pixman_format,
diff --git a/src/cairo-surface.c b/src/cairo-surface.c
index 010195f..eefd60f 100644
--- a/src/cairo-surface.c
+++ b/src/cairo-surface.c
@@ -86,6 +86,7 @@ static DEFINE_NIL_SURFACE(CAIRO_STATUS_FILE_NOT_FOUND, _cairo_surface_nil_file_n
 static DEFINE_NIL_SURFACE(CAIRO_STATUS_TEMP_FILE_ERROR, _cairo_surface_nil_temp_file_error);
 static DEFINE_NIL_SURFACE(CAIRO_STATUS_READ_ERROR, _cairo_surface_nil_read_error);
 static DEFINE_NIL_SURFACE(CAIRO_STATUS_WRITE_ERROR, _cairo_surface_nil_write_error);
+static DEFINE_NIL_SURFACE(CAIRO_STATUS_INVALID_STRIDE, _cairo_surface_nil_invalid_stride);

 static cairo_status_t
 _cairo_surface_copy_pattern_for_destination (const cairo_pattern_t *pattern,
@@ -2427,6 +2428,8 @@ _cairo_surface_create_in_error (cairo_status_t status)
 	return (cairo_surface_t *) &_cairo_surface_nil_file_not_found;
     case CAIRO_STATUS_TEMP_FILE_ERROR:
 	return (cairo_surface_t *) &_cairo_surface_nil_temp_file_error;
+    case CAIRO_STATUS_INVALID_STRIDE:
+	return (cairo_surface_t *) &_cairo_surface_nil_invalid_stride;
     default:
 	_cairo_error_throw (CAIRO_STATUS_NO_MEMORY);
 	return (cairo_surface_t *) &_cairo_surface_nil;
diff --git a/src/cairo.c b/src/cairo.c
index 9535174..5115160 100644
--- a/src/cairo.c
+++ b/src/cairo.c
@@ -67,7 +67,7 @@ static const cairo_t _cairo_nil = {
  * a bit of a pain, but it should be easy to always catch as long as
  * one adds a new test case to test a trigger of the new status value.
  */
-#define CAIRO_STATUS_LAST_STATUS CAIRO_STATUS_TEMP_FILE_ERROR
+#define CAIRO_STATUS_LAST_STATUS CAIRO_STATUS_INVALID_STRIDE

 /**
  * _cairo_error:
@@ -3620,6 +3620,8 @@ cairo_status_to_string (cairo_status_t status)
         return "clip region not representable in desired format";
     case CAIRO_STATUS_TEMP_FILE_ERROR:
 	return "error creating or writing to a temporary file";
+    case CAIRO_STATUS_INVALID_STRIDE:
+	return "invalid value for stride";
     }

     return "<unknown error status>";
diff --git a/src/cairo.h b/src/cairo.h
index 157b6e1..fe40273 100644
--- a/src/cairo.h
+++ b/src/cairo.h
@@ -202,6 +202,7 @@ typedef struct _cairo_user_data_key {
  * @CAIRO_STATUS_INVALID_INDEX: invalid index passed to getter (Since 1.4)
  * @CAIRO_STATUS_CLIP_NOT_REPRESENTABLE: clip region not representable in desired format (Since 1.4)
  * @CAIRO_STATUS_TEMP_FILE_ERROR: error creating or writing to a temporary file (Since 1.6)
+ * @CAIRO_STATUS_INVALID_STRIDE: invalide value for stride (Since 1.6)
  *
  * #cairo_status_t is used to indicate errors that can occur when
  * using Cairo. In some cases it is returned directly by functions.
@@ -235,7 +236,8 @@ typedef enum _cairo_status {
     CAIRO_STATUS_INVALID_DSC_COMMENT,
     CAIRO_STATUS_INVALID_INDEX,
     CAIRO_STATUS_CLIP_NOT_REPRESENTABLE,
-    CAIRO_STATUS_TEMP_FILE_ERROR
+    CAIRO_STATUS_TEMP_FILE_ERROR,
+    CAIRO_STATUS_INVALID_STRIDE
     /* after adding a new error: update CAIRO_STATUS_LAST_STATUS in cairo.c */
 } cairo_status_t;

@@ -1596,6 +1598,10 @@ cairo_image_surface_create (cairo_format_t	format,
 			    int			width,
 			    int			height);

+cairo_public int
+cairo_image_surface_stride_for_width (cairo_format_t	format,
+				      int		width);
+
 cairo_public cairo_surface_t *
 cairo_image_surface_create_for_data (unsigned char	       *data,
 				     cairo_format_t		format,
diff --git a/test/Makefile.am b/test/Makefile.am
index f752fe6..fb56d4c 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -520,11 +520,7 @@ $(REFERENCE_IMAGES)
 # Of course, before any "release" of cairo we should eliminate
 # everything from this list by fixing the bugs. (We don't necessarily
 # have to be that strict for "snapshots" though.)
-#
-# Also, any test listed here should call cairo_test_expect_failure and
-# provide an explanation for the expected failure.
 XFAIL_TESTS =					\
-a8-mask$(EXEEXT)				\
 big-trap$(EXEEXT)				\
 extend-pad$(EXEEXT)				\
 extend-pad-similar$(EXEEXT)			\
diff --git a/test/a8-mask.c b/test/a8-mask.c
index e21b9df..bdfb90a 100644
--- a/test/a8-mask.c
+++ b/test/a8-mask.c
@@ -29,42 +29,142 @@ static cairo_test_draw_function_t draw;

 cairo_test_t test = {
     "a8-mask",
-    "test masks of CAIRO_FORMAT_A8"
-    "\nimage backend fails because libpixman only handles (stride % sizeof(pixman_bits) == 0)",
+    "test masks of CAIRO_FORMAT_A8",
     8, 8,
     draw
 };

-static unsigned char mask[] = {
-    0x0, 0x0, 0xff, 0x0, 0xff, 0x0, 0x0,
-    0x0, 0x0, 0xff, 0x0, 0xff, 0x0, 0x0,
-    0x0, 0x0, 0xff, 0x0, 0xff, 0x0, 0x0,
-    0x0, 0x0, 0xff, 0x0, 0xff, 0x0, 0x0,
-    0x0, 0x0, 0xff, 0x0, 0xff, 0x0, 0x0,
-    0x0, 0x0, 0xff, 0x0, 0xff, 0x0, 0x0,
-    0x0, 0x0, 0xff, 0x0, 0xff, 0x0, 0x0,
-    0x0, 0x0, 0xff, 0x0, 0xff, 0x0, 0x0
+#define MASK_WIDTH 8
+#define MASK_HEIGHT 8
+
+static unsigned char mask[MASK_WIDTH * MASK_HEIGHT] = {
+    0x0, 0x0, 0xff, 0x0, 0xff, 0x0, 0x0, 0x0,
+    0x0, 0x0, 0xff, 0x0, 0xff, 0x0, 0x0, 0x0,
+    0x0, 0x0, 0xff, 0x0, 0xff, 0x0, 0x0, 0x0,
+    0x0, 0x0, 0xff, 0x0, 0xff, 0x0, 0x0, 0x0,
+    0x0, 0x0, 0xff, 0x0, 0xff, 0x0, 0x0, 0x0,
+    0x0, 0x0, 0xff, 0x0, 0xff, 0x0, 0x0, 0x0,
+    0x0, 0x0, 0xff, 0x0, 0xff, 0x0, 0x0, 0x0,
+    0x0, 0x0, 0xff, 0x0, 0xff, 0x0, 0x0, 0x0,
 };

 static cairo_test_status_t
-draw (cairo_t *cr, int width, int height)
+check_status (cairo_status_t status, cairo_status_t expected)
+{
+    if (status == expected)
+	return CAIRO_TEST_SUCCESS;
+
+    cairo_test_log ("Error: Expected status value %d (%s), received %d (%s)\n",
+		    expected,
+		    cairo_status_to_string (expected),
+		    status,
+		    cairo_status_to_string (status));
+    return CAIRO_TEST_FAILURE;
+}
+
+static cairo_test_status_t
+test_surface_with_width_and_stride (int width, int stride,
+				    cairo_status_t expected)
+{
+    cairo_test_status_t status;
+    cairo_surface_t *surface;
+    cairo_t *cr;
+    unsigned char *data;
+
+    cairo_test_log ("Creating surface with width %d and stride %d\n",
+		    width, stride);
+
+    data = malloc (stride);
+
+    surface = cairo_image_surface_create_for_data (data, CAIRO_FORMAT_A8,
+						   width, 1, stride);
+    cr = cairo_create (surface);
+
+    cairo_paint (cr);
+
+    status = check_status (cairo_surface_status (surface), expected);
+    if (status)
+	return status;
+
+    status = check_status (cairo_status (cr), expected);
+    if (status)
+	return status;
+
+    free (data);
+
+    return CAIRO_TEST_SUCCESS;
+}
+
+static cairo_test_status_t
+draw (cairo_t *cr, int dst_width, int dst_height)
 {
+    int test_width, stride, row;
+    unsigned char *src, *dst, *mask_aligned;
     cairo_surface_t *surface;
     cairo_pattern_t *pattern;
+    cairo_test_status_t status;
+    cairo_status_t expected;
+
+    for (test_width = 0; test_width < 40; test_width++) {
+	stride = cairo_image_surface_stride_for_width (CAIRO_FORMAT_A8,
+						       test_width);
+
+	/* First create a surface using the width as the stride, (most
+	 * of these should fail). */
+	expected = (stride == test_width) ?
+	    CAIRO_STATUS_SUCCESS : CAIRO_STATUS_INVALID_STRIDE;

-    cairo_set_source_rgb (cr, 0, 0, 1);
+	status = test_surface_with_width_and_stride (test_width,
+						     test_width,
+						     expected);
+	if (status)
+	    return status;
+
+	/* Then create a surface using the correct stride, (should
+	   always succeed).*/
+	status = test_surface_with_width_and_stride (test_width,
+						     stride,
+						     CAIRO_STATUS_SUCCESS);
+	if (status)
+	    return status;
+    }
+
+    /* Now test actually drawing through our mask data, allocating and
+     * copying with the proper stride. */
+    stride = cairo_image_surface_stride_for_width (CAIRO_FORMAT_A8,
+						   MASK_WIDTH);
+
+    mask_aligned = malloc (stride * MASK_HEIGHT);
+
+    src = mask;
+    dst = mask_aligned;
+    for (row = 0; row < MASK_HEIGHT; row++) {
+	memcpy (dst, src, MASK_WIDTH);
+	src += MASK_WIDTH;
+	dst += stride;
+    }
+
+    surface = cairo_image_surface_create_for_data (mask,
+						   CAIRO_FORMAT_A8,
+						   MASK_WIDTH,
+						   MASK_HEIGHT,
+						   stride);
+
+    /* Paint background blue */
+    cairo_set_source_rgb (cr, 0, 0, 1); /* blue */
     cairo_paint (cr);

-    surface = cairo_image_surface_create_for_data (mask, CAIRO_FORMAT_A8,
-						   7, 8, 7);
+    /* Then paint red through our mask */
     pattern = cairo_pattern_create_for_surface (surface);

-    cairo_set_source_rgb (cr, 1, 0, 0);
+    cairo_set_source_rgb (cr, 1, 0, 0); /* red */
     cairo_mask (cr, pattern);

     cairo_pattern_destroy (pattern);
     cairo_surface_destroy (surface);

+    free (mask_aligned);
+
     return CAIRO_TEST_SUCCESS;
 }

--
1.5.3.2


--Multipart_Sun_Jan_27_21:35:32_2008-1--

--pgp-sign-Multipart_Sun_Jan_27_21:35:32_2008-1
Content-Type: application/pgp-signature
Content-Transfer-Encoding: 7bit

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQBHnWmu6JDdNq8qSWgRAjLcAJ9kQSbnU80I63M+bOFs+7u2a1CwuACglKZ7
FDliQiG27j/pVF0nuGVyLjc=
=75uD
-----END PGP SIGNATURE-----

--pgp-sign-Multipart_Sun_Jan_27_21:35:32_2008-1--


More information about the cairo mailing list