REGRESSION (r284336): system-preview/badge.html is image failing - clip-path on inlines

system-preview/badge.html

Is constantly failing on iOS 15.

History: https://results.webkit.org/?suite=layout-tests&test=system-preview%2Fbadge.html 

Result page: https://build.webkit.org/results/Apple-iOS-15-Simulator-Release-WK2-Tests/r284384%20(215)/results.html#

Created attachment 441652 [details]
Image diff

[Radar WebKit Bug Importer]

<rdar://problem/84391702>

According to the history, it looks like it started from the changes in https://trac.webkit.org/changeset/284336/webkit

[Simon Fraser (smfr)]

Created attachment 441684 [details]
Patch

[Cameron McCormack (:heycam)]

Comment on attachment 441684 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=441684&action=review

Non-reviewer r=me with the restore() question answered. I didn't look at the tests closely.

> Source/WebCore/rendering/RenderLayer.cpp:3160
> +        return std::make_pair(clipPath.pathForReferenceRect(snappedReferenceBox), clipPath.windRule());

Nit: I think it might be more common in the codebase to use `return {..., ...};` rather than `return std::make_pair(..., ...);`.

> Source/WebCore/rendering/RenderLayer.cpp:3169
> +        return std::make_pair(clipPath.pathForReferenceRect(shapeRect), WindRule::NonZero);

Nit: And here.

> Source/WebCore/rendering/RenderLayer.cpp:3172
> +    return std::make_pair(Path(), WindRule::NonZero);

Nit: And here.

> Source/WebCore/rendering/RenderLayer.cpp:3209
> +            context.save();

Is there a restore() missing?

[Simon Fraser (smfr)]

(In reply to Cameron McCormack (:heycam) from comment #5)

> Is there a restore() missing?

It's elsewhere (confusingly). Ideally we'd pass a GraphicsContextStateSaver thingy.

[Antti Koivisto]

Comment on attachment 441684 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=441684&action=review

>> Source/WebCore/rendering/RenderLayer.cpp:3160
>> +        return std::make_pair(clipPath.pathForReferenceRect(snappedReferenceBox), clipPath.windRule());
> 
> Nit: I think it might be more common in the codebase to use `return {..., ...};` rather than `return std::make_pair(..., ...);`.

Even where explicit type is needed std::make_pair is unnecessary as the std::pair constructor can infer the type (since C++17).

[Antti Koivisto]

(In reply to Simon Fraser (smfr) from comment #6)
> (In reply to Cameron McCormack (:heycam) from comment #5)
> 
> > Is there a restore() missing?
> 
> It's elsewhere (confusingly). Ideally we'd pass a GraphicsContextStateSaver
> thingy.

You really should clean this up.

[Simon Fraser (smfr)]

https://trac.webkit.org/changeset/284490/webkit

Will do the GraphicsContxtStateSaver thing separately.

[Simon Fraser (smfr)]

Doing so via bug 231985.

[Jon Lee]

*** Bug 236922 has been marked as a duplicate of this bug. ***