[PATCH 0/2] MR7661: gdiplus: Fix GdipReversePath for mixed Bezier and Line points in subpath

[Bartosz Kosiorek (＠gang65)]

Fixes Reverse_Pie test in Mono:
https://github.com/madewokherd/mono/blob/develop/mcs/class/System.Drawing/Te...

[PATCH 1/2] gdiplus/test: Add GdipReversePath test for Path Pie

[Bartosz Kosiorek]

From: Bartosz Kosiorek

---
 dlls/gdiplus/tests/graphicspath.c | 60 ++++++++++++++++++++++++++++---
 1 file changed, 56 insertions(+), 4 deletions(-)

diff --git a/dlls/gdiplus/tests/graphicspath.c b/dlls/gdiplus/tests/graphicspath.c
index 81b4b40eed4..af9ee3ba853 100644
--- a/dlls/gdiplus/tests/graphicspath.c
+++ b/dlls/gdiplus/tests/graphicspath.c
@@ -1132,6 +1132,17 @@ static void test_addclosedcurve(void)
     GdipDeletePath(path);
 }

+static path_test_t line_path[] = {
+
+    {0.0,  50.0, PathPointTypeStart, 0, 0}, /*0*/
+    {5.0,  45.0, PathPointTypeLine,  0, 0}, /*1*/
+    {0.0,  40.0, PathPointTypeLine,  0, 0}, /*2*/
+    {15.0, 35.0, PathPointTypeLine | PathPointTypeCloseSubpath, 0, 0}, /*3*/
+    {0.0,  30.0, PathPointTypeStart, 0, 0}, /*4*/
+    {25.0, 25.0, PathPointTypeLine,  0, 0}, /*5*/
+    {0.0,  20.0, PathPointTypeLine,  0, 0} /*6*/
+};
+
 static path_test_t reverse_path[] = {
     {0.0,  20.0, PathPointTypeStart, 0, 0}, /*0*/
     {25.0, 25.0, PathPointTypeLine,  0, 0}, /*1*/
@@ -1140,7 +1151,23 @@ static path_test_t reverse_path[] = {
     {0.0,  40.0, PathPointTypeLine,  0, 0}, /*4*/
     {5.0,  45.0, PathPointTypeLine,  0, 0}, /*5*/
     {0.0,  50.0, PathPointTypeLine | PathPointTypeCloseSubpath, 0, 0}  /*6*/
-    };
+};
+
+static path_test_t pie_path[] = {
+    {2.5,  4.0,  PathPointTypeStart,  0, 0}, /*0*/
+    {3.99, 4.26, PathPointTypeLine,   0, 0}, /*1*/
+    {3.96, 4.44, PathPointTypeBezier, 0, 0}, /*2*/
+    {3.93, 4.62, PathPointTypeBezier, 0, 0}, /*3*/
+    {3.88, 4.79, PathPointTypeBezier | PathPointTypeCloseSubpath, 0, 0}  /*4*/
+};
+
+static path_test_t reverse_pie_path[] = {
+    {3.88, 4.79, PathPointTypeStart,  0, 0}, /*0*/
+    {3.93, 4.62, PathPointTypeBezier, 0, 0}, /*1*/
+    {3.96, 4.44, PathPointTypeBezier, 0, 0}, /*2*/
+    {3.99, 4.26, PathPointTypeBezier, 0, 1}, /*3*/
+    {2.5,  4.0,  PathPointTypeLine | PathPointTypeCloseSubpath, 0, 0}  /*4*/
+};

static void test_reverse(void)
 {
@@ -1164,14 +1191,39 @@ static void test_reverse(void)
     status = GdipReversePath(path);
     expect(Ok, status);

-    GdipAddPathLine2(path, pts, 4);
-    GdipClosePathFigure(path);
-    GdipAddPathLine2(path, &(pts[4]), 3);
+    status = GdipAddPathLine2(path, pts, 4);
+    expect(Ok, status);
+    status = GdipClosePathFigure(path);
+    expect(Ok, status);
+    status = GdipAddPathLine2(path, &(pts[4]), 3);
+    expect(Ok, status);
+    ok_path(path, line_path, ARRAY_SIZE(line_path), FALSE);

status = GdipReversePath(path);
     expect(Ok, status);
     ok_path(path, reverse_path, ARRAY_SIZE(reverse_path), FALSE);

+    /* Double reverse should go back to original Line Path */
+    status = GdipReversePath(path);
+    expect(Ok, status);
+    ok_path(path, line_path, ARRAY_SIZE(line_path), FALSE);
+    GdipDeletePath(path);
+
+    /* Reverse test for Path Pie */
+    status = GdipCreatePath(FillModeAlternate, &path);
+    expect(Ok, status);
+    status = GdipAddPathPie(path, 1.0, 2.0, 3.0, 4.0, 10.0, 20.0);
+    expect(Ok, status);
+    ok_path(path, pie_path, ARRAY_SIZE(pie_path), FALSE);
+
+    status = GdipReversePath(path);
+    expect(Ok, status);
+    ok_path(path, reverse_pie_path, ARRAY_SIZE(reverse_pie_path), FALSE);
+
+    /* Double reverse should go back to original Pie Path*/
+    status = GdipReversePath(path);
+    expect(Ok, status);
+    ok_path(path, pie_path, ARRAY_SIZE(pie_path), FALSE);
     GdipDeletePath(path);
 }

[PATCH 2/2] gdiplus: Fix GdipReversePath for mixed Bezier and Line points in subpath

[Bartosz Kosiorek]

From: Bartosz Kosiorek

Fixes Reverse_Pie test in Mono:
https://github.com/madewokherd/mono/blob/develop/mcs/class/System.Drawing/Te...
---
 dlls/gdiplus/graphicspath.c       | 16 ++++++++++------
 dlls/gdiplus/tests/graphicspath.c |  2 +-
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/dlls/gdiplus/graphicspath.c b/dlls/gdiplus/graphicspath.c
index cbf8e74340b..75f08869818 100644
--- a/dlls/gdiplus/graphicspath.c
+++ b/dlls/gdiplus/graphicspath.c
@@ -1749,20 +1749,24 @@ GpStatus WINGDIPAPI GdipReversePath(GpPath* path)
     }

for(i = 0; i < count; i++){
-
         /* find next start point */
         if(path->pathdata.Types[count-i-1] == PathPointTypeStart){
             INT j;
             for(j = start; j <= i; j++){
+                /* copy points coordinate of subpath in reverse order */
                 revpath.Points[j] = path->pathdata.Points[count-j-1];
-                revpath.Types[j] = path->pathdata.Types[count-j-1];
+                /* copy points type shifted by one (j + 1), as the first point type is always PathPointTypeStart */
+                if (j < i)
+                    revpath.Types[j + 1] = path->pathdata.Types[count - j - 1];
             }
-            /* mark start point */
+            /* first point of subpath is always Start point */
             revpath.Types[start] = PathPointTypeStart;
-            /* set 'figure' endpoint type */
+            /* check if subpath contains more than 1 point */
             if(i-start > 1){
-                revpath.Types[i] = path->pathdata.Types[count-start-1] & ~PathPointTypePathTypeMask;
-                revpath.Types[i] |= revpath.Types[i-1];
+                /* add e.g. PathPointTypeCloseSubpath type to endpoint, if existing in original subpath */
+                revpath.Types[i] |= path->pathdata.Types[count - start - 1] & ~PathPointTypePathTypeMask;
+                /* remove e.g. PathPointTypeCloseSubpath type from second point */
+                revpath.Types[start + 1] &= PathPointTypePathTypeMask;
             }
             else
                 revpath.Types[i] = path->pathdata.Types[start];
diff --git a/dlls/gdiplus/tests/graphicspath.c b/dlls/gdiplus/tests/graphicspath.c
index af9ee3ba853..187d47d1603 100644
--- a/dlls/gdiplus/tests/graphicspath.c
+++ b/dlls/gdiplus/tests/graphicspath.c
@@ -1165,7 +1165,7 @@ static path_test_t reverse_pie_path[] = {
     {3.88, 4.79, PathPointTypeStart,  0, 0}, /*0*/
     {3.93, 4.62, PathPointTypeBezier, 0, 0}, /*1*/
     {3.96, 4.44, PathPointTypeBezier, 0, 0}, /*2*/
-    {3.99, 4.26, PathPointTypeBezier, 0, 1}, /*3*/
+    {3.99, 4.26, PathPointTypeBezier, 0, 0}, /*3*/
     {2.5,  4.0,  PathPointTypeLine | PathPointTypeCloseSubpath, 0, 0}  /*4*/
 };

[Bartosz Kosiorek (＠gang65)]

Bartosz Kosiorek (@gang65) commented about dlls/gdiplus/graphicspath.c:
>          for(j = start; j <= i; j++){
> -
>            /* copy points coordinate of subpath in reverse order */
>            revpath.Points[j] = path->pathdata.Points[count-j-1];
> -
>            revpath.Types[j] = path->pathdata.Types[count-j-1];
> -
>            /* copy points type shifted by one (j + 1), as the first point type is always PathPointTypeStart */
> -
>            if (j < i)
> -
>                revpath.Types[j + 1] = path->pathdata.Types[count - j - 1];
>        }
> -
>        /* mark start point */
> -
>        /* first point of subpath is always Start point */
>        revpath.Types[start] = PathPointTypeStart;
> -
>        /* set 'figure' endpoint type */
> -
>        /* check if subpath contains more than 1 point */
>        if(i-start > 1){
> -
>            revpath.Types[i] = path->pathdata.Types[count-start-1] & ~PathPointTypePathTypeMask;
> -
>            revpath.Types[i] |= revpath.Types[i-1];

This line is a workaround, as it duplicate value of for last point type, from previous point.
Current implementation is fixing that by taking real values of types.

[PATCH 0/2] MR7661: gdiplus: Fix GdipReversePath for mixed Bezier and Line points in subpath - approved

[Esme Povirk (＠madewokherd)]

This merge request was approved by Esme Povirk.