[PATCH v4 0/1] MR4165: gdiplus: Fix transformation in GdipIsOutlineVisiblePathPoint

[Bartosz Kosiorek (＠gang65)]

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

--
  v4: gdiplus: Fix transformation in GdipIsOutlineVisiblePathPoint

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

[PATCH v4 1/1] gdiplus: Fix transformation in GdipIsOutlineVisiblePathPoint

[Bartosz Kosiorek]

From: Bartosz Kosiorek

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55717
---
 dlls/gdiplus/graphicspath.c       |  18 ++--
 dlls/gdiplus/tests/graphicspath.c | 143 ++++++++++++++++++++++++++++++
 2 files changed, 149 insertions(+), 12 deletions(-)

diff --git a/dlls/gdiplus/graphicspath.c b/dlls/gdiplus/graphicspath.c
index 8e436f96f4e..54234ea0bd6 100644
--- a/dlls/gdiplus/graphicspath.c
+++ b/dlls/gdiplus/graphicspath.c
@@ -1728,9 +1728,10 @@ GpStatus WINGDIPAPI GdipIsOutlineVisiblePathPoint(GpPath* path, REAL x, REAL y,
 {
     GpStatus stat;
     GpPath *wide_path;
+    GpPointF pt = {x, y};
     GpMatrix *transform = NULL;

-    TRACE("(%p,%0.2f,%0.2f,%p,%p,%p)\n", path, x, y, pen, graphics, result);
+    TRACE("(%p, %0.2f, %0.2f, %p, %p, %p)\n", path, x, y, pen, graphics, result);

if(!path || !pen)
         return InvalidParameter;
@@ -1747,22 +1748,15 @@ GpStatus WINGDIPAPI GdipIsOutlineVisiblePathPoint(GpPath* path, REAL x, REAL y,
         if (stat == Ok)
             stat = get_graphics_transform(graphics, CoordinateSpaceDevice,
                 CoordinateSpaceWorld, transform);
+        if (stat == Ok)
+            GdipTransformMatrixPoints(transform, &pt, 1);
     }

if (stat == Ok)
-        stat = GdipWidenPath(wide_path, pen, transform, 1.0);
-
-    if (pen->unit == UnitPixel && graphics != NULL)
-    {
-        if (stat == Ok)
-            stat = GdipInvertMatrix(transform);
-
-        if (stat == Ok)
-            stat = GdipTransformPath(wide_path, transform);
-    }
+        stat = GdipWidenPath(wide_path, pen, transform, 0.25f);

if (stat == Ok)
-        stat = GdipIsVisiblePathPoint(wide_path, x, y, graphics, result);
+        stat = GdipIsVisiblePathPoint(wide_path, pt.X, pt.Y, graphics, result);

GdipDeleteMatrix(transform);

diff --git a/dlls/gdiplus/tests/graphicspath.c b/dlls/gdiplus/tests/graphicspath.c
index 908ca8bcaf0..89c529555dc 100644
--- a/dlls/gdiplus/tests/graphicspath.c
+++ b/dlls/gdiplus/tests/graphicspath.c
@@ -1847,6 +1847,7 @@ static void test_isvisible(void)
     status = GdipIsVisiblePathPoint(path, 0.0, 0.0, NULL, &result);
     expect(Ok, status);
     expect(FALSE, result);
+
     /* rect */
     status = GdipAddPathRectangle(path, 0.0, 0.0, 10.0, 10.0);
     expect(Ok, status);
@@ -1883,6 +1884,147 @@ static void test_isvisible(void)
     ReleaseDC(0, hdc);
 }

+static void test_is_outline_visible_path_point(void)
+{
+    BOOL result;
+    GpBitmap *bitmap;
+    GpGraphics *graphics = NULL;
+    GpPath *path;
+    GpPen *pen = NULL;
+    GpStatus status;
+    static const int width = 20, height = 20;
+
+    /* Graphics associated with an Image object.*/
+    status = GdipCreateBitmapFromScan0(width, height, 0, PixelFormat32bppRGB, NULL, &bitmap);
+    expect(Ok, status);
+    status = GdipGetImageGraphicsContext((GpImage *)bitmap, &graphics);
+    expect(Ok, status);
+    ok(graphics != NULL, "Expected the graphics context to be initialized.\n");
+
+    status = GdipCreatePath(FillModeAlternate, &path);
+    expect(Ok, status);
+
+    status = GdipAddPathRectangle(path, 2.0, 0.0, 13.0, 15.0);
+    expect(Ok, status);
+
+    status = GdipCreatePen1((ARGB)0xffff00ff, 3.0f, UnitPixel, &pen);
+    expect(Ok, status);
+    ok(pen != NULL, "Expected pen to be initialized\n");
+
+    /* With NULL pen */
+    result = 9;
+    status = GdipIsOutlineVisiblePathPoint(path, 0.0, 1.0, NULL, graphics, &result);
+    expect(InvalidParameter, status);
+    expect(9, result);
+
+    /* Without transformation */
+    result = TRUE;
+    status = GdipIsOutlineVisiblePathPoint(path, 0.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(FALSE, result);
+    result = FALSE;
+    status = GdipIsOutlineVisiblePathPoint(path, 1.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(TRUE, result);
+    result = FALSE;
+    status = GdipIsOutlineVisiblePathPoint(path, 10.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(TRUE, result);
+    result = FALSE;
+    status = GdipIsOutlineVisiblePathPoint(path, 16.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(TRUE, result);
+    result = TRUE;
+    status = GdipIsOutlineVisiblePathPoint(path, 17.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(FALSE, result);
+
+    /* Translating */
+    status = GdipTranslateWorldTransform(graphics, 50.0, 50.0, MatrixOrderPrepend);
+    expect(Ok, status);
+    result = FALSE;
+    status = GdipIsOutlineVisiblePathPoint(path, 10.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(TRUE, result);
+    result = FALSE;
+    status = GdipIsOutlineVisiblePathPoint(path, 15.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(TRUE, result);
+    result = FALSE;
+    status = GdipIsOutlineVisiblePathPoint(path, 16.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(TRUE, result);
+
+    /* Scaling */
+    status = GdipScaleWorldTransform(graphics, 2.0, 2.0, MatrixOrderPrepend);
+    expect(Ok, status);
+    result = TRUE;
+    status = GdipIsOutlineVisiblePathPoint(path, 0.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(FALSE, result);
+    result = TRUE;
+    status = GdipIsOutlineVisiblePathPoint(path, 1.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(FALSE, result);
+    result = FALSE;
+    status = GdipIsOutlineVisiblePathPoint(path, 2.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(TRUE, result);
+    result = TRUE;
+    status = GdipIsOutlineVisiblePathPoint(path, 3.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(FALSE, result);
+    result = TRUE;
+    status = GdipIsOutlineVisiblePathPoint(path, 14.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(FALSE, result);
+    result = FALSE;
+    status = GdipIsOutlineVisiblePathPoint(path, 15.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(TRUE, result);
+    result = TRUE;
+    status = GdipIsOutlineVisiblePathPoint(path, 16.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(FALSE, result);
+
+    /* Page Unit */
+    GdipResetWorldTransform(graphics);
+    status = GdipSetPageUnit(graphics, UnitMillimeter);
+    expect(Ok, status);
+    result = TRUE;
+    status = GdipIsOutlineVisiblePathPoint(path, 0.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(FALSE, result);
+    result = TRUE;
+    status = GdipIsOutlineVisiblePathPoint(path, 1.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(FALSE, result);
+    result = FALSE;
+    status = GdipIsOutlineVisiblePathPoint(path, 2.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(TRUE, result);
+    result = TRUE;
+    status = GdipIsOutlineVisiblePathPoint(path, 3.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(FALSE, result);
+    result = TRUE;
+    status = GdipIsOutlineVisiblePathPoint(path, 14.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(FALSE, result);
+    result = FALSE;
+    status = GdipIsOutlineVisiblePathPoint(path, 15.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(TRUE, result);
+    result = TRUE;
+    status = GdipIsOutlineVisiblePathPoint(path, 16.0, 1.0, pen, graphics, &result);
+    expect(Ok, status);
+    expect(FALSE, result);
+
+    GdipResetWorldTransform(graphics);
+    GdipDeletePath(path);
+    GdipDeleteGraphics(graphics);
+}
+
 static void test_empty_rect(void)
 {
     GpPath *path;
@@ -1976,6 +2118,7 @@ START_TEST(graphicspath)
     test_widen();
     test_widen_cap();
     test_isvisible();
+    test_is_outline_visible_path_point();
     test_empty_rect();

GdiplusShutdown(gdiplusToken);

[Esme Povirk (＠madewokherd)]

This makes sense, but it doesn't seem to fix the Mono test.

[Jeffrey Smith (＠whydoubt)]

I think this is a case where it would be helpful to add the tests in one commit, and the implementation change in another, to make it clear which tests the changes fix.

There also seems to be a lot of repetition in the test that could be reduced.