gfx: Stop cloning clipping regions so much.

Clipping regions can contain vectors and so can be expensive to copy.

5 files changed, 53 insertions, 53 deletions

diff --git a/components/gfx/display_list/mod.rs b/components/gfx/display_list/mod.rs
index aebdc0882c0..9b9c791569f 100644
--- a/components/gfx/display_list/mod.rs
+++ b/components/gfx/display_list/mod.rs
@@ -761,16 +761,13 @@ impl ClippingRegion {
         }
     }
 
-    /// Returns the intersection of this clipping region and the given rectangle.
+    /// Mutates this clipping region to intersect with the given rectangle.
     ///
     /// TODO(pcwalton): This could more eagerly eliminate complex clipping regions, at the cost of
     /// complexity.
     #[inline]
-    pub fn intersect_rect(self, rect: &Rect<Au>) -> ClippingRegion {
-        ClippingRegion {
-            main: self.main.intersection(rect).unwrap_or(Rect::zero()),
-            complex: self.complex,
-        }
+    pub fn intersect_rect(&mut self, rect: &Rect<Au>) {
+        self.main = self.main.intersection(rect).unwrap_or(Rect::zero())
     }
 
     /// Returns true if this clipping region might be nonempty. This can return false positives,
diff --git a/components/layout/block.rs b/components/layout/block.rs
index 56785dd5eda..a348c541ebf 100644
--- a/components/layout/block.rs
+++ b/components/layout/block.rs
@@ -1954,8 +1954,10 @@ impl Flow for BlockFlow {
             self.base.position.size.to_physical(self.base.writing_mode);
 
         // Compute the origin and clipping rectangle for children.
+        //
+        // `clip` is in the child coordinate system.
+        let mut clip;
         let origin_for_children;
-        let clip_in_child_coordinate_system;
         let is_stacking_context = self.fragment.establishes_stacking_context();
         if is_stacking_context {
             // We establish a stacking context, so the position of our children is vertically
@@ -1966,12 +1968,11 @@ impl Flow for BlockFlow {
             // FIXME(pcwalton): Is this vertical-writing-direction-safe?
             let margin = self.fragment.margin.to_physical(self.base.writing_mode);
             origin_for_children = Point2D::new(-margin.left, Au(0));
-            clip_in_child_coordinate_system =
-                self.base.clip.translate(&-self.base.stacking_relative_position);
+            clip = self.base.clip.translate(&-self.base.stacking_relative_position);
         } else {
             let relative_offset = relative_offset.to_physical(self.base.writing_mode);
             origin_for_children = self.base.stacking_relative_position + relative_offset;
-            clip_in_child_coordinate_system = self.base.clip.clone();
+            clip = self.base.clip.clone();
         }
 
         let stacking_relative_position_of_display_port_for_children =
@@ -2003,8 +2004,8 @@ impl Flow for BlockFlow {
                                                   .early_absolute_position_info
                                                   .relative_containing_block_mode,
                                               CoordinateSystem::Own);
-        let clip = self.fragment.clipping_region_for_children(
-            &clip_in_child_coordinate_system,
+        self.fragment.adjust_clipping_region_for_children(
+            &mut clip,
             &stacking_relative_border_box,
             self.base.flags.contains(IS_ABSOLUTELY_POSITIONED));
 
diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs
index b76bdc9ff5d..f1e1b4b806b 100644
--- a/components/layout/display_list_builder.rs
+++ b/components/layout/display_list_builder.rs
@@ -231,19 +231,17 @@ pub trait FragmentDisplayListBuilding {
                           clip: &ClippingRegion,
                           stacking_relative_display_port: &Rect<Au>);
 
-    /// Returns the appropriate clipping region for descendants of this fragment.
-    fn clipping_region_for_children(&self,
-                                    current_clip: &ClippingRegion,
-                                    stacking_relative_border_box: &Rect<Au>,
-                                    is_absolutely_positioned: bool)
-                                    -> ClippingRegion;
-
-    /// Calculates the clipping rectangle for a fragment, taking the `clip` property into account
+    /// Adjusts the clipping region for descendants of this fragment as appropriate.
+    fn adjust_clipping_region_for_children(&self,
+                                           current_clip: &mut ClippingRegion,
+                                           stacking_relative_border_box: &Rect<Au>,
+                                           is_absolutely_positioned: bool);
+
+    /// Adjusts the clipping rectangle for a fragment to take the `clip` property into account
     /// per CSS 2.1 § 11.1.2.
-    fn calculate_style_specified_clip(&self,
-                                      parent_clip: &ClippingRegion,
-                                      stacking_relative_border_box: &Rect<Au>)
-                                      -> ClippingRegion;
+    fn adjust_clip_for_style(&self,
+                             parent_clip: &mut ClippingRegion,
+                             stacking_relative_border_box: &Rect<Au>);
 
     /// Builds the display items necessary to paint the selection and/or caret for this fragment,
     /// if any.
@@ -481,7 +479,8 @@ impl FragmentDisplayListBuilding for Fragment {
             // Clip.
             //
             // TODO: Check the bounds to see if a clip item is actually required.
-            let clip = clip.clone().intersect_rect(&bounds);
+            let mut clip = clip.clone();
+            clip.intersect_rect(&bounds);
 
             // Background image should be positioned on the padding box basis.
             let border = style.logical_border_width().to_physical(style.writing_mode);
@@ -580,7 +579,8 @@ impl FragmentDisplayListBuilding for Fragment {
                                                          clip: &ClippingRegion,
                                                          gradient: &LinearGradient,
                                                          style: &ComputedValues) {
-        let clip = clip.clone().intersect_rect(absolute_bounds);
+        let mut clip = clip.clone();
+        clip.intersect_rect(absolute_bounds);
 
         // This is the distance between the center and the ending point; i.e. half of the distance
         // between the starting point and the ending point.
@@ -893,15 +893,14 @@ impl FragmentDisplayListBuilding for Fragment {
         }), DisplayListSection::Content);
     }
 
-    fn calculate_style_specified_clip(&self,
-                                      parent_clip: &ClippingRegion,
-                                      stacking_relative_border_box: &Rect<Au>)
-                                      -> ClippingRegion {
+    fn adjust_clip_for_style(&self,
+                             parent_clip: &mut ClippingRegion,
+                             stacking_relative_border_box: &Rect<Au>) {
         // Account for `clip` per CSS 2.1 § 11.1.2.
         let style_clip_rect = match (self.style().get_box().position,
                                      self.style().get_effects().clip.0) {
             (position::T::absolute, Some(style_clip_rect)) => style_clip_rect,
-            _ => return (*parent_clip).clone(),
+            _ => return,
         };
 
         // FIXME(pcwalton, #2795): Get the real container size.
@@ -910,7 +909,7 @@ impl FragmentDisplayListBuilding for Fragment {
         let right = style_clip_rect.right.unwrap_or(stacking_relative_border_box.size.width);
         let bottom = style_clip_rect.bottom.unwrap_or(stacking_relative_border_box.size.height);
         let clip_size = Size2D::new(right - clip_origin.x, bottom - clip_origin.y);
-        (*parent_clip).clone().intersect_rect(&Rect::new(clip_origin, clip_size))
+        parent_clip.intersect_rect(&Rect::new(clip_origin, clip_size))
     }
 
     fn build_display_items_for_selection_if_necessary(&self,
@@ -995,7 +994,8 @@ impl FragmentDisplayListBuilding for Fragment {
 
         // Calculate the clip rect. If there's nothing to render at all, don't even construct
         // display list items.
-        let clip = self.calculate_style_specified_clip(clip, &stacking_relative_border_box);
+        let mut clip = (*clip).clone();
+        self.adjust_clip_for_style(&mut clip, &stacking_relative_border_box);
         if !clip.might_intersect_rect(&stacking_relative_border_box) {
             return;
         }
@@ -1424,19 +1424,17 @@ impl FragmentDisplayListBuilding for Fragment {
                                       layer_info))
     }
 
-    fn clipping_region_for_children(&self,
-                                    current_clip: &ClippingRegion,
-                                    stacking_relative_border_box: &Rect<Au>,
-                                    is_absolutely_positioned: bool)
-                                    -> ClippingRegion {
+    fn adjust_clipping_region_for_children(&self,
+                                           current_clip: &mut ClippingRegion,
+                                           stacking_relative_border_box: &Rect<Au>,
+                                           is_absolutely_positioned: bool) {
         // Don't clip if we're text.
         if self.is_scanned_text_fragment() {
-            return (*current_clip).clone()
+            return
         }
 
         // Account for style-specified `clip`.
-        let mut current_clip = self.calculate_style_specified_clip(current_clip,
-                                                                   stacking_relative_border_box);
+        self.adjust_clip_for_style(current_clip, stacking_relative_border_box);
 
         // Clip according to the values of `overflow-x` and `overflow-y`.
         //
@@ -1452,7 +1450,7 @@ impl FragmentDisplayListBuilding for Fragment {
                 let max_x = cmp::min(bounds.max_x(), stacking_relative_border_box.max_x());
                 bounds.origin.x = cmp::max(bounds.origin.x, stacking_relative_border_box.origin.x);
                 bounds.size.width = max_x - bounds.origin.x;
-                current_clip = current_clip.intersect_rect(&bounds)
+                current_clip.intersect_rect(&bounds)
             }
             _ => {}
         }
@@ -1464,12 +1462,10 @@ impl FragmentDisplayListBuilding for Fragment {
                 let max_y = cmp::min(bounds.max_y(), stacking_relative_border_box.max_y());
                 bounds.origin.y = cmp::max(bounds.origin.y, stacking_relative_border_box.origin.y);
                 bounds.size.height = max_y - bounds.origin.y;
-                current_clip = current_clip.intersect_rect(&bounds)
+                current_clip.intersect_rect(&bounds)
             }
             _ => {}
         }
-
-        current_clip
     }
 
     fn build_display_list_for_text_fragment(&self,
@@ -1715,10 +1711,14 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
         };
 
         // Add the box that starts the block context.
-        let clip = if self.fragment.establishes_stacking_context() {
-            self.base.clip.translate(&-self.base.stacking_relative_position)
+        let translated_clip = if self.fragment.establishes_stacking_context() {
+            Some(self.base.clip.translate(&-self.base.stacking_relative_position))
         } else {
-            self.base.clip.clone()
+            None
+        };
+        let clip = match translated_clip {
+            Some(ref translated_clip) => translated_clip,
+            None => &self.base.clip,
         };
 
         self.fragment
@@ -1732,7 +1732,7 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
                                     .relative_containing_block_mode,
                                 border_painting_mode,
                                 background_border_section,
-                                &clip,
+                                clip,
                                 &self.base.stacking_relative_position_of_display_port);
 
         // Add children.
diff --git a/components/layout/flow.rs b/components/layout/flow.rs
index 6dfd78ec841..a02b718d8cd 100644
--- a/components/layout/flow.rs
+++ b/components/layout/flow.rs
@@ -1184,7 +1184,8 @@ impl BaseFlow {
 
         for item in items.iter() {
             let base_item = item.item.base();
-            let paint_bounds = base_item.clip.clone().intersect_rect(&base_item.bounds);
+            let mut paint_bounds = base_item.clip.clone();
+            paint_bounds.intersect_rect(&base_item.bounds);
             if !paint_bounds.might_be_nonempty() {
                 continue;
             }
diff --git a/components/layout/inline.rs b/components/layout/inline.rs
index 291efea790d..1212f8a3cfc 100644
--- a/components/layout/inline.rs
+++ b/components/layout/inline.rs
@@ -1680,9 +1680,10 @@ impl Flow for InlineFlow {
                                                       CoordinateSystem::Parent);
             let stacking_relative_content_box =
                 fragment.stacking_relative_content_box(&stacking_relative_border_box);
-            let clip = fragment.clipping_region_for_children(&self.base.clip,
-                                                             &stacking_relative_border_box,
-                                                             false);
+            let mut clip = self.base.clip.clone();
+            fragment.adjust_clipping_region_for_children(&mut clip,
+                                                         &stacking_relative_border_box,
+                                                         false);
             let is_positioned = fragment.is_positioned();
             match fragment.specific {
                 SpecificFragmentInfo::InlineBlock(ref mut info) => {