Move more code into ClipScrollState during WebRender DL building

This change moves a bit more code into the `ClipScrollState` helper
during WR display list building as well as inlines
build_common_item_properties. This is all in preparation of
modifications to this code which will build a Compositor-side scroll
tree. It should not change any behavior.

components/layout/display_list/webrender_helpers.rs

@@ -10,7 +10,7 @@
 use crate::display_list::items::{BaseDisplayItem, ClipScrollNode, ClipScrollNodeType, ClipType};
 use crate::display_list::items::{DisplayItem, DisplayList, StackingContextType};
 use msg::constellation_msg::PipelineId;
-use webrender_api::units::LayoutPoint;
+use webrender_api::units::{LayoutPoint, LayoutVector2D};
 use webrender_api::{
     self, ClipId, CommonItemProperties, DisplayItem as WrDisplayItem, DisplayListBuilder,
     PrimitiveFlags, PropertyBinding, PushStackingContextDisplayItem, RasterSpace,
@@ -24,6 +24,54 @@ struct ClipScrollState {
     active_spatial_id: SpatialId,
 }
 
+impl ClipScrollState {
+    fn new(size: usize, pipeline_id: webrender_api::PipelineId) -> Self {
+        let root_clip_id = ClipId::root(pipeline_id);
+        let root_scroll_node_id = SpatialId::root_scroll_node(pipeline_id);
+        let root_reference_frame_id = SpatialId::root_reference_frame(pipeline_id);
+        let mut state = ClipScrollState {
+            clip_ids: vec![None; size],
+            spatial_ids: vec![None; size],
+            active_clip_id: root_clip_id,
+            active_spatial_id: root_scroll_node_id,
+        };
+
+        // We need to register the WebRender root reference frame and root scroll node ids
+        // here manually, because WebRender and the CompositorDisplayListInfo create them
+        // automatically. We also follow the "old" WebRender API for clip/scroll for now,
+        // hence both arrays are initialized based on FIRST_SPATIAL_NODE_INDEX, while
+        // FIRST_CLIP_NODE_INDEX is not taken into account.
+        state.spatial_ids[0] = Some(root_reference_frame_id);
+        state.spatial_ids[1] = Some(root_scroll_node_id);
+
+        state.add_clip_node_mapping(0, root_clip_id);
+        state.add_clip_node_mapping(1, root_clip_id);
+
+        state
+    }
+
+    fn webrender_clip_id_for_index(&mut self, index: usize) -> ClipId {
+        self.clip_ids[index].expect("Tried to use WebRender parent ClipId before it was defined.")
+    }
+
+    fn webrender_spatial_id_for_index(&mut self, index: usize) -> SpatialId {
+        self.spatial_ids[index]
+            .expect("Tried to use WebRender parent SpatialId before it was defined.")
+    }
+
+    fn add_clip_node_mapping(&mut self, index: usize, webrender_id: ClipId) {
+        self.clip_ids[index] = Some(webrender_id);
+    }
+
+    fn register_spatial_node(&mut self, index: usize, webrender_id: SpatialId) {
+        self.spatial_ids[index] = Some(webrender_id);
+    }
+
+    fn add_spatial_node_mapping_to_parent_index(&mut self, index: usize, parent_index: usize) {
+        self.spatial_ids[index] = self.spatial_ids[parent_index];
+    }
+}
+
 /// Contentful paint, for the purpose of
 /// https://w3c.github.io/paint-timing/#first-contentful-paint
 /// (i.e. the display list contains items of type text,
@@ -35,27 +83,8 @@ impl DisplayList {
         &mut self,
         pipeline_id: PipelineId,
     ) -> (DisplayListBuilder, IsContentful) {
-        let mut clip_ids = vec![None; self.clip_scroll_nodes.len()];
-        let mut spatial_ids = vec![None; self.clip_scroll_nodes.len()];
-
-        // We need to add the WebRender root reference frame and root scroll node ids
-        // here manually, because WebRender creates these automatically.
-        // We also follow the "old" WebRender API for clip/scroll for now,
-        // hence both arrays are initialized based on FIRST_SPATIAL_NODE_INDEX,
-        // while FIRST_CLIP_NODE_INDEX is not taken into account.
-
         let webrender_pipeline = pipeline_id.to_webrender();
-        clip_ids[0] = Some(ClipId::root(webrender_pipeline));
+        let mut state = ClipScrollState::new(self.clip_scroll_nodes.len(), webrender_pipeline);
-        clip_ids[1] = Some(ClipId::root(webrender_pipeline));
-        spatial_ids[0] = Some(SpatialId::root_reference_frame(webrender_pipeline));
-        spatial_ids[1] = Some(SpatialId::root_scroll_node(webrender_pipeline));
-
-        let mut state = ClipScrollState {
-            clip_ids,
-            spatial_ids,
-            active_clip_id: ClipId::root(webrender_pipeline),
-            active_spatial_id: SpatialId::root_scroll_node(webrender_pipeline),
-        };
 
         let mut builder = DisplayListBuilder::with_capacity(
             webrender_pipeline,
@@ -89,8 +118,8 @@ impl DisplayItem {
         let clip_and_scroll_indices = self.base().clipping_and_scrolling;
         trace!("converting {:?}", clip_and_scroll_indices);
 
-        let cur_spatial_id = state.spatial_ids[clip_and_scroll_indices.scrolling.to_index()]
+        let current_scrolling_index = clip_and_scroll_indices.scrolling.to_index();
-            .expect("Tried to use WebRender SpatialId before it was defined.");
+        let cur_spatial_id = state.webrender_spatial_id_for_index(current_scrolling_index);
         if cur_spatial_id != state.active_spatial_id {
             state.active_spatial_id = cur_spatial_id;
         }
@@ -98,36 +127,50 @@ impl DisplayItem {
         let internal_clip_id = clip_and_scroll_indices
             .clipping
             .unwrap_or(clip_and_scroll_indices.scrolling);
-        let cur_clip_id = state.clip_ids[internal_clip_id.to_index()]
+        let cur_clip_id = state.webrender_clip_id_for_index(internal_clip_id.to_index());
-            .expect("Tried to use WebRender ClipId before it was defined.");
         if cur_clip_id != state.active_clip_id {
             state.active_clip_id = cur_clip_id;
         }
 
+        let build_common_item_properties = |base: &BaseDisplayItem| {
+            let tag = match base.metadata.pointing {
+                Some(cursor) => Some((base.metadata.node.0 as u64, cursor)),
+                None => None,
+            };
+            CommonItemProperties {
+                clip_rect: base.clip_rect,
+                spatial_id: state.active_spatial_id,
+                clip_id: state.active_clip_id,
+                // TODO(gw): Make use of the WR backface visibility functionality.
+                flags: PrimitiveFlags::default(),
+                hit_info: tag,
+            }
+        };
+
         match *self {
             DisplayItem::Rectangle(ref mut item) => {
-                item.item.common = build_common_item_properties(&item.base, state);
+                item.item.common = build_common_item_properties(&item.base);
                 builder.push_item(&WrDisplayItem::Rectangle(item.item));
                 IsContentful(false)
             },
             DisplayItem::Text(ref mut item) => {
-                item.item.common = build_common_item_properties(&item.base, state);
+                item.item.common = build_common_item_properties(&item.base);
                 builder.push_item(&WrDisplayItem::Text(item.item));
                 builder.push_iter(item.data.iter());
                 IsContentful(true)
             },
             DisplayItem::Image(ref mut item) => {
-                item.item.common = build_common_item_properties(&item.base, state);
+                item.item.common = build_common_item_properties(&item.base);
                 builder.push_item(&WrDisplayItem::Image(item.item));
                 IsContentful(true)
             },
             DisplayItem::RepeatingImage(ref mut item) => {
-                item.item.common = build_common_item_properties(&item.base, state);
+                item.item.common = build_common_item_properties(&item.base);
                 builder.push_item(&WrDisplayItem::RepeatingImage(item.item));
                 IsContentful(true)
             },
             DisplayItem::Border(ref mut item) => {
-                item.item.common = build_common_item_properties(&item.base, state);
+                item.item.common = build_common_item_properties(&item.base);
                 if !item.data.is_empty() {
                     builder.push_stops(item.data.as_ref());
                 }
@@ -135,29 +178,29 @@ impl DisplayItem {
                 IsContentful(false)
             },
             DisplayItem::Gradient(ref mut item) => {
-                item.item.common = build_common_item_properties(&item.base, state);
+                item.item.common = build_common_item_properties(&item.base);
                 builder.push_stops(item.data.as_ref());
                 builder.push_item(&WrDisplayItem::Gradient(item.item));
                 IsContentful(false)
             },
             DisplayItem::RadialGradient(ref mut item) => {
-                item.item.common = build_common_item_properties(&item.base, state);
+                item.item.common = build_common_item_properties(&item.base);
                 builder.push_stops(item.data.as_ref());
                 builder.push_item(&WrDisplayItem::RadialGradient(item.item));
                 IsContentful(false)
             },
             DisplayItem::Line(ref mut item) => {
-                item.item.common = build_common_item_properties(&item.base, state);
+                item.item.common = build_common_item_properties(&item.base);
                 builder.push_item(&WrDisplayItem::Line(item.item));
                 IsContentful(false)
             },
             DisplayItem::BoxShadow(ref mut item) => {
-                item.item.common = build_common_item_properties(&item.base, state);
+                item.item.common = build_common_item_properties(&item.base);
                 builder.push_item(&WrDisplayItem::BoxShadow(item.item));
                 IsContentful(false)
             },
             DisplayItem::PushTextShadow(ref mut item) => {
-                let common = build_common_item_properties(&item.base, state);
+                let common = build_common_item_properties(&item.base);
                 builder.push_shadow(
                     &SpaceAndClipInfo {
                         spatial_id: common.spatial_id,
@@ -173,7 +216,7 @@ impl DisplayItem {
                 IsContentful(false)
             },
             DisplayItem::Iframe(ref mut item) => {
-                let common = build_common_item_properties(&item.base, state);
+                let common = build_common_item_properties(&item.base);
                 builder.push_iframe(
                     item.bounds,
                     common.clip_rect,
@@ -212,7 +255,7 @@ impl DisplayItem {
                                 (None, None) => unreachable!(),
                             };
 
-                        let spatial_id = builder.push_reference_frame(
+                        let new_spatial_id = builder.push_reference_frame(
                             stacking_context.bounds.origin,
                             state.active_spatial_id,
                             stacking_context.transform_style,
@@ -220,11 +263,12 @@ impl DisplayItem {
                             ref_frame,
                         );
 
-                        state.spatial_ids[frame_index.to_index()] = Some(spatial_id);
+                        let index = frame_index.to_index();
-                        state.clip_ids[frame_index.to_index()] = Some(cur_clip_id);
+                        state.add_clip_node_mapping(index, cur_clip_id);
+                        state.register_spatial_node(index, new_spatial_id);
 
                         bounds.origin = LayoutPoint::zero();
-                        spatial_id
+                        new_spatial_id
                     } else {
                         state.active_spatial_id
                     };
@@ -264,13 +308,13 @@ impl DisplayItem {
                 IsContentful(false)
             },
             DisplayItem::DefineClipScrollNode(ref mut item) => {
-                let node = &clip_scroll_nodes[item.node_index.to_index()];
+                let index = item.node_index.to_index();
+                let node = &clip_scroll_nodes[index];
                 let item_rect = node.clip.main;
 
-                let parent_spatial_id = state.spatial_ids[node.parent_index.to_index()]
+                let parent_index = node.parent_index.to_index();
-                    .expect("Tried to use WebRender parent SpatialId before it was defined.");
+                let parent_spatial_id = state.webrender_spatial_id_for_index(parent_index);
-                let parent_clip_id = state.clip_ids[node.parent_index.to_index()]
+                let parent_clip_id = state.webrender_clip_id_for_index(parent_index);
-                    .expect("Tried to use WebRender parent ClipId before it was defined.");
 
                 match node.node_type {
                     ClipScrollNodeType::Clip(clip_type) => {
@@ -278,7 +322,7 @@ impl DisplayItem {
                             clip_id: parent_clip_id,
                             spatial_id: parent_spatial_id,
                         };
-                        let id = match clip_type {
+                        let clip_id = match clip_type {
                             ClipType::Rect => {
                                 builder.define_clip_rect(&space_and_clip_info, item_rect)
                             },
@@ -287,8 +331,8 @@ impl DisplayItem {
                             },
                         };
 
-                        state.spatial_ids[item.node_index.to_index()] = Some(parent_spatial_id);
+                        state.add_clip_node_mapping(index, clip_id);
-                        state.clip_ids[item.node_index.to_index()] = Some(id);
+                        state.add_spatial_node_mapping_to_parent_index(index, parent_index);
                     },
                     ClipScrollNodeType::ScrollFrame(scroll_sensitivity, external_id) => {
                         let space_clip_info = builder.define_scroll_frame(
@@ -298,14 +342,13 @@ impl DisplayItem {
                             },
                             Some(external_id),
                             node.content_rect,
-                            node.clip.main,
+                            item_rect,
                             scroll_sensitivity,
-                            webrender_api::units::LayoutVector2D::zero(),
+                            LayoutVector2D::zero(),
                         );
 
-                        state.clip_ids[item.node_index.to_index()] = Some(space_clip_info.clip_id);
+                        state.register_spatial_node(index, space_clip_info.spatial_id);
-                        state.spatial_ids[item.node_index.to_index()] =
+                        state.add_clip_node_mapping(index, space_clip_info.clip_id);
-                            Some(space_clip_info.spatial_id);
                     },
                     ClipScrollNodeType::StickyFrame(ref sticky_data) => {
                         // TODO: Add define_sticky_frame_with_parent to WebRender.
@@ -315,11 +358,11 @@ impl DisplayItem {
                             sticky_data.margins,
                             sticky_data.vertical_offset_bounds,
                             sticky_data.horizontal_offset_bounds,
-                            webrender_api::units::LayoutVector2D::zero(),
+                            LayoutVector2D::zero(),
                         );
 
-                        state.spatial_ids[item.node_index.to_index()] = Some(id);
+                        state.add_clip_node_mapping(index, parent_clip_id);
-                        state.clip_ids[item.node_index.to_index()] = Some(parent_clip_id);
+                        state.register_spatial_node(index, id);
                     },
                     ClipScrollNodeType::Placeholder => {
                         unreachable!("Found DefineClipScrollNode for Placeholder type node.");
@@ -330,21 +373,3 @@ impl DisplayItem {
         }
     }
 }
-
-fn build_common_item_properties(
-    base: &BaseDisplayItem,
-    state: &ClipScrollState,
-) -> CommonItemProperties {
-    let tag = match base.metadata.pointing {
-        Some(cursor) => Some((base.metadata.node.0 as u64, cursor)),
-        None => None,
-    };
-    CommonItemProperties {
-        clip_rect: base.clip_rect,
-        spatial_id: state.active_spatial_id,
-        clip_id: state.active_clip_id,
-        // TODO(gw): Make use of the WR backface visibility functionality.
-        flags: PrimitiveFlags::default(),
-        hit_info: tag,
-    }
-}