servo: Merge #11900 - Various DisplayList cleanup (from Ms2ger:displaylist-cleanup); r=nox
authorMs2ger <Ms2ger@gmail.com>
Wed, 29 Jun 2016 03:32:52 -0500
changeset 339150 db0b6d1ace8c78c662e74f06ca2080b933a1fa3c
parent 339149 88f9c9e6a11c81a3e4e74df60ade498c0953580a
child 339151 f4299f3601667cb2a643cdf364de22a5fbf3edac
push id31307
push usergszorc@mozilla.com
push dateSat, 04 Feb 2017 00:59:06 +0000
treeherdermozilla-central@94079d43835f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnox
servo: Merge #11900 - Various DisplayList cleanup (from Ms2ger:displaylist-cleanup); r=nox Source-Repo: https://github.com/servo/servo Source-Revision: ffb8b35d3549ead90e0a05d492558bf8cabfbbe1
servo/components/gfx/display_list/mod.rs
servo/components/layout/query.rs
servo/components/layout_thread/lib.rs
--- a/servo/components/gfx/display_list/mod.rs
+++ b/servo/components/gfx/display_list/mod.rs
@@ -210,23 +210,18 @@ impl<K, V> Visitor for FnvHashMapVisitor
 pub struct DisplayList {
     pub list: Vec<DisplayItem>,
     pub offsets: FnvHashMap<StackingContextId, StackingContextOffsets>,
     pub root_stacking_context: StackingContext,
 }
 
 impl DisplayList {
     pub fn new(mut root_stacking_context: StackingContext,
-               items: &mut Option<Vec<DisplayItem>>)
+               items: Vec<DisplayItem>)
                -> DisplayList {
-        let items = match items.take() {
-            Some(items) => items,
-            None => panic!("Tried to create empty display list."),
-        };
-
         let mut offsets = FnvHashMap(HashMap::with_hasher(Default::default()));
         DisplayList::sort_and_count_stacking_contexts(&mut root_stacking_context, &mut offsets, 0);
 
         let mut display_list = DisplayList {
             list: items,
             offsets: offsets,
             root_stacking_context: root_stacking_context,
         };
@@ -480,29 +475,27 @@ impl DisplayList {
             paint_subcontext.pop_clip_if_applicable();
         }
 
         draw_target.set_transform(&old_transform);
         paint_context.draw_temporary_draw_target_if_necessary(
             &draw_target, &stacking_context.filters, stacking_context.blend_mode);
     }
 
-    /// Places all nodes containing the point of interest into `result`, topmost first. Respects
-    /// the `pointer-events` CSS property If `topmost_only` is true, stops after placing one node
-    /// into the list. `result` must be empty upon entry to this function.
+    /// Return all nodes containing the point of interest, bottommost first,
+    /// and respecting the `pointer-events` CSS property.
     pub fn hit_test(&self, point: &Point2D<Au>, scroll_offsets: &ScrollOffsetMap)
                     -> Vec<DisplayItemMetadata> {
         let mut traversal = DisplayListTraversal {
             display_list: self,
             current_item_index: 0,
             last_item_index: self.list.len() - 1,
         };
         let mut result = Vec::new();
         self.root_stacking_context.hit_test(&mut traversal, point, scroll_offsets, &mut result);
-        result.reverse();
         result
     }
 }
 
 fn transformed_tile_rect(tile_rect: TypedRect<ScreenPx, usize>, transform: &Matrix4D<f32>) -> Rect<Au> {
     // Invert the current transform, then use this to back transform
     // the tile rect (placed at the origin) into the space of this
     // stacking context.
@@ -636,23 +629,27 @@ impl StackingContext {
             if let Some(scroll_offset) = scroll_offsets.get(&self.id) {
                 point.x -= Au::from_f32_px(scroll_offset.x);
                 point.y -= Au::from_f32_px(scroll_offset.y);
             }
         }
 
         for child in self.children.iter() {
             while let Some(item) = traversal.advance(self) {
-                item.hit_test(point, result);
+                if let Some(meta) = item.hit_test(point) {
+                    result.push(meta);
+                }
             }
             child.hit_test(traversal, &point, scroll_offsets, result);
         }
 
         while let Some(item) = traversal.advance(self) {
-            item.hit_test(point, result);
+            if let Some(meta) = item.hit_test(point) {
+                result.push(meta);
+            }
         }
     }
 
     pub fn print_with_tree(&self, print_tree: &mut PrintTree) {
         print_tree.new_level(format!("{:?}", self));
         for kid in self.children.iter() {
             kid.print_with_tree(print_tree);
         }
@@ -1331,31 +1328,31 @@ impl DisplayItem {
     pub fn debug_with_level(&self, level: u32) {
         let mut indent = String::new();
         for _ in 0..level {
             indent.push_str("| ")
         }
         println!("{}+ {:?}", indent, self);
     }
 
-    fn hit_test(&self, point: Point2D<Au>, result: &mut Vec<DisplayItemMetadata>) {
+    fn hit_test(&self, point: Point2D<Au>) -> Option<DisplayItemMetadata> {
         // TODO(pcwalton): Use a precise algorithm here. This will allow us to properly hit
         // test elements with `border-radius`, for example.
         let base_item = self.base();
         if !base_item.clip.might_intersect_point(&point) {
             // Clipped out.
-            return;
+            return None;
         }
         if !self.bounds().contains(&point) {
             // Can't possibly hit.
-            return;
+            return None;
         }
         if base_item.metadata.pointing.is_none() {
             // `pointer-events` is `none`. Ignore this item.
-            return;
+            return None;
         }
 
         match *self {
             DisplayItem::BorderClass(ref border) => {
                 // If the point is inside the border, it didn't hit the border!
                 let interior_rect =
                     Rect::new(
                         Point2D::new(border.base.bounds.origin.x +
@@ -1364,28 +1361,27 @@ impl DisplayItem {
                                      border.border_widths.top),
                         Size2D::new(border.base.bounds.size.width -
                                     (border.border_widths.left +
                                      border.border_widths.right),
                                     border.base.bounds.size.height -
                                     (border.border_widths.top +
                                      border.border_widths.bottom)));
                 if interior_rect.contains(&point) {
-                    return;
+                    return None;
                 }
             }
             DisplayItem::BoxShadowClass(_) => {
                 // Box shadows can never be hit.
-                return
+                return None;
             }
             _ => {}
         }
 
-        // We found a hit!
-        result.push(base_item.metadata);
+        Some(base_item.metadata)
     }
 }
 
 impl fmt::Debug for DisplayItem {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(f, "{} @ {:?} {:?}",
             match *self {
                 DisplayItem::SolidColorClass(ref solid_color) =>
--- a/servo/components/layout/query.rs
+++ b/servo/components/layout/query.rs
@@ -145,16 +145,17 @@ impl LayoutRPC for LayoutRPCImpl {
                     display_list.hit_test(&point, &rw_data.stacking_context_scroll_offsets)
                 }
             };
 
             result
         };
 
         nodes_from_point_list.iter()
+           .rev()
            .map(|metadata| metadata.node.to_untrusted_node_address())
            .collect()
     }
 
     fn node_geometry(&self) -> NodeGeometryResponse {
         let &LayoutRPCImpl(ref rw_data) = self;
         let rw_data = rw_data.lock().unwrap();
         NodeGeometryResponse {
--- a/servo/components/layout_thread/lib.rs
+++ b/servo/components/layout_thread/lib.rs
@@ -913,18 +913,17 @@ impl LayoutThread {
                 root_stacking_context.overflow = origin;
                 root_stacking_context.layer_info =
                     Some(LayerInfo::new(layout_root.layer_id(),
                                         ScrollPolicy::Scrollable,
                                         None,
                                         root_background_color));
 
                 rw_data.display_list =
-                    Some(Arc::new(DisplayList::new(root_stacking_context,
-                                                   &mut Some(display_list_entries))))
+                    Some(Arc::new(DisplayList::new(root_stacking_context, display_list_entries)))
             }
 
             if data.goal == ReflowGoal::ForDisplay {
                 let display_list = (*rw_data.display_list.as_ref().unwrap()).clone();
 
                 if opts::get().dump_display_list {
                     display_list.print();
                 }
@@ -1152,27 +1151,21 @@ impl LayoutThread {
                     rw_data.content_box_response = process_content_box_request(node, &mut root_flow);
                 },
                 ReflowQueryType::ContentBoxesQuery(node) => {
                     let node = unsafe { ServoLayoutNode::new(&node) };
                     rw_data.content_boxes_response = process_content_boxes_request(node, &mut root_flow);
                 },
                 ReflowQueryType::HitTestQuery(point, update_cursor) => {
                     let point = Point2D::new(Au::from_f32_px(point.x), Au::from_f32_px(point.y));
-                    let result = match rw_data.display_list {
-                        None => panic!("Tried to hit test with no display list"),
-                        Some(ref display_list) => {
-                            display_list.hit_test(&point, &rw_data.stacking_context_scroll_offsets)
-                        }
-                    };
-                    rw_data.hit_test_response = if result.len() > 0 {
-                        (Some(result[0]), update_cursor)
-                    } else {
-                        (None, update_cursor)
-                    };
+                    let result = rw_data.display_list
+                                        .as_ref()
+                                        .expect("Tried to hit test with no display list")
+                                        .hit_test(&point, &rw_data.stacking_context_scroll_offsets);
+                    rw_data.hit_test_response = (result.last().cloned(), update_cursor);
                 },
                 ReflowQueryType::NodeGeometryQuery(node) => {
                     let node = unsafe { ServoLayoutNode::new(&node) };
                     rw_data.client_rect_response = process_node_geometry_request(node, &mut root_flow);
                 },
                 ReflowQueryType::NodeScrollGeometryQuery(node) => {
                     let node = unsafe { ServoLayoutNode::new(&node) };
                     rw_data.scroll_area_response = process_node_scroll_area_request(node, &mut root_flow);