Bug 828173 patch 6: Remove calls to ForceLayerRerendering from the miniflush code (UpdateThrottledStyles, which flushes animations whose main thread updates are throttled without updating any other styles). r=mattwoodrow

I've been wanting to remove this code for a while.  I think this code is
problematic for three reasons:

 (1) It's in the middle of code where it doesn't belong, and which ought
     to be handling purely-style-system things.  (This is blocking me
     from reusing that code elsewhere, e.g., in bug 977991 and
     bug 960465, both of which could use it in some form.)

 (2) It defeats the optimization from bug 790505 whenever we do a
     miniflush (in other words, whenever we have any style change,
     whether or not it's related)

 (3) It means the conditions for when we decide to ship a new set of
     animation data to a layer doesn't cover all the cases the layer
     needs it.  In particular, we only run this miniflush code when we
     have a currently running animation or transition that's running on
     the compositor thread.  On the other hand, the UpdateTransformLayer
     style change handling in DoApplyRenderingChangeToTree depends on
     whether the frame currently has a transform layer, which can
     continue to be true for a bit after the animation stops.  So if we
     need to send animations to the layer because of a transform style
     change that happens soon after an animation completes, our style
     change handling will find the existing layer and call its
     SetBaseTransformForNextTransaction method but never do anything
     that triggers layer construction.  The style throttling code, in
     turn, will never stop doing main thread updates because the
     animation generation on the layer is out-of-date, and these main
     thread updates will keep the layer active, but they'll never show
     up because the stale animation data overrides the new transform
     that we've been setting.  (At least, I think that's what was
     happening; it makes sense to me and matches the behavior I was
     observing.  I didn't verify which main thread updates and which
     layer updates were actually happening, though.)

     This shows up, for example, in the animation in attachment 8384813
     just halting at a corner if I'm careful not to disturb it.  (I'm
     testing on Linux, with both accelerated layers and OMT animations
     explicitly enabled.)

I think there are probably some other things that can be removed as
followups to removing this code, because I think we made some boundary
conditions intentionally incorrect so that problem (3) above wouldn't be
as bad as it otherwise would have been.
This commit is contained in:
L. David Baron 2014-03-04 20:13:22 -08:00
parent 598c0d4122
commit 503c4c2bbe

View File

@ -191,28 +191,6 @@ CommonAnimationManager::ReparentBeforeAndAfter(dom::Element* aElement,
}
}
// Ensure that the next repaint rebuilds the layer tree for aFrame. That
// means that changes to animations on aFrame's layer are propagated to
// the compositor, which is needed for correct behaviour of new
// transitions.
static void
ForceLayerRerendering(nsIFrame* aFrame, CommonElementAnimationData* aData)
{
if (aData->HasAnimationOfProperty(eCSSProperty_opacity)) {
if (Layer* layer = FrameLayerBuilder::GetDedicatedLayer(
aFrame, nsDisplayItem::TYPE_OPACITY)) {
layer->RemoveUserData(nsIFrame::LayerIsPrerenderedDataKey());
}
}
if (aData->HasAnimationOfProperty(eCSSProperty_transform)) {
if (Layer* layer = FrameLayerBuilder::GetDedicatedLayer(
aFrame, nsDisplayItem::TYPE_TRANSFORM)) {
layer->RemoveUserData(nsIFrame::LayerIsPrerenderedDataKey());
}
}
}
nsStyleContext*
CommonAnimationManager::UpdateThrottledStyle(dom::Element* aElement,
nsStyleContext* aParentStyle,
@ -254,9 +232,6 @@ CommonAnimationManager::UpdateThrottledStyle(dom::Element* aElement,
mPresContext->AnimationManager()->EnsureStyleRuleFor(ea);
curRule.mRule = ea->mStyleRule;
// FIXME (bug 828173): maybe not needed anymore:
ForceLayerRerendering(primaryFrame, ea);
} else if (curRule.mLevel == nsStyleSet::eTransitionSheet) {
ElementTransitions *et =
mPresContext->TransitionManager()->GetElementTransitions(
@ -268,9 +243,6 @@ CommonAnimationManager::UpdateThrottledStyle(dom::Element* aElement,
et->EnsureStyleRuleFor(mPresContext->RefreshDriver()->MostRecentRefresh());
curRule.mRule = et->mStyleRule;
// FIXME (bug 828173): maybe not needed anymore:
ForceLayerRerendering(primaryFrame, et);
} else {
curRule.mRule = ruleNode->GetRule();
}