Avoid updating UI-side layer positions for no-op transactions#63293
Conversation
|
EWS run on previous version of this PR (hash 21eb1ea) Details
|
|
|
||
| m_needsApplyLayerPositions = false; | ||
|
|
||
| Locker locker { m_treeLock }; |
There was a problem hiding this comment.
Do we need to lock before the check? Otherwise won't we have potentially multiple applyLayerPositions() queued that will still think they need to apply later positions even after the first one goes through?
| @@ -125,6 +125,8 @@ class ScrollingTree : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<Scr | |||
| bool commitTreeStateInternal(std::unique_ptr<ScrollingStateTree>&&, std::optional<LayerHostingContextIdentifier>) WTF_REQUIRES_LOCK(m_treeLock); | |||
|
|
|||
| WEBCORE_EXPORT virtual void applyLayerPositions(); | |||
| void setNeedsApplyLayerPositions() { m_needsApplyLayerPositions = true; } | |||
| bool needsApplyLayerPositions() const { return m_needsApplyLayerPositions; } | |||
| if (!m_needsApplyLayerPositions) | ||
| return; | ||
|
|
||
| m_needsApplyLayerPositions = false; |
There was a problem hiding this comment.
Since std::atomic<bool> m_needsApplyLayerPositions, shouldn't you atomically, get and reset m_needsApplyLayerPositions to false? Then the winning thread can proceed.
There was a problem hiding this comment.
Updated to use m_needsApplyLayerPositions.exchange()
21eb1ea to
50ea6df
Compare
|
EWS run on current version of this PR (hash 50ea6df) Details
|
https://bugs.webkit.org/show_bug.cgi?id=312959 rdar://175308880 Reviewed by Brent Fulgham. Currently, after every RemoteLayerTree commit that is received by the UI process, we call `applyScrollingTreeLayerPositionsAfterCommit()`, which traverses the scrolling tree and updates CALayer positions. If the transaction contains no scrolling tree changes, then this is not necessary; `applyLayerPositions()` only needs to be called after a scrolling tree state change, or after a user scroll. Track this state via `ScrollingTree::m_needsApplyLayerPositions()`, and set the flag under `commitTreeState()` and `scrollingTreeNodeDidScroll()`. It's the `commitTreeState(()` call that's skipped when `stateTree->hasChangedProperties()` is false. This should be a minor power saving. * Source/WebCore/page/scrolling/ScrollingTree.cpp: (WebCore::ScrollingTree::scrollingTreeNodeDidScroll): (WebCore::ScrollingTree::commitTreeStateInternal): (WebCore::ScrollingTree::applyLayerPositions): * Source/WebCore/page/scrolling/ScrollingTree.h: (WebCore::ScrollingTree::setNeedsApplyLayerPositions): Canonical link: https://commits.webkit.org/311816@main
50ea6df to
8ff9ee2
Compare
|
Committed 311816@main (8ff9ee2): https://commits.webkit.org/311816@main Reviewed commits have been landed. Closing PR #63293 and removing active labels. |
🧪 api-mac-debug
8ff9ee2
50ea6df
🧪 api-mac-debug