Signed-off-by: Jeff Smith whydoubt@gmail.com --- v2: - Check if points and types parameters are valid, and add some tests to exercise these. - Got rid of expectp macro.
dlls/gdiplus/graphicspath.c | 24 ++++++- dlls/gdiplus/tests/graphicspath.c | 103 ++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 1 deletion(-)
diff --git a/dlls/gdiplus/graphicspath.c b/dlls/gdiplus/graphicspath.c index 55545a449a..df6a32d22d 100644 --- a/dlls/gdiplus/graphicspath.c +++ b/dlls/gdiplus/graphicspath.c @@ -1213,14 +1213,34 @@ GpStatus WINGDIPAPI GdipCreatePath(GpFillMode fill, GpPath **path) GpStatus WINGDIPAPI GdipCreatePath2(GDIPCONST GpPointF* points, GDIPCONST BYTE* types, INT count, GpFillMode fill, GpPath **path) { + int i; + TRACE("(%p, %p, %d, %d, %p)\n", points, types, count, fill, path);
- if(!path) + if(!points || !types || !path) return InvalidParameter;
+ if(count <= 0) { + *path = NULL; + return OutOfMemory; + } + *path = heap_alloc_zero(sizeof(GpPath)); if(!*path) return OutOfMemory;
+ for(i = 1; i < count; i++) { + if((types[i] & PathPointTypePathTypeMask) == PathPointTypeBezier) { + if(i+2 < count && + (types[i+1] & PathPointTypePathTypeMask) == PathPointTypeBezier && + (types[i+2] & PathPointTypePathTypeMask) == PathPointTypeBezier) + i += 2; + else { + count = 0; + break; + } + } + } + (*path)->pathdata.Points = heap_alloc_zero(count * sizeof(PointF)); (*path)->pathdata.Types = heap_alloc_zero(count);
@@ -1233,6 +1253,8 @@ GpStatus WINGDIPAPI GdipCreatePath2(GDIPCONST GpPointF* points,
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;
diff --git a/dlls/gdiplus/tests/graphicspath.c b/dlls/gdiplus/tests/graphicspath.c index 17e6a7cafb..1b3a9a161f 100644 --- a/dlls/gdiplus/tests/graphicspath.c +++ b/dlls/gdiplus/tests/graphicspath.c @@ -174,6 +174,108 @@ static void test_getpathdata(void) GdipDeletePath(path); }
+static void test_createpath2(void) +{ + GpStatus status; + GpPath* path = NULL; + GpPathData data; + INT i, count, expect_count; + + PointF test_line_points[] = {{1.0,1.0}, {2.0,1.0}}; + BYTE test_line_types[] = {PathPointTypeStart, PathPointTypeLine}; + + PointF test_bez_points[] = {{1.0,1.0}, {2.0,1.0}, {3.0,1.0}, {4.0,1.0}, + {5.0,1.0}, {6.0,1.0}, {7.0,1.0}}; + BYTE test_bez_types[] = {PathPointTypeStart, PathPointTypeBezier, + PathPointTypeBezier, PathPointTypeBezier, PathPointTypeBezier, + PathPointTypeBezier, PathPointTypeBezier}; + + status = GdipCreatePath2(test_line_points, test_line_types, 2, FillModeAlternate, &path); + expect(Ok, status); + status = GdipGetPointCount(path, &count); + expect(Ok, status); + expect(2, count); + GdipDeletePath(path); + + status = GdipCreatePath2(test_line_points, test_line_types, 1, FillModeAlternate, &path); + expect(Ok, status); + status = GdipGetPointCount(path, &count); + expect(Ok, status); + expect(1, count); + GdipDeletePath(path); + + path = (void *)0xdeadbeef; + status = GdipCreatePath2(test_line_points, test_line_types, 0, FillModeAlternate, &path); + expect(OutOfMemory, status); + ok(!path, "Expected NULL, got %p\n", path); + if(path && path != (void *)0xdeadbeef) + GdipDeletePath(path); + + path = (void *)0xdeadbeef; + status = GdipCreatePath2(test_line_points, test_line_types, -1, FillModeAlternate, &path); + expect(OutOfMemory, status); + ok(!path, "Expected NULL, got %p\n", path); + if(path && path != (void *)0xdeadbeef) + GdipDeletePath(path); + + path = (void *)0xdeadbeef; + status = GdipCreatePath2(NULL, test_line_types, 2, FillModeAlternate, &path); + expect(InvalidParameter, status); + ok(path == (void *)0xdeadbeef, "Expected %p, got %p\n", (void *)0xdeadbeef, path); + if(path && path != (void *)0xdeadbeef) + GdipDeletePath(path); + + path = (void *)0xdeadbeef; + status = GdipCreatePath2(test_line_points, NULL, 2, FillModeAlternate, &path); + expect(InvalidParameter, status); + ok(path == (void *)0xdeadbeef, "Expected %p, got %p\n", (void *)0xdeadbeef, path); + if(path && path != (void *)0xdeadbeef) + GdipDeletePath(path); + + status = GdipCreatePath2(test_line_points, test_line_types, 2, FillModeAlternate, NULL); + expect(InvalidParameter, status); + + /* Zero-length line points do not get altered */ + path = NULL; + test_line_points[1].X = test_line_points[0].X; + test_line_points[1].Y = test_line_points[0].Y; + status = GdipCreatePath2(test_line_points, test_line_types, 2, FillModeAlternate, &path); + expect(Ok, status); + status = GdipGetPointCount(path, &count); + expect(Ok, status); + expect(2, count); + GdipDeletePath(path); + + /* The type of the first point is always converted to PathPointTypeStart */ + test_line_types[0] = PathPointTypeLine; + status = GdipCreatePath2(test_line_points, test_line_types, 1, FillModeAlternate, &path); + expect(Ok, status); + status = GdipGetPointCount(path, &count); + expect(Ok, status); + expect(1, count); + data.Count = count; + data.Types = GdipAlloc(sizeof(BYTE) * count); + data.Points = GdipAlloc(sizeof(PointF) * count); + status = GdipGetPathData(path, &data); + expect(Ok, status); + expect((data.Points[0].X == 1.0) && (data.Points[0].Y == 1.0), TRUE); + expect(data.Types[0], PathPointTypeStart); + GdipFree(data.Points); + GdipFree(data.Types); + GdipDeletePath(path); + + /* Bezier points must come in groups of three */ + for(i = 2; i <= 7; i++) { + expect_count = (i % 3 == 1) ? i : 0; + status = GdipCreatePath2(test_bez_points, test_bez_types, i, FillModeAlternate, &path); + expect(Ok, status); + status = GdipGetPointCount(path, &count); + expect(Ok, status); + expect(expect_count, count); + GdipDeletePath(path); + } +} + static path_test_t line2_path[] = { {0.0, 50.0, PathPointTypeStart, 0, 0}, /*0*/ {5.0, 45.0, PathPointTypeLine, 0, 0}, /*1*/ @@ -1663,6 +1765,7 @@ START_TEST(graphicspath)
test_constructor_destructor(); test_getpathdata(); + test_createpath2(); test_line2(); test_arc(); test_worldbounds();
Signed-off-by: Vincent Povirk vincent@codeweavers.com
It seems like you're going out of your way to make the tests robust. I don't think that's necessary. It's ok if the tests crash when things fail in ways that aren't known to happen on any existing implementation.
On Fri, Apr 10, 2020 at 10:20 AM Esme (they/them) vincent@codeweavers.com wrote:
Signed-off-by: Vincent Povirk vincent@codeweavers.com
It seems like you're going out of your way to make the tests robust. I don't think that's necessary. It's ok if the tests crash when things fail in ways that aren't known to happen on any existing implementation.
Yes, perhaps I am. Sometimes it's hard to strike the right balance.
Though if I was REALLY going out of my way to be robust, I would be setting path before EVERY time I called GdipCreatePath2, instead of just around the times I am interested in the expected failure.
-
Esme (they/them) -
Jeff Smith