[PATCH v5 0/1] MR3288: gdiplus: Clip polygons before drawing them
Validate user data before passing it to PolyDraw. The program in the bug requests to draw figures outrageously outside the DC's region after presumably, some uninitialized values happen as a result of a missing font. Native gdiplus seems to handle this gracefully so we probably also should. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=41617 Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> -- v5: gdiplus: Clip polygons before drawing them https://gitlab.winehq.org/wine/wine/-/merge_requests/3288
From: David Kahurani <k.kahurani(a)gmail.com> This should reduce the load on GDI when an application is trying to draw to regions outside of selected DC's region Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=41617 Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> --- dlls/gdiplus/graphics.c | 234 ++++++++++++++++++++++++++++++++-------- 1 file changed, 187 insertions(+), 47 deletions(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 6dc34707bbf..f9f54026eec 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -1920,6 +1920,192 @@ static void shorten_bezier_amt(GpPointF * pt, REAL amt, BOOL rev) } } +static GpStatus get_graphics_device_bounds(GpGraphics* graphics, GpRectF* rect) +{ + RECT wnd_rect; + GpStatus stat=Ok; + GpUnit unit; + + if(graphics->hwnd) { + if(!GetClientRect(graphics->hwnd, &wnd_rect)) + return GenericError; + + rect->X = wnd_rect.left; + rect->Y = wnd_rect.top; + rect->Width = wnd_rect.right - wnd_rect.left; + rect->Height = wnd_rect.bottom - wnd_rect.top; + }else if (graphics->image){ + stat = GdipGetImageBounds(graphics->image, rect, &unit); + if (stat == Ok && unit != UnitPixel) + FIXME("need to convert from unit %i\n", unit); + }else if (GetObjectType(graphics->hdc) == OBJ_MEMDC){ + HBITMAP hbmp; + BITMAP bmp; + + rect->X = 0; + rect->Y = 0; + + hbmp = GetCurrentObject(graphics->hdc, OBJ_BITMAP); + if (hbmp && GetObjectW(hbmp, sizeof(bmp), &bmp)) + { + rect->Width = bmp.bmWidth; + rect->Height = bmp.bmHeight; + } + else + { + /* FIXME: ??? */ + rect->Width = 1; + rect->Height = 1; + } + }else{ + rect->X = 0; + rect->Y = 0; + rect->Width = GetDeviceCaps(graphics->hdc, HORZRES); + rect->Height = GetDeviceCaps(graphics->hdc, VERTRES); + } + + return stat; +} + +static void poly_clip(GpGraphics *graphics, POINT *pti, BYTE *tp, int count) +{ + RectF rect; + BYTE *tpcopy = heap_alloc_zero(count * sizeof(BYTE)); + + memcpy(tpcopy, tp, count); + get_graphics_device_bounds(graphics, &rect); + + /* Ignore data that is not meant for drawing */ + if (rect.Width == 1 && rect.Height == 1) + return; + + for (int i = 0; i < count; i++) + { + RectF bounds; + + switch(tp[i] &~ PT_CLOSEFIGURE) + { + case PT_BEZIERTO: + { + int j, left_most = pti[i - 1].x, right_most = pti[i - 1].x, + top = pti[i - 1].y, bottom = pti[i - 1].y; + + for (j = 0; j < 3; j++) + { + if (pti[i + j].x < left_most) + left_most = pti[i + j].x; + + if (pti[i + j].x > right_most) + right_most = pti[i + j].x; + + if (pti[i + j].y < top) + top = pti[i + j].y; + + if (pti[i + j].y > bottom) + bottom = pti[i + j].y; + } + + set_rect(&bounds, left_most, top, right_most - left_most, bottom - top); + + /* properties of intersect rectangle */ + left_most = (rect.X < bounds.X) ? bounds.X : rect.X; + top = (rect.Y < bounds.Y) ? bounds.Y : rect.Y; + + right_most = (rect.X + rect.Width < bounds.X + bounds.Width) + ? rect.X + rect.Width : bounds.X + bounds.Width; + bottom = (rect.Y + rect.Height < bounds.Y + bounds.Height) + ? rect.Y + rect.Height : bounds.Y + bounds.Height; + + if (left_most < right_most && top < bottom) + { + i += 2; + break; + } + else + for(j = 0; j < 3; j++) + tp[i + j] = PT_MOVETO; + + i += 2; + break; + } + case PT_LINETO: + { + POINT tmp[4]; + + int j, left_most = pti[i - 1].x, right_most = pti[i - 1].x, + top = pti[i - 1].y, bottom = pti[i - 1].y; + + /* widen the path by a width of 10 */ + if (pti[i - 1].y != pti[i].y) + { + tmp[0].x = pti[i - 1].x + 5; + tmp[0].y = pti[i - 1].y; + tmp[1].x = pti[i - 1].x - 5; + tmp[1].y = pti[i - 1].y; + + tmp[2].x = pti[i].x + 5; + tmp[2].y = pti[i].y; + tmp[3].x = pti[i].x - 5; + tmp[3].y = pti[i].y; + } + else + { + tmp[0].x = pti[i - 1].x; + tmp[0].y = pti[i - 1].y + 5; + tmp[1].x = pti[i - 1].x; + tmp[1].y = pti[i - 1].y - 5; + + tmp[2].x = pti[i].x; + tmp[2].y = pti[i].y + 5; + tmp[3].x = pti[i].x; + tmp[3].y = pti[i].y - 5; + } + + for (j = 0; j < 4; j++) + { + if (tmp[j].x < left_most) + left_most = tmp[j].x; + + if (tmp[j].x > right_most) + right_most = tmp[j].x; + + if (tmp[j].y < top) + top = tmp[j].y; + + if (tmp[j].y > bottom) + bottom = tmp[j].y; + } + + set_rect(&bounds, left_most, top, right_most - left_most, bottom - top); + + left_most = (rect.X < bounds.X) ? bounds.X : rect.X; + top = (rect.Y < bounds.Y) ? bounds.Y : rect.Y; + + right_most = (rect.X + rect.Width < bounds.X + bounds.Width) + ? rect.X + rect.Width : bounds.X + bounds.Width; + bottom = (rect.Y + rect.Height < bounds.Y + bounds.Height) + ? rect.Y + rect.Height : bounds.Y + bounds.Height; + + if (left_most < right_most && top < bottom) + { + break; + } + + tp[i] = PT_MOVETO; + + break; + } + case PT_MOVETO: + break; + + default: + ERR("Bad point type \n"); + } + } + + free(tpcopy); +} + /* Draws a combination of bezier curves and lines between points. */ static GpStatus draw_poly(GpGraphics *graphics, GpPen *pen, GDIPCONST GpPointF * pt, GDIPCONST BYTE * types, INT count, BOOL caps) @@ -2036,6 +2222,7 @@ static GpStatus draw_poly(GpGraphics *graphics, GpPen *pen, GDIPCONST GpPointF * tp[i] = convert_path_point_type(types[i]); } + poly_clip(graphics, pti, tp, count); PolyDraw(graphics->hdc, pti, tp, count); status = Ok; @@ -2153,53 +2340,6 @@ static GpStatus restore_container(GpGraphics* graphics, return Ok; } -static GpStatus get_graphics_device_bounds(GpGraphics* graphics, GpRectF* rect) -{ - RECT wnd_rect; - GpStatus stat=Ok; - GpUnit unit; - - if(graphics->hwnd) { - if(!GetClientRect(graphics->hwnd, &wnd_rect)) - return GenericError; - - rect->X = wnd_rect.left; - rect->Y = wnd_rect.top; - rect->Width = wnd_rect.right - wnd_rect.left; - rect->Height = wnd_rect.bottom - wnd_rect.top; - }else if (graphics->image){ - stat = GdipGetImageBounds(graphics->image, rect, &unit); - if (stat == Ok && unit != UnitPixel) - FIXME("need to convert from unit %i\n", unit); - }else if (GetObjectType(graphics->hdc) == OBJ_MEMDC){ - HBITMAP hbmp; - BITMAP bmp; - - rect->X = 0; - rect->Y = 0; - - hbmp = GetCurrentObject(graphics->hdc, OBJ_BITMAP); - if (hbmp && GetObjectW(hbmp, sizeof(bmp), &bmp)) - { - rect->Width = bmp.bmWidth; - rect->Height = bmp.bmHeight; - } - else - { - /* FIXME: ??? */ - rect->Width = 1; - rect->Height = 1; - } - }else{ - rect->X = 0; - rect->Y = 0; - rect->Width = GetDeviceCaps(graphics->hdc, HORZRES); - rect->Height = GetDeviceCaps(graphics->hdc, VERTRES); - } - - return stat; -} - static GpStatus get_graphics_bounds(GpGraphics* graphics, GpRectF* rect) { GpStatus stat = get_graphics_device_bounds(graphics, rect); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3288
Esme Povirk (@madewokherd) commented about dlls/gdiplus/graphics.c:
+ rect->Height = GetDeviceCaps(graphics->hdc, VERTRES); + } + + return stat; +} + +static void poly_clip(GpGraphics *graphics, POINT *pti, BYTE *tp, int count) +{ + RectF rect; + BYTE *tpcopy = heap_alloc_zero(count * sizeof(BYTE)); + + memcpy(tpcopy, tp, count); + get_graphics_device_bounds(graphics, &rect); + + /* Ignore data that is not meant for drawing */ + if (rect.Width == 1 && rect.Height == 1) This doesn't seem right to me. A 1x1 image or window could still be valid.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3288#note_39516
I don't think this will interact correctly with PT_CLOSEFIGURE. According to MSDN, PT_CLOSEFIGURE draws a line or bezier to the most recent PT_MOVETO points, so by changing points to PT_MOVETO, the behavior of PT_CLOSEFIGURE can be changed. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3288#note_39517
Given some other similar bugs in other drawing operations, I think a better approach may be to optimize the conversion from GpRegion to HRGN to only convert the parts within a known bounding rectangle. We could then use SOFTWARE_GdipFillPath which would benefit from that. Unfortunately, that requires rewriting the conversion so it doesn't rely on gdi32. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3288#note_39518
On Fri Dec 15 14:18:24 2023 +0000, Esme Povirk wrote:
Given some other similar bugs in other drawing operations, I think a better approach may be to optimize the conversion from GpRegion to HRGN to only convert the parts within a known bounding rectangle. We could then use SOFTWARE_GdipFillPath which would benefit from that. Unfortunately, that requires rewriting the conversion so it doesn't rely on gdi32. Hello,
Sorry, got carried away with other things so yeah, haven't managed to continue working on this until recently. It looks like it would be a great idea to convert this so as not to rely on gdi32 but it doesn't look like `SOFTWARE_GdipFillPath` is the right way to go. \*FillPath methods will fill the path while `DrawPoly` only outlines. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3288#note_56081
participants (3)
-
David Kahurani -
David Kahurani (＠kahurani)
-
Esme Povirk (＠madewokherd)