Bug 1357320 - Deal with the lifetime of compositor animations, r?kats draft
authorpeter chang <pchang@mozilla.com>
Wed, 19 Apr 2017 17:54:11 +0800
changeset 569912 7ea3d70fb84337cb5dc633248d87bbba126176d6
parent 565098 9c846a7169412b6e2635c1873c9d1b0b6ec3b367
child 569913 f2ac84b0ffd60b5492d04fcc93abbec2ab6577e3
push id56310
push userbmo:howareyou322@gmail.com
push dateFri, 28 Apr 2017 02:14:55 +0000
reviewerskats
bugs1357320
milestone55.0a1
Bug 1357320 - Deal with the lifetime of compositor animations, r?kats First, hook the Layer's ClearAnimation API to delete unnecessary animations in next layer transaction. Second, add another async DeleteCompositorAnimations API to delete animations on the compositor, especially calling this API before WebRenderLayerManager got destroyed. MozReview-Commit-ID: 4mbj5IgsXYa
gfx/layers/Layers.h
gfx/layers/ipc/PWebRenderBridge.ipdl
gfx/layers/ipc/WebRenderMessages.ipdlh
gfx/layers/wr/WebRenderBridgeParent.cpp
gfx/layers/wr/WebRenderBridgeParent.h
gfx/layers/wr/WebRenderContainerLayer.cpp
gfx/layers/wr/WebRenderContainerLayer.h
gfx/layers/wr/WebRenderLayerManager.cpp
--- a/gfx/layers/Layers.h
+++ b/gfx/layers/Layers.h
@@ -1233,17 +1233,17 @@ public:
     }
   }
 
   // Call AddAnimation to add a new animation to this layer from layout code.
   // Caller must fill in all the properties of the returned animation.
   // A later animation overrides an earlier one.
   Animation* AddAnimation();
   // ClearAnimations clears animations on this layer.
-  void ClearAnimations();
+  virtual void ClearAnimations();
   // This is only called when the layer tree is updated. Do not call this from
   // layout code.  To add an animation to this layer, use AddAnimation.
   void SetCompositorAnimations(const CompositorAnimations& aCompositorAnimations);
   // Go through all animations in this layer and its children and, for
   // any animations with a null start time, update their start time such
   // that at |aReadyTime| the animation's current time corresponds to its
   // 'initial current time' value.
   void StartPendingAnimations(const TimeStamp& aReadyTime);
--- a/gfx/layers/ipc/PWebRenderBridge.ipdl
+++ b/gfx/layers/ipc/PWebRenderBridge.ipdl
@@ -40,16 +40,17 @@ parent:
   sync Create(IntSize aSize);
   sync AddImage(ImageKey aImageKey, IntSize aSize, uint32_t aStride,
                 SurfaceFormat aFormat, ByteBuffer aBytes);
   async AddBlobImage(ImageKey aImageKey, IntSize aSize, uint32_t aStride,
                      SurfaceFormat aFormat, ByteBuffer aBytes);
   sync UpdateImage(ImageKey aImageKey, IntSize aSize,
                    SurfaceFormat aFormat, ByteBuffer aBytes);
   sync DeleteImage(ImageKey aImageKey);
+  async DeleteCompositorAnimations(uint64_t aId);
   async AddRawFont(FontKey aFontKey, ByteBuffer aBytes, uint32_t aFontIndex);
   async DeleteFont(FontKey aFontKey);
   async DPBegin(IntSize aSize);
   async DPEnd(IntSize aSize, WebRenderParentCommand[] commands, OpDestroy[] toDestroy, uint64_t fwdTransactionId, uint64_t transactionId,
               ByteBuffer aDL, WrBuiltDisplayListDescriptor aDLDesc, ByteBuffer aAux, WrAuxiliaryListsDescriptor aAuxDesc);
   sync DPSyncEnd(IntSize aSize, WebRenderParentCommand[] commands, OpDestroy[] toDestroy, uint64_t fwdTransactionId, uint64_t transactionId,
                  ByteBuffer aDL, WrBuiltDisplayListDescriptor aDLDesc, ByteBuffer aAux, WrAuxiliaryListsDescriptor aAuxDesc);
   sync DPGetSnapshot(PTexture texture);
--- a/gfx/layers/ipc/WebRenderMessages.ipdlh
+++ b/gfx/layers/ipc/WebRenderMessages.ipdlh
@@ -36,21 +36,16 @@ struct OpAddExternalImage {
   uint64_t externalImageId;
   ImageKey key;
 };
 
 struct OpAddCompositorAnimations {
   CompositorAnimations data;
 };
 
-struct OpRemoveCompositorAnimations {
-  uint64_t id;
-};
-
 union WebRenderParentCommand {
   OpAddExternalImage;
   CompositableOperation;
   OpAddCompositorAnimations;
-  OpRemoveCompositorAnimations;
 };
 
 } // namespace
 } // namespace
--- a/gfx/layers/wr/WebRenderBridgeParent.cpp
+++ b/gfx/layers/wr/WebRenderBridgeParent.cpp
@@ -264,16 +264,33 @@ WebRenderBridgeParent::RecvDeleteImage(c
   if (mActiveKeys.Get(wr::AsUint64(aImageKey), nullptr)) {
     mActiveKeys.Remove(wr::AsUint64(aImageKey));
   }
   mKeysToDelete.push_back(aImageKey);
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
+WebRenderBridgeParent::RecvDeleteCompositorAnimations(const uint64_t& aId)
+{
+  if (mDestroyed) {
+    return IPC_OK();
+  }
+  uint64_t id = mWidget ? 0 : mPipelineId.mHandle;
+  CompositorAnimationStorage* storage =
+    mCompositorBridge->GetAnimationStorage(id);
+
+  MOZ_ASSERT(storage);
+  storage->ClearById(aId);
+
+  return IPC_OK();
+}
+
+
+mozilla::ipc::IPCResult
 WebRenderBridgeParent::RecvDPBegin(const gfx::IntSize& aSize)
 {
   if (mDestroyed) {
     return IPC_OK();
   }
   return IPC_OK();
 }
 
@@ -424,28 +441,16 @@ WebRenderBridgeParent::ProcessWebRenderC
           CompositorAnimationStorage* storage =
             mCompositorBridge->GetAnimationStorage(id);
           if (storage) {
             storage->SetAnimations(data.id(), data.animations());
           }
         }
         break;
       }
-      case WebRenderParentCommand::TOpRemoveCompositorAnimations: {
-        const OpRemoveCompositorAnimations& op = cmd.get_OpRemoveCompositorAnimations();
-        if (op.id()) {
-          uint64_t id = mWidget ? 0 : mPipelineId.mHandle;
-          CompositorAnimationStorage* storage =
-            mCompositorBridge->GetAnimationStorage(id);
-          if (storage) {
-            storage->ClearById(op.id());
-          }
-        }
-        break;
-      }
       default: {
         // other commands are handle on the child
         break;
       }
     }
   }
   if (mWidget) {
     LayoutDeviceIntSize size = mWidget->GetClientSize();
--- a/gfx/layers/wr/WebRenderBridgeParent.h
+++ b/gfx/layers/wr/WebRenderBridgeParent.h
@@ -73,16 +73,17 @@ public:
                                            const uint32_t& aStride,
                                            const gfx::SurfaceFormat& aFormat,
                                            const ByteBuffer& aBuffer) override;
   mozilla::ipc::IPCResult RecvUpdateImage(const wr::ImageKey& aImageKey,
                                           const gfx::IntSize& aSize,
                                           const gfx::SurfaceFormat& aFormat,
                                           const ByteBuffer& aBuffer) override;
   mozilla::ipc::IPCResult RecvDeleteImage(const wr::ImageKey& a1) override;
+  mozilla::ipc::IPCResult RecvDeleteCompositorAnimations(const uint64_t& aId) override;
   mozilla::ipc::IPCResult RecvAddRawFont(const wr::FontKey& aFontKey,
                                          const ByteBuffer& aBuffer,
                                          const uint32_t& aFontIndex) override;
   mozilla::ipc::IPCResult RecvDeleteFont(const wr::FontKey& aFontKey) override;
   mozilla::ipc::IPCResult RecvDPBegin(const gfx::IntSize& aSize) override;
   mozilla::ipc::IPCResult RecvDPEnd(const gfx::IntSize& aSize,
                                     InfallibleTArray<WebRenderParentCommand>&& aCommands,
                                     InfallibleTArray<OpDestroy>&& aToDestroy,
--- a/gfx/layers/wr/WebRenderContainerLayer.cpp
+++ b/gfx/layers/wr/WebRenderContainerLayer.cpp
@@ -10,16 +10,28 @@
 #include "LayersLogging.h"
 #include "mozilla/layers/WebRenderBridgeChild.h"
 #include "mozilla/webrender/WebRenderTypes.h"
 
 namespace mozilla {
 namespace layers {
 
 void
+WebRenderContainerLayer::ClearAnimations()
+{
+
+  if (!GetAnimations().IsEmpty()) {
+    mManager->AsWebRenderLayerManager()->
+      AddCompositorAnimationsIdForDiscard(GetCompositorAnimationsId());
+  }
+
+  Layer::ClearAnimations();
+}
+
+void
 WebRenderContainerLayer::RenderLayer(wr::DisplayListBuilder& aBuilder)
 {
   nsTArray<LayerPolygon> children = SortChildrenBy3DZOrder(SortMode::WITHOUT_GEOMETRY);
 
   gfx::Matrix4x4 transform = GetTransform();
   float opacity = GetLocalOpacity();
   gfx::Rect relBounds = GetWrRelBounds();
   gfx::Rect clip(0, 0, relBounds.width, relBounds.height);
@@ -33,17 +45,17 @@ WebRenderContainerLayer::RenderLayer(wr:
                   this->GetLayer(),
                   Stringify(relBounds).c_str(),
                   Stringify(clip).c_str(),
                   Stringify(transform).c_str(),
                   Stringify(mixBlendMode).c_str());
   }
 
   if (gfxPrefs::WebRenderOMTAEnabled() &&
-      GetAnimations().Length()) {
+      !GetAnimations().IsEmpty()) {
     MOZ_ASSERT(GetCompositorAnimationsId());
 
     CompositorAnimations anim;
     anim.animations() = GetAnimations();
     anim.id() = GetCompositorAnimationsId();
     WrBridge()->AddWebRenderParentCommand(OpAddCompositorAnimations(anim));
 
     float* maybeOpacity = HasOpacityAnimation() ? nullptr : &opacity;
--- a/gfx/layers/wr/WebRenderContainerLayer.h
+++ b/gfx/layers/wr/WebRenderContainerLayer.h
@@ -22,29 +22,30 @@ public:
     MOZ_COUNT_CTOR(WebRenderContainerLayer);
   }
 
 protected:
   virtual ~WebRenderContainerLayer()
   {
 
     if (gfxPrefs::WebRenderOMTAEnabled() &&
-        GetAnimations().Length()) {
+        !GetAnimations().IsEmpty()) {
       mManager->AsWebRenderLayerManager()->
         AddCompositorAnimationsIdForDiscard(GetCompositorAnimationsId());
     }
 
     ContainerLayer::RemoveAllChildren();
     MOZ_COUNT_DTOR(WebRenderContainerLayer);
   }
 
 public:
   Layer* GetLayer() override { return this; }
   void RenderLayer(wr::DisplayListBuilder& aBuilder) override;
 
+  void ClearAnimations() override;
   virtual void ComputeEffectiveTransforms(const gfx::Matrix4x4& aTransformToSurface) override
   {
     DefaultComputeEffectiveTransforms(aTransformToSurface);
   }
 };
 
 class WebRenderRefLayer : public WebRenderLayer,
                           public RefLayer {
--- a/gfx/layers/wr/WebRenderLayerManager.cpp
+++ b/gfx/layers/wr/WebRenderLayerManager.cpp
@@ -277,16 +277,17 @@ void
 WebRenderLayerManager::Destroy()
 {
   if (IsDestroyed()) {
     return;
   }
 
   LayerManager::Destroy();
   DiscardImages();
+  DiscardCompositorAnimations();
   WrBridge()->Destroy();
 
   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;
     uint64_t id = mLatestTransactionId;
@@ -460,17 +461,17 @@ WebRenderLayerManager::AddCompositorAnim
 {
   mDiscardedCompositorAnimationsIds.push_back(aId);
 }
 
 void
 WebRenderLayerManager::DiscardCompositorAnimations()
 {
   for (auto id : mDiscardedCompositorAnimationsIds) {
-    WrBridge()->AddWebRenderParentCommand(OpRemoveCompositorAnimations(id));
+    WrBridge()->SendDeleteCompositorAnimations(id);
   }
   mDiscardedCompositorAnimationsIds.clear();
 }
 
 
 void
 WebRenderLayerManager::Hold(Layer* aLayer)
 {