[PATCH v2 0/2] MR4638: gdiplus: Partially implement GdipDrawImageFX.

[Jinoh Kang (＠iamahuman)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55945

Based on https://gitlab.winehq.org/gang65/wine/-/merge_requests/12.

cc/ @gang65

--
  v2: gdiplus: Partially implement GdipDrawImageFX.

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

[PATCH v2 1/2] gdiplus/tests: Add GdipDrawImageFX tests except effects or attributes.

[Bartosz Kosiorek]

From: Bartosz Kosiorek

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

diff --git a/dlls/gdiplus/tests/graphics.c b/dlls/gdiplus/tests/graphics.c
index d71bc78c2ae..6d7f0ec1ea8 100644
--- a/dlls/gdiplus/tests/graphics.c
+++ b/dlls/gdiplus/tests/graphics.c
@@ -1307,6 +1307,68 @@ static void test_GdipDrawLineI(void)
     ReleaseDC(hwnd, hdc);
 }

+static void test_GdipDrawImageFX(void)
+{
+    GpStatus status;
+    GpRectF source;
+    GpMatrix *transform;
+    GpGraphics *graphics = NULL;
+    GpBitmap *bm = NULL;
+    BYTE buff[400];
+    BITMAPINFOHEADER bmihdr;
+    HDC hdc = GetDC( hwnd );
+    if (!hdc)
+        return;
+
+    memset(&bmihdr, 0, sizeof(bmihdr));
+    bmihdr.biSize = sizeof(BITMAPINFOHEADER);
+    bmihdr.biWidth = 10;
+    bmihdr.biHeight = 10;
+    bmihdr.biPlanes = 1;
+    bmihdr.biBitCount = 32;
+    bmihdr.biCompression = BI_RGB;
+    status = GdipCreateBitmapFromGdiDib((BITMAPINFO*)&bmihdr, buff, &bm);
+    expect(Ok, status);
+    ok(NULL != bm, "Expected bitmap to be initialized\n");
+    status = GdipCreateFromHDC(hdc, &graphics);
+    expect(Ok, status);
+
+    status = GdipCreateMatrix2(2.0, 0.0, 0.0, 1.0, 10.0, 20.0, &transform);
+    expect(Ok, status);
+
+    /* DrawImageFX with source rectangle */
+    status = GdipDrawImageFX(graphics, NULL, &source, NULL, NULL, NULL, UnitPixel);
+    todo_wine
+    expect(InvalidParameter, status);
+
+    /* DrawImageFX with source bitmap */
+    status = GdipDrawImageFX(graphics, (GpImage*)bm, NULL, NULL, NULL, NULL, UnitPixel);
+    todo_wine
+    expect(Ok, status);
+
+    /* DrawImageFX with source bitmap and transform */
+    status = GdipDrawImageFX(graphics, (GpImage*)bm, NULL, transform, NULL, NULL, UnitPixel);
+    todo_wine
+    expect(Ok, status);
+
+    /* DrawImageFX with source bitmap and source rectangle */
+    source.X = source.Y = 0.0;
+    source.Height = source.Width = 10.0;
+
+    status = GdipDrawImageFX(graphics, (GpImage*)bm, &source, NULL, NULL, NULL, UnitPixel);
+    todo_wine
+    expect(Ok, status);
+
+    /* DrawImageFX with source bitmap, source rectangle, and transform */
+    status = GdipDrawImageFX(graphics, (GpImage*)bm, &source, transform, NULL, NULL, UnitPixel);
+    todo_wine
+    expect(Ok, status);
+
+    GdipDisposeImage((GpImage*)bm);
+    GdipDeleteGraphics(graphics);
+    ReleaseDC(hwnd, hdc);
+}
+
 static void test_GdipDrawImagePointsRect(void)
 {
     GpStatus status;
@@ -7395,6 +7457,7 @@ START_TEST(graphics)
     test_GdipDrawCurve3I();
     test_GdipDrawLineI();
     test_GdipDrawLinesI();
+    test_GdipDrawImageFX();
     test_GdipDrawImagePointsRect();
     test_GdipFillClosedCurve();
     test_GdipFillClosedCurveI();

[PATCH v2 2/2] gdiplus: Partially implement GdipDrawImageFX.

[Bartosz Kosiorek]

From: Bartosz Kosiorek

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55945
---
 dlls/gdiplus/graphics.c       | 40 +++++++++++++++++++++++++++++++----
 dlls/gdiplus/tests/graphics.c |  5 -----
 2 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c
index 5101378d0e1..3c43487274f 100644
--- a/dlls/gdiplus/graphics.c
+++ b/dlls/gdiplus/graphics.c
@@ -2947,13 +2947,45 @@ GpStatus WINGDIPAPI GdipDrawImage(GpGraphics *graphics, GpImage *image, REAL x,
                                   0.0, 0.0, (REAL)width, (REAL)height, UnitPixel);
 }

-GpStatus WINGDIPAPI GdipDrawImageFX(GpGraphics *graphics, GpImage *image, GpRectF *src,
+GpStatus WINGDIPAPI GdipDrawImageFX(GpGraphics *graphics, GpImage *image, GpRectF *src_rect,
     GpMatrix *transform, CGpEffect *effect, GpImageAttributes *imageattr,
-    GpUnit srcUnit)
+    GpUnit src_unit)
 {
-    FIXME("(%p, %p, %p, %p, %p, %p, %d): stub\n", graphics, image, src, transform, effect, imageattr, srcUnit);
+    GpRectF src_rect_buf;
+    GpPointF points[3];
+    GpStatus status;

-    return NotImplemented;
+    TRACE("(%p, %p, %p, %p, %p, %p, %d)\n", graphics, image, src_rect, transform, effect, imageattr, src_unit);
+
+    if (!graphics || !image)
+        return InvalidParameter;
+
+    if (effect)
+        FIXME("effect not implemented\n");
+
+    if (!src_rect)
+    {
+        if (image->type != ImageTypeBitmap)
+            FIXME("src rect is NULL and source is not bitmap\n");
+
+        if ((status = GdipGetImageBounds(image, &src_rect_buf, &src_unit)) != Ok)
+            return status;
+
+        src_rect = &src_rect_buf;
+    }
+
+    points[0].X = points[2].X = src_rect->X;
+    points[0].Y = points[1].Y = src_rect->Y;
+    points[1].X = src_rect->X + src_rect->Width;
+    points[2].Y = src_rect->Y + src_rect->Height;
+
+    if (transform)
+        GdipTransformMatrixPoints(transform, points, 3);
+
+    return GdipDrawImagePointsRect(graphics, image, points, 3,
+                                   src_rect->X, src_rect->Y,
+                                   src_rect->Width, src_rect->Height,
+                                   src_unit, imageattr, NULL, NULL);
 }

GpStatus WINGDIPAPI GdipDrawImageI(GpGraphics *graphics, GpImage *image, INT x,
diff --git a/dlls/gdiplus/tests/graphics.c b/dlls/gdiplus/tests/graphics.c
index 6d7f0ec1ea8..fe2ab34f562 100644
--- a/dlls/gdiplus/tests/graphics.c
+++ b/dlls/gdiplus/tests/graphics.c
@@ -1338,17 +1338,14 @@ static void test_GdipDrawImageFX(void)

/* DrawImageFX with source rectangle */
     status = GdipDrawImageFX(graphics, NULL, &source, NULL, NULL, NULL, UnitPixel);
-    todo_wine
     expect(InvalidParameter, status);

/* DrawImageFX with source bitmap */
     status = GdipDrawImageFX(graphics, (GpImage*)bm, NULL, NULL, NULL, NULL, UnitPixel);
-    todo_wine
     expect(Ok, status);

/* DrawImageFX with source bitmap and transform */
     status = GdipDrawImageFX(graphics, (GpImage*)bm, NULL, transform, NULL, NULL, UnitPixel);
-    todo_wine
     expect(Ok, status);

/* DrawImageFX with source bitmap and source rectangle */
@@ -1356,12 +1353,10 @@ static void test_GdipDrawImageFX(void)
     source.Height = source.Width = 10.0;

status = GdipDrawImageFX(graphics, (GpImage*)bm, &source, NULL, NULL, NULL, UnitPixel);
-    todo_wine
     expect(Ok, status);

/* DrawImageFX with source bitmap, source rectangle, and transform */
     status = GdipDrawImageFX(graphics, (GpImage*)bm, &source, transform, NULL, NULL, UnitPixel);
-    todo_wine
     expect(Ok, status);

GdipDisposeImage((GpImage*)bm);

[Jinoh Kang (＠iamahuman)]

> I don't know what should be the naming convention.

We stick to snake case usually for new code.
> We already have `srcUnit`, but here we have `src_argp`. To be aligned, we should gave `srcArgp`.

Good catch! I renamed `srcUnit` to `src_unit`.
> What name `src_argp` means?

It meant argument pointer to source rect, that is overriden by `src_rect` local variable if the pointer is NULL.

I renamed `src_argp` to `src_rect`, and made it hold the final source rect (either the argument as-is or `&src_rect_buf`).

[Esme Povirk (＠madewokherd)]

Esme Povirk (@madewokherd) commented about dlls/gdiplus/tests/graphics.c:
> - status = GdipCreateBitmapFromGdiDib((BITMAPINFO*)&bmihdr, buff, &bm);
> - expect(Ok, status);
> - ok(NULL != bm, "Expected bitmap to be initialized\n");
> - status = GdipCreateFromHDC(hdc, &graphics);
> - expect(Ok, status);
> -
> - status = GdipCreateMatrix2(2.0, 0.0, 0.0, 1.0, 10.0, 20.0, &transform);
> - expect(Ok, status);
> -
> - /* DrawImageFX with source rectangle */
> - status = GdipDrawImageFX(graphics, NULL, &source, NULL, NULL, NULL, UnitPixel);
> - todo_wine
> - expect(InvalidParameter, status);
> -
> - /* DrawImageFX with source bitmap */
> - status = GdipDrawImageFX(graphics, (GpImage*)bm, NULL, NULL, NULL, NULL, UnitPixel);

This is going to use the contents of `buff` uninitialized.

[Esme Povirk (＠madewokherd)]

Esme Povirk (@madewokherd) commented about dlls/gdiplus/tests/graphics.c:
> -
>    return;
> -
> - memset(&bmihdr, 0, sizeof(bmihdr));
> - bmihdr.biSize = sizeof(BITMAPINFOHEADER);
> - bmihdr.biWidth = 10;
> - bmihdr.biHeight = 10;
> - bmihdr.biPlanes = 1;
> - bmihdr.biBitCount = 32;
> - bmihdr.biCompression = BI_RGB;
> - status = GdipCreateBitmapFromGdiDib((BITMAPINFO*)&bmihdr, buff, &bm);
> - expect(Ok, status);
> - ok(NULL != bm, "Expected bitmap to be initialized\n");
> - status = GdipCreateFromHDC(hdc, &graphics);
> - expect(Ok, status);
> -
> - status = GdipCreateMatrix2(2.0, 0.0, 0.0, 1.0, 10.0, 20.0, &transform);

`transform` is never deleted.

[Esme Povirk (＠madewokherd)]

Esme Povirk (@madewokherd) commented about dlls/gdiplus/graphics.c:
> - GpPointF points[3];
> - GpStatus status;
> - return NotImplemented;
> - TRACE("(%p, %p, %p, %p, %p, %p, %d)\n", graphics, image, src_rect, transform, effect, imageattr, src_unit);
> -
> - if (!graphics || !image)
> -
>    return InvalidParameter;
> -
> - if (effect)
> -
>    FIXME("effect not implemented\n");
> -
> - if (!src_rect)
> - {
> -
>    if (image->type != ImageTypeBitmap)
> -
>        FIXME("src rect is NULL and source is not bitmap\n");

Why do we need it to be a bitmap? GdipGetImageBounds works for metafiles.

[Esme Povirk (＠madewokherd)]

Can you set your clone of wine to public? When it's internal, I have to be logged in to access the pipeline results which means I can't just paste the URL into gvim to view it.