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

[Zhiyi Zhang]

Hi Vincent,

I tested open paths for path to region function manually. It works on Windows and Wine.
So we have to consider empty path on Wine only. In case any error happens in
SelectClipPath(), further operation is canceled and error is reported to the
caller. V2 is sent.

Thanks,
Zhiyi

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
>>>>
>>>>
>>>>