From 16a2fc58ba2007e08b4d77e02cf0d45d0ba0b108 Mon Sep 17 00:00:00 2001 From: Jehan Date: Mon, 23 Sep 2024 01:48:37 +0200 Subject: [PATCH] libgimp: fixing "palette" libgimp API unit testing. It was an equality because I think the conversion from palette format to "RGBA float" should go through the exact same babl code path, however it is done in the API. And that is the case on my machine where I get indeed perfect equality. But apparently not on the CI. Though I guess investigating further would not be a bad idea, let's consider this a minor issue (as a general rule, comparing float with epsilon is still a recommended software usage), and just add a comment. --- libgimp/tests/test-palette.c | 17 +++++++++++++---- libgimp/tests/test-palette.py | 19 ++++++++++--------- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/libgimp/tests/test-palette.c b/libgimp/tests/test-palette.c index ac292b18e5..72523b08e8 100644 --- a/libgimp/tests/test-palette.c +++ b/libgimp/tests/test-palette.c @@ -5,6 +5,7 @@ #define GIMP_TEST_COLOR_R_U8 72 #define GIMP_TEST_COLOR_G_U8 56 #define GIMP_TEST_COLOR_B_U8 56 +#define EPSILON 1e-6f static GimpValueArray * gimp_c_test_run (GimpProcedure *procedure, @@ -74,16 +75,24 @@ gimp_c_test_run (GimpProcedure *procedure, format2 = babl_format ("RGBA float"); format2_bpp = babl_format_get_bytes_per_pixel (format2); fcolormap = (gfloat *) gimp_palette_get_colormap (palette, format2, &n_colormap_colors, &n_bytes); - GIMP_TEST_END(fcolormap != NULL && n_colormap_colors == n_colors && + GIMP_TEST_END(fcolormap != NULL && n_colormap_colors == n_colors && format2 != format && format2_bpp > format_bpp && n_colormap_colors * format2_bpp == n_bytes) GIMP_TEST_START("Comparing fourth palette color's RGB components from colormap in float format") gegl_color_get_pixel (color, format2, rgba); - GIMP_TEST_END(fcolormap[4 * GIMP_TEST_COLOR_IDX] == rgba[0] && - fcolormap[4 * GIMP_TEST_COLOR_IDX + 1] == rgba[1] && - fcolormap[4 * GIMP_TEST_COLOR_IDX + 2]== rgba[2]) + /* XXX: whether to a colormap or to a GeglColor, I was expecting the + * conversion from palette format to target format to be done through + * exact same babl code path, hence equality to be perfect (i.e. no + * epsilon needed for float comparison). This is true on my machine, + * not on CI. Is it again some SSE2 inconsistency where sometimes + * conversion goes to SSE2, sometimes not, for the same run and same + * babl fish? Something else? To be further investigated. + */ + GIMP_TEST_END(fabsf (fcolormap[4 * GIMP_TEST_COLOR_IDX] - rgba[0]) < EPSILON && + fabsf (fcolormap[4 * GIMP_TEST_COLOR_IDX + 1] - rgba[1]) < EPSILON && + fabsf (fcolormap[4 * GIMP_TEST_COLOR_IDX + 2] - rgba[2]) < EPSILON) g_free (fcolormap); diff --git a/libgimp/tests/test-palette.py b/libgimp/tests/test-palette.py index dfcce9dfb5..2a656a72d7 100644 --- a/libgimp/tests/test-palette.py +++ b/libgimp/tests/test-palette.py @@ -9,6 +9,7 @@ GIMP_TEST_COLOR_FORMAT = "R'G'B' u8" GIMP_TEST_COLOR_R_U8 = 72 GIMP_TEST_COLOR_G_U8 = 56 GIMP_TEST_COLOR_B_U8 = 56 +EPSILON = 1e-6 pal = Gimp.Palette.get_by_name(GIMP_TEST_PALETTE) gimp_assert('gimp_palette_get_by_name()', type(pal) == Gimp.Palette) @@ -52,19 +53,19 @@ gimp_assert('gimp_palette_get_colormap() in "RGBA float"', start = f2_bpp * GIMP_TEST_COLOR_IDX -colormap_r = struct.unpack('f', colormap[start:start + 4]) -colormap_g = struct.unpack('f', colormap[start + 4:start + 8]) -colormap_b = struct.unpack('f', colormap[start + 8:start + 12]) +colormap_r = struct.unpack('f', colormap[start:start + 4])[0] +colormap_g = struct.unpack('f', colormap[start + 4:start + 8])[0] +colormap_b = struct.unpack('f', colormap[start + 8:start + 12])[0] rgb_bytes = colors[GIMP_TEST_COLOR_IDX].get_bytes(f2) rgb = rgb_bytes.get_data() -palette_r = struct.unpack('f', rgb[:4]) -palette_g = struct.unpack('f', rgb[4:8]) -palette_b = struct.unpack('f', rgb[8:12]) +palette_r = struct.unpack('f', rgb[:4])[0] +palette_g = struct.unpack('f', rgb[4:8])[0] +palette_b = struct.unpack('f', rgb[8:12])[0] gimp_assert("Comparing fourth palette color's RGB components from colormap in float format", - colormap_r == palette_r and - colormap_g == palette_g and - colormap_b == palette_b) + abs(colormap_r - palette_r) < EPSILON and + abs(colormap_g - palette_g) < EPSILON and + abs(colormap_b - palette_b) < EPSILON) # Run the same tests through PDB: