Bug 231916

Summary: REGRESSION (r284336): system-preview/badge.html is image failing - clip-path on inlines
Product: WebKit Reporter: ayumi_kojima
Component: Layout and RenderingAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, changseok, esprehn+autocc, ews-watchlist, fred.wang, glenn, heycam, jonlee, koivisto, kondapallykalyan, pdr, simon.fraser, thorton, webkit-bot-watchers-bugzilla, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: iPhone / iPad   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=231852
Attachments:
Description Flags
Image diff
none
Patch koivisto: review+

Attachments
Image diff (31.45 KB, image/png)
2021-10-18 15:59 PDT, ayumi_kojima 		no flags
Details
Patch (14.25 KB, patch)
2021-10-18 20:32 PDT, Simon Fraser (smfr) 		koivisto: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
ayumi_kojima
Comment 1 2021-10-18 15:59:42 PDT
Created attachment 441652 [details] Image diff
Radar WebKit Bug Importer
Comment 2 2021-10-18 16:00:07 PDT
<rdar://problem/84391702>
ayumi_kojima
Comment 3 2021-10-18 16:01:15 PDT
According to the history, it looks like it started from the changes in https://trac.webkit.org/changeset/284336/webkit
Simon Fraser (smfr)
Comment 4 2021-10-18 20:32:42 PDT
Created attachment 441684 [details] Patch
Cameron McCormack (:heycam)
Comment 5 2021-10-18 20:50:23 PDT
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)
Comment 6 2021-10-18 20:52:43 PDT
(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 7 2021-10-18 22:11:07 PDT
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
Comment 8 2021-10-18 22:13:12 PDT
(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)
Comment 9 2021-10-19 13:45:27 PDT
https://trac.webkit.org/changeset/284490/webkit Will do the GraphicsContxtStateSaver thing separately.
Simon Fraser (smfr)
Comment 10 2021-10-19 14:48:06 PDT
Doing so via bug 231985.
Jon Lee
Comment 11 2022-04-01 15:48:05 PDT
*** Bug 236922 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.