[PATCH v2 0/1] MR3288: gdiplus: Clip polygons before drawing them

[David Kahurani (＠kahurani)]

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>

--
  v2: gdiplus: Clip polygons before drawing them
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3288

[PATCH v2 1/1] gdiplus: Clip polygons before drawing them

[David Kahurani]

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 | 181 +++++++++++++++++++++++++++++-----------
 1 file changed, 134 insertions(+), 47 deletions(-)

diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c
index 6dc34707bbf..3b7f4cb7c85 100644
--- a/dlls/gdiplus/graphics.c
+++ b/dlls/gdiplus/graphics.c
@@ -1920,6 +1920,139 @@ 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 = malloc(count);
+
+    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++)
+    {
+         switch(tp[i] &~ PT_CLOSEFIGURE)
+         {
+         case PT_BEZIERTO:
+         {
+             int j;
+
+             for (j = 0; j < 3; j++)
+             {
+                  REAL x = pti[i + j].x;
+                  REAL y = pti[i + j].y;
+
+                  /* At least part of the rectangle encompassing the bezier lies within the DC's region */
+                  if (rect.X <= x && rect.Y <= y && x <= rect.X + rect.Width && y <= rect.Y + rect.Height)
+                      break;
+             }
+
+             if (j == 3)
+             {
+                 for(j = 0; j < 3; j++)
+                     tp[i + j] = PT_MOVETO;
+             }
+
+             i += 2;
+             break;
+         }
+         case PT_LINETO:
+         {
+             GpPath *path;
+             GpPen *pen;
+             GpRegion *line_region;
+             BOOL empty;
+
+             GdipCreatePath(FillModeAlternate, &path);
+
+             if (i == 0)
+                 GdipAddPathLine(path, 0, 0, pti[i].x, pti[i].y);
+             else
+                 GdipAddPathLine(path, pti[i - 1].x, pti[i - 1].y, pti[i].x, pti[i].y);
+
+             GdipCreatePen1(0xffffffff, 10.0, UnitPixel, &pen);
+             GdipWidenPath(path, pen, NULL, FlatnessDefault);
+             GdipCreateRegionPath(path, &line_region);
+             GdipCombineRegionRect(line_region, &rect, CombineModeIntersect);
+             GdipIsEmptyRegion(line_region, graphics, &empty);
+
+             if (!empty)
+             {
+                 if (i != 0)
+                 {
+                     tp[i - 1] = tpcopy[i - 1];
+                     tp[i] = tpcopy[i];
+                 }
+             }
+             else
+                  tp[i] = PT_MOVETO;
+
+             GdipDeleteRegion(line_region);
+             GdipDeletePen(pen);
+             GdipDeletePath(path);
+             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 +2169,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 +2287,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

[David Kahurani (＠kahurani)]

On Mon Jul 10 16:41:30 2023 +0000, Esme Povirk wrote:
> There's also the possibility of line width bringing the rendered line
> into the region if it's near the edge.
> I updated this to use regions but I believe that line thickness(artificially created) may lead to some false positives.... around the edges and corners of the rectangle but it looks to me like these are unlikely to cause any serious issues.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3288#note_38832