Bug 1377187 - Simplify code to define clips and scroll layers. r?mstange draft
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 08 May 2018 09:07:30 -0400
changeset 792511 86170dec8abf8881e2a3f1aa077653df10b113b4
parent 792510 7048b011cc74d49e4db88b273892d6cb5cc3de04
child 792512 305c0f4f1121da55e63879ede69d317f80190811
push id109117
push userkgupta@mozilla.com
push dateTue, 08 May 2018 13:09:28 +0000
reviewersmstange
bugs1377187
milestone62.0a1
Bug 1377187 - Simplify code to define clips and scroll layers. r?mstange This is a refactoring that moves the "parent clip id" determination from bindings.rs into WebRenderAPI.cpp. This should be functionally identical. MozReview-Commit-ID: 36rQmsH5E7J
gfx/webrender_bindings/WebRenderAPI.cpp
gfx/webrender_bindings/src/bindings.rs
gfx/webrender_bindings/webrender_ffi_generated.h
--- a/gfx/webrender_bindings/WebRenderAPI.cpp
+++ b/gfx/webrender_bindings/WebRenderAPI.cpp
@@ -807,26 +807,24 @@ DisplayListBuilder::PopStackingContext()
 
 wr::WrClipId
 DisplayListBuilder::DefineClip(const Maybe<wr::WrScrollId>& aAncestorScrollId,
                                const Maybe<wr::WrClipId>& aAncestorClipId,
                                const wr::LayoutRect& aClipRect,
                                const nsTArray<wr::ComplexClipRegion>* aComplex,
                                const wr::WrImageMask* aMask)
 {
-  const size_t* ancestorScrollId = nullptr;
+  const size_t* parentId = nullptr;
   if (aAncestorScrollId) {
-    ancestorScrollId = &(aAncestorScrollId.ref().id);
-  }
-  const size_t* ancestorClipId = nullptr;
-  if (aAncestorClipId) {
-    ancestorClipId = &(aAncestorClipId.ref().id);
+    parentId = &(aAncestorScrollId.ref().id);
+  } else if (aAncestorClipId) {
+    parentId = &(aAncestorClipId.ref().id);
   }
   size_t clip_id = wr_dp_define_clip(mWrState,
-      ancestorScrollId, ancestorClipId,
+      parentId,
       aClipRect,
       aComplex ? aComplex->Elements() : nullptr,
       aComplex ? aComplex->Length() : 0,
       aMask);
   WRDL_LOG("DefineClip id=%zu as=%s ac=%s r=%s m=%p b=%s complex=%zu\n", mWrState,
       clip_id,
       aAncestorScrollId ? Stringify(aAncestorScrollId.ref().id).c_str() : "(nil)",
       aAncestorClipId ? Stringify(aAncestorClipId.ref().id).c_str() : "(nil)",
@@ -977,33 +975,38 @@ DisplayListBuilder::GetScrollIdForDefine
 
 wr::WrScrollId
 DisplayListBuilder::DefineScrollLayer(const layers::FrameMetrics::ViewID& aViewId,
                                       const Maybe<wr::WrScrollId>& aAncestorScrollId,
                                       const Maybe<wr::WrClipId>& aAncestorClipId,
                                       const wr::LayoutRect& aContentRect,
                                       const wr::LayoutRect& aClipRect)
 {
+  const size_t* parentId = nullptr;
+  if (aAncestorScrollId) {
+    parentId = &(aAncestorScrollId.ref().id);
+  } else if (aAncestorClipId) {
+    parentId = &(aAncestorClipId.ref().id);
+  }
   WRDL_LOG("DefineScrollLayer id=%" PRIu64 " as=%s ac=%s co=%s cl=%s\n", mWrState,
       aViewId,
       aAncestorScrollId ? Stringify(aAncestorScrollId.ref().id).c_str() : "(nil)",
       aAncestorClipId ? Stringify(aAncestorClipId.ref().id).c_str() : "(nil)",
       Stringify(aContentRect).c_str(), Stringify(aClipRect).c_str());
 
   auto it = mScrollIds.find(aViewId);
   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,
-      aAncestorScrollId ? &(aAncestorScrollId->id) : nullptr,
-      aAncestorClipId ? &(aAncestorClipId->id) : nullptr,
+      parentId,
       aContentRect,
       aClipRect);
    auto wrScrollId = wr::WrScrollId { numericScrollId };
    mScrollIds[aViewId] = wrScrollId;
    return wrScrollId;
 }
 
 void
--- a/gfx/webrender_bindings/src/bindings.rs
+++ b/gfx/webrender_bindings/src/bindings.rs
@@ -1652,59 +1652,35 @@ pub extern "C" fn wr_dp_push_stacking_co
 }
 
 #[no_mangle]
 pub extern "C" fn wr_dp_pop_stacking_context(state: &mut WrState) {
     debug_assert!(unsafe { !is_in_render_thread() });
     state.frame_builder.dl_builder.pop_stacking_context();
 }
 
-fn make_scroll_info(state: &mut WrState,
-                    scroll_id: Option<&usize>,
-                    clip_id: Option<&usize>)
-                    -> Option<ClipAndScrollInfo> {
-    if let Some(&sid) = scroll_id {
-        if let Some(&cid) = clip_id {
-            Some(ClipAndScrollInfo::new(
-                ClipId::Clip(sid , state.pipeline_id),
-                ClipId::Clip(cid, state.pipeline_id)))
-        } else {
-            Some(ClipAndScrollInfo::simple(
-                ClipId::Clip(sid, state.pipeline_id)))
-        }
-    } else if let Some(&cid) = clip_id {
-        Some(ClipAndScrollInfo::simple(
-            ClipId::Clip(cid, state.pipeline_id)))
-    } else {
-        None
-    }
-}
-
 #[no_mangle]
 pub extern "C" fn wr_dp_define_clip(state: &mut WrState,
-                                    ancestor_scroll_id: *const usize,
-                                    ancestor_clip_id: *const usize,
+                                    parent_id: *const usize,
                                     clip_rect: LayoutRect,
                                     complex: *const ComplexClipRegion,
                                     complex_count: usize,
                                     mask: *const WrImageMask)
                                     -> usize {
     debug_assert!(unsafe { is_in_main_thread() });
 
-    let info = make_scroll_info(state,
-                                unsafe { ancestor_scroll_id.as_ref() },
-                                unsafe { ancestor_clip_id.as_ref() });
-
+    let parent_id = unsafe { parent_id.as_ref() };
     let complex_slice = make_slice(complex, complex_count);
     let complex_iter = complex_slice.iter().cloned();
     let mask : Option<ImageMask> = unsafe { mask.as_ref() }.map(|x| x.into());
 
-    let clip_id = if info.is_some() {
+    let clip_id = if let Some(&pid) = parent_id {
         state.frame_builder.dl_builder.define_clip_with_parent(
-            info.unwrap().scroll_node_id, clip_rect, complex_iter, mask)
+            ClipId::Clip(pid, state.pipeline_id),
+            clip_rect, complex_iter, mask)
     } else {
         state.frame_builder.dl_builder.define_clip(clip_rect, complex_iter, mask)
     };
     // return the usize id value from inside the ClipId::Clip(..)
     match clip_id {
         ClipId::Clip(id, pipeline_id) => {
             assert!(pipeline_id == state.pipeline_id);
             id
@@ -1753,30 +1729,27 @@ pub extern "C" fn wr_dp_define_sticky_fr
         },
         _ => panic!("Got unexpected clip id type"),
     }
 }
 
 #[no_mangle]
 pub extern "C" fn wr_dp_define_scroll_layer(state: &mut WrState,
                                             scroll_id: u64,
-                                            ancestor_scroll_id: *const usize,
-                                            ancestor_clip_id: *const usize,
+                                            parent_id: *const usize,
                                             content_rect: LayoutRect,
                                             clip_rect: LayoutRect)
                                             -> usize {
     assert!(unsafe { is_in_main_thread() });
 
-    let info = make_scroll_info(state,
-                                unsafe { ancestor_scroll_id.as_ref() },
-                                unsafe { ancestor_clip_id.as_ref() });
+    let parent_id = unsafe { parent_id.as_ref() };
 
-    let new_id = if info.is_some() {
+    let new_id = if let Some(&pid) = parent_id {
         state.frame_builder.dl_builder.define_scroll_frame_with_parent(
-            info.unwrap().scroll_node_id,
+            ClipId::Clip(pid, state.pipeline_id),
             Some(ExternalScrollId(scroll_id, state.pipeline_id)),
             content_rect,
             clip_rect,
             vec![],
             None,
             ScrollSensitivity::Script
         )
     } else {
@@ -1813,19 +1786,27 @@ pub extern "C" fn wr_dp_pop_scroll_layer
     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,
                                                   clip_id: *const usize) {
     debug_assert!(unsafe { is_in_main_thread() });
-    let info = make_scroll_info(state, Some(&scroll_id), unsafe { clip_id.as_ref() });
-    debug_assert!(info.is_some());
-    state.frame_builder.dl_builder.push_clip_and_scroll_info(info.unwrap());
+
+    let clip_id = unsafe { clip_id.as_ref() };
+    let info = if let Some(&cid) = clip_id {
+        ClipAndScrollInfo::new(
+            ClipId::Clip(scroll_id, state.pipeline_id),
+            ClipId::Clip(cid, state.pipeline_id))
+    } else {
+        ClipAndScrollInfo::simple(
+            ClipId::Clip(scroll_id, state.pipeline_id))
+    };
+    state.frame_builder.dl_builder.push_clip_and_scroll_info(info);
 }
 
 #[no_mangle]
 pub extern "C" fn wr_dp_pop_clip_and_scroll_info(state: &mut WrState) {
     debug_assert!(unsafe { is_in_main_thread() });
     state.frame_builder.dl_builder.pop_clip_id();
 }
 
--- a/gfx/webrender_bindings/webrender_ffi_generated.h
+++ b/gfx/webrender_bindings/webrender_ffi_generated.h
@@ -1105,29 +1105,27 @@ void wr_dec_ref_arc(const VecU8 *aArc)
 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 *aAncestorScrollId,
-                            const uintptr_t *aAncestorClipId,
+                            const uintptr_t *aParentId,
                             LayoutRect aClipRect,
                             const ComplexClipRegion *aComplex,
                             uintptr_t aComplexCount,
                             const WrImageMask *aMask)
 WR_FUNC;
 
 WR_INLINE
 uintptr_t wr_dp_define_scroll_layer(WrState *aState,
                                     uint64_t aScrollId,
-                                    const uintptr_t *aAncestorScrollId,
-                                    const uintptr_t *aAncestorClipId,
+                                    const uintptr_t *aParentId,
                                     LayoutRect aContentRect,
                                     LayoutRect aClipRect)
 WR_FUNC;
 
 WR_INLINE
 uintptr_t wr_dp_define_sticky_frame(WrState *aState,
                                     LayoutRect aContentRect,
                                     const float *aTopMargin,