[PATCH] gdiplus: Check return value of SelectClipPath in brush_fill_path().

Vincent Povirk vincent at codeweavers.com
Sat Jun 16 10:26:58 CDT 2018


If converting a path to a region fails for an open path, then our code
in get_path_hrgn is also wrong. From reading gdi32 code, think it will
work as long as there are points in the path, but the only way to be
sure is to add a test.

If we check for failure in SelectClipPath, I agree that we should not
continue the operation for any error. I think we should report failure
to the caller if it's an error we don't expect. That would also help
explain why it's correct to silently ignore the failure.

On Sat, Jun 16, 2018 at 9:31 AM, Zhiyi Zhang <zzhang at codeweavers.com> wrote:
> 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 <zzhang at codeweavers.com> 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 <zzhang at codeweavers.com>
>>> ---
>>>  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
>>>
>>>
>>>



More information about the wine-devel mailing list