[PATCH v5 0/4] MR4803: dlls/gdiplus: Assorted path changes

[David Kahurani (＠kahurani)]

Signed-off-by: David Kahurani [e-mail]

--
  v5: dlls/gdiplus: Use path_list to path helper in GdipWidenPath.
      dlls/gdiplus: Use path_list to path helper in GdipFlattenPath.
      dlls/gdiplus: Use GdipCreatePath2 when serializing paths
      dlls/gdiplus: Use GdipCreatePath2 in GdipClonePath

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

[PATCH v5 1/4] dlls/gdiplus: Use GdipCreatePath2 in GdipClonePath

[David Kahurani]

From: David Kahurani

This seems like a more effective interface and avoids
code duplication.

Signed-off-by: David Kahurani 
---
 dlls/gdiplus/graphicspath.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/dlls/gdiplus/graphicspath.c b/dlls/gdiplus/graphicspath.c
index 9e186aa43d1..dabd458b312 100644
--- a/dlls/gdiplus/graphicspath.c
+++ b/dlls/gdiplus/graphicspath.c
@@ -1185,24 +1185,15 @@ GpStatus WINGDIPAPI GdipClonePath(GpPath* path, GpPath **clone)
     if(!path || !clone)
         return InvalidParameter;

-    *clone = malloc(sizeof(GpPath));
-    if(!*clone) return OutOfMemory;
-
-    **clone = *path;
-
-    (*clone)->pathdata.Points = malloc(path->datalen * sizeof(PointF));
-    (*clone)->pathdata.Types = malloc(path->datalen);
-    if(!(*clone)->pathdata.Points || !(*clone)->pathdata.Types){
-        free((*clone)->pathdata.Points);
-        free((*clone)->pathdata.Types);
-        free(*clone);
-        return OutOfMemory;
+    if (path->pathdata.Count)
+      return GdipCreatePath2(path->pathdata.Points, path->pathdata.Types, path->pathdata.Count,
+                             path->fill, clone);
+    else
+    {
+        *clone = calloc(1, sizeof(GpPath));
+        if(!*clone) return OutOfMemory;
     }

-    memcpy((*clone)->pathdata.Points, path->pathdata.Points,
-           path->datalen * sizeof(PointF));
-    memcpy((*clone)->pathdata.Types, path->pathdata.Types, path->datalen);
-
     return Ok;
 }

[PATCH v5 2/4] dlls/gdiplus: Use GdipCreatePath2 when serializing paths

[David Kahurani]

From: David Kahurani

This seems like a better interface and avoids duplicating code.

Signed-off-by: David Kahurani 
---
 dlls/gdiplus/metafile.c | 46 ++++++++++++++++++-----------------------
 1 file changed, 20 insertions(+), 26 deletions(-)

diff --git a/dlls/gdiplus/metafile.c b/dlls/gdiplus/metafile.c
index 5c50d0d1d37..b6d0e64e0cb 100644
--- a/dlls/gdiplus/metafile.c
+++ b/dlls/gdiplus/metafile.c
@@ -2006,7 +2006,6 @@ static GpStatus metafile_deserialize_image(const BYTE *record_data, UINT data_si
 static GpStatus metafile_deserialize_path(const BYTE *record_data, UINT data_size, GpPath **path)
 {
     EmfPlusPath *data = (EmfPlusPath *)record_data;
-    GpStatus status;
     BYTE *types;
     UINT size;
     DWORD i;
@@ -2035,40 +2034,35 @@ static GpStatus metafile_deserialize_path(const BYTE *record_data, UINT data_siz
     if (data_size < size)
         return InvalidParameter;

-    status = GdipCreatePath(FillModeAlternate, path);
-    if (status != Ok)
-        return status;
-
-    (*path)->pathdata.Count = data->PathPointCount;
-    (*path)->pathdata.Points = malloc(data->PathPointCount * sizeof(*(*path)->pathdata.Points));
-    (*path)->pathdata.Types = malloc(data->PathPointCount * sizeof(*(*path)->pathdata.Types));
-    (*path)->datalen = (*path)->pathdata.Count;
-
-    if (!(*path)->pathdata.Points || !(*path)->pathdata.Types)
+    if (data->PathPointCount)
     {
-        GdipDeletePath(*path);
-        return OutOfMemory;
-    }
+        if (data->PathPointFlags & 0x4000) /* C */
+        {
+            EmfPlusPoint *points = (EmfPlusPoint *)data->data;
+            GpPointF *temp = malloc(sizeof(GpPointF) * data->PathPointCount);

-    if (data->PathPointFlags & 0x4000) /* C */
-    {
-        EmfPlusPoint *points = (EmfPlusPoint *)data->data;
-        for (i = 0; i < data->PathPointCount; i++)
+            for (i = 0; i < data->PathPointCount; i++)
+            {
+                temp[i].X = points[i].X;
+                temp[i].Y = points[i].Y;
+            }
+            
+            types = (BYTE *)(points + i);
+            GdipCreatePath2(temp, types, data->PathPointCount, FillModeAlternate, path);
+            free(temp);
+        }
+        else
         {
-            (*path)->pathdata.Points[i].X = points[i].X;
-            (*path)->pathdata.Points[i].Y = points[i].Y;
+            EmfPlusPointF *points = (EmfPlusPointF *)data->data;
+            types = (BYTE *)(points + data->PathPointCount);
+            return GdipCreatePath2((GpPointF*)points, types, data->PathPointCount, FillModeAlternate, path);
         }
-        types = (BYTE *)(points + i);
     }
     else
     {
-        EmfPlusPointF *points = (EmfPlusPointF *)data->data;
-        memcpy((*path)->pathdata.Points, points, sizeof(*points) * data->PathPointCount);
-        types = (BYTE *)(points + data->PathPointCount);
+        return GdipCreatePath(FillModeAlternate, path);
     }

-    memcpy((*path)->pathdata.Types, types, sizeof(*types) * data->PathPointCount);
-
     return Ok;
 }

[PATCH v5 3/4] dlls/gdiplus: Use path_list to path helper in GdipFlattenPath.

[David Kahurani]

From: David Kahurani

This avoids a situation where the lengthening code, assuming the
data in the path is valid proceeds to lengthen the path further
while transforming data from a path_list into a path.

Signed-off-by: David Kahurani 
---
 dlls/gdiplus/graphicspath.c | 53 ++++++++++++++++++++++++++++---------
 1 file changed, 40 insertions(+), 13 deletions(-)

diff --git a/dlls/gdiplus/graphicspath.c b/dlls/gdiplus/graphicspath.c
index dabd458b312..15db862b1f8 100644
--- a/dlls/gdiplus/graphicspath.c
+++ b/dlls/gdiplus/graphicspath.c
@@ -103,6 +103,45 @@ static INT path_list_count(path_list_node_t *node)
     return count;
 }

+static BOOL path_list_to_path(path_list_node_t *node, GpPath *path)
+{
+    INT i, count = path_list_count(node);
+    GpPointF *Points;
+    BYTE *Types;
+
+    if (count == 0)
+    {
+        path->pathdata.Count = count;
+        return TRUE;
+    }
+
+    Points = calloc(count, sizeof(GpPointF));
+    Types = calloc(1, count);
+    
+    if (!Points || !Types)
+    {
+        free(Points);
+        free(Types);
+        return FALSE;
+    }
+
+    for(i = 0; i < count; i++){
+        Points[i] = node->pt;
+        Types[i] = node->type;
+        node = node->next;
+    }
+
+    free(path->pathdata.Points);
+    free(path->pathdata.Types);
+
+    path->pathdata.Points = Points;
+    path->pathdata.Types = Types;
+    path->pathdata.Count = count;
+    path->datalen = count;
+
+    return TRUE;
+}
+
 struct flatten_bezier_job
 {
     path_list_node_t *start;
@@ -1417,19 +1456,7 @@ GpStatus WINGDIPAPI GdipFlattenPath(GpPath *path, GpMatrix* matrix, REAL flatnes
         ++i;
     }/* while */

-    /* store path data back */
-    i = path_list_count(list);
-    if(!lengthen_path(path, i))
-        goto memout;
-    path->pathdata.Count = i;
-
-    node = list;
-    for(i = 0; i < path->pathdata.Count; i++){
-        path->pathdata.Points[i] = node->pt;
-        path->pathdata.Types[i]  = node->type;
-        node = node->next;
-    }
-
+    if (!path_list_to_path(list, path)) goto memout;
     free_path_list(list);
     return Ok;

[PATCH v5 4/4] dlls/gdiplus: Use path_list to path helper in GdipWidenPath.

[David Kahurani]

From: David Kahurani

The data is the path is invalid and therefore caution has to be
taken when adding data to this path to avoid lengthening it
unnecessarily. Also, don't assume there's a head node on the list
while counting number of nodes.

Signed-off-by: David Kahurani 
---
 dlls/gdiplus/graphicspath.c | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/dlls/gdiplus/graphicspath.c b/dlls/gdiplus/graphicspath.c
index 15db862b1f8..68954716f49 100644
--- a/dlls/gdiplus/graphicspath.c
+++ b/dlls/gdiplus/graphicspath.c
@@ -95,10 +95,13 @@ static path_list_node_t* add_path_list_node(path_list_node_t *node, REAL x, REAL
 /* returns element count */
 static INT path_list_count(path_list_node_t *node)
 {
-    INT count = 1;
-
-    while((node = node->next))
+    INT count = 0;
+  
+    while(node)
+    {
         ++count;
+        node = node->next;
+    }

return count;
 }
@@ -2501,7 +2504,7 @@ GpStatus WINGDIPAPI GdipWidenPath(GpPath *path, GpPen *pen, GpMatrix *matrix,
     GpPath *flat_path=NULL;
     GpStatus status;
     path_list_node_t *points=NULL, *last_point=NULL;
-    int i, subpath_start=0, new_length;
+    int i, subpath_start=0;

TRACE("(%p,%p,%s,%0.2f)\n", path, pen, debugstr_matrix(matrix), flatness);

@@ -2584,23 +2587,8 @@ GpStatus WINGDIPAPI GdipWidenPath(GpPath *path, GpPen *pen, GpMatrix *matrix,
             }
         }

-        new_length = path_list_count(points)-1;
-
-        if (!lengthen_path(path, new_length))
+        if (!path_list_to_path(points->next, path))
             status = OutOfMemory;
-    }
-
-    if (status == Ok)
-    {
-        path->pathdata.Count = new_length;
-
-        last_point = points->next;
-        for (i = 0; i < new_length; i++)
-        {
-            path->pathdata.Points[i] = last_point->pt;
-            path->pathdata.Types[i] = last_point->type;
-            last_point = last_point->next;
-        }

path->fill = FillModeWinding;
     }

[Esme Povirk (＠madewokherd)]

Esme Povirk (@madewokherd) commented about dlls/gdiplus/metafile.c:
>      if (data->PathPointFlags & 0x4000) /* C */
>      {
>          EmfPlusPoint *points = (EmfPlusPoint *)data->data;
> -
>        GpPointF *temp = malloc(sizeof(GpPointF) * data->PathPointCount);
> -
>        for (i = 0; i < data->PathPointCount; i++)
>        {
> -
>        (*path)->pathdata.Points[i].X = points[i].X;
> -
>        (*path)->pathdata.Points[i].Y = points[i].Y;
> -
>            temp[i].X = points[i].X;
> -
>            temp[i].Y = points[i].Y;
>        }
> -
>        
>        types = (BYTE *)(points + i);
> -
>        GdipCreatePath2(temp, types, data->PathPointCount, FillModeAlternate, path);
> -
>        free(temp);

We can use GdipCreatePath2I in this case.

[Esme Povirk (＠madewokherd)]

Testbot seems to be complaining about whitespace errors, not sure if that can be trivially fixed when merging or not.

[PATCH v5 0/4] MR4803: dlls/gdiplus: Assorted path changes - approved

[Esme Povirk (＠madewokherd)]

This merge request was approved by Esme Povirk.