| Summary: | Don't call invalidateRectsForAllMarkers() for every layer in the updateLayerPositions() traversal | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||
| Component: | Layout and Rendering | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | bfulgham, changseok, esprehn+autocc, ews-watchlist, fred.wang, glenn, koivisto, kondapallykalyan, pdr, sam, simon.fraser, webkit-bug-importer, zalan | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Simon Fraser (smfr)
2022-01-13 17:00:01 PST
Created attachment 449126 [details]
Patch
Comment on attachment 449126 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449126&action=review > Source/WebCore/rendering/RenderLayer.cpp:942 > + renderer().document().markers().invalidateRectsForAllMarkers(); Why doesn't this one use willUpdateLayerPositions()? Comment on attachment 449126 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449126&action=review > Source/WebCore/rendering/RenderLayer.cpp:937 > + willUpdateLayerPositions(); > + recursiveUpdateLayerPositions(nullptr, flagsForUpdateLayerPositions(*this)); I don't think I could come up with better names, but these 2 lines will surely confuse me in the future. Created attachment 449130 [details]
Patch
Committed r288008 (246034@main): <https://commits.webkit.org/246034@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 449130 [details]. Comment on attachment 449126 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449126&action=review > Source/WebCore/rendering/RenderLayer.cpp:1063 > + child->recursiveUpdateLayerPositions(geometryMap, flags); could this be something like updateLayerPositionsForSubtree? (In reply to Antti Koivisto from comment #7) > Comment on attachment 449126 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=449126&action=review > > > Source/WebCore/rendering/RenderLayer.cpp:1063 > > + child->recursiveUpdateLayerPositions(geometryMap, flags); > > could this be something like updateLayerPositionsForSubtree? It could, but that doesn't make it obvious that it's call recursively. > It could, but that doesn't make it obvious that it's call recursively.
Why does that matter? If it was refactored to do the same thing iteratively would the callers care?
If it were iterative, I would rename it of course, and the distinction between the "called-once" and the "called-one-per-layer" function wouldn't be necessary. |