Bug 1402439 - Redo how we discard compositor animation ids. r?pchang
Instead of always discarding the compositor animation id, and then
sometimes un-discarding it (which involves a linear lookup in nsTArray),
this patch now has the WebRenderLayerManager keep a set of active
animation ids, and uses that to avoid discarding the same animation
twice.
In addition, because the display item can be destroyed at any time (e.g.
in the middle of an animation), we were previously "leaking" compositor
animations in that the compositor side never got notified to discard the
IDs. This resulted in infinite composition loops. This patch solves this
problem by having any unused WebRenderAnimationData trigger discard of
the animation id during destruction. This way, even if the nsDisplayItem
is deleted in the middle of the animation we have a fallback mechanism
to discard the id.
MozReview-Commit-ID: 8G3EYHcg9Kl
--- a/gfx/layers/AnimationInfo.cpp
+++ b/gfx/layers/AnimationInfo.cpp
@@ -38,21 +38,16 @@ AnimationInfo::AddAnimation()
// Here generates a new id when the first animation is added and
// this id is used to represent the animations in this layer.
EnsureAnimationsId();
MOZ_ASSERT(!mPendingAnimations, "should have called ClearAnimations first");
Animation* anim = mAnimations.AppendElement();
- if (mManager->AsWebRenderLayerManager()) {
- mManager->AsWebRenderLayerManager()->
- KeepCompositorAnimationsIdAlive(mCompositorAnimationsId);
- }
-
mMutated = true;
return anim;
}
Animation*
AnimationInfo::AddAnimationForNextTransaction()
{
@@ -68,21 +63,16 @@ void
AnimationInfo::ClearAnimations()
{
mPendingAnimations = nullptr;
if (mAnimations.IsEmpty() && mAnimationData.IsEmpty()) {
return;
}
- if (mManager->AsWebRenderLayerManager()) {
- mManager->AsWebRenderLayerManager()->
- AddCompositorAnimationsIdForDiscard(mCompositorAnimationsId);
- }
-
mAnimations.Clear();
mAnimationData.Clear();
mMutated = true;
}
void
AnimationInfo::ClearAnimationsForNextTransaction()
--- a/gfx/layers/wr/WebRenderLayerManager.cpp
+++ b/gfx/layers/wr/WebRenderLayerManager.cpp
@@ -104,16 +104,22 @@ WebRenderLayerManager::DoDestroy(bool aI
// Just clear ImageKeys, they are deleted during WebRenderAPI destruction.
mImageKeysToDeleteLater.Clear();
mImageKeysToDelete.Clear();
// CompositorAnimations are cleared by WebRenderBridgeParent.
mDiscardedCompositorAnimationsIds.Clear();
WrBridge()->Destroy(aIsSync);
}
+ // Clear this before calling RemoveUnusedAndResetWebRenderUserData(),
+ // otherwise that function might destroy some WebRenderAnimationData instances
+ // which will put stuff back into mDiscardedCompositorAnimationsIds. If
+ // mActiveCompositorAnimationIds is empty that won't happen.
+ mActiveCompositorAnimationIds.clear();
+
mLastCanvasDatas.Clear();
RemoveUnusedAndResetWebRenderUserData();
if (mTransactionIdAllocator) {
// Make sure to notify the refresh driver just in case it's waiting on a
// pending transaction. Do this at the top of the event loop so we don't
// cause a paint to occur during compositor shutdown.
RefPtr<TransactionIdAllocator> allocator = mTransactionIdAllocator;
@@ -951,25 +957,40 @@ WebRenderLayerManager::DiscardImages()
resources.DeleteImage(key);
}
mImageKeysToDeleteLater.Clear();
mImageKeysToDelete.Clear();
WrBridge()->UpdateResources(resources);
}
void
-WebRenderLayerManager::AddCompositorAnimationsIdForDiscard(uint64_t aId)
+WebRenderLayerManager::AddActiveCompositorAnimationId(uint64_t aId)
{
- mDiscardedCompositorAnimationsIds.AppendElement(aId);
+ // In layers-free mode we track the active compositor animation ids on the
+ // client side so that we don't try to discard the same animation id multiple
+ // times. We could just ignore the multiple-discard on the parent side, but
+ // checking on the content side reduces IPC traffic.
+ MOZ_ASSERT(IsLayersFreeTransaction());
+ mActiveCompositorAnimationIds.insert(aId);
}
void
-WebRenderLayerManager::KeepCompositorAnimationsIdAlive(uint64_t aId)
+WebRenderLayerManager::AddCompositorAnimationsIdForDiscard(uint64_t aId)
{
- mDiscardedCompositorAnimationsIds.RemoveElement(aId);
+ if (!IsLayersFreeTransaction()) {
+ // For layers-full we don't track the active animation id in
+ // mActiveCompositorAnimationIds, we just call this on layer destruction and
+ // don't need to worry about discarding the same id multiple times.
+ mDiscardedCompositorAnimationsIds.AppendElement(aId);
+ } else if (mActiveCompositorAnimationIds.erase(aId)) {
+ // For layers-free ensure we don't try to discard an animation id that wasn't
+ // active. We also remove it from mActiveCompositorAnimationIds so we don't
+ // discard it again unless it gets re-activated.
+ mDiscardedCompositorAnimationsIds.AppendElement(aId);
+ }
}
void
WebRenderLayerManager::DiscardCompositorAnimations()
{
if (WrBridge()->IPCOpen() &&
!mDiscardedCompositorAnimationsIds.IsEmpty()) {
WrBridge()->
--- a/gfx/layers/wr/WebRenderLayerManager.h
+++ b/gfx/layers/wr/WebRenderLayerManager.h
@@ -1,16 +1,17 @@
/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#ifndef GFX_WEBRENDERLAYERMANAGER_H
#define GFX_WEBRENDERLAYERMANAGER_H
+#include <unordered_set>
#include <vector>
#include "gfxPrefs.h"
#include "Layers.h"
#include "mozilla/MozPromise.h"
#include "mozilla/layers/APZTestData.h"
#include "mozilla/layers/FocusTarget.h"
#include "mozilla/layers/StackingContextHelper.h"
@@ -166,22 +167,21 @@ public:
{ return mPaintedLayerCallbackData; }
// adds an imagekey to a list of keys that will be discarded on the next
// transaction or destruction
void AddImageKeyForDiscard(wr::ImageKey);
void DiscardImages();
void DiscardLocalImages();
- // Before destroying a layer with animations, add its compositorAnimationsId
- // to a list of ids that will be discarded on the next transaction
+ // Methods to manage the compositor animation ids. Active animations are still
+ // going, and when they end we discard them and remove them from the active
+ // list.
+ void AddActiveCompositorAnimationId(uint64_t aId);
void AddCompositorAnimationsIdForDiscard(uint64_t aId);
- // If the animations are valid and running on the compositor,
- // we should keep the compositorAnimationsId alive on the compositor side.
- void KeepCompositorAnimationsIdAlive(uint64_t aId);
void DiscardCompositorAnimations();
WebRenderBridgeChild* WrBridge() const { return mWrChild; }
virtual void Mutated(Layer* aLayer) override;
virtual void MutatedSimple(Layer* aLayer) override;
void Hold(Layer* aLayer);
@@ -292,16 +292,22 @@ private:
private:
nsIWidget* MOZ_NON_OWNING_REF mWidget;
nsTArray<wr::ImageKey> mImageKeysToDelete;
// TODO - This is needed because we have some code that creates image keys
// and enqueues them for deletion right away which is bad not only because
// of poor texture cache usage, but also because images end up deleted before
// they are used. This should hopfully be temporary.
nsTArray<wr::ImageKey> mImageKeysToDeleteLater;
+
+ // Set of compositor animation ids for which there are active animations (as
+ // of the last transaction) on the compositor side.
+ std::unordered_set<uint64_t> mActiveCompositorAnimationIds;
+ // Compositor animation ids for animations that are done now and that we want
+ // the compositor to discard information for.
nsTArray<uint64_t> mDiscardedCompositorAnimationsIds;
/* PaintedLayer callbacks; valid at the end of a transaciton,
* while rendering */
DrawPaintedLayerCallback mPaintedLayerCallback;
void *mPaintedLayerCallbackData;
RefPtr<WebRenderBridgeChild> mWrChild;
--- a/gfx/layers/wr/WebRenderUserData.cpp
+++ b/gfx/layers/wr/WebRenderUserData.cpp
@@ -215,16 +215,28 @@ WebRenderFallbackData::SetGeometry(nsAut
WebRenderAnimationData::WebRenderAnimationData(WebRenderLayerManager* aWRManager, nsDisplayItem* aItem,
WebRenderUserDataRefTable* aTable)
: WebRenderUserData(aWRManager, aItem, aTable)
, mAnimationInfo(aWRManager)
{
}
+WebRenderAnimationData::~WebRenderAnimationData()
+{
+ // It may be the case that nsDisplayItem that created this WebRenderUserData
+ // gets destroyed without getting a chance to discard the compositor animation
+ // id, so we should do it as part of cleanup here.
+ uint64_t animationId = mAnimationInfo.GetCompositorAnimationsId();
+ // animationId might be 0 if mAnimationInfo never held any active animations.
+ if (animationId) {
+ mWRManager->AddCompositorAnimationsIdForDiscard(animationId);
+ }
+}
+
WebRenderCanvasData::WebRenderCanvasData(WebRenderLayerManager* aWRManager, nsDisplayItem* aItem,
WebRenderUserDataRefTable* aTable)
: WebRenderUserData(aWRManager, aItem, aTable)
{
}
WebRenderCanvasData::~WebRenderCanvasData()
{
--- a/gfx/layers/wr/WebRenderUserData.h
+++ b/gfx/layers/wr/WebRenderUserData.h
@@ -133,17 +133,17 @@ protected:
bool mInvalid;
};
class WebRenderAnimationData : public WebRenderUserData
{
public:
explicit WebRenderAnimationData(WebRenderLayerManager* aWRManager, nsDisplayItem* aItem,
WebRenderUserDataRefTable* aTable);
- virtual ~WebRenderAnimationData() {}
+ virtual ~WebRenderAnimationData();
virtual UserDataType GetType() override { return UserDataType::eAnimation; }
static UserDataType Type() { return UserDataType::eAnimation; }
AnimationInfo& GetAnimationInfo() { return mAnimationInfo; }
protected:
AnimationInfo mAnimationInfo;
};
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -6260,27 +6260,32 @@ nsDisplayOpacity::CreateWebRenderCommand
float* opacityForSC = &mOpacity;
RefPtr<WebRenderAnimationData> animationData = aManager->CreateOrRecycleWebRenderUserData<WebRenderAnimationData>(this);
AnimationInfo& animationInfo = animationData->GetAnimationInfo();
AddAnimationsForProperty(Frame(), aDisplayListBuilder,
this, eCSSProperty_opacity,
animationInfo, false);
animationInfo.StartPendingAnimations(aManager->GetAnimationReadyTime());
- uint64_t animationsId = 0;
+
+ // Note that animationsId can be 0 (uninitialized in AnimationInfo) if there
+ // are no active animations.
+ uint64_t animationsId = animationInfo.GetCompositorAnimationsId();
if (!animationInfo.GetAnimations().IsEmpty()) {
- animationsId = animationInfo.GetCompositorAnimationsId();
opacityForSC = nullptr;
OptionalOpacity opacityForCompositor = mOpacity;
OpAddCompositorAnimations
anim(CompositorAnimations(animationInfo.GetAnimations(), animationsId),
void_t(), opacityForCompositor);
aManager->WrBridge()->AddWebRenderParentCommand(anim);
+ aManager->AddActiveCompositorAnimationId(animationsId);
+ } else if (animationsId) {
+ aManager->AddCompositorAnimationsIdForDiscard(animationsId);
}
nsTArray<mozilla::wr::WrFilterOp> filters;
StackingContextHelper sc(aSc,
aBuilder,
aDisplayListBuilder,
this,
&mList,
@@ -8023,34 +8028,38 @@ nsDisplayTransform::CreateWebRenderComma
RefPtr<WebRenderAnimationData> animationData = aManager->CreateOrRecycleWebRenderUserData<WebRenderAnimationData>(this);
AnimationInfo& animationInfo = animationData->GetAnimationInfo();
AddAnimationsForProperty(Frame(), aDisplayListBuilder,
this, eCSSProperty_transform,
animationInfo, false);
animationInfo.StartPendingAnimations(aManager->GetAnimationReadyTime());
- uint64_t animationsId = 0;
+
+ // Note that animationsId can be 0 (uninitialized in AnimationInfo) if there
+ // are no active animations.
+ uint64_t animationsId = animationInfo.GetCompositorAnimationsId();
if (!animationInfo.GetAnimations().IsEmpty()) {
- animationsId = animationInfo.GetCompositorAnimationsId();
-
// Update transfrom as nullptr in stacking context if there exists
// transform animation, the transform value will be resolved
// after animation sampling on the compositor
transformForSC = nullptr;
// Pass default transform to compositor in case gecko fails to
// get animated value after animation sampling.
OptionalTransform transformForCompositor = newTransformMatrix;
OpAddCompositorAnimations
anim(CompositorAnimations(animationInfo.GetAnimations(), animationsId),
transformForCompositor, void_t());
aManager->WrBridge()->AddWebRenderParentCommand(anim);
+ aManager->AddActiveCompositorAnimationId(animationsId);
+ } else if (animationsId) {
+ aManager->AddCompositorAnimationsIdForDiscard(animationsId);
}
gfx::Matrix4x4Typed<LayerPixel, LayerPixel> boundTransform = ViewAs<gfx::Matrix4x4Typed<LayerPixel, LayerPixel>>(newTransformMatrix);
nsTArray<mozilla::wr::WrFilterOp> filters;
StackingContextHelper sc(aSc,
aBuilder,
aDisplayListBuilder,