[PATCH v3 0/2] MR11714: gdiplus/metafile: Allow drawimage w/o attributes

[Tobi Laskowski (＠tobil)]

As stated in the documentation, the attribute id is optional: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-emfplus/778...

Without the patch, emf+ files that contain records with an empty image attribute field do not render properly since those records are rejected.

--
  v3: gdiplus/metafile: Allow drawimage w/o attributes
      gdiplus/tests: Check drawimage w/o attributes
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/11714

[PATCH v3 1/2] gdiplus/tests: Check drawimage w/o attributes

[Tobiasz Laskowski]

From: Tobiasz Laskowski <[e-mail]>

---
 dlls/gdiplus/tests/metafile.c | 168 ++++++++++++++++++++++++++++++++++
 1 file changed, 168 insertions(+)

diff --git a/dlls/gdiplus/tests/metafile.c b/dlls/gdiplus/tests/metafile.c
index 9ee052b23d3..3cee0695667 100644
--- a/dlls/gdiplus/tests/metafile.c
+++ b/dlls/gdiplus/tests/metafile.c
@@ -2655,6 +2655,172 @@ static void test_drawimage(void)
     expect(Ok, stat);
 }
 
+typedef struct {
+    EmfPlusRecordHeader Header;
+    DWORD ImageAttributesID;
+} DrawImageBaseRecord;
+
+typedef struct {
+    EmfPlusRecordHeader Header;
+    DWORD ImageAttributesID;
+    DWORD SrcUnit;
+    GpRectF SrcRect;
+    GpRectF rectF;
+} DrawImageRecord;
+
+typedef struct {
+    EmfPlusRecordHeader Header;
+    DWORD ImageAttributesID;
+    DWORD SrcUnit;
+    GpRectF SrcRect;
+    DWORD count;
+    GpPointF pointsF[3];
+} DrawImagePointsRecord;
+
+typedef struct {
+    GpMetafile* metafile;
+    DrawImageBaseRecord* record;
+    BOOL executed;
+} play_draw_image_ctx;
+
+static BOOL CALLBACK play_drawimage_proc(EmfPlusRecordType recordType, UINT flags,
+    UINT dataSize, const BYTE* data, void* callbackData) {
+    play_draw_image_ctx* ctx = callbackData;
+    GpStatus stat;
+
+    if (recordType == EmfPlusRecordTypeObject && !ctx->executed) {
+        stat = GdipPlayMetafileRecord(ctx->metafile, recordType, flags, dataSize, data);
+        expect(Ok, stat);
+
+        ctx->executed = TRUE;
+        ctx->record->Header.Flags = flags;
+
+        /* Out of bounds image attribute ids are ignored */
+        ctx->record->ImageAttributesID = (DWORD)-1;
+        stat = GdipPlayMetafileRecord(ctx->metafile, ctx->record->Header.Type,
+            ctx->record->Header.Flags, ctx->record->Header.DataSize,
+            (const BYTE*)&ctx->record->ImageAttributesID);
+        todo_wine
+        expect(Ok, stat);
+
+        ctx->record->ImageAttributesID = (DWORD)-2;
+        stat = GdipPlayMetafileRecord(ctx->metafile, ctx->record->Header.Type,
+            ctx->record->Header.Flags, ctx->record->Header.DataSize,
+            (const BYTE*)&ctx->record->ImageAttributesID);
+        todo_wine
+        expect(Ok, stat);
+
+        ctx->record->ImageAttributesID = (DWORD)64;
+        stat = GdipPlayMetafileRecord(ctx->metafile, ctx->record->Header.Type,
+            ctx->record->Header.Flags, ctx->record->Header.DataSize,
+            (const BYTE*)&ctx->record->ImageAttributesID);
+        todo_wine
+        expect(Ok, stat);
+
+        /* Within bounds, but missing */
+        ctx->record->ImageAttributesID = (DWORD)1;
+        stat = GdipPlayMetafileRecord(ctx->metafile, ctx->record->Header.Type,
+            ctx->record->Header.Flags, ctx->record->Header.DataSize,
+            (const BYTE*)&ctx->record->ImageAttributesID);
+        todo_wine
+        expect(Ok, stat);
+    }
+
+    return TRUE;
+}
+
+static void check_play_drawimage(DrawImageBaseRecord* record) {
+    static const GpRectF frame = { 0.0f, 0.0f, 100.0f, 100.0f };
+    GpMetafile* metafile;
+    GpGraphics* graphics;
+    GpBitmap* bitmap;
+    play_draw_image_ctx ctx;
+    GpStatus stat;
+    HDC hdc;
+
+    hdc = CreateCompatibleDC(0);
+    stat = GdipRecordMetafile(hdc, EmfTypeEmfPlusOnly, &frame, MetafileFrameUnitPixel, description, &metafile);
+    expect(Ok, stat);
+
+    stat = GdipGetImageGraphicsContext((GpImage*)metafile, &graphics);
+    expect(Ok, stat);
+
+    stat = GdipCreateBitmapFromScan0(1, 1, 0, PixelFormat32bppARGB, NULL, &bitmap);
+    expect(Ok, stat);
+
+    stat = GdipDrawImage(graphics, (GpImage*)bitmap, 0.0f, 0.0f);
+    expect(Ok, stat);
+
+    stat = GdipDisposeImage((GpImage*)bitmap);
+    expect(Ok, stat);
+    stat = GdipDeleteGraphics(graphics);
+    expect(Ok, stat);
+
+    stat = GdipCreateFromHDC(hdc, &graphics);
+    expect(Ok, stat);
+
+    ctx.metafile = metafile;
+    ctx.record = record;
+    ctx.executed = FALSE;
+
+    stat = GdipEnumerateMetafileDestRect(graphics, metafile, &frame,
+        play_drawimage_proc, &ctx, NULL);
+    expect(Ok, stat);
+    expect(TRUE, ctx.executed);
+
+    GdipDeleteGraphics(graphics);
+    GdipDisposeImage((GpImage*)metafile);
+    DeleteDC(hdc);
+}
+
+static void test_drawimage_record(void) {
+    DrawImageRecord record;
+
+    record.Header.Type = EmfPlusRecordTypeDrawImage;
+    record.Header.Flags = 0;
+    record.Header.Size = sizeof(record);
+    record.Header.DataSize = sizeof(record) - sizeof(EmfPlusRecordHeader);
+
+    record.ImageAttributesID = (DWORD)-1;
+    record.SrcUnit = UnitPixel;
+    record.SrcRect.X = 0.0f;
+    record.SrcRect.Y = 0.0f;
+    record.SrcRect.Width = 100.0f;
+    record.SrcRect.Height = 100.0f;
+    record.rectF.X = 0.0f;
+    record.rectF.Y = 0.0f;
+    record.rectF.Width = 100.0f;
+    record.rectF.Height = 100.0f;
+
+    check_play_drawimage((DrawImageBaseRecord*)&record);
+}
+
+static void test_drawimagepoints_record(void) {
+    DrawImagePointsRecord record;
+
+    record.Header.Type = EmfPlusRecordTypeDrawImagePoints;
+    record.Header.Flags = 0;
+    record.Header.Size = sizeof(record);
+    record.Header.DataSize = sizeof(record) - sizeof(EmfPlusRecordHeader);
+
+    record.ImageAttributesID = (DWORD)-1;
+    record.SrcUnit = UnitPixel;
+    record.SrcRect.X = 0.0f;
+    record.SrcRect.Y = 0.0f;
+    record.SrcRect.Width = 100.0f;
+    record.SrcRect.Height = 100.0f;
+
+    record.count = 3;
+    record.pointsF[0].X = 0.0f;
+    record.pointsF[0].Y = 0.0f;
+    record.pointsF[1].X = 100.0f;
+    record.pointsF[1].Y = 0.0f;
+    record.pointsF[2].X = 0.0f;
+    record.pointsF[2].Y = 100.0f;
+
+    check_play_drawimage((DrawImageBaseRecord*)&record);
+}
+
 static const emfplus_record properties_records[] = {
     { EMR_HEADER },
     { EmfPlusRecordTypeHeader },
@@ -4041,6 +4207,8 @@ START_TEST(metafile)
     test_clipping();
     test_gditransform();
     test_drawimage();
+    test_drawimage_record();
+    test_drawimagepoints_record();
     test_properties();
     test_drawpath();
     test_fillpath();
-- 
GitLab


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

[PATCH v3 2/2] gdiplus/metafile: Allow drawimage w/o attributes

[Tobiasz Laskowski]

From: Tobiasz Laskowski <[e-mail]>

As stated in the documentation, the attribute id is optional:
https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-emfplus/778...
---
 dlls/gdiplus/metafile.c       | 14 ++++++++++----
 dlls/gdiplus/tests/metafile.c |  4 ----
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/dlls/gdiplus/metafile.c b/dlls/gdiplus/metafile.c
index 83484b9a00e..b9323559582 100644
--- a/dlls/gdiplus/metafile.c
+++ b/dlls/gdiplus/metafile.c
@@ -3218,6 +3218,7 @@ GpStatus WINGDIPAPI GdipPlayMetafileRecord(GDIPCONST GpMetafile *metafile,
         {
             EmfPlusDrawImage *draw = (EmfPlusDrawImage *)header;
             BYTE image = flags & 0xff;
+            GpImageAttributes *attributes;
             GpPointF points[3];
 
             if (image >= EmfPlusObjectTableSize || real_metafile->objtable[image].type != ObjectTypeImage)
@@ -3229,7 +3230,9 @@ GpStatus WINGDIPAPI GdipPlayMetafileRecord(GDIPCONST GpMetafile *metafile,
 
             if (draw->ImageAttributesID >= EmfPlusObjectTableSize ||
                     real_metafile->objtable[draw->ImageAttributesID].type != ObjectTypeImageAttributes)
-                return InvalidParameter;
+                attributes = NULL;
+            else
+                attributes = real_metafile->objtable[draw->ImageAttributesID].u.image_attributes;
 
             if (flags & 0x4000) /* C */
             {
@@ -3252,7 +3255,7 @@ GpStatus WINGDIPAPI GdipPlayMetafileRecord(GDIPCONST GpMetafile *metafile,
 
             return GdipDrawImagePointsRect(real_metafile->playback_graphics, real_metafile->objtable[image].u.image,
                 points, 3, draw->SrcRect.X, draw->SrcRect.Y, draw->SrcRect.Width, draw->SrcRect.Height, draw->SrcUnit,
-                real_metafile->objtable[draw->ImageAttributesID].u.image_attributes, NULL, NULL);
+                attributes, NULL, NULL);
         }
         case EmfPlusRecordTypeDrawImagePoints:
         {
@@ -3260,6 +3263,7 @@ GpStatus WINGDIPAPI GdipPlayMetafileRecord(GDIPCONST GpMetafile *metafile,
             static const UINT fixed_part_size = FIELD_OFFSET(EmfPlusDrawImagePoints, PointData) -
                 FIELD_OFFSET(EmfPlusDrawImagePoints, ImageAttributesID);
             BYTE image = flags & 0xff;
+            GpImageAttributes* attributes;
             GpPointF points[3];
             unsigned int i;
             UINT size;
@@ -3273,7 +3277,9 @@ GpStatus WINGDIPAPI GdipPlayMetafileRecord(GDIPCONST GpMetafile *metafile,
 
             if (draw->ImageAttributesID >= EmfPlusObjectTableSize ||
                     real_metafile->objtable[draw->ImageAttributesID].type != ObjectTypeImageAttributes)
-                return InvalidParameter;
+                attributes = NULL;
+            else
+                attributes = real_metafile->objtable[draw->ImageAttributesID].u.image_attributes;
 
             if (draw->count != 3)
                 return InvalidParameter;
@@ -3314,7 +3320,7 @@ GpStatus WINGDIPAPI GdipPlayMetafileRecord(GDIPCONST GpMetafile *metafile,
 
             return GdipDrawImagePointsRect(real_metafile->playback_graphics, real_metafile->objtable[image].u.image,
                 points, 3, draw->SrcRect.X, draw->SrcRect.Y, draw->SrcRect.Width, draw->SrcRect.Height, draw->SrcUnit,
-                real_metafile->objtable[draw->ImageAttributesID].u.image_attributes, NULL, NULL);
+                attributes, NULL, NULL);
         }
         case EmfPlusRecordTypeFillPath:
         {
diff --git a/dlls/gdiplus/tests/metafile.c b/dlls/gdiplus/tests/metafile.c
index 3cee0695667..b0315acbf98 100644
--- a/dlls/gdiplus/tests/metafile.c
+++ b/dlls/gdiplus/tests/metafile.c
@@ -2700,21 +2700,18 @@ static BOOL CALLBACK play_drawimage_proc(EmfPlusRecordType recordType, UINT flag
         stat = GdipPlayMetafileRecord(ctx->metafile, ctx->record->Header.Type,
             ctx->record->Header.Flags, ctx->record->Header.DataSize,
             (const BYTE*)&ctx->record->ImageAttributesID);
-        todo_wine
         expect(Ok, stat);
 
         ctx->record->ImageAttributesID = (DWORD)-2;
         stat = GdipPlayMetafileRecord(ctx->metafile, ctx->record->Header.Type,
             ctx->record->Header.Flags, ctx->record->Header.DataSize,
             (const BYTE*)&ctx->record->ImageAttributesID);
-        todo_wine
         expect(Ok, stat);
 
         ctx->record->ImageAttributesID = (DWORD)64;
         stat = GdipPlayMetafileRecord(ctx->metafile, ctx->record->Header.Type,
             ctx->record->Header.Flags, ctx->record->Header.DataSize,
             (const BYTE*)&ctx->record->ImageAttributesID);
-        todo_wine
         expect(Ok, stat);
 
         /* Within bounds, but missing */
@@ -2722,7 +2719,6 @@ static BOOL CALLBACK play_drawimage_proc(EmfPlusRecordType recordType, UINT flag
         stat = GdipPlayMetafileRecord(ctx->metafile, ctx->record->Header.Type,
             ctx->record->Header.Flags, ctx->record->Header.DataSize,
             (const BYTE*)&ctx->record->ImageAttributesID);
-        todo_wine
         expect(Ok, stat);
     }
 
-- 
GitLab

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

[PATCH v3 0/2] MR11714: gdiplus/metafile: Allow drawimage w/o attributes - approved

[Esme Povirk (＠madewokherd)]

This merge request was approved by Esme Povirk.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/11714