[PATCH 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.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=41617

Signed-off-by: David Kahurani

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

[David Kahurani]

From: David Kahurani

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 
---
 dlls/gdiplus/graphics.c | 93 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c
index 835c2889bd1..7475557280c 100644
--- a/dlls/gdiplus/graphics.c
+++ b/dlls/gdiplus/graphics.c
@@ -1920,6 +1920,98 @@ static void shorten_bezier_amt(GpPointF * pt, REAL amt, BOOL rev)
     }
 }

+static void poly_clip(GpGraphics *graphics, POINT *pti, BYTE *tp, int count)
+{
+    RectF rect;
+    BITMAP structBitmapHeader = {0};
+    HGDIOBJ hBitmap = GetCurrentObject(graphics->hdc, OBJ_BITMAP);
+    BYTE *tpcopy = malloc(count);
+
+    memcpy(tpcopy, tp, count);
+    GetObjectW(hBitmap, sizeof(BITMAP), &structBitmapHeader);
+    set_rect(&rect, 0, 0, structBitmapHeader.bmWidth, structBitmapHeader.bmHeight);
+
+    /* Ignore data that is not meant for drawing */
+    if (rect.Width == 1 && rect.Height == 1)
+        return;
+
+    /* DC attributes were not set, assume client area */
+    if (rect.Width == 0 && rect.Height == 0)
+    {
+        RECT lpRect;
+        GetClientRect(graphics->hwnd, &lpRect);
+        rect.Width = lpRect.right;
+        rect.Height = lpRect.bottom;
+    }
+
+    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:
+         {
+             int x1, y1, x2 = pti[i].x, y2 = pti[i].y;
+
+             if (i == 0)
+             {
+                 x1 = y1 = 0;
+             }
+             else
+             {
+                 x1 = pti[i - 1].x;
+                 y1 = pti[i - 1].y;
+             }
+
+             /* If any of the connecting points lies within DC's region */
+             if ((rect.X <= x1 && rect.Y <= y1 && x1 <= rect.X + rect.Width && y1 <= rect.Y + rect.Height) ||
+                 (rect.X <= x2 && rect.Y <= y2 && x2 <= rect.X + rect.Width && y2 <= rect.Y + rect.Height))
+             {
+                 if (i != 0)
+                 {
+                     tp[i - 1] = tpcopy[i - 1];
+                     tp[i] = tpcopy[i];
+                 }
+                 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 +2128,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;

[Esme Povirk (＠madewokherd)]

Esme Povirk (@madewokherd) commented about dlls/gdiplus/graphics.c:
> - memcpy(tpcopy, tp, count);
> - GetObjectW(hBitmap, sizeof(BITMAP), &structBitmapHeader);
> - set_rect(&rect, 0, 0, structBitmapHeader.bmWidth, structBitmapHeader.bmHeight);
> -
> - /* Ignore data that is not meant for drawing */
> - if (rect.Width == 1 && rect.Height == 1)
> -
>    return;
> -
> - /* DC attributes were not set, assume client area */
> - if (rect.Width == 0 && rect.Height == 0)
> - {
> -
>    RECT lpRect;
> -
>    GetClientRect(graphics->hwnd, &lpRect);
> -
>    rect.Width = lpRect.right;
> -
>    rect.Height = lpRect.bottom;
> - }

We already have the `get_graphics_device_bounds` function for this.

[Esme Povirk (＠madewokherd)]

It's possible for a line to fall inside the HDC region without any of the points being in the region, if the points are on opposite sides of it.

[Esme Povirk (＠madewokherd)]

On Mon Jul 10 16:40:13 2023 +0000, Esme Povirk wrote:
> It's possible for a line to fall inside the HDC region without any of
> the points being in the region, if the points are on opposite sides of it.

There's also the possibility of line width bringing the rendered line into the region if it's near the edge.

[Esme Povirk (＠madewokherd)]

@huw I wanted you to be in this discussion because I'm not as familiar with PolyDraw input (though at the moment the transformation of those inputs looks flawed to me so you can probably hold off on that), and I'd like to know if this is something that would make more sense in gdi32.

[David Kahurani (＠kahurani)]

I might not be qualified to comment on this and are not sure exactly what is the question. I am going to assume the question is whether the changes that this code is making to the input improve things for gdi32. My answer is yes. The code essentially replaces invalid points with the PT_MOVETO type. PolyDraw(at least the wine version) ignores PT_MOVETO type points or uses them as a starting point to draw.

When we generate consequent PT_MOVETO type points, all will get ignored except the last which will be used as the starting point for a valid line segment.

To sum it up, PT_MOVETO type of points are ultra cheap hence why I used them here instead of complicating everything while trying to come up with a new set of updated points.

[David Kahurani (＠kahurani)]

The relevant code can be seen here:

https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/win32u/painting.c#L15...

I don't see any other relevant codes that care for type of a point.