From dc67de3d8b6154b74a243cd7b63e45f343520256 Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Sat, 26 Jan 2008 23:12:14 -0800 Subject: [PATCH] Add cairo_image_surface_stride_for_width 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 | 136 +++++++++++++++++++++++++++++++++----- 6 files changed, 181 insertions(+), 34 deletions(-) diff --git a/src/cairo-image-surface.c b/src/cairo-image-surface.c index 9273f178b..d6a3a95e9 100644 --- a/src/cairo-image-surface.c +++ b/src/cairo-image-surface.c @@ -394,17 +394,57 @@ _cairo_image_surface_create_with_content (cairo_content_t content, width, height); } +/** + * 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: + * + * + * 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); + * + * + * 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 288346065..9716c06e3 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, @@ -2430,6 +2431,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 8ad1893e1..6ab384efd 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: @@ -3634,6 +3634,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 ""; diff --git a/src/cairo.h b/src/cairo.h index 177d2b17d..bfd928408 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; @@ -1624,6 +1626,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 f752fe6c3..fb56d4cfd 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 e21b9df6b..bdfb90a55 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) { - cairo_surface_t *surface; - cairo_pattern_t *pattern; + 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_set_source_rgb (cr, 0, 0, 1); cairo_paint (cr); - surface = cairo_image_surface_create_for_data (mask, CAIRO_FORMAT_A8, - 7, 8, 7); + 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; + + 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); + + /* 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; }