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

[Zhiyi Zhang]

Thanks. I'll work on it as you suggest.

On Sat 6 16 23:26, Vincent Povirk wrote:
> 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
>>>>
>>>>
>>>>
> 
>