[PATCH v6 0/4] MR4803: dlls/gdiplus: Assorted path changes
Signed-off-by: David Kahurani k.kahurani(a)gmail.com -- v6: 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
From: David Kahurani <k.kahurani(a)gmail.com> This seems like a more effective interface and avoids code duplication. Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> --- 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; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4803
From: David Kahurani <k.kahurani(a)gmail.com> This seems like a better interface and avoids duplicating code. Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> --- 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..6ccfc469c25 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; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4803
From: David Kahurani <k.kahurani(a)gmail.com> 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 <k.kahurani(a)gmail.com> --- 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..502c5c6b645 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; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4803
From: David Kahurani <k.kahurani(a)gmail.com> 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 <k.kahurani(a)gmail.com> --- dlls/gdiplus/graphicspath.c | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/dlls/gdiplus/graphicspath.c b/dlls/gdiplus/graphicspath.c index 502c5c6b645..09cd99e4467 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; + INT count = 0; - while((node = node->next)) + 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; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4803
On Fri Jan 26 20:01:22 2024 +0000, Esme Povirk wrote:
We can use GdipCreatePath2I in this case. I'm not sure this will be possible. Looking at EmfPlusPoint:
`typedef struct EmfPlusPoint { short X; short Y; } EmfPlusPoint;` On the other hand, GpPoint, which is a typedef for Point is defined as: `typedef struct Point { INT X; INT Y; } Point;` It seems to me that the difference in internal types means casting EmfPlusPoint to GpPoint will give us memory corruption. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4803#note_59371
On Mon Jan 29 16:17:52 2024 +0000, David Kahurani wrote:
I'm not sure this will be possible. Looking at EmfPlusPoint: `typedef struct EmfPlusPoint { short X; short Y; } EmfPlusPoint;` On the other hand, GpPoint, which is a typedef for Point is defined as: `typedef struct Point { INT X; INT Y; } Point;` It seems to me that the difference in internal types means casting EmfPlusPoint to GpPoint will give us memory corruption. Oh, you're right, never mind.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4803#note_59394
participants (3)
-
David Kahurani -
David Kahurani (＠kahurani)
-
Esme Povirk (＠madewokherd)