Bug 1514384 - Pass the current clip chain id instead of clipping to the filter bounds. r=mattwoodrow

Differential Revision: https://phabricator.services.mozilla.com/D16206

gfx/layers/wr/AsyncImagePipelineManager.cpp

@@ -400,7 +400,8 @@ void AsyncImagePipelineManager::ApplyAsyncImageForPipeline(
 
   float opacity = 1.0f;
   Maybe<wr::WrSpatialId> referenceFrameId = builder.PushStackingContext(
-      wr::ToRoundedLayoutRect(aPipeline->mScBounds), nullptr, nullptr, &opacity,
+      wr::ToRoundedLayoutRect(aPipeline->mScBounds),
+      wr::WrStackingContextClip::None(), nullptr, &opacity,
       aPipeline->mScTransform.IsIdentity() ? nullptr : &aPipeline->mScTransform,
       wr::TransformStyle::Flat, nullptr, aPipeline->mMixBlendMode,
       nsTArray<wr::FilterOp>(), true,

gfx/layers/wr/StackingContextHelper.cpp

@@ -31,7 +31,7 @@ StackingContextHelper::StackingContextHelper(
     const gfx::CompositionOp& aMixBlendMode, bool aBackfaceVisible,
     bool aIsPreserve3D,
     const Maybe<nsDisplayTransform*>& aDeferredTransformItem,
-    const wr::WrClipId* aClipNodeId, bool aAnimated)
+    const wr::WrStackingContextClip& aClip, bool aAnimated)
     : mBuilder(&aBuilder),
       mScale(1.0f, 1.0f),
       mDeferredTransformItem(aDeferredTransformItem),
@@ -68,8 +68,7 @@ StackingContextHelper::StackingContextHelper(
           : wr::RasterSpace::Screen();
 
   mReferenceFrameId = mBuilder->PushStackingContext(
-      wr::ToLayoutRect(aBounds), aClipNodeId, aAnimation, aOpacityPtr,
-      aTransformPtr,
+      wr::ToLayoutRect(aBounds), aClip, aAnimation, aOpacityPtr, aTransformPtr,
       aIsPreserve3D ? wr::TransformStyle::Preserve3D : wr::TransformStyle::Flat,
       aPerspectivePtr, wr::ToMixBlendMode(aMixBlendMode), aFilters,
       aBackfaceVisible, rasterSpace);

gfx/layers/wr/StackingContextHelper.h

@@ -41,7 +41,8 @@ class MOZ_RAII StackingContextHelper {
       const gfx::CompositionOp& aMixBlendMode = gfx::CompositionOp::OP_OVER,
       bool aBackfaceVisible = true, bool aIsPreserve3D = false,
       const Maybe<nsDisplayTransform*>& aDeferredTransformItem = Nothing(),
-      const wr::WrClipId* aClipNodeId = nullptr, bool aAnimated = false);
+      const wr::WrStackingContextClip& = wr::WrStackingContextClip::None(),
+      bool aAnimated = false);
   // This version of the constructor should only be used at the root level
   // of the tree, so that we have a StackingContextHelper to pass down into
   // the RenderLayer traversal, but don't actually want it to push a stacking

gfx/webrender_bindings/WebRenderAPI.cpp

@@ -20,7 +20,8 @@
 
 //#define WRDL_LOG(...) printf_stderr("WRDL(%p): " __VA_ARGS__)
 
-//#define WRDL_LOG(...) if (XRE_IsContentProcess()) printf_stderr("WRDL(%p): " __VA_ARGS__)
+//#define WRDL_LOG(...) if (XRE_IsContentProcess()) printf_stderr("WRDL(%p): "
+//__VA_ARGS__)
 
 namespace mozilla {
 namespace wr {
@@ -672,7 +673,7 @@ void DisplayListBuilder::Finalize(wr::LayoutSize& aOutContentSize,
 }
 
 Maybe<wr::WrSpatialId> DisplayListBuilder::PushStackingContext(
-    const wr::LayoutRect& aBounds, const wr::WrClipId* aClipNodeId,
+    const wr::LayoutRect& aBounds, const wr::WrStackingContextClip& aClip,
     const WrAnimationProperty* aAnimation, const float* aOpacity,
     const gfx::Matrix4x4* aTransform, wr::TransformStyle aTransformStyle,
     const gfx::Matrix4x4* aPerspective, const wr::MixBlendMode& aMixBlendMode,
@@ -698,8 +699,8 @@ Maybe<wr::WrSpatialId> DisplayListBuilder::PushStackingContext(
            aTransform ? Stringify(*aTransform).c_str() : "none");
 
   auto spatialId = wr_dp_push_stacking_context(
-      mWrState, aBounds, mCurrentSpaceAndClipChain.space, aClipNodeId,
-      aAnimation, aOpacity, maybeTransform, aTransformStyle, maybePerspective,
+      mWrState, aBounds, mCurrentSpaceAndClipChain.space, &aClip, aAnimation,
+      aOpacity, maybeTransform, aTransformStyle, maybePerspective,
       aMixBlendMode, aFilters.Elements(), aFilters.Length(), aIsBackfaceVisible,
       aRasterSpace);
 

gfx/webrender_bindings/WebRenderAPI.h

@@ -329,9 +329,8 @@ class DisplayListBuilder {
                 wr::BuiltDisplayList& aOutDisplayList);
 
   Maybe<wr::WrSpatialId> PushStackingContext(
-      const wr::LayoutRect&
-          aBounds,  // TODO: We should work with strongly typed rects
-      const wr::WrClipId* aClipNodeId,
+      // TODO: We should work with strongly typed rects
+      const wr::LayoutRect& aBounds, const wr::WrStackingContextClip& aClip,
       const wr::WrAnimationProperty* aAnimation, const float* aOpacity,
       const gfx::Matrix4x4* aTransform, wr::TransformStyle aTransformStyle,
       const gfx::Matrix4x4* aPerspective, const wr::MixBlendMode& aMixBlendMode,
@@ -486,6 +485,10 @@ class DisplayListBuilder {
                      const wr::BorderRadius& aBorderRadius,
                      const wr::BoxShadowClipMode& aClipMode);
 
+  uint64_t CurrentClipChainId() const {
+    return mCurrentSpaceAndClipChain.clip_chain;
+  }
+
   // Checks to see if the innermost enclosing fixed pos item has the same
   // ASR. If so, it returns the scroll target for that fixed-pos item.
   // Otherwise, it returns Nothing().

gfx/webrender_bindings/src/bindings.rs

@@ -140,6 +140,15 @@ impl WrSpaceAndClip {
     }
 }
 
+#[inline]
+fn clip_chain_id_to_webrender(id: u64, pipeline_id: WrPipelineId) -> ClipId {
+    if id == ROOT_CLIP_CHAIN {
+        ClipId::root(pipeline_id)
+    } else {
+        ClipId::ClipChain(ClipChainId(id, pipeline_id))
+    }
+}
+
 #[repr(C)]
 pub struct WrSpaceAndClipChain {
     space: WrSpatialId,
@@ -151,15 +160,27 @@ impl WrSpaceAndClipChain {
         //Warning: special case here to support dummy clip chain
         SpaceAndClipInfo {
             spatial_id: self.space.to_webrender(pipeline_id),
-            clip_id: if self.clip_chain == ROOT_CLIP_CHAIN {
-                ClipId::root(pipeline_id)
-            } else {
-                ClipId::ClipChain(ClipChainId(self.clip_chain, pipeline_id))
-            },
+            clip_id: clip_chain_id_to_webrender(self.clip_chain, pipeline_id),
         }
     }
 }
 
+#[repr(C)]
+pub enum WrStackingContextClip {
+    None,
+    ClipId(WrClipId),
+    ClipChain(u64),
+}
+
+impl WrStackingContextClip {
+    fn to_webrender(&self, pipeline_id: WrPipelineId) -> Option<ClipId> {
+        match *self {
+            WrStackingContextClip::None => None,
+            WrStackingContextClip::ClipChain(id) => Some(clip_chain_id_to_webrender(id, pipeline_id)),
+            WrStackingContextClip::ClipId(id) => Some(id.to_webrender(pipeline_id)),
+        }
+    }
+}
 
 fn make_slice<'a, T>(ptr: *const T, len: usize) -> &'a [T] {
     if ptr.is_null() {
@@ -1874,10 +1895,11 @@ pub extern "C" fn wr_dp_clear_save(state: &mut WrState) {
 }
 
 #[no_mangle]
-pub extern "C" fn wr_dp_push_stacking_context(state: &mut WrState,
+pub extern "C" fn wr_dp_push_stacking_context(
+    state: &mut WrState,
     mut bounds: LayoutRect,
     spatial_id: WrSpatialId,
-                                              clip_node_id: *const WrClipId,
+    clip: &WrStackingContextClip,
     animation: *const WrAnimationProperty,
     opacity: *const f32,
     transform: *const LayoutTransform,
@@ -1896,8 +1918,6 @@ pub extern "C" fn wr_dp_push_stacking_context(state: &mut WrState,
                                                            *c_filter
     }).collect();
 
-    let clip_node_id_ref = unsafe { clip_node_id.as_ref() };
-
     let transform_ref = unsafe { transform.as_ref() };
     let mut transform_binding = match transform_ref {
         Some(transform) => Some(PropertyBinding::Value(transform.clone())),
@@ -1942,7 +1962,7 @@ pub extern "C" fn wr_dp_push_stacking_context(state: &mut WrState,
     };
 
     let mut wr_spatial_id = spatial_id.to_webrender(state.pipeline_id);
-    let wr_clip_id = clip_node_id_ref.map(|id| id.to_webrender(state.pipeline_id));
+    let wr_clip_id = clip.to_webrender(state.pipeline_id);
 
     let is_reference_frame = transform_binding.is_some() || perspective.is_some();
     // Note: 0 has special meaning in WR land, standing for ROOT_REFERENCE_FRAME.

gfx/webrender_bindings/webrender_ffi_generated.h

@@ -839,6 +839,81 @@ struct Shadow {
   }
 };
 
+struct WrStackingContextClip {
+  enum class Tag {
+    None,
+    ClipId,
+    ClipChain,
+
+    Sentinel /* this must be last for serialization purposes. */
+  };
+
+  struct ClipId_Body {
+    WrClipId _0;
+
+    bool operator==(const ClipId_Body& aOther) const {
+      return _0 == aOther._0;
+    }
+  };
+
+  struct ClipChain_Body {
+    uint64_t _0;
+
+    bool operator==(const ClipChain_Body& aOther) const {
+      return _0 == aOther._0;
+    }
+  };
+
+  Tag tag;
+  union {
+    ClipId_Body clip_id;
+    ClipChain_Body clip_chain;
+  };
+
+  static WrStackingContextClip None() {
+    WrStackingContextClip result;
+    result.tag = Tag::None;
+    return result;
+  }
+
+  static WrStackingContextClip ClipId(const WrClipId &a0) {
+    WrStackingContextClip result;
+    result.clip_id._0 = a0;
+    result.tag = Tag::ClipId;
+    return result;
+  }
+
+  static WrStackingContextClip ClipChain(const uint64_t &a0) {
+    WrStackingContextClip result;
+    result.clip_chain._0 = a0;
+    result.tag = Tag::ClipChain;
+    return result;
+  }
+
+  bool IsNone() const {
+    return tag == Tag::None;
+  }
+
+  bool IsClipId() const {
+    return tag == Tag::ClipId;
+  }
+
+  bool IsClipChain() const {
+    return tag == Tag::ClipChain;
+  }
+
+  bool operator==(const WrStackingContextClip& aOther) const {
+    if (tag != aOther.tag) {
+      return false;
+    }
+    switch (tag) {
+      case Tag::ClipId: return clip_id == aOther.clip_id;
+      case Tag::ClipChain: return clip_chain == aOther.clip_chain;
+      default: return true;
+    }
+  }
+};
+
 struct WrAnimationProperty {
   WrAnimationType effect_type;
   uint64_t id;
@@ -1937,7 +2012,7 @@ WR_INLINE
 WrSpatialId wr_dp_push_stacking_context(WrState *aState,
                                         LayoutRect aBounds,
                                         WrSpatialId aSpatialId,
-                                        const WrClipId *aClipNodeId,
+                                        const WrStackingContextClip *aClip,
                                         const WrAnimationProperty *aAnimation,
                                         const float *aOpacity,
                                         const LayoutTransform *aTransform,

layout/painting/nsDisplayList.cpp

@@ -7878,7 +7878,7 @@ bool nsDisplayTransform::CreateWebRenderCommands(
       animationsId ? &prop : nullptr, nullptr, transformForSC, nullptr,
       gfx::CompositionOp::OP_OVER, !BackfaceIsHidden(),
       mFrame->Extend3DContext() && !mNoExtendContext, deferredTransformItem,
-      nullptr, animated);
+      wr::WrStackingContextClip::None(), animated);
 
   return mStoredList.CreateWebRenderCommands(aBuilder, aResources, sc, aManager,
                                              aDisplayListBuilder);
@@ -9071,7 +9071,7 @@ bool nsDisplayMasksAndClipPaths::CreateWebRenderCommands(
                   /*aBackfaceVisible: */ true,
                   /*aIsPreserve3D: */ false,
                   /*aTransformForScrollData: */ Nothing(),
-                  /*aClipNodeId: */ &clipId);
+                  /*aClip: */ wr::WrStackingContextClip::ClipId(clipId));
     sc = layer.ptr();
   }
 
@@ -9348,15 +9348,13 @@ bool nsDisplayFilters::CreateWebRenderCommands(
     return false;
   }
 
-  wr::WrClipId clipId =
-      aBuilder.DefineClip(Nothing(), wr::ToLayoutRect(postFilterBounds));
-
   float opacity = mFrame->StyleEffects()->mOpacity;
   StackingContextHelper sc(
       aSc, GetActiveScrolledRoot(), mFrame, this, aBuilder, wrFilters,
       LayoutDeviceRect(), nullptr, nullptr,
       opacity != 1.0f && mHandleOpacity ? &opacity : nullptr, nullptr, nullptr,
-      gfx::CompositionOp::OP_OVER, true, false, Nothing(), &clipId);
+      gfx::CompositionOp::OP_OVER, true, false, Nothing(),
+      wr::WrStackingContextClip::ClipChain(aBuilder.CurrentClipChainId()));
 
   nsDisplayEffectsBase::CreateWebRenderCommands(aBuilder, aResources, sc,
                                                 aManager, aDisplayListBuilder);