From: Zhiyi Zhang Subject: Re: [PATCH] gdiplus: Check return value of SelectClipPath in brush_fill_path(). Message-Id: <1a25a0bf-1335-1b2f-04e6-14a2e2b692d6@codeweavers.com> Date: Sat, 16 Jun 2018 22:31:28 +0800 In-Reply-To: References: <73e868d5-11d6-984e-3a85-22184fa39d07@codeweavers.com> Hi Vincent, Yes, we could check if a path is empty just like get_path_hrgn did. I also worried that a not empty but open path will also make SelectClipPath() fail. Is there any function to check if a path is closed? I couldn't found any. As for SelectClipPath() last error, I don't think any further operation should be continued if any error occurs. So we add a check for empty path and also skip further operation if SelectClipPath() returns any error. Is that enough? Thanks, Zhiyi On Fri 6 15 23:55, Vincent Povirk wrote: > It looks like we also encountered this case in get_path_hrgn. Maybe we > shouldn't call brush_fill_pixels if the path is empty? If we're going > to ignore errors, I think we should at least check GetLastError() to > make sure it's one we expect. > > On Fri, Jun 15, 2018 at 4:04 AM, Zhiyi Zhang wrote: >> For Crossover bug 16126. >> >> When GraphicPath is empty, filling path with gdi32 will >> result in a DC with empty path. When SelectClipPath() is >> called with such a DC, it will fail because it requires >> a closed path in DC. Thus further operation should be canceled. >> >> Signed-off-by: Zhiyi Zhang >> --- >> dlls/gdiplus/graphics.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c >> index 76aabe74bf..2a95d686fa 100644 >> --- a/dlls/gdiplus/graphics.c >> +++ b/dlls/gdiplus/graphics.c >> @@ -1052,6 +1052,7 @@ static BOOL brush_can_fill_path(GpBrush *brush, BOOL is_fill) >> >> static void brush_fill_path(GpGraphics *graphics, GpBrush* brush) >> { >> + BOOL success; >> switch (brush->bt) >> { >> case BrushTypeSolidColor: >> @@ -1064,8 +1065,8 @@ static void brush_fill_path(GpGraphics *graphics, GpBrush* brush) >> RECT rc; >> /* partially transparent fill */ >> >> - SelectClipPath(graphics->hdc, RGN_AND); >> - if (GetClipBox(graphics->hdc, &rc) != NULLREGION) >> + success = SelectClipPath(graphics->hdc, RGN_AND); >> + if (success && GetClipBox(graphics->hdc, &rc) != NULLREGION) >> { >> HDC hdc = CreateCompatibleDC(NULL); >> >> -- >> 2.17.1 >> >> >>