[PATCH v5 0/2] MR4165: gdiplus: Fix transformation in GdipIsOutlineVisiblePathPoint

[Bartosz Kosiorek (＠gang65)]

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

--
  v5: gdiplus: Fix transformation in GdipIsOutlineVisiblePathPoint
      gdiplus/test: Add test for GdipIsOutlineVisiblePathPoint
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4165

[PATCH v5 1/2] gdiplus/test: Add test for GdipIsOutlineVisiblePathPoint

[Bartosz Kosiorek]

From: Bartosz Kosiorek <gang65(a)poczta.onet.pl>

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55717
---
 dlls/gdiplus/tests/graphicspath.c | 143 ++++++++++++++++++++++++++++++
 1 file changed, 143 insertions(+)

diff --git a/dlls/gdiplus/tests/graphicspath.c b/dlls/gdiplus/tests/graphicspath.c
index 908ca8bcaf0..ff69e036aaf 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);
+    todo_wine 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);
+    todo_wine 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);
+    todo_wine 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);
+    todo_wine 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);
-- 
GitLab


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

[PATCH v5 2/2] gdiplus: Fix transformation in GdipIsOutlineVisiblePathPoint

[Bartosz Kosiorek]

From: Bartosz Kosiorek <gang65(a)poczta.onet.pl>

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55717
---
 dlls/gdiplus/graphicspath.c       | 18 ++++++------------
 dlls/gdiplus/tests/graphicspath.c |  8 ++++----
 2 files changed, 10 insertions(+), 16 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 ff69e036aaf..89c529555dc 100644
--- a/dlls/gdiplus/tests/graphicspath.c
+++ b/dlls/gdiplus/tests/graphicspath.c
@@ -1965,7 +1965,7 @@ static void test_is_outline_visible_path_point(void)
     result = TRUE;
     status = GdipIsOutlineVisiblePathPoint(path, 1.0, 1.0, pen, graphics, &result);
     expect(Ok, status);
-    todo_wine expect(FALSE, result);
+    expect(FALSE, result);
     result = FALSE;
     status = GdipIsOutlineVisiblePathPoint(path, 2.0, 1.0, pen, graphics, &result);
     expect(Ok, status);
@@ -1977,7 +1977,7 @@ static void test_is_outline_visible_path_point(void)
     result = TRUE;
     status = GdipIsOutlineVisiblePathPoint(path, 14.0, 1.0, pen, graphics, &result);
     expect(Ok, status);
-    todo_wine expect(FALSE, result);
+    expect(FALSE, result);
     result = FALSE;
     status = GdipIsOutlineVisiblePathPoint(path, 15.0, 1.0, pen, graphics, &result);
     expect(Ok, status);
@@ -2002,7 +2002,7 @@ static void test_is_outline_visible_path_point(void)
     result = FALSE;
     status = GdipIsOutlineVisiblePathPoint(path, 2.0, 1.0, pen, graphics, &result);
     expect(Ok, status);
-    todo_wine expect(TRUE, result);
+    expect(TRUE, result);
     result = TRUE;
     status = GdipIsOutlineVisiblePathPoint(path, 3.0, 1.0, pen, graphics, &result);
     expect(Ok, status);
@@ -2014,7 +2014,7 @@ static void test_is_outline_visible_path_point(void)
     result = FALSE;
     status = GdipIsOutlineVisiblePathPoint(path, 15.0, 1.0, pen, graphics, &result);
     expect(Ok, status);
-    todo_wine expect(TRUE, result);
+    expect(TRUE, result);
     result = TRUE;
     status = GdipIsOutlineVisiblePathPoint(path, 16.0, 1.0, pen, graphics, &result);
     expect(Ok, status);
-- 
GitLab

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

[Bartosz Kosiorek (＠gang65)]

On Tue Oct 24 22:06:09 2023 +0000, Jeffrey Smith wrote:
> 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.
> I have splitted the work to tests and implementation.
> Now it is clearly visible what kind of issues were solved with the fix.

@whydoubt Could you please try to reproduce Mono issue under Wine tests?

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

[Bartosz Kosiorek (＠gang65)]

On Wed Oct 25 06:15:55 2023 +0000, Esme Povirk wrote:
> This makes sense, but it doesn't seem to fix the Mono test.
> It seems it is fixing other issue, not covered by Mono tests.
> Can we integrate this changes as a starting point. I would like to continue working on that (first of all the issue needs to be reproduced in Wine).

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

[Jeffrey Smith (＠whydoubt)]

On Tue Oct 24 22:07:30 2023 +0000, Bartosz Kosiorek wrote:
> I have splitted the work to tests and implementation.
> Now it is clearly visible what kind of issues were solved with the fix.
> @whydoubt Could you please try to reproduce Mono issue under Wine tests?
> Unfortunately I have problems with it. It is a starting point for the
> final implementation.
> That is trivial to reproduce.  I am away from my development environment ATM, but I believe it would be this:

``` c
GdipAddPathRectangle(path, 10.0, 0.5, 4.0, 1.0);

GdipScaleWorldTransform(graphics, 2.0, 2.0, MatrixOrderPrepend);
GdipTranslateWorldTransform(graphics, 50.0, -50.0, MatrixOrderPrepend);
GdipSetPageUnit(graphics, UnitMillimeter);
GdipSetPageScale(graphics, 2.0);

GdipIsVisiblePathPoint(path, 10.0, 1.0, graphics, &result);
```

I have created some better, more general tests that I may share on the bug item when I am able.

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

[Jeffrey Smith (＠whydoubt)]

On Wed Oct 25 06:26:43 2023 +0000, Bartosz Kosiorek wrote:
> It seems it is fixing other issue, not covered by Mono tests.
> Can we integrate this changes (it is also improves performance)? 
> I would like to continue working on that (the issue needs to be
> reproduced in Wine).
> Then I don't believe Wine-Bug belongs on the commits.

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

[Esme Povirk (＠madewokherd)]

On Wed Oct 25 12:49:00 2023 +0000, Jeffrey Smith wrote:
> That is trivial to reproduce.  I am away from my development environment
> ATM, but I believe it would be this:
> ``` c
> GdipAddPathRectangle(path, 10.0, 0.5, 4.0, 1.0);
> GdipScaleWorldTransform(graphics, 2.0, 2.0, MatrixOrderPrepend);
> GdipTranslateWorldTransform(graphics, 50.0, -50.0, MatrixOrderPrepend);
> GdipSetPageUnit(graphics, UnitMillimeter);
> GdipSetPageScale(graphics, 2.0);
> GdipIsVisiblePathPoint(path, 10.0, 1.0, graphics, &result);
> ```
> I have created some better, more general tests that I may share on the
> bug item when I am able.
> That may be my fault for giving bad instructions, I just commented on the bug.

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

[Esme Povirk (＠madewokherd)]

On Wed Oct 25 15:50:28 2023 +0000, Esme Povirk wrote:
> That may be my fault for giving bad instructions, I just commented on
> the bug.
> FWIW the full "System.Drawing" test suite can be run with `run-tests System.Drawing`.

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

[Esme Povirk (＠madewokherd)]

On Wed Oct 25 15:54:17 2023 +0000, Esme Povirk wrote:
> FWIW the full "System.Drawing" test suite can be run with `run-tests System.Drawing`.
> Individual test results will only be listed at the end if they differ from the "expected" result, so it'll tell you if you either broke or fixed something.

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

[Esme Povirk (＠madewokherd)]

On Wed Oct 25 12:50:20 2023 +0000, Jeffrey Smith wrote:
> Then I don't believe Wine-Bug belongs on the commits.
> If Wine-Bug: is removed, I think this does make sense.

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

[Esme Povirk (＠madewokherd)]

Esme Povirk (@madewokherd) commented about dlls/gdiplus/tests/graphicspath.c:
> status = GdipIsVisiblePathPoint(path, 0.0, 0.0, NULL, &result);
>      expect(Ok, status);
>      expect(FALSE, result);
> +

Unrelated whitespace change.

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