[PATCH v3 0/3] MR4803: dlls/gdiplus: Assorted path changes
Signed-off-by: David Kahurani k.kahurani(a)gmail.com -- v3: dlls/gdiplus: Use path_list to path helper in GdipWidenPath. dlls/gdiplus: Use path_list to path helper in GdipFlattenPath. dlls/gdiplus: Add helper function for strictly allocating to a path https://gitlab.winehq.org/wine/wine/-/merge_requests/4803
From: David Kahurani <k.kahurani(a)gmail.com> Serves to strictly allocate the requested size to a path unlike lengthen_path whose algorithm allocates more than strictly required Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> --- dlls/gdiplus/gdiplus.c | 24 ++++++++++++++++++++++++ dlls/gdiplus/gdiplus_private.h | 1 + dlls/gdiplus/graphicspath.c | 28 +++------------------------- dlls/gdiplus/metafile.c | 16 +++------------- 4 files changed, 31 insertions(+), 38 deletions(-) diff --git a/dlls/gdiplus/gdiplus.c b/dlls/gdiplus/gdiplus.c index 32e287b0409..8e7b291f6a6 100644 --- a/dlls/gdiplus/gdiplus.c +++ b/dlls/gdiplus/gdiplus.c @@ -442,6 +442,30 @@ BOOL lengthen_path(GpPath *path, INT len) return TRUE; } +BOOL lengthen_path_strict(GpPath **path, INT len) +{ + /* Has to be a new unallocated path */ + *path = calloc(1, sizeof(GpPath)); + + if (!*path) return FALSE; + + /* Strictly allocate the requested memory size */ + (*path)->pathdata.Points = calloc(len, sizeof(PointF)); + (*path)->pathdata.Types = calloc(1, len); + + if (!(*path)->pathdata.Points || !(*path)->pathdata.Types) + { + free((*path)->pathdata.Points); + free((*path)->pathdata.Types); + free(*path); + return FALSE; + } + + (*path)->datalen = len; + + return TRUE; +} + void convert_32bppARGB_to_32bppPARGB(UINT width, UINT height, BYTE *dst_bits, INT dst_stride, const BYTE *src_bits, INT src_stride) { diff --git a/dlls/gdiplus/gdiplus_private.h b/dlls/gdiplus/gdiplus_private.h index 6f7e72124c2..2cfb93c62e5 100644 --- a/dlls/gdiplus/gdiplus_private.h +++ b/dlls/gdiplus/gdiplus_private.h @@ -130,6 +130,7 @@ extern void calc_curve_bezier_endp(REAL xend, REAL yend, REAL xadj, REAL yadj, extern void free_installed_fonts(void); extern BOOL lengthen_path(GpPath *path, INT len); +extern BOOL lengthen_path_strict(GpPath **path, INT len); extern DWORD write_region_data(const GpRegion *region, void *data); extern DWORD write_path_data(GpPath *path, void *data); diff --git a/dlls/gdiplus/graphicspath.c b/dlls/gdiplus/graphicspath.c index 9e186aa43d1..cbf2d074b20 100644 --- a/dlls/gdiplus/graphicspath.c +++ b/dlls/gdiplus/graphicspath.c @@ -1185,19 +1185,9 @@ GpStatus WINGDIPAPI GdipClonePath(GpPath* path, GpPath **clone) if(!path || !clone) return InvalidParameter; - *clone = malloc(sizeof(GpPath)); - if(!*clone) return OutOfMemory; + if (!lengthen_path_strict(clone, path->datalen)) 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; - } + (*clone)->pathdata.Count = path->pathdata.Count; memcpy((*clone)->pathdata.Points, path->pathdata.Points, path->datalen * sizeof(PointF)); @@ -1271,9 +1261,6 @@ GpStatus WINGDIPAPI GdipCreatePath2(GDIPCONST GpPointF* points, return OutOfMemory; } - *path = calloc(1, sizeof(GpPath)); - if(!*path) return OutOfMemory; - if(count > 1 && (types[count-1] & PathPointTypePathTypeMask) == PathPointTypeStart) count = 0; @@ -1290,22 +1277,13 @@ GpStatus WINGDIPAPI GdipCreatePath2(GDIPCONST GpPointF* points, } } - (*path)->pathdata.Points = malloc(count * sizeof(PointF)); - (*path)->pathdata.Types = malloc(count); - - if(!(*path)->pathdata.Points || !(*path)->pathdata.Types){ - free((*path)->pathdata.Points); - free((*path)->pathdata.Types); - free(*path); - return OutOfMemory; - } + if (!lengthen_path_strict(path, count)) return OutOfMemory; memcpy((*path)->pathdata.Points, points, count * sizeof(PointF)); memcpy((*path)->pathdata.Types, types, count); if(count > 0) (*path)->pathdata.Types[0] = PathPointTypeStart; (*path)->pathdata.Count = count; - (*path)->datalen = count; (*path)->fill = fill; (*path)->newfigure = TRUE; diff --git a/dlls/gdiplus/metafile.c b/dlls/gdiplus/metafile.c index 5c50d0d1d37..897d4a83702 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,20 +2034,11 @@ 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; + if (!lengthen_path_strict(path, data->PathPointCount)) return OutOfMemory; + (*path)->fill = FillModeAlternate; + (*path)->newfigure = TRUE; (*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) - { - GdipDeletePath(*path); - return OutOfMemory; - } if (data->PathPointFlags & 0x4000) /* C */ { -- 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 | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/dlls/gdiplus/graphicspath.c b/dlls/gdiplus/graphicspath.c index ed04b66ac5f..f3ad7018234 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; } @@ -2488,7 +2491,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); @@ -2571,23 +2574,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
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 cbf2d074b20..ed04b66ac5f 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; @@ -1404,19 +1443,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
Esme Povirk (@madewokherd) commented about dlls/gdiplus/gdiplus.c:
return TRUE; }
+BOOL lengthen_path_strict(GpPath **path, INT len)
This should probably be renamed since it creates a new path and doesn't lengthen anything. By convention, the out parameter should be the last one. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4803#note_58441
On Mon Jan 22 18:51:20 2024 +0000, Esme Povirk wrote:
This should probably be renamed since it creates a new path and doesn't lengthen anything. By convention, the out parameter should be the last one. On second thought, I think it would be better to call GdipCreatePath2 in the other places (GdipClonePath and the metafile code). It feels to me like the correct interface for this.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4803#note_58442
participants (3)
-
David Kahurani -
David Kahurani (＠kahurani)
-
Esme Povirk (＠madewokherd)