From 4a2f4eaddcb693f7cffba0d884674d3384a8d43b Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Mon, 14 Aug 2017 19:59:59 +0900 Subject: [PATCH] Bug 1388692 - When doing hit testing, only flush animations if there are throttled compositor animations. r=birtles This patch changes UpdateAnimationOnlyStyles to only flush animation styles if there are throttled animation styles that could affect hit-testing and renames the function to UpdateAnimationStylesForHitTesting at the same time. For GeckoRestyleManager, the original UpdateAnimationOnlyStyles which flushes animation styles if there are any pending animation styles, is renamed to UpdateAnimationStyles for consistency. MozReview-Commit-ID: 89UleXjI2OE --HG-- extra : rebase_source : cac59f9d48b096aee718f89ffe203d38d39027fd --- layout/base/GeckoRestyleManager.cpp | 35 +++++++++++++++++++++++------ layout/base/GeckoRestyleManager.h | 17 +++++++------- layout/base/PresShell.cpp | 8 +++---- layout/base/RestyleManager.h | 12 +++++++++- layout/base/RestyleManagerInlines.h | 13 +++++++++-- layout/base/ServoRestyleManager.cpp | 10 +++------ layout/base/ServoRestyleManager.h | 15 +------------ 7 files changed, 66 insertions(+), 44 deletions(-) diff --git a/layout/base/GeckoRestyleManager.cpp b/layout/base/GeckoRestyleManager.cpp index 75abb9f5b683..577ab839315d 100644 --- a/layout/base/GeckoRestyleManager.cpp +++ b/layout/base/GeckoRestyleManager.cpp @@ -550,27 +550,27 @@ GeckoRestyleManager::ProcessPendingRestyles() mHavePendingNonAnimationRestyles || mDoRebuildAllStyleData; if (haveNonAnimation) { ++mAnimationGeneration; - UpdateOnlyAnimationStyles(); + UpdateAnimationStyles(); } else { // If we don't have non-animation style updates, then we have queued // up animation style updates from the refresh driver tick. This // doesn't necessarily include *all* animation style updates, since // we might be suppressing main-thread updates for some animations, - // so we don't want to call UpdateOnlyAnimationStyles, which updates + // so we don't want to call UpdateAnimationStyles, which updates // all animations. In other words, the work that we're about to do // to process the pending restyles queue is a *subset* of the work - // that UpdateOnlyAnimationStyles would do, since we're *not* + // that UpdateAnimationStyles would do, since we're *not* // updating transitions that are running on the compositor thread // and suppressed on the main thread. // // But when we update those styles, we want to suppress updates to - // transitions just like we do in UpdateOnlyAnimationStyles. So we + // transitions just like we do in UpdateAnimationStyles. So we // want to tell the transition manager to act as though we're in - // UpdateOnlyAnimationStyles. + // UpdateAnimationStyles. // // FIXME: In the future, we might want to refactor the way the // animation and transition manager do their refresh driver ticks so - // that we can use UpdateOnlyAnimationStyles, with a different + // that we can use UpdateAnimationStyles, with a different // boolean argument, for this update as well, instead of having them // post style updates in their WillRefresh methods. PresContext()->TransitionManager()->SetInAnimationOnlyStyleUpdate(true); @@ -642,7 +642,7 @@ GeckoRestyleManager::EndProcessingRestyles() } void -GeckoRestyleManager::UpdateOnlyAnimationStyles() +GeckoRestyleManager::UpdateAnimationStyles() { bool doCSS = PresContext()->EffectCompositor()->HasPendingStyleUpdates(); @@ -679,6 +679,27 @@ GeckoRestyleManager::UpdateOnlyAnimationStyles() transitionManager->SetInAnimationOnlyStyleUpdate(false); } +void +GeckoRestyleManager::UpdateAnimationStylesForHitTesting() +{ + MOZ_ASSERT(PresContext()->EffectCompositor()->HasThrottledStyleUpdates(), + "Should have throttled animation"); + + nsTransitionManager* transitionManager = PresContext()->TransitionManager(); + + transitionManager->SetInAnimationOnlyStyleUpdate(true); + + RestyleTracker tracker(ELEMENT_HAS_PENDING_ANIMATION_ONLY_RESTYLE | + ELEMENT_IS_POTENTIAL_ANIMATION_ONLY_RESTYLE_ROOT); + tracker.Init(this); + + PresContext()->EffectCompositor()->AddStyleUpdatesTo(tracker); + + ProcessRestyles(tracker); + + transitionManager->SetInAnimationOnlyStyleUpdate(false); +} + void GeckoRestyleManager::PostRestyleEventInternal() { diff --git a/layout/base/GeckoRestyleManager.h b/layout/base/GeckoRestyleManager.h index d8d7e9268f2f..7cb5fee5b7dc 100644 --- a/layout/base/GeckoRestyleManager.h +++ b/layout/base/GeckoRestyleManager.h @@ -217,15 +217,13 @@ private: void EndProcessingRestyles(); public: - // Update styles for animations that are running on the compositor and - // whose updating is suppressed on the main thread (to save - // unnecessary work), while leaving all other aspects of style - // out-of-date. + // Perform any pending animation restyles. // - // Performs an animation-only style flush to make styles from - // throttled transitions up-to-date prior to processing an unrelated - // style change, so that any transitions triggered by that style - // change produce correct results. + // This is a superset of the work performed by + // UpdateAnimationStylesForHitTesting since it includes not only any + // animations whose restyles might have been suppressed on the main thread + // because they are running on the compositor, but *any* animations with + // pending restyles including SMIL animation restyles. // // In more detail: when we're able to run animations on the // compositor, we sometimes "throttle" these animations by skipping @@ -241,7 +239,8 @@ public: // process the queued style updates we'll have correct old data to // compare against. When we do this, we don't bother touching frames // other than primary frames. - void UpdateOnlyAnimationStyles(); + void UpdateAnimationStyles(); + void UpdateAnimationStylesForHitTesting(); // Rebuilds all style data by throwing out the old rule tree and // building a new one, and additionally applying aExtraHint (which diff --git a/layout/base/PresShell.cpp b/layout/base/PresShell.cpp index 7dd794146ae6..f112bc533fc0 100644 --- a/layout/base/PresShell.cpp +++ b/layout/base/PresShell.cpp @@ -6929,17 +6929,17 @@ nsIFrame* GetNearestFrameContainingPresShell(nsIPresShell* aPresShell) } static bool -FlushThrottledStyles(nsIDocument *aDocument, void *aData) +FlushAnimationsForHitTesting(nsIDocument *aDocument, void *aData) { nsIPresShell* shell = aDocument->GetShell(); if (shell && shell->IsVisible()) { nsPresContext* presContext = shell->GetPresContext(); if (presContext) { - presContext->RestyleManager()->UpdateOnlyAnimationStyles(); + presContext->RestyleManager()->UpdateAnimationStylesForHitTesting(); } } - aDocument->EnumerateSubDocuments(FlushThrottledStyles, nullptr); + aDocument->EnumerateSubDocuments(FlushAnimationsForHitTesting, nullptr); return true; } @@ -7365,7 +7365,7 @@ PresShell::HandleEvent(nsIFrame* aFrame, AutoWeakFrame weakFrame(frame); { // scope for scriptBlocker. nsAutoScriptBlocker scriptBlocker; - FlushThrottledStyles(GetRootPresShell()->GetDocument(), nullptr); + FlushAnimationsForHitTesting(GetRootPresShell()->GetDocument(), nullptr); } diff --git a/layout/base/RestyleManager.h b/layout/base/RestyleManager.h index 114ee45cea47..7c7eab1f60fe 100644 --- a/layout/base/RestyleManager.h +++ b/layout/base/RestyleManager.h @@ -192,7 +192,17 @@ public: const nsAttrValue* aOldValue); inline nsresult ReparentStyleContext(nsIFrame* aFrame); - inline void UpdateOnlyAnimationStyles(); + // Update styles for animations if there are animations that are running on + // the compositor and whose updating is suppressed on the main thread (to save + // unnecessary work). This is needed to ensure that transform animations + // running on the compositor are updated on the main thread giving elements + // their up-to-date geometry before we attempt to perform hit-testing. + // + // Although SMIL animations may also be throttled, such animations do not + // affect hit-testing since only animations on elements in display:none + // subtree are throttled for SMIL. As a result, this method does not update + // throttled SMIL animations. + inline void UpdateAnimationStylesForHitTesting(); // Get a counter that increments on every style change, that we use to // track whether off-main-thread animations are up-to-date. diff --git a/layout/base/RestyleManagerInlines.h b/layout/base/RestyleManagerInlines.h index 6e2e70e58469..44cdcad2fb6e 100644 --- a/layout/base/RestyleManagerInlines.h +++ b/layout/base/RestyleManagerInlines.h @@ -80,9 +80,18 @@ RestyleManager::ReparentStyleContext(nsIFrame* aFrame) } void -RestyleManager::UpdateOnlyAnimationStyles() +RestyleManager::UpdateAnimationStylesForHitTesting() { - MOZ_STYLO_FORWARD(UpdateOnlyAnimationStyles, ()); + // Throttled SMIL samples never affect hit-testing since they only apply to + // display:none subtrees so we don't need to handle them here. + // + // Bug 1353212: We don't need to flush opacity animations running on the + // compositor since opacity animations don't affect hit-testing. + if (!PresContext()->EffectCompositor()->HasThrottledStyleUpdates()) { + return; + } + + MOZ_STYLO_FORWARD(UpdateAnimationStylesForHitTesting, ()); } } // namespace mozilla diff --git a/layout/base/ServoRestyleManager.cpp b/layout/base/ServoRestyleManager.cpp index 2f12ce00f441..89de548883ed 100644 --- a/layout/base/ServoRestyleManager.cpp +++ b/layout/base/ServoRestyleManager.cpp @@ -1163,14 +1163,10 @@ ServoRestyleManager::ProcessPendingRestyles() } void -ServoRestyleManager::UpdateOnlyAnimationStyles() +ServoRestyleManager::UpdateAnimationStylesForHitTesting() { - // Bug 1365855: We also need to implement this for SMIL. - bool doCSS = PresContext()->EffectCompositor()->HasPendingStyleUpdates(); - if (!doCSS) { - return; - } - + MOZ_ASSERT(PresContext()->EffectCompositor()->HasThrottledStyleUpdates(), + "Should have throttled animation"); DoProcessPendingRestyles(ServoTraversalFlags::FlushThrottledAnimations); } diff --git a/layout/base/ServoRestyleManager.h b/layout/base/ServoRestyleManager.h index 0081946726e7..d322484bde13 100644 --- a/layout/base/ServoRestyleManager.h +++ b/layout/base/ServoRestyleManager.h @@ -186,20 +186,7 @@ public: nsRestyleHint aRestyleHint); void ProcessPendingRestyles(); - /** - * Performs a Servo animation-only traversal to compute style for all nodes - * with the animation-only dirty bit in the document. - * - * This processes just the traversal for animation-only restyles and skips the - * normal traversal for other restyles unrelated to animations. - * This is used to bring throttled animations up-to-date such as when we need - * to get correct position for transform animations that are throttled because - * they are running on the compositor. - * - * This will traverse all of the document's style roots (that is, its document - * element, and the roots of the document-level native anonymous content). - */ - void UpdateOnlyAnimationStyles(); + void UpdateAnimationStylesForHitTesting(); void ContentStateChanged(nsIContent* aContent, EventStates aStateMask); void AttributeWillChange(dom::Element* aElement,