Bug 1474300 - Update for API changes in WR PR 2871. r=Gankro
☠☠ backed out by f97ba5c963ff ☠ ☠
authorKartikaya Gupta <kgupta@mozilla.com>
Thu, 12 Jul 2018 10:36:04 -0400
changeset 426526 f7659b60b7b02ccdcf902f34273f7c386620cafd
parent 426525 a65429a135c7a385c3614d24b6d79cb7a9e3b3d1
child 426527 24a8cf2131d1964a65b96c770728f03467818dce
push id66342
push userkgupta@mozilla.com
push dateFri, 13 Jul 2018 15:50:29 +0000
treeherderautoland@f7659b60b7b0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGankro
bugs1474300
milestone63.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 1474300 - Update for API changes in WR PR 2871. r=Gankro This change has WrClipId contain the ClipId type (except for clip chains, which are handled separately) in the least significant bit of the size_t. On 32-bit systems this limits the number of clip and spatial nodes to 2,147,483,648 which is likely more than what WebRender can handle. MozReview-Commit-ID: 8ohMKqTZcKT
gfx/webrender_bindings/src/bindings.rs
gfx/webrender_bindings/webrender_ffi_generated.h
--- a/gfx/webrender_bindings/src/bindings.rs
+++ b/gfx/webrender_bindings/src/bindings.rs
@@ -1651,17 +1651,17 @@ pub extern "C" fn wr_dp_push_stacking_co
                                                                c_filter.argument,
                                                                c_filter.color),
             WrFilterOpType::ColorMatrix => FilterOp::ColorMatrix(c_filter.matrix),
         }
     }).collect();
 
     let clip_node_id_ref = unsafe { clip_node_id.as_ref() };
     let clip_node_id = match clip_node_id_ref {
-        Some(clip_node_id) => Some(ClipId::Clip(*clip_node_id, state.pipeline_id)),
+        Some(clip_node_id) => Some(unpack_clip_id(*clip_node_id, state.pipeline_id)),
         None => None,
     };
 
     let transform_ref = unsafe { transform.as_ref() };
     let mut transform_binding = match transform_ref {
         Some(transform) => Some(PropertyBinding::Value(transform.clone())),
         None => None,
     };
@@ -1705,23 +1705,17 @@ pub extern "C" fn wr_dp_push_stacking_co
 
     let mut prim_info = LayoutPrimitiveInfo::new(bounds);
 
     *out_is_reference_frame = transform_binding.is_some() || perspective.is_some();
     if *out_is_reference_frame {
         let ref_frame_id = state.frame_builder
             .dl_builder
             .push_reference_frame(&prim_info, transform_binding, perspective);
-        match ref_frame_id {
-            ClipId::Clip(id, pipeline_id) => {
-                assert!(pipeline_id == state.pipeline_id);
-                *out_reference_frame_id = id;
-            },
-            _ => panic!("Pushing a reference frame must produce a ClipId::Clip"),
-        }
+        *out_reference_frame_id = pack_clip_id(ref_frame_id);
 
         prim_info.rect.origin = LayoutPoint::zero();
         prim_info.clip_rect.origin = LayoutPoint::zero();
         state.frame_builder.dl_builder.push_clip_id(ref_frame_id);
     }
 
     prim_info.is_backface_visible = is_backface_visible;
     prim_info.tag = state.current_tag;
@@ -1750,17 +1744,21 @@ pub extern "C" fn wr_dp_pop_stacking_con
 #[no_mangle]
 pub extern "C" fn wr_dp_define_clipchain(state: &mut WrState,
                                          parent_clipchain_id: *const u64,
                                          clips: *const usize,
                                          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 clips_slice : Vec<ClipId> = make_slice(clips, clips_count).iter().map(|id| ClipId::Clip(*id, state.pipeline_id)).collect();
+    let clips_slice : Vec<ClipId> = make_slice(clips, clips_count)
+        .iter()
+        .map(|id| unpack_clip_id(*id, state.pipeline_id))
+        .collect();
+
     let clipchain_id = state.frame_builder.dl_builder.define_clip_chain(parent, clips_slice);
     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,
@@ -1773,36 +1771,29 @@ pub extern "C" fn wr_dp_define_clip(stat
 
     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 let Some(&pid) = parent_id {
         state.frame_builder.dl_builder.define_clip_with_parent(
-            ClipId::Clip(pid, state.pipeline_id),
+            unpack_clip_id(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
-        },
-        _ => panic!("Got unexpected clip id type"),
-    }
+    pack_clip_id(clip_id)
 }
 
 #[no_mangle]
 pub extern "C" fn wr_dp_push_clip(state: &mut WrState,
                                   clip_id: usize) {
     debug_assert!(unsafe { is_in_main_thread() });
-    state.frame_builder.dl_builder.push_clip_id(ClipId::Clip(clip_id, state.pipeline_id));
+    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();
 }
 
@@ -1821,39 +1812,33 @@ pub extern "C" fn wr_dp_define_sticky_fr
     let clip_id = state.frame_builder.dl_builder.define_sticky_frame(
         content_rect, SideOffsets2D::new(
             unsafe { top_margin.as_ref() }.cloned(),
             unsafe { right_margin.as_ref() }.cloned(),
             unsafe { bottom_margin.as_ref() }.cloned(),
             unsafe { left_margin.as_ref() }.cloned()
         ),
         vertical_bounds, horizontal_bounds, applied_offset);
-    match clip_id {
-        ClipId::Clip(id, pipeline_id) => {
-            assert!(pipeline_id == state.pipeline_id);
-            id
-        },
-        _ => panic!("Got unexpected clip id type"),
-    }
+    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,
                                             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 {
         state.frame_builder.dl_builder.define_scroll_frame_with_parent(
-            ClipId::Clip(pid, state.pipeline_id),
+            unpack_clip_id(pid, state.pipeline_id),
             Some(ExternalScrollId(scroll_id, state.pipeline_id)),
             content_rect,
             clip_rect,
             vec![],
             None,
             ScrollSensitivity::Script
         )
     } else {
@@ -1862,30 +1847,24 @@ pub extern "C" fn wr_dp_define_scroll_la
             content_rect,
             clip_rect,
             vec![],
             None,
             ScrollSensitivity::Script
         )
     };
 
-    match new_id {
-        ClipId::Clip(id, pipeline_id) => {
-            assert!(pipeline_id == state.pipeline_id);
-            id
-        },
-        _ => panic!("Got unexpected clip id type"),
-    }
+    pack_clip_id(new_id)
 }
 
 #[no_mangle]
 pub extern "C" fn wr_dp_push_scroll_layer(state: &mut WrState,
                                           scroll_id: usize) {
     debug_assert!(unsafe { is_in_main_thread() });
-    let clip_id = ClipId::Clip(scroll_id, state.pipeline_id);
+    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();
 }
@@ -1894,21 +1873,20 @@ pub extern "C" fn wr_dp_pop_scroll_layer
 pub extern "C" fn wr_dp_push_clip_and_scroll_info(state: &mut WrState,
                                                   scroll_id: usize,
                                                   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(
-            ClipId::Clip(scroll_id, state.pipeline_id),
+            unpack_clip_id(scroll_id, state.pipeline_id),
             ClipId::ClipChain(ClipChainId(ccid, state.pipeline_id)))
     } else {
-        ClipAndScrollInfo::simple(
-            ClipId::Clip(scroll_id, state.pipeline_id))
+        ClipAndScrollInfo::simple(unpack_clip_id(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();
@@ -2458,13 +2436,32 @@ extern "C" {
                                output: MutByteSlice)
                                -> bool;
 }
 
 #[no_mangle]
 pub extern "C" fn wr_root_scroll_node_id() -> usize {
     // The PipelineId doesn't matter here, since we just want the numeric part of the id
     // produced for any given root reference frame.
-    match ClipId::root_scroll_node(PipelineId(0, 0)) {
-        ClipId::Clip(id, _) => id,
-        _ => unreachable!("Got a non Clip ClipId for root reference frame."),
+    pack_clip_id(ClipId::root_scroll_node(PipelineId(0, 0)))
+}
+
+fn pack_clip_id(id: ClipId) -> usize {
+    let (id, type_value) = match id {
+        ClipId::Spatial(id, _) => (id, 0),
+        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;
+
+    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
@@ -396,24 +396,27 @@ using LayoutSize = TypedSize2D<float, La
 // items.
 struct BuiltDisplayListDescriptor {
   // The first IPC time stamp: before any work has been done
   uint64_t builder_start_time;
   // The second IPC time stamp: after serialization
   uint64_t builder_finish_time;
   // The third IPC time stamp: just before sending
   uint64_t send_start_time;
-  // The amount of clips ids assigned while building this display list.
-  uintptr_t total_clip_ids;
+  // The amount of clipping nodes created while building this display list.
+  uintptr_t total_clip_nodes;
+  // The amount of spatial nodes created while building this display list.
+  uintptr_t total_spatial_nodes;
 
   bool operator==(const BuiltDisplayListDescriptor& aOther) const {
     return builder_start_time == aOther.builder_start_time &&
            builder_finish_time == aOther.builder_finish_time &&
            send_start_time == aOther.send_start_time &&
-           total_clip_ids == aOther.total_clip_ids;
+           total_clip_nodes == aOther.total_clip_nodes &&
+           total_spatial_nodes == aOther.total_spatial_nodes;
   }
 };
 
 struct WrVecU8 {
   uint8_t *data;
   uintptr_t length;
   uintptr_t capacity;