Bug 1509554. Define WrClipId in bindings.rs. r=kats
authorJeff Muizelaar <jrmuizel@gmail.com>
Fri, 23 Nov 2018 19:49:56 +0000
changeset 507103 856f149c4faa4ebbe466f58d0c56dbb2f9ba83d0
parent 507102 7bc33731a895a6d634bef92a9da8c4148eb269a9
child 507104 deed2b6607e9b77e0b5cc92ac16ed734629b37f0
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats
bugs1509554
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1509554. Define WrClipId in bindings.rs. r=kats This lets us avoid having to put usize in the exported signatures from bindings.rs It also avoids a heap allocation when defining a clip chain. Differential Revision: https://phabricator.services.mozilla.com/D12785
gfx/layers/wr/ClipManager.h
gfx/webrender_bindings/WebRenderAPI.cpp
gfx/webrender_bindings/WebRenderTypes.cpp
gfx/webrender_bindings/WebRenderTypes.h
gfx/webrender_bindings/src/bindings.rs
gfx/webrender_bindings/webrender_ffi_generated.h
--- a/gfx/layers/wr/ClipManager.h
+++ b/gfx/layers/wr/ClipManager.h
@@ -114,18 +114,17 @@ private:
   // Any time ClipManager wants to define a new clip as a child of ASR X, it
   // should first check the cache overrides to see if there is a cache override
   // item ((a) or (b) above) that is already a child of X, and then define that
   // clip as a child of Y instead. This map stores X -> Y, which allows
   // ClipManager to do the necessary lookup. Note that there theoretically might
   // be multiple different "Y" clips (in case of nested cache overrides), which
   // is why we need a stack.
   std::unordered_map<wr::WrClipId,
-                     std::stack<wr::WrClipId>,
-                     wr::WrClipId::HashFn> mASROverride;
+                     std::stack<wr::WrClipId>> mASROverride;
 
   // This holds some clip state for a single nsDisplayItem
   struct ItemClips {
     ItemClips(const ActiveScrolledRoot* aASR,
               const DisplayItemClipChain* aChain,
               bool aSeparateLeaf);
 
     // These are the "inputs" - they come from the nsDisplayItem
--- a/gfx/webrender_bindings/WebRenderAPI.cpp
+++ b/gfx/webrender_bindings/WebRenderAPI.cpp
@@ -875,23 +875,22 @@ DisplayListBuilder::PushStackingContext(
   }
   const wr::LayoutTransform* maybeTransform = aTransform ? &matrix : nullptr;
   wr::LayoutTransform perspective;
   if (aPerspective) {
     perspective = ToLayoutTransform(*aPerspective);
   }
 
   const wr::LayoutTransform* maybePerspective = aPerspective ? &perspective : nullptr;
-  const size_t* maybeClipNodeId = aClipNodeId ? &aClipNodeId->id : nullptr;
   WRDL_LOG("PushStackingContext b=%s t=%s\n", mWrState, Stringify(aBounds).c_str(),
       aTransform ? Stringify(*aTransform).c_str() : "none");
 
   bool outIsReferenceFrame = false;
   uintptr_t outReferenceFrameId = 0;
-  wr_dp_push_stacking_context(mWrState, aBounds, maybeClipNodeId, aAnimation,
+  wr_dp_push_stacking_context(mWrState, aBounds, aClipNodeId, aAnimation,
                               aOpacity, maybeTransform, aTransformStyle,
                               maybePerspective, aMixBlendMode,
                               aFilters.Elements(), aFilters.Length(),
                               aIsBackfaceVisible, aRasterSpace,
                               &outIsReferenceFrame, &outReferenceFrameId);
   return outIsReferenceFrame ? Some(wr::WrClipId { outReferenceFrameId }) : Nothing();
 }
 
@@ -901,55 +900,51 @@ DisplayListBuilder::PopStackingContext(b
   WRDL_LOG("PopStackingContext\n", mWrState);
   wr_dp_pop_stacking_context(mWrState, aIsReferenceFrame);
 }
 
 wr::WrClipChainId
 DisplayListBuilder::DefineClipChain(const Maybe<wr::WrClipChainId>& aParent,
                                     const nsTArray<wr::WrClipId>& aClips)
 {
-  nsTArray<size_t> clipIds;
-  for (wr::WrClipId id : aClips) {
-    clipIds.AppendElement(id.id);
-  }
   uint64_t clipchainId = wr_dp_define_clipchain(mWrState,
       aParent ? &(aParent->id) : nullptr,
-      clipIds.Elements(), clipIds.Length());
+      aClips.Elements(), aClips.Length());
   WRDL_LOG("DefineClipChain id=%" PRIu64 " p=%s clips=%zu\n", mWrState,
       clipchainId,
       aParent ? Stringify(aParent->id).c_str() : "(nil)",
       clipIds.Length());
   return wr::WrClipChainId{ clipchainId };
 }
 
 wr::WrClipId
 DisplayListBuilder::DefineClip(const Maybe<wr::WrClipId>& aParentId,
                                const wr::LayoutRect& aClipRect,
                                const nsTArray<wr::ComplexClipRegion>* aComplex,
                                const wr::WrImageMask* aMask)
 {
   size_t clip_id = wr_dp_define_clip(mWrState,
-      aParentId ? &(aParentId->id) : nullptr,
+      aParentId.ptrOr(nullptr),
       aClipRect,
       aComplex ? aComplex->Elements() : nullptr,
       aComplex ? aComplex->Length() : 0,
       aMask);
   WRDL_LOG("DefineClip id=%zu p=%s r=%s m=%p b=%s complex=%zu\n", mWrState,
       clip_id, aParentId ? Stringify(aParentId->id).c_str() : "(nil)",
       Stringify(aClipRect).c_str(), aMask,
       aMask ? Stringify(aMask->rect).c_str() : "none",
       aComplex ? aComplex->Length() : 0);
   return wr::WrClipId { clip_id };
 }
 
 void
 DisplayListBuilder::PushClip(const wr::WrClipId& aClipId)
 {
   WRDL_LOG("PushClip id=%zu\n", mWrState, aClipId.id);
-  wr_dp_push_clip(mWrState, aClipId.id);
+  wr_dp_push_clip(mWrState, aClipId);
 }
 
 void
 DisplayListBuilder::PopClip()
 {
   WRDL_LOG("PopClip\n", mWrState);
   wr_dp_pop_clip(mWrState);
 }
@@ -979,17 +974,17 @@ DisplayListBuilder::DefineStickyFrame(co
       Stringify(aAppliedOffset).c_str());
   return wr::WrClipId { id };
 }
 
 Maybe<wr::WrClipId>
 DisplayListBuilder::GetScrollIdForDefinedScrollLayer(layers::ScrollableLayerGuid::ViewID aViewId) const
 {
   if (aViewId == layers::ScrollableLayerGuid::NULL_SCROLL_ID) {
-    return Some(wr::WrClipId::RootScrollNode());
+    return Some(wr::RootScrollNode());
   }
 
   auto it = mScrollIds.find(aViewId);
   if (it == mScrollIds.end()) {
     return Nothing();
   }
 
   return Some(it->second);
@@ -1005,17 +1000,17 @@ DisplayListBuilder::DefineScrollLayer(co
   if (it != mScrollIds.end()) {
     return it->second;
   }
 
   // We haven't defined aViewId before, so let's define it now.
   size_t numericScrollId = wr_dp_define_scroll_layer(
       mWrState,
       aViewId,
-      aParentId ? &(aParentId->id) : nullptr,
+      aParentId.ptrOr(nullptr),
       aContentRect,
       aClipRect);
 
   WRDL_LOG("DefineScrollLayer id=%" PRIu64 "/%zu p=%s co=%s cl=%s\n", mWrState,
       aViewId, numericScrollId,
       aParentId ? Stringify(aParentId->id).c_str() : "(nil)",
       Stringify(aContentRect).c_str(), Stringify(aClipRect).c_str());
 
@@ -1027,17 +1022,17 @@ DisplayListBuilder::DefineScrollLayer(co
 void
 DisplayListBuilder::PushClipAndScrollInfo(const wr::WrClipId* aScrollId,
                                           const wr::WrClipChainId* aClipChainId,
                                           const Maybe<wr::LayoutRect>& aClipChainLeaf)
 {
   if (aScrollId) {
     WRDL_LOG("PushClipAndScroll s=%zu c=%s\n", mWrState, aScrollId->id,
         aClipChainId ? Stringify(aClipChainId->id).c_str() : "none");
-    wr_dp_push_clip_and_scroll_info(mWrState, aScrollId->id,
+    wr_dp_push_clip_and_scroll_info(mWrState, *aScrollId,
         aClipChainId ? &(aClipChainId->id) : nullptr);
   }
   mClipChainLeaf = aClipChainLeaf;
 }
 
 void
 DisplayListBuilder::PopClipAndScrollInfo(const wr::WrClipId* aScrollId)
 {
--- a/gfx/webrender_bindings/WebRenderTypes.cpp
+++ b/gfx/webrender_bindings/WebRenderTypes.cpp
@@ -28,14 +28,14 @@ Assign_WrVecU8(wr::WrVecU8& aVec, mozill
   aVec.length = aOther.mLen;
   aVec.capacity = aOther.mCapacity;
   aOther.mData = nullptr;
   aOther.mLen = 0;
   aOther.mCapacity = 0;
 }
 
 /*static*/ WrClipId
-WrClipId::RootScrollNode() {
+RootScrollNode() {
   return WrClipId { wr_root_scroll_node_id() };
 }
 
 } // namespace wr
 } // namespace mozilla
--- a/gfx/webrender_bindings/WebRenderTypes.h
+++ b/gfx/webrender_bindings/WebRenderTypes.h
@@ -823,44 +823,17 @@ static inline wr::WrFilterOpType ToWrFil
       return wr::WrFilterOpType::Sepia;
     case NS_STYLE_FILTER_DROP_SHADOW:
       return wr::WrFilterOpType::DropShadow;
   }
   MOZ_ASSERT_UNREACHABLE("Tried to convert unknown filter type.");
   return wr::WrFilterOpType::Grayscale;
 }
 
-// Corresponds to an "internal" webrender clip id. That is, a
-// ClipId::Clip(x,pipeline_id) maps to a WrClipId{x}. We use a struct wrapper
-// instead of a typedef so that this is a distinct type from ids generated
-// by scroll and position:sticky nodes and the compiler will catch accidental
-// conversions between them.
-struct WrClipId {
-  size_t id;
-
-  bool operator==(const WrClipId& other) const {
-    return id == other.id;
-  }
-
-  bool operator!=(const WrClipId& other) const {
-    return !(*this == other);
-  }
-
-  static WrClipId RootScrollNode();
-
-  // Helper struct that allows this class to be used as a key in
-  // std::unordered_map like so:
-  //   std::unordered_map<WrClipId, ValueType, WrClipId::HashFn> myMap;
-  struct HashFn {
-    std::size_t operator()(const WrClipId& aKey) const
-    {
-      return std::hash<size_t>{}(aKey.id);
-    }
-  };
-};
+extern WrClipId RootScrollNode();
 
 // Corresponds to a clip id for a clip chain in webrender. Similar to
 // WrClipId but a separate struct so we don't get them mixed up in C++.
 struct WrClipChainId {
   uint64_t id;
 
   bool operator==(const WrClipChainId& other) const {
     return id == other.id;
@@ -907,9 +880,19 @@ static inline wr::SyntheticItalics Degre
   wr::SyntheticItalics synthetic_italics;
   synthetic_italics.angle = int16_t(std::min(std::max(aDegrees, -89.0f), 89.0f) * 256.0f);
   return synthetic_italics;
 }
 
 } // namespace wr
 } // namespace mozilla
 
+namespace std
+{
+    template<> struct hash<mozilla::wr::WrClipId>
+    {
+        std::size_t operator()(mozilla::wr::WrClipId const& aKey) const noexcept {
+          return std::hash<size_t>{}(aKey.id);
+        }
+    };
+}
+
 #endif /* GFX_WEBRENDERTYPES_H */
--- a/gfx/webrender_bindings/src/bindings.rs
+++ b/gfx/webrender_bindings/src/bindings.rs
@@ -1826,17 +1826,17 @@ pub extern "C" fn wr_dp_restore(state: &
 #[no_mangle]
 pub extern "C" fn wr_dp_clear_save(state: &mut WrState) {
     state.frame_builder.dl_builder.clear_save();
 }
 
 #[no_mangle]
 pub extern "C" fn wr_dp_push_stacking_context(state: &mut WrState,
                                               bounds: LayoutRect,
-                                              clip_node_id: *const usize,
+                                              clip_node_id: *const WrClipId,
                                               animation: *const WrAnimationProperty,
                                               opacity: *const f32,
                                               transform: *const LayoutTransform,
                                               transform_style: TransformStyle,
                                               perspective: *const LayoutTransform,
                                               mix_blend_mode: MixBlendMode,
                                               filters: *const WrFilterOp,
                                               filter_count: usize,
@@ -1952,34 +1952,34 @@ pub extern "C" fn wr_dp_pop_stacking_con
         state.frame_builder.dl_builder.pop_clip_id();
         state.frame_builder.dl_builder.pop_reference_frame();
     }
 }
 
 #[no_mangle]
 pub extern "C" fn wr_dp_define_clipchain(state: &mut WrState,
                                          parent_clipchain_id: *const u64,
-                                         clips: *const usize,
+                                         clips: *const WrClipId,
                                          clips_count: usize)
                                          -> u64 {
     debug_assert!(unsafe { is_in_main_thread() });
     let parent = unsafe { parent_clipchain_id.as_ref() }.map(|id| ClipChainId(*id, state.pipeline_id));
     let pipeline_id = state.pipeline_id;
     let clips = make_slice(clips, clips_count)
         .iter()
         .map(|id| unpack_clip_id(*id, pipeline_id));
 
     let clipchain_id = state.frame_builder.dl_builder.define_clip_chain(parent, clips);
     assert!(clipchain_id.1 == state.pipeline_id);
     clipchain_id.0
 }
 
 #[no_mangle]
 pub extern "C" fn wr_dp_define_clip(state: &mut WrState,
-                                    parent_id: *const usize,
+                                    parent_id: *const WrClipId,
                                     clip_rect: LayoutRect,
                                     complex: *const ComplexClipRegion,
                                     complex_count: usize,
                                     mask: *const WrImageMask)
                                     -> usize {
     debug_assert!(unsafe { is_in_main_thread() });
 
     let parent_id = unsafe { parent_id.as_ref() };
@@ -1994,17 +1994,17 @@ pub extern "C" fn wr_dp_define_clip(stat
     } else {
         state.frame_builder.dl_builder.define_clip(clip_rect, complex_iter, mask)
     };
     pack_clip_id(clip_id)
 }
 
 #[no_mangle]
 pub extern "C" fn wr_dp_push_clip(state: &mut WrState,
-                                  clip_id: usize) {
+                                  clip_id: WrClipId) {
     debug_assert!(unsafe { is_in_main_thread() });
     state.frame_builder.dl_builder.push_clip_id(unpack_clip_id(clip_id, state.pipeline_id));
 }
 
 #[no_mangle]
 pub extern "C" fn wr_dp_pop_clip(state: &mut WrState) {
     debug_assert!(unsafe { !is_in_render_thread() });
     state.frame_builder.dl_builder.pop_clip_id();
@@ -2031,17 +2031,17 @@ pub extern "C" fn wr_dp_define_sticky_fr
         ),
         vertical_bounds, horizontal_bounds, applied_offset);
     pack_clip_id(clip_id)
 }
 
 #[no_mangle]
 pub extern "C" fn wr_dp_define_scroll_layer(state: &mut WrState,
                                             scroll_id: u64,
-                                            parent_id: *const usize,
+                                            parent_id: *const WrClipId,
                                             content_rect: LayoutRect,
                                             clip_rect: LayoutRect)
                                             -> usize {
     assert!(unsafe { is_in_main_thread() });
 
     let parent_id = unsafe { parent_id.as_ref() };
 
     let new_id = if let Some(&pid) = parent_id {
@@ -2065,31 +2065,31 @@ pub extern "C" fn wr_dp_define_scroll_la
         )
     };
 
     pack_clip_id(new_id)
 }
 
 #[no_mangle]
 pub extern "C" fn wr_dp_push_scroll_layer(state: &mut WrState,
-                                          scroll_id: usize) {
+                                          scroll_id: WrClipId) {
     debug_assert!(unsafe { is_in_main_thread() });
     let clip_id = unpack_clip_id(scroll_id, state.pipeline_id);
     state.frame_builder.dl_builder.push_clip_id(clip_id);
 }
 
 #[no_mangle]
 pub extern "C" fn wr_dp_pop_scroll_layer(state: &mut WrState) {
     debug_assert!(unsafe { is_in_main_thread() });
     state.frame_builder.dl_builder.pop_clip_id();
 }
 
 #[no_mangle]
 pub extern "C" fn wr_dp_push_clip_and_scroll_info(state: &mut WrState,
-                                                  scroll_id: usize,
+                                                  scroll_id: WrClipId,
                                                   clip_chain_id: *const u64) {
     debug_assert!(unsafe { is_in_main_thread() });
 
     let clip_chain_id = unsafe { clip_chain_id.as_ref() };
     let info = if let Some(&ccid) = clip_chain_id {
         ClipAndScrollInfo::new(
             unpack_clip_id(scroll_id, state.pipeline_id),
             ClipId::ClipChain(ClipChainId(ccid, state.pipeline_id)))
@@ -2698,19 +2698,25 @@ fn pack_clip_id(id: ClipId) -> usize {
         ClipId::Clip(id, _) => (id, 1),
         ClipId::ClipChain(..) => unreachable!("Tried to pack a clip chain id"),
     };
 
     assert!(id <= usize::max_value() >> 1);
     return (id << 1) + type_value;
 }
 
-fn unpack_clip_id(id: usize, pipeline_id: PipelineId) -> ClipId {
-    let type_value = id & 0b01;
-    let id = id >> 1;
+#[repr(C)]
+#[derive(Clone, Copy)]
+pub struct WrClipId {
+    id: usize
+}
+
+fn unpack_clip_id(id: WrClipId, pipeline_id: PipelineId) -> ClipId {
+    let type_value = id.id & 0b01;
+    let id = id.id >> 1;
 
     match type_value {
         0 => ClipId::Spatial(id, pipeline_id),
         1 => ClipId::Clip(id, pipeline_id),
         _ => unreachable!("Got a bizarre value for the clip type"),
     }
 }
 
--- a/gfx/webrender_bindings/webrender_ffi_generated.h
+++ b/gfx/webrender_bindings/webrender_ffi_generated.h
@@ -629,16 +629,24 @@ struct TypedPoint2D {
   bool operator==(const TypedPoint2D& aOther) const {
     return x == aOther.x &&
            y == aOther.y;
   }
 };
 
 using WorldPoint = TypedPoint2D<float, WorldPixel>;
 
+struct WrClipId {
+  uintptr_t id;
+
+  bool operator==(const WrClipId& aOther) const {
+    return id == aOther.id;
+  }
+};
+
 struct WrDebugFlags {
   uint32_t mBits;
 
   bool operator==(const WrDebugFlags& aOther) const {
     return mBits == aOther.mBits;
   }
 };
 
@@ -1288,34 +1296,34 @@ void wr_device_delete(Device *aDevice)
 WR_DESTRUCTOR_SAFE_FUNC;
 
 WR_INLINE
 void wr_dp_clear_save(WrState *aState)
 WR_FUNC;
 
 WR_INLINE
 uintptr_t wr_dp_define_clip(WrState *aState,
-                            const uintptr_t *aParentId,
+                            const WrClipId *aParentId,
                             LayoutRect aClipRect,
                             const ComplexClipRegion *aComplex,
                             uintptr_t aComplexCount,
                             const WrImageMask *aMask)
 WR_FUNC;
 
 WR_INLINE
 uint64_t wr_dp_define_clipchain(WrState *aState,
                                 const uint64_t *aParentClipchainId,
-                                const uintptr_t *aClips,
+                                const WrClipId *aClips,
                                 uintptr_t aClipsCount)
 WR_FUNC;
 
 WR_INLINE
 uintptr_t wr_dp_define_scroll_layer(WrState *aState,
                                     uint64_t aScrollId,
-                                    const uintptr_t *aParentId,
+                                    const WrClipId *aParentId,
                                     LayoutRect aContentRect,
                                     LayoutRect aClipRect)
 WR_FUNC;
 
 WR_INLINE
 uintptr_t wr_dp_define_sticky_frame(WrState *aState,
                                     LayoutRect aContentRect,
                                     const float *aTopMargin,
@@ -1425,22 +1433,22 @@ WR_FUNC;
 WR_INLINE
 void wr_dp_push_clear_rect(WrState *aState,
                            LayoutRect aRect,
                            LayoutRect aClip)
 WR_FUNC;
 
 WR_INLINE
 void wr_dp_push_clip(WrState *aState,
-                     uintptr_t aClipId)
+                     WrClipId aClipId)
 WR_FUNC;
 
 WR_INLINE
 void wr_dp_push_clip_and_scroll_info(WrState *aState,
-                                     uintptr_t aScrollId,
+                                     WrClipId aScrollId,
                                      const uint64_t *aClipChainId)
 WR_FUNC;
 
 WR_INLINE
 void wr_dp_push_iframe(WrState *aState,
                        LayoutRect aRect,
                        LayoutRect aClip,
                        bool aIsBackfaceVisible,
@@ -1505,31 +1513,31 @@ void wr_dp_push_rect(WrState *aState,
                      LayoutRect aRect,
                      LayoutRect aClip,
                      bool aIsBackfaceVisible,
                      ColorF aColor)
 WR_FUNC;
 
 WR_INLINE
 void wr_dp_push_scroll_layer(WrState *aState,
-                             uintptr_t aScrollId)
+                             WrClipId aScrollId)
 WR_FUNC;
 
 WR_INLINE
 void wr_dp_push_shadow(WrState *aState,
                        LayoutRect aBounds,
                        LayoutRect aClip,
                        bool aIsBackfaceVisible,
                        Shadow aShadow)
 WR_FUNC;
 
 WR_INLINE
 void wr_dp_push_stacking_context(WrState *aState,
                                  LayoutRect aBounds,
-                                 const uintptr_t *aClipNodeId,
+                                 const WrClipId *aClipNodeId,
                                  const WrAnimationProperty *aAnimation,
                                  const float *aOpacity,
                                  const LayoutTransform *aTransform,
                                  TransformStyle aTransformStyle,
                                  const LayoutTransform *aPerspective,
                                  MixBlendMode aMixBlendMode,
                                  const WrFilterOp *aFilters,
                                  uintptr_t aFilterCount,