From 9b82bfae9eaad955f8ccc3d8baedfa94e68ae73e Mon Sep 17 00:00:00 2001 From: Pekka Paalanen Date: Mon, 13 Jun 2022 13:45:58 +0300 Subject: [PATCH] tests/color-icc-output: extract image-iter.h Move the struct image_header and get_image_prop() into a header where we can share these useful helpers between more test code. While doing that, drop the unused field 'depth', rename into image_header_from(), and introduce a helper to get u32 pointer to the beginning of a row. These helpers should make pixel iterating code easier to read and safer (less room for mistakes in address computations, and asserts). Use the shared 'struct image_header' instead of the local one. The intention is to make the code easier to read by using the same helpers everywhere. Width, height and stride use type 'int' because Pixman API uses that too. Signed-off-by: Pekka Paalanen --- tests/color-icc-output-test.c | 79 ++++++++++------------------------- tests/image-iter.h | 75 +++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 57 deletions(-) create mode 100644 tests/image-iter.h diff --git a/tests/color-icc-output-test.c b/tests/color-icc-output-test.c index 0efdcfe2..429a7ce0 100644 --- a/tests/color-icc-output-test.c +++ b/tests/color-icc-output-test.c @@ -34,6 +34,7 @@ #include "weston-test-client-helper.h" #include "weston-test-fixture-compositor.h" #include "color_util.h" +#include "image-iter.h" #include "lcms_util.h" struct lcms_pipeline { @@ -166,26 +167,6 @@ static const struct setup_args my_setup_args[] = { { { "sRGB->adobeRGB" }, 1, &pipeline_adobeRGB, 1, 17, PTYPE_CLUT, 0.0065 }, }; -struct image_header { - int width; - int height; - int stride; - int depth; - pixman_format_code_t pix_format; - uint32_t *data; -}; - -static void -get_image_prop(struct buffer *buf, struct image_header *header) -{ - header->width = pixman_image_get_width(buf->image); - header->height = pixman_image_get_height(buf->image); - header->stride = pixman_image_get_stride(buf->image); - header->depth = pixman_image_get_depth(buf->image); - header->pix_format = pixman_image_get_format (buf->image); - header->data = pixman_image_get_data(buf->image); -} - static void test_roundtrip(uint8_t r, uint8_t g, uint8_t b, cmsPipeline *pip, struct rgb_diff_stat *stat) @@ -486,7 +467,7 @@ fixture_setup(struct weston_test_harness *harness, const struct setup_args *arg) DECLARE_FIXTURE_SETUP_WITH_ARG(fixture_setup, my_setup_args, meta); static void -gen_ramp_rgb(const struct image_header *header, int bitwidth, int width_bar) +gen_ramp_rgb(pixman_image_t *image, int bitwidth, int width_bar) { static const int hue[][COLOR_CHAN_NUM] = { { 1, 1, 1 }, /* White */ @@ -499,6 +480,7 @@ gen_ramp_rgb(const struct image_header *header, int bitwidth, int width_bar) }; const int num_hues = ARRAY_LENGTH(hue); + struct image_header ih = image_header_from(image); float val_max; int x, y; int hue_index; @@ -511,14 +493,15 @@ gen_ramp_rgb(const struct image_header *header, int bitwidth, int width_bar) val_max = (1 << bitwidth) - 1; - for (y = 0; y < header->height; y++) { - hue_index = (y * num_hues) / (header->height - 1); + for (y = 0; y < ih.height; y++) { + hue_index = (y * num_hues) / (ih.height - 1); hue_index = MIN(hue_index, num_hues - 1); - for (x = 0; x < header->width; x++) { + pixel = image_header_get_row_u32(&ih, y); + for (x = 0; x < ih.width; x++, pixel++) { struct color_float rgb = { .rgb = { 0, 0, 0 } }; - value = (float)x / (float)(header->width - 1); + value = (float)x / (float)(ih.width - 1); if (width_bar > 1) value = floor(value * n_steps) / n_steps; @@ -534,7 +517,6 @@ gen_ramp_rgb(const struct image_header *header, int bitwidth, int width_bar) g = round(rgb.g * val_max); b = round(rgb.b * val_max); - pixel = header->data + (y * header->stride / 4) + x; *pixel = (255U << 24) | (r << 16) | (g << 8) | b; } } @@ -585,8 +567,8 @@ compare_float(float ref, float dst, int x, const char *chan, } static bool -process_pipeline_comparison(const struct image_header *src, - const struct image_header *shot, +process_pipeline_comparison(const struct buffer *src_buf, + const struct buffer *shot_buf, const struct setup_args * arg) { const char *const chan_name[COLOR_CHAN_NUM] = { "r", "g", "b" }; @@ -595,18 +577,23 @@ process_pipeline_comparison(const struct image_header *src, float max_allow_diff = arg->tolerance / max_pixel_value; float max_err = 0.0f; bool ok = true; - uint32_t *row_ptr, *row_ptr_shot; + struct image_header ih_src = image_header_from(src_buf->image); + struct image_header ih_shot = image_header_from(shot_buf->image); int y, x; int chan; struct color_float pix_src; struct color_float pix_src_pipeline; struct color_float pix_shot; - for (y = 0; y < src->height; y++) { - row_ptr = (uint32_t*)((uint8_t*)src->data + (src->stride * y)); - row_ptr_shot = (uint32_t*)((uint8_t*)shot->data + (shot->stride * y)); + /* no point to compare different images */ + assert(ih_src.width == ih_shot.width); + assert(ih_src.height == ih_shot.height); + + for (y = 0; y < ih_src.height; y++) { + uint32_t *row_ptr = image_header_get_row_u32(&ih_src, y); + uint32_t *row_ptr_shot = image_header_get_row_u32(&ih_shot, y); - for (x = 0; x < src->width; x++) { + for (x = 0; x < ih_src.width; x++) { pix_src = a8r8g8b8_to_float(row_ptr[x]); pix_shot = a8r8g8b8_to_float(row_ptr_shot[x]); /* do pipeline processing */ @@ -639,26 +626,6 @@ process_pipeline_comparison(const struct image_header *src, return ok; } -static bool -check_process_pattern_ex(struct buffer *src, struct buffer *shot, - const struct setup_args * arg) -{ - struct image_header header_src; - struct image_header header_shot; - bool ok; - - get_image_prop(src, &header_src); - get_image_prop(shot, &header_shot); - - /* no point to compare different images */ - assert(header_src.width == header_shot.width); - assert(header_src.height == header_shot.height); - - ok = process_pipeline_comparison(&header_src, &header_shot, arg); - - return ok; -} - /* * Test that opaque client pixels produce the expected output when converted * from the implicit sRGB input to ICC profile described output. @@ -680,7 +647,6 @@ TEST(opaque_pixel_conversion) struct buffer *buf; struct buffer *shot; struct wl_surface *surface; - struct image_header image; bool match; client = create_client_and_test_surface(0, 0, width, height); @@ -688,8 +654,7 @@ TEST(opaque_pixel_conversion) surface = client->surface->wl_surface; buf = create_shm_buffer_a8r8g8b8(client, width, height); - get_image_prop(buf, &image); - gen_ramp_rgb(&image, bitwidth, width_bar); + gen_ramp_rgb(buf->image, bitwidth, width_bar); wl_surface_attach(surface, buf->proxy, 0, 0); wl_surface_damage(surface, 0, 0, width, height); @@ -700,7 +665,7 @@ TEST(opaque_pixel_conversion) match = verify_image(shot, "shaper_matrix", arg->ref_image_index, NULL, seq_no); - assert(check_process_pattern_ex(buf, shot, arg)); + assert(process_pipeline_comparison(buf, shot, arg)); assert(match); buffer_destroy(shot); buffer_destroy(buf); diff --git a/tests/image-iter.h b/tests/image-iter.h new file mode 100644 index 00000000..ab9b0830 --- /dev/null +++ b/tests/image-iter.h @@ -0,0 +1,75 @@ +/* + * Copyright 2021 Advanced Micro Devices, Inc. + * Copyright 2022 Collabora, Ltd. + * + * 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 (including the + * next paragraph) 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. + */ + +#pragma once + +#include +#include +#include + +/** A collection of basic information extracted from a pixman_image_t */ +struct image_header { + int width; + int height; + pixman_format_code_t pixman_format; + + int stride_bytes; + unsigned char *data; +}; + +/** Populate image_header from pixman_image_t */ +static inline struct image_header +image_header_from(pixman_image_t *image) +{ + struct image_header h; + + h.width = pixman_image_get_width(image); + h.height = pixman_image_get_height(image); + h.pixman_format = pixman_image_get_format(image); + h.stride_bytes = pixman_image_get_stride(image); + h.data = (void *)pixman_image_get_data(image); + + return h; +} + +/** Get pointer to the beginning of the row + * + * \param header Header describing the Pixman image. + * \param y Index of the desired row, starting from zero. + * \return Pointer to the first pixel of the row. + * + * Asserts that y is within image height, and that pixel format uses 32 bits + * per pixel. + */ +static inline uint32_t * +image_header_get_row_u32(const struct image_header *header, int y) +{ + assert(y >= 0); + assert(y < header->height); + assert(PIXMAN_FORMAT_BPP(header->pixman_format) == 32); + + return (uint32_t *)(header->data + y * header->stride_bytes); +}