[PATCH 0/5] MR3524: gdiplus: Add missing PNG metadata properties

[Jeffrey Smith (＠whydoubt)]

This adds the unit, date-time, and palette histogram metadata properties for PNG images to GDI+.  With these changes, the properties returned match Windows for all files in the PNG samples suite found at http://www.schaik.com/pngsuite/ .

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3524

[PATCH 1/5] gdiplus/tests: Factor out check_properties

[Jeff Smith]

From: Jeff Smith <whydoubt(a)gmail.com>

---
 dlls/gdiplus/tests/image.c | 445 +++++++++++++++----------------------
 1 file changed, 177 insertions(+), 268 deletions(-)

diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c
index bced4e0ca39..8d5151d1421 100644
--- a/dlls/gdiplus/tests/image.c
+++ b/dlls/gdiplus/tests/image.c
@@ -3654,6 +3654,157 @@ static GpImage *load_image(const BYTE *image_data, UINT image_size, BOOL valid_d
     return image;
 }
 
+struct property_test_data
+{
+    ULONG type, id, length;
+    const BYTE value[32];
+    BOOL broken_length;
+    BOOL broken_data;
+};
+
+#ifndef PropertyTagTypeSByte
+#define PropertyTagTypeSByte  6
+#define PropertyTagTypeSShort 8
+#define PropertyTagTypeFloat  11
+#define PropertyTagTypeDouble 12
+#endif
+
+static UINT documented_type(UINT type)
+{
+    /* Win7 stopped using proper but not documented types, and it
+       looks broken since TypeFloat and TypeDouble now reported as
+       TypeUndefined, and signed types reported as unsigned. */
+    switch (type)
+    {
+    case PropertyTagTypeSByte: return PropertyTagTypeByte;
+    case PropertyTagTypeSShort: return PropertyTagTypeShort;
+    case PropertyTagTypeFloat: return PropertyTagTypeUndefined;
+    case PropertyTagTypeDouble: return PropertyTagTypeUndefined;
+    default: return type;
+    }
+}
+
+static void check_properties_id_list(GpImage *image, const struct property_test_data *td, UINT count,
+    const struct property_test_data *td_broken, UINT count_broken, UINT *prop_size)
+{
+    GpStatus status;
+    UINT prop_count, size, i;
+    PROPID *prop_id;
+    PropertyItem *prop_item;
+
+    prop_count = 0xdeadbeef;
+    status = GdipGetPropertyCount(image, &prop_count);
+    expect(Ok, status);
+    ok(count == prop_count || broken(count_broken != ~0 && count_broken == prop_count),
+       "expected property count %u, got %u\n", count, prop_count);
+    if (count_broken != ~0 && count_broken == prop_count)
+        td = td_broken;
+
+    prop_id = HeapAlloc(GetProcessHeap(), 0, prop_count * sizeof(*prop_id));
+
+    status = GdipGetPropertyIdList(image, prop_count, prop_id);
+    expect(Ok, status);
+
+    if (prop_size)
+        *prop_size = 0;
+
+    for (i = 0; i < prop_count; i++)
+    {
+        winetest_push_context("prop %u", i);
+
+        status = GdipGetPropertyItemSize(image, prop_id[i], &size);
+        expect(Ok, status);
+        if (status != Ok)
+            break;
+        ok(size > sizeof(*prop_item), "%u: too small item length %u\n", i, size);
+
+        if (prop_size)
+            *prop_size += size;
+
+        prop_item = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, size);
+        status = GdipGetPropertyItem(image, prop_id[i], size, prop_item);
+        size -= sizeof(*prop_item);
+        expect(Ok, status);
+
+        ok(prop_item->value == prop_item + 1, "expected item->value %p, got %p\n",
+           prop_item + 1, prop_item->value);
+        ok(td[i].type == prop_item->type ||
+           broken(documented_type(td[i].type) == prop_item->type),
+           "expected type %lu, got %u\n", td[i].type, prop_item->type);
+        ok(td[i].id == prop_item->id, "expected id %#lx, got %#lx\n", td[i].id, prop_item->id);
+        ok(prop_item->length == size, "expected length %u, got %lu\n", size, prop_item->length);
+        ok(td[i].length == prop_item->length || broken(td[i].broken_length),
+           "expected length %lu, got %lu\n", td[i].length, prop_item->length);
+        ok(td[i].length == size || broken(td[i].broken_length),
+           "expected length %lu, got %u\n", td[i].length, size);
+        if (td[i].length == prop_item->length)
+        {
+            int match = !memcmp(td[i].value, prop_item->value, td[i].length);
+            ok(match || broken(td[i].broken_data), "data mismatch\n");
+            if (!match)
+                trace("(id %#lx) %s\n", prop_item->id, dbgstr_hexdata(prop_item->value, prop_item->length));
+        }
+
+        HeapFree(GetProcessHeap(), 0, prop_item);
+
+        winetest_pop_context();
+    }
+
+    HeapFree(GetProcessHeap(), 0, prop_id);
+}
+
+static void check_properties_get_all(GpImage *image, const struct property_test_data *td, UINT count,
+    const struct property_test_data *td_broken, UINT count_broken, UINT prop_size)
+{
+    GpStatus status;
+    UINT total_count, total_size, i;
+    PropertyItem *prop_item;
+    const BYTE *item_data;
+
+    total_size = 0xdeadbeef;
+    total_count = 0xdeadbeef;
+    status = GdipGetPropertySize(image, &total_size, &total_count);
+    expect(Ok, status);
+    ok(prop_size == total_size,
+       "expected total property size %u, got %u\n", prop_size, total_size);
+    ok(count == total_count || broken(count_broken != ~0 && count_broken == total_count),
+       "expected total property count %u, got %u\n", count, total_count);
+    if (count_broken != ~0 && count_broken == total_count)
+        td = td_broken;
+
+    prop_item = HeapAlloc(GetProcessHeap(), 0, total_size);
+    status = GdipGetAllPropertyItems(image, total_size, total_count, prop_item);
+    expect(Ok, status);
+
+    item_data = (const BYTE *)(prop_item + total_count);
+    for (i = 0; i < total_count && i < count; i++)
+    {
+        winetest_push_context("prop %u", i);
+
+        ok(prop_item[i].value == item_data,
+           "expected value %p, got %p\n", item_data, prop_item[i].value);
+        ok(td[i].type == prop_item[i].type ||
+           broken(documented_type(td[i].type) == prop_item[i].type),
+           "expected type %lu, got %u\n", td[i].type, prop_item[i].type);
+        ok(td[i].id == prop_item[i].id,
+           "expected id %#lx, got %#lx\n", td[i].id, prop_item[i].id);
+        ok(td[i].length == prop_item[i].length || broken(td[i].broken_length),
+           "expected length %lu, got %lu\n", td[i].length, prop_item[i].length);
+        if (td[i].length == prop_item[i].length)
+        {
+            int match = !memcmp(td[i].value, prop_item[i].value, td[i].length);
+            ok(match || broken(td[i].broken_data), "data mismatch\n");
+            if (!match)
+                trace("(id %#lx) %s\n", prop_item[i].id, dbgstr_hexdata(prop_item[i].value, prop_item[i].length));
+        }
+        item_data += prop_item[i].length;
+
+        winetest_pop_context();
+    }
+
+    HeapFree(GetProcessHeap(), 0, prop_item);
+}
+
 static void test_image_properties(void)
 {
     static const struct test_data
@@ -3861,25 +4012,6 @@ static void test_image_properties(void)
 #define IFD_FLOAT     11
 #define IFD_DOUBLE    12
 
-#ifndef PropertyTagTypeSByte
-#define PropertyTagTypeSByte  6
-#define PropertyTagTypeSShort 8
-#define PropertyTagTypeFloat  11
-#define PropertyTagTypeDouble 12
-#endif
-
-static UINT documented_type(UINT type)
-{
-    switch (type)
-    {
-    case PropertyTagTypeSByte: return PropertyTagTypeByte;
-    case PropertyTagTypeSShort: return PropertyTagTypeShort;
-    case PropertyTagTypeFloat: return PropertyTagTypeUndefined;
-    case PropertyTagTypeDouble: return PropertyTagTypeUndefined;
-    default: return type;
-    }
-}
-
 #include "pshpack2.h"
 struct IFD_entry
 {
@@ -3974,12 +4106,6 @@ static const struct tiff_data
 };
 #include "poppack.h"
 
-struct property_test_data
-{
-    ULONG type, id, length;
-    const BYTE value[32];
-};
-
 static void test_tiff_properties(void)
 {
     static const struct property_test_data td[31] =
@@ -3999,20 +4125,20 @@ static void test_tiff_properties(void)
         { PropertyTagTypeShort, 0x128, 2, { 2 } },
         { PropertyTagTypeByte, 0xf001, 1, { 0x44 } },
         { PropertyTagTypeByte, 0xf002, 4, { 0x44,0x33,0x22,0x11 } },
-        { PropertyTagTypeSByte, 0xf003, 1, { 0x44 } },
-        { PropertyTagTypeSShort, 0xf004, 2, { 0x44,0x33 } },
-        { PropertyTagTypeSShort, 0xf005, 4, { 0x44,0x33,0x22,0x11 } },
-        { PropertyTagTypeSLONG, 0xf006, 4, { 0x44,0x33,0x22,0x11 } },
-        { PropertyTagTypeFloat, 0xf007, 4, { 0x44,0x33,0x22,0x11 } },
+        { PropertyTagTypeSByte, 0xf003, 1, { 0x44 }, FALSE, TRUE },
+        { PropertyTagTypeSShort, 0xf004, 2, { 0x44,0x33 }, FALSE, TRUE },
+        { PropertyTagTypeSShort, 0xf005, 4, { 0x44,0x33,0x22,0x11 }, FALSE, TRUE },
+        { PropertyTagTypeSLONG, 0xf006, 4, { 0x44,0x33,0x22,0x11 }, FALSE, TRUE },
+        { PropertyTagTypeFloat, 0xf007, 4, { 0x44,0x33,0x22,0x11 }, FALSE, TRUE },
         { PropertyTagTypeDouble, 0xf008, 8, { 0x2c,0x52,0x86,0xb4,0x80,0x65,0xd2,0x41 } },
         { PropertyTagTypeSRational, 0xf009, 8, { 0x4d, 0x3c, 0x2b, 0x1a, 0x8d, 0x7c, 0x6b, 0x5a } },
-        { PropertyTagTypeByte, 0xf00a, 13, { 'H','e','l','l','o',' ','W','o','r','l','d','!',0 } },
+        { PropertyTagTypeByte, 0xf00a, 13, "Hello World!" },
         { PropertyTagTypeSShort, 0xf00b, 8, { 0x01,0x01,0x02,0x02,0x03,0x03,0x04,0x04 } },
         { PropertyTagTypeSLONG, 0xf00c, 8, { 0x44,0x33,0x22,0x11,0x88,0x77,0x66,0x55 } },
-        { PropertyTagTypeASCII, 0xf00e, 13, { 'H','e','l','l','o',' ','W','o','r','l','d','!',0 } },
-        { PropertyTagTypeASCII, 0xf00f, 5, { 'a','b','c','d' } },
-        { PropertyTagTypeUndefined, 0xf010, 13, { 'H','e','l','l','o',' ','W','o','r','l','d','!',0 } },
-        { PropertyTagTypeUndefined, 0xf011, 4, { 'a','b','c','d' } },
+        { PropertyTagTypeASCII, 0xf00e, 13, "Hello World!" },
+        { PropertyTagTypeASCII, 0xf00f, 5, "abcd", TRUE },
+        { PropertyTagTypeUndefined, 0xf010, 13, "Hello World!" },
+        { PropertyTagTypeUndefined, 0xf011, 4, { 'a','b','c','d' }, FALSE, TRUE },
         { PropertyTagTypeSRational, 0xf016, 24,
           { 0x04,0x03,0x02,0x01,0x08,0x07,0x06,0x05,
             0x40,0x30,0x20,0x10,0x80,0x70,0x60,0x50,
@@ -4023,9 +4149,7 @@ static void test_tiff_properties(void)
     GpStatus status;
     GpImage *image;
     GUID guid;
-    UINT dim_count, frame_count, prop_count, prop_size, i;
-    PROPID *prop_id;
-    PropertyItem *prop_item;
+    UINT dim_count, frame_count;
 
     image = load_image((const BYTE *)&TIFF_data, sizeof(TIFF_data), TRUE, FALSE);
     if (!image)
@@ -4047,53 +4171,9 @@ static void test_tiff_properties(void)
     expect(Ok, status);
     expect(1, frame_count);
 
-    prop_count = 0xdeadbeef;
-    status = GdipGetPropertyCount(image, &prop_count);
-    expect(Ok, status);
-    ok(prop_count == ARRAY_SIZE(td) ||
-       broken(prop_count == ARRAY_SIZE(td) - 1) /* Win7 SP0 */,
-       "expected property count %u, got %u\n", (UINT) ARRAY_SIZE(td), prop_count);
-
-    prop_id = HeapAlloc(GetProcessHeap(), 0, prop_count * sizeof(*prop_id));
-
-    status = GdipGetPropertyIdList(image, prop_count, prop_id);
-    expect(Ok, status);
-
-    for (i = 0; i < prop_count; i++)
-    {
-        status = GdipGetPropertyItemSize(image, prop_id[i], &prop_size);
-        expect(Ok, status);
-        if (status != Ok) break;
-        ok(prop_size > sizeof(*prop_item), "%u: too small item length %u\n", i, prop_size);
-
-        prop_item = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, prop_size);
-        status = GdipGetPropertyItem(image, prop_id[i], prop_size, prop_item);
-        expect(Ok, status);
-        ok(prop_item->value == prop_item + 1, "expected item->value %p, got %p\n", prop_item + 1, prop_item->value);
-        ok(td[i].type == prop_item->type ||
-           /* Win7 stopped using proper but not documented types, and it
-              looks broken since TypeFloat and TypeDouble now reported as
-              TypeUndefined, and signed types reported as unsigned. */
-           broken(prop_item->type == documented_type(td[i].type)),
-            "%u: expected type %lu, got %u\n", i, td[i].type, prop_item->type);
-        ok(td[i].id == prop_item->id, "%u: expected id %#lx, got %#lx\n", i, td[i].id, prop_item->id);
-        prop_size -= sizeof(*prop_item);
-        ok(prop_item->length == prop_size, "%u: expected length %u, got %lu\n", i, prop_size, prop_item->length);
-        ok(td[i].length == prop_item->length || broken(td[i].id == 0xf00f && td[i].length == prop_item->length+1) /* XP */,
-           "%u: expected length %lu, got %lu\n", i, td[i].length, prop_item->length);
-        ok(td[i].length == prop_size || broken(td[i].id == 0xf00f && td[i].length == prop_size+1) /* XP */,
-           "%u: expected length %lu, got %u\n", i, td[i].length, prop_size);
-        if (td[i].length == prop_item->length)
-        {
-            int match = memcmp(td[i].value, prop_item->value, td[i].length) == 0;
-            ok(match || broken(td[i].length <= 4 && !match), "%u: data mismatch\n", i);
-            if (!match)
-                trace("id %#lx:%s\n", prop_item->id, dbgstr_hexdata(prop_item->value, prop_item->length));
-        }
-        HeapFree(GetProcessHeap(), 0, prop_item);
-    }
-
-    HeapFree(GetProcessHeap(), 0, prop_id);
+    winetest_push_context("%s", __FUNCTION__);
+    check_properties_id_list(image, td, ARRAY_SIZE(td), td, ARRAY_SIZE(td) - 1 /* Win7 SP0 */, NULL);
+    winetest_pop_context();
 
     GdipDisposeImage(image);
 }
@@ -4122,11 +4202,7 @@ static void test_GdipGetAllPropertyItems(void)
     GpStatus status;
     GpImage *image;
     GUID guid;
-    UINT dim_count, frame_count, prop_count, prop_size, i;
-    UINT total_size, total_count;
-    PROPID *prop_id;
-    PropertyItem *prop_item;
-    const char *item_data;
+    UINT dim_count, frame_count, prop_size;
 
     image = load_image(tiffimage, sizeof(tiffimage), TRUE, FALSE);
     ok(image != 0, "Failed to load TIFF image data\n");
@@ -4146,83 +4222,10 @@ static void test_GdipGetAllPropertyItems(void)
     expect(Ok, status);
     expect(1, frame_count);
 
-    prop_count = 0xdeadbeef;
-    status = GdipGetPropertyCount(image, &prop_count);
-    expect(Ok, status);
-    ok(prop_count == ARRAY_SIZE(td),
-       "expected property count %u, got %u\n", (UINT) ARRAY_SIZE(td), prop_count);
-
-    prop_id = HeapAlloc(GetProcessHeap(), 0, prop_count * sizeof(*prop_id));
-
-    status = GdipGetPropertyIdList(image, prop_count, prop_id);
-    expect(Ok, status);
-
-    prop_size = 0;
-    for (i = 0; i < prop_count; i++)
-    {
-        UINT size;
-        status = GdipGetPropertyItemSize(image, prop_id[i], &size);
-        expect(Ok, status);
-        if (status != Ok) break;
-        ok(size > sizeof(*prop_item), "%u: too small item length %u\n", i, size);
-
-        prop_size += size;
-
-        prop_item = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, size);
-        status = GdipGetPropertyItem(image, prop_id[i], size, prop_item);
-        expect(Ok, status);
-        ok(prop_item->value == prop_item + 1, "expected item->value %p, got %p\n", prop_item + 1, prop_item->value);
-        ok(td[i].type == prop_item->type,
-            "%u: expected type %lu, got %u\n", i, td[i].type, prop_item->type);
-        ok(td[i].id == prop_item->id, "%u: expected id %#lx, got %#lx\n", i, td[i].id, prop_item->id);
-        size -= sizeof(*prop_item);
-        ok(prop_item->length == size, "%u: expected length %u, got %lu\n", i, size, prop_item->length);
-        ok(td[i].length == prop_item->length, "%u: expected length %lu, got %lu\n", i, td[i].length, prop_item->length);
-        if (td[i].length == prop_item->length)
-        {
-            int match = memcmp(td[i].value, prop_item->value, td[i].length) == 0;
-            ok(match, "%u: data mismatch\n", i);
-            if (!match)
-                trace("id %#lx:%s\n", prop_item->id, dbgstr_hexdata(prop_item->value, prop_item->length));
-        }
-        HeapFree(GetProcessHeap(), 0, prop_item);
-    }
-
-    HeapFree(GetProcessHeap(), 0, prop_id);
-
-    total_size = 0xdeadbeef;
-    total_count = 0xdeadbeef;
-    status = GdipGetPropertySize(image, &total_size, &total_count);
-    expect(Ok, status);
-    ok(prop_count == total_count,
-       "expected total property count %u, got %u\n", prop_count, total_count);
-    ok(prop_size == total_size,
-       "expected total property size %u, got %u\n", prop_size, total_size);
-
-    prop_item = HeapAlloc(GetProcessHeap(), 0, prop_size);
-    status = GdipGetAllPropertyItems(image, prop_size, prop_count, prop_item);
-    expect(Ok, status);
-
-    item_data = (const char *)(prop_item + prop_count);
-    for (i = 0; i < prop_count; i++)
-    {
-        ok(prop_item[i].value == item_data, "%u: expected value %p, got %p\n",
-           i, item_data, prop_item[i].value);
-        ok(td[i].type == prop_item[i].type,
-            "%u: expected type %lu, got %u\n", i, td[i].type, prop_item[i].type);
-        ok(td[i].id == prop_item[i].id, "%u: expected id %#lx, got %#lx\n", i, td[i].id, prop_item[i].id);
-        ok(td[i].length == prop_item[i].length, "%u: expected length %lu, got %lu\n", i, td[i].length, prop_item[i].length);
-        if (td[i].length == prop_item[i].length)
-        {
-            int match = memcmp(td[i].value, prop_item[i].value, td[i].length) == 0;
-            ok(match, "%u: data mismatch\n", i);
-            if (!match)
-                trace("id %#lx:%s\n", prop_item[i].id, dbgstr_hexdata(prop_item[i].value, prop_item[i].length));
-        }
-        item_data += prop_item[i].length;
-    }
-
-    HeapFree(GetProcessHeap(), 0, prop_item);
+    winetest_push_context("%s", __FUNCTION__);
+    check_properties_id_list(image, td, ARRAY_SIZE(td), NULL, ~0, &prop_size);
+    check_properties_get_all(image, td, ARRAY_SIZE(td), NULL, ~0, prop_size);
+    winetest_pop_context();
 
     GdipDisposeImage(image);
 }
@@ -4943,18 +4946,14 @@ static void test_gif_properties(void)
     GpStatus status;
     GpImage *image;
     GUID guid;
-    UINT dim_count, frame_count, prop_count, prop_size, i, j;
-    UINT total_size, total_count;
-    PROPID *prop_id;
-    PropertyItem *prop_item;
-    const char *item_data;
+    UINT dim_count, frame_count, prop_size, i;
 
     for (i = 0; i < ARRAY_SIZE(td); i++)
     {
         winetest_push_context("test %u", i);
 
         image = load_image(td[i].image_data, td[i].image_size, TRUE, FALSE);
-        if (!image) /* XP fails to load this GIF image */
+        if (!image) /* XP fails to load most of these GIF images */
         {
             trace("Failed to load GIF image data\n");
             winetest_pop_context();
@@ -4976,104 +4975,14 @@ static void test_gif_properties(void)
         status = GdipImageSelectActiveFrame(image, &guid, td[i].frame_count - 1);
         expect(Ok, status);
 
-        status = GdipGetPropertyCount(image, &prop_count);
-        expect(Ok, status);
-        ok(prop_count == td[i].prop_count || broken(prop_count == 1) /* before win7 */,
-           "expected property count %u, got %u\n", (UINT) td[i].prop_count, prop_count);
-
-        if (prop_count != td[i].prop_count)
-        {
-            win_skip("Property count mismatch.\n");
-            GdipDisposeImage(image);
-            winetest_pop_context();
-            continue;
-        }
-
-        prop_id = HeapAlloc(GetProcessHeap(), 0, prop_count * sizeof(*prop_id));
-
-        status = GdipGetPropertyIdList(image, prop_count, prop_id);
-        expect(Ok, status);
-
-        prop_size = 0;
-        for (j = 0; j < prop_count; j++)
-        {
-            UINT size;
-
-            winetest_push_context("property %u", j);
-
-            status = GdipGetPropertyItemSize(image, prop_id[j], &size);
-            expect(Ok, status);
-            if (status != Ok) break;
-            ok(size > sizeof(*prop_item), "too small item length %u\n", size);
-
-            prop_size += size;
-
-            prop_item = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, size);
-            status = GdipGetPropertyItem(image, prop_id[j], size, prop_item);
-            expect(Ok, status);
-            ok(prop_item->value == prop_item + 1, "expected item->value %p, got %p\n", prop_item + 1, prop_item->value);
-            ok(td[i].prop_item[j].type == prop_item->type,
-                "expected type %lu, got %u\n", td[i].prop_item[j].type, prop_item->type);
-            ok(td[i].prop_item[j].id == prop_item->id, "expected id %#lx, got %#lx\n", td[i].prop_item[j].id, prop_item->id);
-            size -= sizeof(*prop_item);
-            ok(prop_item->length == size, "expected length %u, got %lu\n", size, prop_item->length);
-            ok(td[i].prop_item[j].length == prop_item->length, "expected length %lu, got %lu\n", td[i].prop_item[j].length, prop_item->length);
-            if (td[i].prop_item[j].length == prop_item->length)
-            {
-                int match = memcmp(td[i].prop_item[j].value, prop_item->value, td[i].prop_item[j].length) == 0;
-                ok(match, "data mismatch\n");
-                if (!match)
-                    trace("id %#lx:%s\n", prop_item->id, dbgstr_hexdata(prop_item->value, prop_item->length));
-            }
-            HeapFree(GetProcessHeap(), 0, prop_item);
-
-            winetest_pop_context();
-        }
-
-        HeapFree(GetProcessHeap(), 0, prop_id);
-
-        total_size = 0xdeadbeef;
-        total_count = 0xdeadbeef;
-        status = GdipGetPropertySize(image, &total_size, &total_count);
-        expect(Ok, status);
-        ok(prop_count == total_count,
-           "expected total property count %u, got %u\n", prop_count, total_count);
-        ok(prop_size == total_size,
-           "expected total property size %u, got %u\n", prop_size, total_size);
-
-        prop_item = HeapAlloc(GetProcessHeap(), 0, prop_size);
-
-        status = GdipGetAllPropertyItems(image, prop_size, prop_count, prop_item);
-        expect(Ok, status);
-
-        item_data = (const char *)(prop_item + prop_count);
-        for (j = 0; j < prop_count; j++)
-        {
-            winetest_push_context("property %u", j);
-
-            ok(prop_item[j].value == item_data, "expected value %p, got %p\n",
-               item_data, prop_item[j].value);
-            ok(td[i].prop_item[j].type == prop_item[j].type,
-                "expected type %lu, got %u\n", td[i].prop_item[j].type, prop_item[j].type);
-            ok(td[i].prop_item[j].id == prop_item[j].id, "expected id %#lx, got %#lx\n", td[i].prop_item[j].id, prop_item[j].id);
-            ok(td[i].prop_item[j].length == prop_item[j].length, "expected length %lu, got %lu\n", td[i].prop_item[j].length, prop_item[j].length);
-            if (td[i].prop_item[j].length == prop_item[j].length)
-            {
-                int match = memcmp(td[i].prop_item[j].value, prop_item[j].value, td[i].prop_item[j].length) == 0;
-                ok(match, "data mismatch\n");
-                if (!match)
-                    trace("id %#lx:%s\n", prop_item[j].id, dbgstr_hexdata(prop_item[j].value, prop_item[j].length));
-            }
-            item_data += prop_item[j].length;
-
-            winetest_pop_context();
-        }
+        winetest_pop_context();
 
-        HeapFree(GetProcessHeap(), 0, prop_item);
+        winetest_push_context("%s test %u", __FUNCTION__, i);
+        check_properties_id_list(image, td[i].prop_item, td[i].prop_count, td[i].prop_item, 1, &prop_size);
+        check_properties_get_all(image, td[i].prop_item, td[i].prop_count, td[i].prop_item, 1, prop_size);
+        winetest_pop_context();
 
         GdipDisposeImage(image);
-
-        winetest_pop_context();
     }
 }
 
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/3524

[PATCH 2/5] gdiplus: Refactor png_metadata_reader.

[Jeff Smith]

From: Jeff Smith <whydoubt(a)gmail.com>

---
 dlls/gdiplus/image.c | 294 +++++++++++++++++++++++++------------------
 1 file changed, 174 insertions(+), 120 deletions(-)

diff --git a/dlls/gdiplus/image.c b/dlls/gdiplus/image.c
index 56a3cfa1d18..bef2a336986 100644
--- a/dlls/gdiplus/image.c
+++ b/dlls/gdiplus/image.c
@@ -3451,17 +3451,15 @@ static ULONG get_ulong_by_index(IWICMetadataReader* reader, ULONG index)
     return result;
 }
 
-static void png_metadata_reader(GpBitmap *bitmap, IWICBitmapDecoder *decoder, UINT active_frame)
+static HRESULT png_read_text(IWICMetadataReader *reader, GpBitmap *bitmap, BOOL **seen_text)
 {
     HRESULT hr;
-    IWICBitmapFrameDecode *frame;
-    IWICMetadataBlockReader *block_reader;
-    IWICMetadataReader *reader;
-    UINT block_count, i, j;
-    struct keyword_info {
+    UINT i;
+    PROPVARIANT name, value;
+    PropertyItem* item;
+    static const struct keyword_info {
         const char* name;
         PROPID propid;
-        BOOL seen;
     } keywords[] = {
         { "Title", PropertyTagImageTitle },
         { "Author", PropertyTagArtist },
@@ -3471,134 +3469,190 @@ static void png_metadata_reader(GpBitmap *bitmap, IWICBitmapDecoder *decoder, UI
         { "Source", PropertyTagEquipModel },
         { "Comment", PropertyTagExifUserComment },
     };
+
+    if (*seen_text == NULL)
+        *seen_text = heap_alloc_zero(sizeof(BOOL) * ARRAY_SIZE(keywords));
+    if (*seen_text == NULL)
+        return E_OUTOFMEMORY;
+
+    hr = IWICMetadataReader_GetValueByIndex(reader, 0, NULL, &name, &value);
+    if (FAILED(hr))
+        return hr;
+
+    if (name.vt == VT_LPSTR)
+    {
+        for (i = 0; i < ARRAY_SIZE(keywords); i++)
+        {
+            if (!strcmp(keywords[i].name, name.pszVal))
+                break;
+        }
+        if (i < ARRAY_SIZE(keywords) && !(*seen_text)[i])
+        {
+            (*seen_text)[i] = TRUE;
+            item = create_prop(keywords[i].propid, &value);
+            if (item)
+                add_property(bitmap, item);
+            heap_free(item);
+        }
+    }
+
+    PropVariantClear(&name);
+    PropVariantClear(&value);
+
+    return S_OK;
+}
+
+static HRESULT png_read_gamma(IWICMetadataReader *reader, GpBitmap *bitmap)
+{
+    PropertyItem* item;
+    ULONG *rational;
+
+    item = heap_alloc_zero(sizeof(*item) + sizeof(ULONG) * 2);
+    if (!item)
+        return E_OUTOFMEMORY;
+
+    item->length = sizeof(ULONG) * 2;
+    item->type = PropertyTagTypeRational;
+    item->id = PropertyTagGamma;
+    rational = item->value = item + 1;
+    rational[0] = 100000;
+    rational[1] = get_ulong_by_index(reader, 0);
+    add_property(bitmap, item);
+    heap_free(item);
+
+    return S_OK;
+}
+
+static HRESULT png_read_whitepoint(IWICMetadataReader *reader, GpBitmap *bitmap)
+{
+    PropertyItem* item;
+    ULONG *rational;
+
+    item = heap_alloc_zero(sizeof(*item) + sizeof(ULONG) * 4);
+    if (!item)
+        return E_OUTOFMEMORY;
+
+    item->length = sizeof(ULONG) * 4;
+    item->type = PropertyTagTypeRational;
+    item->id = PropertyTagWhitePoint;
+    rational = item->value = item + 1;
+    rational[0] = get_ulong_by_index(reader, 0);
+    rational[1] = 100000;
+    rational[2] = get_ulong_by_index(reader, 1);
+    rational[3] = 100000;
+    add_property(bitmap, item);
+    heap_free(item);
+
+    return S_OK;
+}
+
+static HRESULT png_read_chromaticity(IWICMetadataReader *reader, GpBitmap *bitmap)
+{
+    PropertyItem* item;
+    ULONG *rational;
+
+    item = heap_alloc_zero(sizeof(*item) + sizeof(ULONG) * 12);
+    if (!item)
+        return E_OUTOFMEMORY;
+
+    item->length = sizeof(ULONG) * 12;
+    item->type = PropertyTagTypeRational;
+    item->id = PropertyTagPrimaryChromaticities;
+    rational = item->value = item + 1;
+    rational[0] = get_ulong_by_index(reader, 2);
+    rational[1] = 100000;
+    rational[2] = get_ulong_by_index(reader, 3);
+    rational[3] = 100000;
+    rational[4] = get_ulong_by_index(reader, 4);
+    rational[5] = 100000;
+    rational[6] = get_ulong_by_index(reader, 5);
+    rational[7] = 100000;
+    rational[8] = get_ulong_by_index(reader, 6);
+    rational[9] = 100000;
+    rational[10] = get_ulong_by_index(reader, 7);
+    rational[11] = 100000;
+    add_property(bitmap, item);
+    heap_free(item);
+
+    return S_OK;
+}
+
+static void png_metadata_reader(GpBitmap *bitmap, IWICBitmapDecoder *decoder, UINT active_frame)
+{
+    HRESULT hr;
+    IWICBitmapFrameDecode *frame;
+    IWICMetadataBlockReader *block_reader;
+    UINT block_count, i;
     BOOL seen_gamma=FALSE, seen_whitepoint=FALSE, seen_chrm=FALSE;
+    BOOL *seen_text = NULL;
 
     hr = IWICBitmapDecoder_GetFrame(decoder, active_frame, &frame);
-    if (hr != S_OK) return;
+    if (hr != S_OK)
+        return;
 
     hr = IWICBitmapFrameDecode_QueryInterface(frame, &IID_IWICMetadataBlockReader, (void **)&block_reader);
-    if (hr == S_OK)
+    if (hr != S_OK)
     {
-        hr = IWICMetadataBlockReader_GetCount(block_reader, &block_count);
-        if (hr == S_OK)
-        {
-            for (i = 0; i < block_count; i++)
-            {
-                hr = IWICMetadataBlockReader_GetReaderByIndex(block_reader, i, &reader);
-                if (hr == S_OK)
-                {
-                    GUID format;
-
-                    hr = IWICMetadataReader_GetMetadataFormat(reader, &format);
-                    if (SUCCEEDED(hr) && IsEqualGUID(&GUID_MetadataFormatChunktEXt, &format))
-                    {
-                        PROPVARIANT name, value;
-                        PropertyItem* item;
+        IWICBitmapFrameDecode_Release(frame);
+        return;
+    }
 
-                        hr = IWICMetadataReader_GetValueByIndex(reader, 0, NULL, &name, &value);
+    hr = IWICMetadataBlockReader_GetCount(block_reader, &block_count);
+    if (hr != S_OK)
+    {
+        IWICMetadataBlockReader_Release(block_reader);
+        IWICBitmapFrameDecode_Release(frame);
+        return;
+    }
 
-                        if (SUCCEEDED(hr))
-                        {
-                            if (name.vt == VT_LPSTR)
-                            {
-                                for (j = 0; j < ARRAY_SIZE(keywords); j++)
-                                    if (!strcmp(keywords[j].name, name.pszVal))
-                                        break;
-                                if (j < ARRAY_SIZE(keywords) && !keywords[j].seen)
-                                {
-                                    keywords[j].seen = TRUE;
-                                    item = create_prop(keywords[j].propid, &value);
-                                    if (item)
-                                        add_property(bitmap, item);
-                                    heap_free(item);
-                                }
-                            }
-
-                            PropVariantClear(&name);
-                            PropVariantClear(&value);
-                        }
-                    }
-                    else if (SUCCEEDED(hr) && IsEqualGUID(&GUID_MetadataFormatChunkgAMA, &format))
-                    {
-                        PropertyItem* item;
+    for (i = 0; i < block_count; i++)
+    {
+        IWICMetadataReader *reader;
+        GUID format;
 
-                        if (!seen_gamma)
-                        {
-                            item = heap_alloc_zero(sizeof(PropertyItem) + sizeof(ULONG) * 2);
-                            if (item)
-                            {
-                                ULONG *rational;
-                                item->length = sizeof(ULONG) * 2;
-                                item->type = PropertyTagTypeRational;
-                                item->id = PropertyTagGamma;
-                                rational = item->value = item + 1;
-                                rational[0] = 100000;
-                                rational[1] = get_ulong_by_index(reader, 0);
-                                add_property(bitmap, item);
-                                seen_gamma = TRUE;
-                                heap_free(item);
-                            }
-                        }
-                    }
-                    else if (SUCCEEDED(hr) && IsEqualGUID(&GUID_MetadataFormatChunkcHRM, &format))
-                    {
-                        PropertyItem* item;
+        hr = IWICMetadataBlockReader_GetReaderByIndex(block_reader, i, &reader);
+        if (hr != S_OK)
+            continue;
 
-                        if (!seen_whitepoint)
-                        {
-                            item = GdipAlloc(sizeof(PropertyItem) + sizeof(ULONG) * 4);
-                            if (item)
-                            {
-                                ULONG *rational;
-                                item->length = sizeof(ULONG) * 4;
-                                item->type = PropertyTagTypeRational;
-                                item->id = PropertyTagWhitePoint;
-                                rational = item->value = item + 1;
-                                rational[0] = get_ulong_by_index(reader, 0);
-                                rational[1] = 100000;
-                                rational[2] = get_ulong_by_index(reader, 1);
-                                rational[3] = 100000;
-                                add_property(bitmap, item);
-                                seen_whitepoint = TRUE;
-                                GdipFree(item);
-                            }
-                        }
-                        if (!seen_chrm)
-                        {
-                            item = GdipAlloc(sizeof(PropertyItem) + sizeof(ULONG) * 12);
-                            if (item)
-                            {
-                                ULONG *rational;
-                                item->length = sizeof(ULONG) * 12;
-                                item->type = PropertyTagTypeRational;
-                                item->id = PropertyTagPrimaryChromaticities;
-                                rational = item->value = item + 1;
-                                rational[0] = get_ulong_by_index(reader, 2);
-                                rational[1] = 100000;
-                                rational[2] = get_ulong_by_index(reader, 3);
-                                rational[3] = 100000;
-                                rational[4] = get_ulong_by_index(reader, 4);
-                                rational[5] = 100000;
-                                rational[6] = get_ulong_by_index(reader, 5);
-                                rational[7] = 100000;
-                                rational[8] = get_ulong_by_index(reader, 6);
-                                rational[9] = 100000;
-                                rational[10] = get_ulong_by_index(reader, 7);
-                                rational[11] = 100000;
-                                add_property(bitmap, item);
-                                seen_chrm = TRUE;
-                                GdipFree(item);
-                            }
-                        }
-                    }
+        hr = IWICMetadataReader_GetMetadataFormat(reader, &format);
+        if (FAILED(hr))
+        {
+            IWICMetadataReader_Release(reader);
+            continue;
+        }
 
-                    IWICMetadataReader_Release(reader);
-                }
+        if (IsEqualGUID(&GUID_MetadataFormatChunktEXt, &format))
+            png_read_text(reader, bitmap, &seen_text);
+        else if (IsEqualGUID(&GUID_MetadataFormatChunkgAMA, &format))
+        {
+            if (!seen_gamma)
+            {
+                hr = png_read_gamma(reader, bitmap);
+                seen_gamma = SUCCEEDED(hr);
             }
         }
-        IWICMetadataBlockReader_Release(block_reader);
+        else if (IsEqualGUID(&GUID_MetadataFormatChunkcHRM, &format))
+        {
+            if (!seen_whitepoint)
+            {
+                hr = png_read_whitepoint(reader, bitmap);
+                seen_whitepoint = SUCCEEDED(hr);
+            }
+            if (!seen_chrm)
+            {
+                hr = png_read_chromaticity(reader, bitmap);
+                seen_chrm = SUCCEEDED(hr);
+            }
+        }
+
+        IWICMetadataReader_Release(reader);
     }
 
+    if (seen_text)
+        heap_free(seen_text);
+
+    IWICMetadataBlockReader_Release(block_reader);
+
     IWICBitmapFrameDecode_Release(frame);
 }
 
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/3524

[PATCH 3/5] gdiplus: Set PNG unit properties always.

[Jeff Smith]

From: Jeff Smith <whydoubt(a)gmail.com>

---
 dlls/gdiplus/image.c       | 51 ++++++++++++++++++++
 dlls/gdiplus/tests/image.c | 96 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 146 insertions(+), 1 deletion(-)

diff --git a/dlls/gdiplus/image.c b/dlls/gdiplus/image.c
index bef2a336986..599f576ca4d 100644
--- a/dlls/gdiplus/image.c
+++ b/dlls/gdiplus/image.c
@@ -3577,6 +3577,55 @@ static HRESULT png_read_chromaticity(IWICMetadataReader *reader, GpBitmap *bitma
     return S_OK;
 }
 
+static HRESULT png_add_unit_properties(IWICBitmapFrameDecode *frame, GpBitmap *bitmap)
+{
+    HRESULT hr;
+    double dpiX, dpiY;
+    PropertyItem *unit, *unitX, *unitY;
+
+    hr = IWICBitmapFrameDecode_GetResolution(frame, &dpiX, &dpiY);
+    if (FAILED(hr))
+        return hr;
+
+    unit = heap_alloc_zero(sizeof(*unit) + 1);
+    unitX = heap_alloc_zero(sizeof(*unitX) + 4);
+    unitY = heap_alloc_zero(sizeof(*unitY) + 4);
+
+    if (!unit || !unitX || !unitY)
+    {
+        heap_free(unit);
+        heap_free(unitX);
+        heap_free(unitY);
+        return E_OUTOFMEMORY;
+    }
+
+    unit->type = PropertyTagTypeByte;
+    unit->id = PropertyTagPixelUnit;
+    unit->length = 1;
+    unit->value = unit + 1;
+    *(BYTE *)unit->value = 1;
+    add_property(bitmap, unit);
+    heap_free(unit);
+
+    unitX->type = PropertyTagTypeLong;
+    unitX->id = PropertyTagPixelPerUnitX;
+    unitX->length = 4;
+    unitX->value = unitX + 1;
+    *(ULONG *)unitX->value = (dpiX == 96.0) ? 0 : gdip_round(dpiX / 0.0254);
+    add_property(bitmap, unitX);
+    heap_free(unitX);
+
+    unitY->type = PropertyTagTypeLong;
+    unitY->id = PropertyTagPixelPerUnitY;
+    unitY->length = 4;
+    unitY->value = unitY + 1;
+    *(ULONG *)unitY->value = (dpiY == 96.0) ? 0 : gdip_round(dpiY / 0.0254);
+    add_property(bitmap, unitY);
+    heap_free(unitY);
+
+    return S_OK;
+}
+
 static void png_metadata_reader(GpBitmap *bitmap, IWICBitmapDecoder *decoder, UINT active_frame)
 {
     HRESULT hr;
@@ -3651,6 +3700,8 @@ static void png_metadata_reader(GpBitmap *bitmap, IWICBitmapDecoder *decoder, UI
     if (seen_text)
         heap_free(seen_text);
 
+    png_add_unit_properties(frame, bitmap);
+
     IWICMetadataBlockReader_Release(block_reader);
 
     IWICBitmapFrameDecode_Release(frame);
diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c
index 8d5151d1421..5eb35ec85ad 100644
--- a/dlls/gdiplus/tests/image.c
+++ b/dlls/gdiplus/tests/image.c
@@ -3765,7 +3765,7 @@ static void check_properties_get_all(GpImage *image, const struct property_test_
     total_count = 0xdeadbeef;
     status = GdipGetPropertySize(image, &total_size, &total_count);
     expect(Ok, status);
-    ok(prop_size == total_size,
+    ok(prop_size == total_size || prop_size == ~0,
        "expected total property size %u, got %u\n", prop_size, total_size);
     ok(count == total_count || broken(count_broken != ~0 && count_broken == total_count),
        "expected total property count %u, got %u\n", count, total_count);
@@ -5787,6 +5787,99 @@ static void test_png_save_palette(void)
     GlobalFree(hglob);
 }
 
+static const BYTE png_minimal[] = {
+  0x89,'P','N','G',0x0d,0x0a,0x1a,0x0a,
+  0x00,0x00,0x00,0x0d,'I','H','D','R',0x00,0x00,0x00,0x01,0x00,0x00,0x00,0x01,0x08,0x02,0x00,0x00,0x00,0xff,0xff,0xff,0xff,
+  0x00,0x00,0x00,0x0c,'I','D','A','T',0x08,0xd7,0x63,0xf8,0xff,0xff,0x3f,0x00,0x05,0xfe,0x02,0xfe,0xdc,0xcc,0x59,0xe7,
+  0x00,0x00,0x00,0x00,'I','E','N','D',0xae,0x42,0x60,0x82
+};
+
+static const BYTE png_phys[] = {
+  0x89,'P','N','G',0x0d,0x0a,0x1a,0x0a,
+  0x00,0x00,0x00,0x0d,'I','H','D','R',0x00,0x00,0x00,0x01,0x00,0x00,0x00,0x01,0x08,0x02,0x00,0x00,0x00,0x90,0x77,0x53,0xde,
+  0x00,0x00,0x00,0x09,'p','H','Y','s',0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0xff,0xff,0xff,0xff,
+  0x00,0x00,0x00,0x0c,'I','D','A','T',0x08,0xd7,0x63,0xf8,0xff,0xff,0x3f,0x00,0x05,0xfe,0x02,0xfe,0xdc,0xcc,0x59,0xe7,
+  0x00,0x00,0x00,0x00,'I','E','N','D',0xae,0x42,0x60,0x82
+};
+
+static void test_png_unit_properties(void)
+{
+    GpImage *image;
+    UINT pHYs_off = 0, i;
+    static const struct {
+        BYTE unit;
+        ULONG unitX;
+        ULONG unitY;
+    } td[] =
+    {
+        {},
+        {1, 0, 0},
+        {0, 1000, 1000},
+        {1, 1000, 1000},
+        {1, 3780, 3780},
+    };
+    struct property_test_data prop_td[][3] =
+    {
+       {{ PropertyTagTypeByte, PropertyTagPixelUnit, 1, { 1 } },
+        { PropertyTagTypeLong, PropertyTagPixelPerUnitX, 4, { 0,0,0,0 } },
+        { PropertyTagTypeLong, PropertyTagPixelPerUnitY, 4, { 0,0,0,0 } }},
+       {{ PropertyTagTypeByte, PropertyTagPixelUnit, 1, { 1 } },
+        { PropertyTagTypeLong, PropertyTagPixelPerUnitX, 4, { 0,0,0,0 } },
+        { PropertyTagTypeLong, PropertyTagPixelPerUnitY, 4, { 0,0,0,0 } }},
+       {{ PropertyTagTypeByte, PropertyTagPixelUnit, 1, { 1 }, FALSE, TRUE },
+        { PropertyTagTypeLong, PropertyTagPixelPerUnitX, 4, { 0,0,0,0 }, FALSE, TRUE },
+        { PropertyTagTypeLong, PropertyTagPixelPerUnitY, 4, { 0,0,0,0 }, FALSE, TRUE }},
+       {{ PropertyTagTypeByte, PropertyTagPixelUnit, 1, { 1 } },
+        { PropertyTagTypeLong, PropertyTagPixelPerUnitX, 4, { 0xe8,0x03,0,0 } },
+        { PropertyTagTypeLong, PropertyTagPixelPerUnitY, 4, { 0xe8,0x03,0,0 } }},
+       {{ PropertyTagTypeByte, PropertyTagPixelUnit, 1, { 1 } },
+        { PropertyTagTypeLong, PropertyTagPixelPerUnitX, 4, { 0xc4,0x0e,0,0 } },
+        { PropertyTagTypeLong, PropertyTagPixelPerUnitY, 4, { 0xc4,0x0e,0,0 } }},
+    };
+    BYTE buf[sizeof(png_phys)];
+
+
+    for (i = 0; i < sizeof(png_phys) - 4; i++)
+    {
+        if (!memcmp(png_phys + i, "pHYs", 4))
+            pHYs_off = i;
+    }
+
+    ok(pHYs_off, "pHYs offset %d\n", pHYs_off);
+    if (!pHYs_off)
+        return;
+
+    for (i = 0; i < ARRAY_SIZE(td); i++)
+    {
+        if (i == 0)
+            image = load_image(png_minimal, sizeof(png_minimal), TRUE, FALSE);
+        else
+        {
+            memcpy(buf, png_phys, sizeof(png_phys));
+            buf[pHYs_off + 4] = (td[i].unitX >> 24) & 0xff;
+            buf[pHYs_off + 5] = (td[i].unitX >> 16) & 0xff;
+            buf[pHYs_off + 6] = (td[i].unitX >> 8) & 0xff;
+            buf[pHYs_off + 7] = td[i].unitX & 0xff;
+            buf[pHYs_off + 8] = (td[i].unitY >> 24) & 0xff;
+            buf[pHYs_off + 9] = (td[i].unitY >> 16) & 0xff;
+            buf[pHYs_off + 10] = (td[i].unitY >> 8) & 0xff;
+            buf[pHYs_off + 11] = td[i].unitY & 0xff;
+            buf[pHYs_off + 12] = td[i].unit;
+            image = load_image(buf, sizeof(buf), TRUE, FALSE);
+        }
+
+        ok(image != NULL, "%u: Failed to load PNG image data\n", i);
+        if (!image)
+            continue;
+
+        winetest_push_context("%s test %u", __FUNCTION__, i);
+        check_properties_get_all(image, prop_td[i], 3, NULL, 0, ~0);
+        winetest_pop_context();
+
+        GdipDisposeImage(image);
+    }
+}
+
 static void test_GdipLoadImageFromStream(void)
 {
     IStream *stream;
@@ -6031,6 +6124,7 @@ START_TEST(image)
     test_GdipInitializePalette();
     test_png_color_formats();
     test_png_save_palette();
+    test_png_unit_properties();
     test_supported_encoders();
     test_CloneBitmapArea();
     test_ARGB_conversion();
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/3524

[PATCH 4/5] gdiplus: Set PNG date-time property when present.

[Jeff Smith]

From: Jeff Smith <whydoubt(a)gmail.com>

---
 dlls/gdiplus/image.c       | 53 +++++++++++++++++++++++++++++++++++++-
 dlls/gdiplus/tests/image.c | 33 +++++++++++++++++++++++-
 2 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/dlls/gdiplus/image.c b/dlls/gdiplus/image.c
index 599f576ca4d..4837b4b1a3f 100644
--- a/dlls/gdiplus/image.c
+++ b/dlls/gdiplus/image.c
@@ -3577,6 +3577,49 @@ static HRESULT png_read_chromaticity(IWICMetadataReader *reader, GpBitmap *bitma
     return S_OK;
 }
 
+static HRESULT png_read_time(IWICMetadataReader *reader, GpBitmap *bitmap)
+{
+    HRESULT hr;
+    UINT item_size, i;
+    PropertyItem* item;
+    PROPVARIANT value;
+    USHORT datetime[6];
+
+    for (i = 0; i < 6; i++)
+    {
+        hr = IWICMetadataReader_GetValueByIndex(reader, i, NULL, NULL, &value);
+        if (FAILED(hr))
+            return hr;
+        if (i == 0 && value.vt == VT_UI2)
+            datetime[0] = value.uiVal;
+        else if (i > 0 && value.vt == VT_UI1)
+            datetime[i] = value.bVal;
+        else
+        {
+            PropVariantClear(&value);
+            return E_FAIL;
+        }
+        PropVariantClear(&value);
+    }
+
+    item_size = 20;
+    item = heap_alloc_zero(sizeof(*item) + item_size);
+    if (!item)
+        return E_OUTOFMEMORY;
+
+    item->id = PropertyTagDateTime;
+    item->type = PropertyTagTypeASCII;
+    item->length = item_size;
+    item->value = item + 1;
+    snprintf(item->value, item_size, "%04u:%02u:%02u %02u:%02u:%02u",
+        datetime[0], datetime[1], datetime[2], datetime[3], datetime[4], datetime[5]);
+
+    add_property(bitmap, item);
+    heap_free(item);
+
+    return S_OK;
+}
+
 static HRESULT png_add_unit_properties(IWICBitmapFrameDecode *frame, GpBitmap *bitmap)
 {
     HRESULT hr;
@@ -3632,7 +3675,7 @@ static void png_metadata_reader(GpBitmap *bitmap, IWICBitmapDecoder *decoder, UI
     IWICBitmapFrameDecode *frame;
     IWICMetadataBlockReader *block_reader;
     UINT block_count, i;
-    BOOL seen_gamma=FALSE, seen_whitepoint=FALSE, seen_chrm=FALSE;
+    BOOL seen_gamma=FALSE, seen_whitepoint=FALSE, seen_chrm=FALSE, seen_time=FALSE;
     BOOL *seen_text = NULL;
 
     hr = IWICBitmapDecoder_GetFrame(decoder, active_frame, &frame);
@@ -3693,6 +3736,14 @@ static void png_metadata_reader(GpBitmap *bitmap, IWICBitmapDecoder *decoder, UI
                 seen_chrm = SUCCEEDED(hr);
             }
         }
+        else if (IsEqualGUID(&GUID_MetadataFormatChunktIME, &format))
+        {
+            if (!seen_time)
+            {
+                hr = png_read_time(reader, bitmap);
+                seen_time = SUCCEEDED(hr);
+            }
+        }
 
         IWICMetadataReader_Release(reader);
     }
diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c
index 5eb35ec85ad..b240e58ba43 100644
--- a/dlls/gdiplus/tests/image.c
+++ b/dlls/gdiplus/tests/image.c
@@ -3877,7 +3877,7 @@ static void test_image_properties(void)
 
         status = GdipGetPropertyCount(image, &prop_count);
         ok(status == Ok, "GdipGetPropertyCount error %d\n", status);
-        todo_wine_if(td[i].image_data == pngimage || td[i].image_data == jpgimage)
+        todo_wine_if(td[i].image_data == jpgimage)
         ok(td[i].prop_count == prop_count || (td[i].prop_count2 != ~0 && td[i].prop_count2 == prop_count),
            "expected property count %u or %u, got %u\n",
            td[i].prop_count, td[i].prop_count2, prop_count);
@@ -5802,6 +5802,14 @@ static const BYTE png_phys[] = {
   0x00,0x00,0x00,0x00,'I','E','N','D',0xae,0x42,0x60,0x82
 };
 
+static const BYTE png_time[] = {
+  0x89,'P','N','G',0x0d,0x0a,0x1a,0x0a,
+  0x00,0x00,0x00,0x0d,'I','H','D','R',0x00,0x00,0x00,0x01,0x00,0x00,0x00,0x01,0x08,0x02,0x00,0x00,0x00,0x90,0x77,0x53,0xde,
+  0x00,0x00,0x00,0x07,'t','I','M','E',0x07,0xb2,0x01,0x01,0x00,0x00,0x00,0xff,0xff,0xff,0xff,
+  0x00,0x00,0x00,0x0c,'I','D','A','T',0x08,0xd7,0x63,0xf8,0xff,0xff,0x3f,0x00,0x05,0xfe,0x02,0xfe,0xdc,0xcc,0x59,0xe7,
+  0x00,0x00,0x00,0x00,'I','E','N','D',0xae,0x42,0x60,0x82
+};
+
 static void test_png_unit_properties(void)
 {
     GpImage *image;
@@ -5880,6 +5888,28 @@ static void test_png_unit_properties(void)
     }
 }
 
+static void test_png_datetime_property(void)
+{
+    struct property_test_data td[] =
+    {
+        { PropertyTagTypeASCII, PropertyTagDateTime, 20, { "1970:01:01 00:00:00" } },
+        { PropertyTagTypeByte, PropertyTagPixelUnit, 1, { 1 } },
+        { PropertyTagTypeLong, PropertyTagPixelPerUnitX, 4, { 0,0,0,0 } },
+        { PropertyTagTypeLong, PropertyTagPixelPerUnitY, 4, { 0,0,0,0 } },
+    };
+
+    GpImage *image = load_image(png_time, sizeof(png_time), TRUE, FALSE);
+    ok(image != NULL, "Failed to load PNG image data\n");
+    if (!image)
+        return;
+
+    winetest_push_context("%s", __FUNCTION__);
+    check_properties_get_all(image, td, ARRAY_SIZE(td), td, 1, ~0);
+    winetest_pop_context();
+
+    GdipDisposeImage(image);
+}
+
 static void test_GdipLoadImageFromStream(void)
 {
     IStream *stream;
@@ -6125,6 +6155,7 @@ START_TEST(image)
     test_png_color_formats();
     test_png_save_palette();
     test_png_unit_properties();
+    test_png_datetime_property();
     test_supported_encoders();
     test_CloneBitmapArea();
     test_ARGB_conversion();
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/3524

[PATCH 5/5] gdiplus: Set PNG palette histogram property when present.

[Jeff Smith]

From: Jeff Smith <whydoubt(a)gmail.com>

---
 dlls/gdiplus/image.c       | 30 +++++++++++++++++++++++++++++-
 dlls/gdiplus/tests/image.c | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/dlls/gdiplus/image.c b/dlls/gdiplus/image.c
index 4837b4b1a3f..67a7fdfab13 100644
--- a/dlls/gdiplus/image.c
+++ b/dlls/gdiplus/image.c
@@ -3620,6 +3620,26 @@ static HRESULT png_read_time(IWICMetadataReader *reader, GpBitmap *bitmap)
     return S_OK;
 }
 
+static HRESULT png_read_histogram(IWICMetadataReader *reader, GpBitmap *bitmap)
+{
+    HRESULT hr;
+    PropertyItem* item;
+    PROPVARIANT value;
+
+    hr = IWICMetadataReader_GetValueByIndex(reader, 0, NULL, NULL, &value);
+    if (FAILED(hr))
+        return hr;
+
+    item = create_prop(PropertyTagPaletteHistogram, &value);
+    if (item)
+        add_property(bitmap, item);
+    heap_free(item);
+
+    PropVariantClear(&value);
+
+    return S_OK;
+}
+
 static HRESULT png_add_unit_properties(IWICBitmapFrameDecode *frame, GpBitmap *bitmap)
 {
     HRESULT hr;
@@ -3675,7 +3695,7 @@ static void png_metadata_reader(GpBitmap *bitmap, IWICBitmapDecoder *decoder, UI
     IWICBitmapFrameDecode *frame;
     IWICMetadataBlockReader *block_reader;
     UINT block_count, i;
-    BOOL seen_gamma=FALSE, seen_whitepoint=FALSE, seen_chrm=FALSE, seen_time=FALSE;
+    BOOL seen_gamma=FALSE, seen_whitepoint=FALSE, seen_chrm=FALSE, seen_time=FALSE, seen_histogram=FALSE;
     BOOL *seen_text = NULL;
 
     hr = IWICBitmapDecoder_GetFrame(decoder, active_frame, &frame);
@@ -3744,6 +3764,14 @@ static void png_metadata_reader(GpBitmap *bitmap, IWICBitmapDecoder *decoder, UI
                 seen_time = SUCCEEDED(hr);
             }
         }
+        else if (IsEqualGUID(&GUID_MetadataFormatChunkhIST, &format))
+        {
+            if (!seen_histogram)
+            {
+                hr = png_read_histogram(reader, bitmap);
+                seen_histogram = SUCCEEDED(hr);
+            }
+        }
 
         IWICMetadataReader_Release(reader);
     }
diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c
index b240e58ba43..22a7786ed1d 100644
--- a/dlls/gdiplus/tests/image.c
+++ b/dlls/gdiplus/tests/image.c
@@ -5810,6 +5810,14 @@ static const BYTE png_time[] = {
   0x00,0x00,0x00,0x00,'I','E','N','D',0xae,0x42,0x60,0x82
 };
 
+static const BYTE png_hist[] = {
+  0x89,'P','N','G',0x0d,0x0a,0x1a,0x0a,
+  0x00,0x00,0x00,0x0d,'I','H','D','R',0x00,0x00,0x00,0x01,0x00,0x00,0x00,0x01,0x08,0x02,0x00,0x00,0x00,0x90,0x77,0x53,0xde,
+  0x00,0x00,0x00,0x08,'h','I','S','T',0x00,0x01,0x00,0x02,0x00,0x03,0x00,0x04,0xff,0xff,0xff,0xff,
+  0x00,0x00,0x00,0x0c,'I','D','A','T',0x08,0xd7,0x63,0xf8,0xff,0xff,0x3f,0x00,0x05,0xfe,0x02,0xfe,0xdc,0xcc,0x59,0xe7,
+  0x00,0x00,0x00,0x00,'I','E','N','D',0xae,0x42,0x60,0x82
+};
+
 static void test_png_unit_properties(void)
 {
     GpImage *image;
@@ -5910,6 +5918,30 @@ static void test_png_datetime_property(void)
     GdipDisposeImage(image);
 }
 
+static void test_png_histogram_property(void)
+{
+    struct property_test_data td[] =
+    {
+        { PropertyTagTypeShort, PropertyTagPaletteHistogram, 8, { 1,0,2,0,3,0,4,0 } },
+        { PropertyTagTypeByte, PropertyTagPixelUnit, 1, { 1 } },
+        { PropertyTagTypeLong, PropertyTagPixelPerUnitX, 4, { 0,0,0,0 } },
+        { PropertyTagTypeLong, PropertyTagPixelPerUnitY, 4, { 0,0,0,0 } },
+    };
+
+    GpImage *image = load_image(png_hist, sizeof(png_hist), TRUE, FALSE);
+    if (!image)
+    {
+        win_skip("broken PNG histogram support\n");
+        return;
+    }
+
+    winetest_push_context("%s", __FUNCTION__);
+    check_properties_get_all(image, td, ARRAY_SIZE(td), NULL, 0, ~0);
+    winetest_pop_context();
+
+    GdipDisposeImage(image);
+}
+
 static void test_GdipLoadImageFromStream(void)
 {
     IStream *stream;
@@ -6156,6 +6188,7 @@ START_TEST(image)
     test_png_save_palette();
     test_png_unit_properties();
     test_png_datetime_property();
+    test_png_histogram_property();
     test_supported_encoders();
     test_CloneBitmapArea();
     test_ARGB_conversion();
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/3524

[Esme Povirk (＠madewokherd)]

Esme Povirk (@madewokherd) commented about dlls/gdiplus/tests/image.c:
> +    prop_id = HeapAlloc(GetProcessHeap(), 0, prop_count * sizeof(*prop_id));
> +
> +    status = GdipGetPropertyIdList(image, prop_count, prop_id);
> +    expect(Ok, status);
> +
> +    if (prop_size)
> +        *prop_size = 0;
> +
> +    for (i = 0; i < prop_count; i++)
> +    {
> +        winetest_push_context("prop %u", i);
> +
> +        status = GdipGetPropertyItemSize(image, prop_id[i], &size);
> +        expect(Ok, status);
> +        if (status != Ok)
> +            break;
> I think this desyncs winetest_push_context and winetest_pop_context calls.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3524#note_41526

[Esme Povirk (＠madewokherd)]

Esme Povirk (@madewokherd) commented about dlls/gdiplus/tests/image.c:
> +
> +    status = GdipGetPropertyIdList(image, prop_count, prop_id);
> +    expect(Ok, status);
> +
> +    if (prop_size)
> +        *prop_size = 0;
> +
> +    for (i = 0; i < prop_count; i++)
> +    {
> +        winetest_push_context("prop %u", i);
> +
> +        status = GdipGetPropertyItemSize(image, prop_id[i], &size);
> +        expect(Ok, status);
> +        if (status != Ok)
> +            break;
> +        ok(size > sizeof(*prop_item), "%u: too small item length %u\n", i, size);
> `i` should already be shown via winetest_push_context.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3524#note_41527