[PATCH v12 0/3] MR1164: gdiplus: add initial CompoundLine support to fix application crash

[Bartosz Kosiorek (＠gang65)]

Wine-bug: https://bugs.winehq.org/show_bug.cgi?id=52196

--
  v12: gdiplus: add GdipGetPenCompoundArray implementation
       gdiplus: add GdipGetPenCompoundCount implementation
       gdiplus: add GdipSetPenCompoundArray implementation

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

[PATCH v12 1/3] gdiplus: add GdipSetPenCompoundArray implementation

[Bartosz Kosiorek]

From: Bartosz Kosiorek

Wine-bug: https://bugs.winehq.org/show_bug.cgi?id=52196
---
 dlls/gdiplus/gdiplus_private.h |  2 ++
 dlls/gdiplus/graphicspath.c    |  3 +++
 dlls/gdiplus/pen.c             | 27 +++++++++++++++++++++++----
 dlls/gdiplus/tests/pen.c       | 15 ++++++++++++++-
 4 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/dlls/gdiplus/gdiplus_private.h b/dlls/gdiplus/gdiplus_private.h
index 03bfa5513cb..0c55af6614a 100644
--- a/dlls/gdiplus/gdiplus_private.h
+++ b/dlls/gdiplus/gdiplus_private.h
@@ -235,6 +235,8 @@ struct GpPen{
     GpBrush *brush;
     GpPenAlignment align;
     GpMatrix transform;
+    REAL *compound_array;
+    INT compound_array_size;
 };

struct GpGraphics{
diff --git a/dlls/gdiplus/graphicspath.c b/dlls/gdiplus/graphicspath.c
index ce2666eedab..d98a5ae4bf6 100644
--- a/dlls/gdiplus/graphicspath.c
+++ b/dlls/gdiplus/graphicspath.c
@@ -2356,6 +2356,9 @@ GpStatus WINGDIPAPI GdipWidenPath(GpPath *path, GpPen *pen, GpMatrix *matrix,
         if (pen->align != PenAlignmentCenter)
             FIXME("unimplemented pen alignment %d\n", pen->align);

+        if (pen->compound_array_size != 0)
+            FIXME("unimplemented pen compoundline. Solid line will be drawn instead: %d\n", pen->compound_array_size);
+
         for (i=0; i < flat_path->pathdata.Count; i++)
         {
             if ((types[i]&PathPointTypePathTypeMask) == PathPointTypeStart)
diff --git a/dlls/gdiplus/pen.c b/dlls/gdiplus/pen.c
index 32579adc484..162b73ee64e 100644
--- a/dlls/gdiplus/pen.c
+++ b/dlls/gdiplus/pen.c
@@ -171,6 +171,8 @@ GpStatus WINGDIPAPI GdipCreatePen2(GpBrush *brush, REAL width, GpUnit unit,
     gp_pen->offset = 0.0;
     gp_pen->customstart = NULL;
     gp_pen->customend = NULL;
+    gp_pen->compound_array = NULL;
+    gp_pen->compound_array_size = 0;
     GdipSetMatrixElements(&gp_pen->transform, 1.0, 0.0, 0.0, 1.0, 0.0, 0.0);

if(!((gp_pen->unit == UnitWorld) || (gp_pen->unit == UnitPixel))) {
@@ -198,6 +200,7 @@ GpStatus WINGDIPAPI GdipDeletePen(GpPen *pen)
     GdipDeleteBrush(pen->brush);
     GdipDeleteCustomLineCap(pen->customstart);
     GdipDeleteCustomLineCap(pen->customend);
+    heap_free(pen->compound_array);
     heap_free(pen->dashes);
     heap_free(pen);

@@ -533,15 +536,31 @@ GpStatus WINGDIPAPI GdipGetPenCompoundCount(GpPen *pen, INT *count)
     return NotImplemented;
 }

-GpStatus WINGDIPAPI GdipSetPenCompoundArray(GpPen *pen, GDIPCONST REAL *dash,
+GpStatus WINGDIPAPI GdipSetPenCompoundArray(GpPen *pen, GDIPCONST REAL *compoundarray,
     INT count)
 {
-    FIXME("(%p, %p, %i): stub\n", pen, dash, count);
+    INT i;
+    TRACE("(%p, %p, %i)\n", pen, compoundarray, count);

-    if (!pen || !dash || count < 2 || count%2 == 1)
+    if(!pen || !compoundarray || count < 2 || count%2 == 1 || *compoundarray < 0.0 || *compoundarray > 1.0)
         return InvalidParameter;

-    return NotImplemented;
+    for(i = 1; i<count; i++)
+    {
+        if((compoundarray[i] < compoundarray[i - 1]) || (compoundarray[i] > 1.0))
+            return InvalidParameter;
+    }
+
+    heap_free(pen->compound_array);
+    pen->compound_array = heap_alloc_zero(count * sizeof(REAL));
+    if(!pen->compound_array)
+    {
+        pen->compound_array_size = 0;
+        return OutOfMemory;
+    }
+    memcpy(pen->compound_array, compoundarray, count * sizeof(REAL));
+    pen->compound_array_size = count;
+    return Ok;
 }

GpStatus WINGDIPAPI GdipSetPenCustomEndCap(GpPen *pen, GpCustomLineCap* customCap)
diff --git a/dlls/gdiplus/tests/pen.c b/dlls/gdiplus/tests/pen.c
index cd3e45f9b41..e46859235f1 100644
--- a/dlls/gdiplus/tests/pen.c
+++ b/dlls/gdiplus/tests/pen.c
@@ -351,6 +351,9 @@ static void test_compoundarray(void)
     GpStatus status;
     GpPen *pen;
     static const REAL testvalues[] = {0.2, 0.4, 0.6, 0.8};
+    static const REAL notSortedValues[] = {0.2, 0.6, 0.4, 0.8};
+    static const REAL negativeValues[] = {-1.2, 0.4, 0.6, 0.8};
+    static const REAL tooLargeValues[] = {0.2, 0.4, 0.6, 2.8};
     INT count;

status = GdipSetPenCompoundArray(NULL, testvalues, 4);
@@ -382,8 +385,18 @@ todo_wine {
     status = GdipSetPenCompoundArray(pen, testvalues, -2);
     expect(InvalidParameter, status);

+    status = GdipSetPenCompoundArray(pen, notSortedValues, 4);
+    expect(InvalidParameter, status);
+    status = GdipSetPenCompoundArray(pen, negativeValues, 4);
+    expect(InvalidParameter, status);
+    status = GdipSetPenCompoundArray(pen, tooLargeValues, 4);
+    expect(InvalidParameter, status);
+
     status = GdipSetPenCompoundArray(pen, testvalues, 4);
-    todo_wine expect(Ok, status);
+    expect(Ok, status);
+    // If compound count is negative, return Ok and don't update compound
+    status = GdipSetPenCompoundArray(pen, testvalues, -10);
+    expect(Ok, status);
     status = GdipSetPenCompoundArray(pen, NULL, 0);
     expect(InvalidParameter, status);

[PATCH v12 2/3] gdiplus: add GdipGetPenCompoundCount implementation

[Bartosz Kosiorek]

From: Bartosz Kosiorek

Wine-bug: https://bugs.winehq.org/show_bug.cgi?id=52196
---
 dlls/gdiplus/pen.c       | 6 +++---
 dlls/gdiplus/tests/pen.c | 3 +--
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/dlls/gdiplus/pen.c b/dlls/gdiplus/pen.c
index 162b73ee64e..fb53fe9889e 100644
--- a/dlls/gdiplus/pen.c
+++ b/dlls/gdiplus/pen.c
@@ -528,12 +528,12 @@ GpStatus WINGDIPAPI GdipSetPenColor(GpPen *pen, ARGB argb)

GpStatus WINGDIPAPI GdipGetPenCompoundCount(GpPen *pen, INT *count)
 {
-    FIXME("(%p, %p): stub\n", pen, count);
+    TRACE("(%p, %p)\n", pen, count);

if (!pen || !count)
         return InvalidParameter;
-
-    return NotImplemented;
+    *count = pen->compound_array_size;
+    return Ok;
 }

GpStatus WINGDIPAPI GdipSetPenCompoundArray(GpPen *pen, GDIPCONST REAL *compoundarray,
diff --git a/dlls/gdiplus/tests/pen.c b/dlls/gdiplus/tests/pen.c
index e46859235f1..495fe76b4aa 100644
--- a/dlls/gdiplus/tests/pen.c
+++ b/dlls/gdiplus/tests/pen.c
@@ -370,10 +370,9 @@ static void test_compoundarray(void)

count = 10;
     status = GdipGetPenCompoundCount(pen, &count);
-todo_wine {
     expect(Ok, status);
     ok(count == 0, "Unexpected compound count %d\n", count);
-}
+
     status = GdipSetPenCompoundArray(pen, NULL, 0);
     expect(InvalidParameter, status);
     status = GdipSetPenCompoundArray(pen, NULL, 4);

[PATCH v12 3/3] gdiplus: add GdipGetPenCompoundArray implementation

[Bartosz Kosiorek]

From: Bartosz Kosiorek

Wine-bug: https://bugs.winehq.org/show_bug.cgi?id=52196
---
 dlls/gdiplus/gdiplus.spec |  2 +-
 dlls/gdiplus/pen.c        | 11 +++++++++++
 dlls/gdiplus/tests/pen.c  | 20 ++++++++++++++++++--
 include/gdiplusflat.h     |  1 +
 4 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/dlls/gdiplus/gdiplus.spec b/dlls/gdiplus/gdiplus.spec
index 15d9b5d81e8..d4545abf319 100644
--- a/dlls/gdiplus/gdiplus.spec
+++ b/dlls/gdiplus/gdiplus.spec
@@ -334,7 +334,7 @@
 334 stdcall GdipGetPathWorldBoundsI(ptr ptr ptr ptr)
 335 stdcall GdipGetPenBrushFill(ptr ptr)
 336 stdcall GdipGetPenColor(ptr ptr)
-337 stub GdipGetPenCompoundArray
+337 stdcall GdipGetPenCompoundArray(ptr ptr long)
 338 stdcall GdipGetPenCompoundCount(ptr ptr)
 339 stdcall GdipGetPenCustomEndCap(ptr ptr)
 340 stdcall GdipGetPenCustomStartCap(ptr ptr)
diff --git a/dlls/gdiplus/pen.c b/dlls/gdiplus/pen.c
index fb53fe9889e..b3098c618c1 100644
--- a/dlls/gdiplus/pen.c
+++ b/dlls/gdiplus/pen.c
@@ -526,6 +526,17 @@ GpStatus WINGDIPAPI GdipSetPenColor(GpPen *pen, ARGB argb)
     return GdipSetSolidFillColor(((GpSolidFill*)pen->brush), argb);
 }

+GpStatus WINGDIPAPI GdipGetPenCompoundArray(GpPen *pen, REAL *compoundarray, INT count)
+{
+    TRACE("(%p, %p, %i)\n", pen, compoundarray, count);
+
+    if (!pen || !compoundarray || count > pen->compound_array_size)
+        return InvalidParameter;
+    if (pen->compound_array && count)
+        memcpy(compoundarray, pen->compound_array, count * sizeof(REAL));
+    return Ok;
+}
+
 GpStatus WINGDIPAPI GdipGetPenCompoundCount(GpPen *pen, INT *count)
 {
     TRACE("(%p, %p)\n", pen, count);
diff --git a/dlls/gdiplus/tests/pen.c b/dlls/gdiplus/tests/pen.c
index 495fe76b4aa..4f398bf7027 100644
--- a/dlls/gdiplus/tests/pen.c
+++ b/dlls/gdiplus/tests/pen.c
@@ -350,6 +350,7 @@ static void test_compoundarray(void)
 {
     GpStatus status;
     GpPen *pen;
+    REAL *returnvalues;
     static const REAL testvalues[] = {0.2, 0.4, 0.6, 0.8};
     static const REAL notSortedValues[] = {0.2, 0.6, 0.4, 0.8};
     static const REAL negativeValues[] = {-1.2, 0.4, 0.6, 0.8};
@@ -401,10 +402,25 @@ static void test_compoundarray(void)

count = 0;
     status = GdipGetPenCompoundCount(pen, &count);
-todo_wine {
     expect(Ok, status);
     ok(count == 4, "Unexpected compound count %d\n", count);
-}
+
+    returnvalues = calloc(5, sizeof(REAL));
+    // When count larger than stored array return error
+    status = GdipGetPenCompoundArray(pen, returnvalues, 40);
+    expect(InvalidParameter, status);
+    status = GdipGetPenCompoundArray(NULL, returnvalues, 4);
+    expect(InvalidParameter, status);
+    // When count is zero, it should do nothing
+    status = GdipGetPenCompoundArray(pen, returnvalues, 0);
+    expect(Ok, status);
+    ok(returnvalues[0] == 0.0, "Unexpected compound array %f\n", returnvalues[0]);
+
+    status = GdipGetPenCompoundArray(pen, returnvalues, 4);
+    expect(Ok, status);
+    ok(memcmp(returnvalues, testvalues, 4 * sizeof(REAL)) == 0, "Unexpected compound array\n");
+
+    free(returnvalues);
     GdipDeletePen(pen);
 }

diff --git a/include/gdiplusflat.h b/include/gdiplusflat.h
index 5c9600ed30b..10f4d435a85 100644
--- a/include/gdiplusflat.h
+++ b/include/gdiplusflat.h
@@ -629,6 +629,7 @@ GpStatus WINGDIPAPI GdipCreatePen2(GpBrush*,REAL,GpUnit,GpPen**);
 GpStatus WINGDIPAPI GdipDeletePen(GpPen*);
 GpStatus WINGDIPAPI GdipGetPenBrushFill(GpPen*,GpBrush**);
 GpStatus WINGDIPAPI GdipGetPenColor(GpPen*,ARGB*);
+GpStatus WINGDIPAPI GdipGetPenCompoundArray(GpPen*,REAL*,INT);
 GpStatus WINGDIPAPI GdipGetPenCompoundCount(GpPen*,INT*);
 GpStatus WINGDIPAPI GdipGetPenCustomStartCap(GpPen*,GpCustomLineCap**);
 GpStatus WINGDIPAPI GdipGetPenCustomEndCap(GpPen*,GpCustomLineCap**);

[PATCH v12 3/3] gdiplus: add GdipGetPenCompoundArray implementation

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=125519

Your paranoid android.

=== w7u_2qxl (32 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w7u_adm (32 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w7u_el (32 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w8 (32 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w8adm (32 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w864 (32 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w1064v1507 (32 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w1064v1809 (32 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w1064 (32 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w1064_tsign (32 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w10pro64 (32 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w864 (64 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w1064v1507 (64 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w1064v1809 (64 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w1064 (64 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w1064_2qxl (64 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w1064_adm (64 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w1064_tsign (64 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w10pro64 (64 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w10pro64_en_AE_u8 (64 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w10pro64_ar (64 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w10pro64_ja (64 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== w10pro64_zh_CN (64 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== debian11 (32 bit report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== debian11 (32 bit ar:MA report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== debian11 (32 bit de report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== debian11 (32 bit fr report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== debian11 (32 bit he:IL report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== debian11 (32 bit hi:IN report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== debian11 (32 bit ja:JP report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

=== debian11 (32 bit zh:CN report) ===

gdiplus:
pen.c:399: Test failed: Expected 00000000, got 00000002

[Bartosz Kosiorek (＠gang65)]

On Fri Oct 28 18:30:23 2022 +0000, Esme Povirk wrote:
> I've been told in the past that an old array shouldn't be freed until we
> have successfully allocated a new array to replace it.

My initial implementation was different, but I would like to keep consistency with:
`GdipSetPenDashArray function.`