gfx: Clip the background properly when border-radius is used.

Improves Reddit, GitHub, etc.

components/gfx/display_list/mod.rs

@@ -33,10 +33,11 @@ use servo_msg::compositor_msg::LayerId;
 use servo_net::image::base::Image;
 use servo_util::cursor::Cursor;
 use servo_util::dlist as servo_dlist;
-use servo_util::geometry::{mod, Au, ZERO_POINT};
+use servo_util::geometry::{mod, Au, MAX_RECT, ZERO_POINT, ZERO_RECT};
 use servo_util::range::Range;
 use servo_util::smallvec::{SmallVec, SmallVec8};
 use std::fmt;
+use std::num::Zero;
 use std::slice::Items;
 use style::ComputedValues;
 use style::computed_values::border_style;
@@ -205,7 +206,7 @@ impl StackingContext {
                 page_rect: paint_context.page_rect,
                 screen_rect: paint_context.screen_rect,
                 clip_rect: clip_rect,
-                transient_clip_rect: None,
+                transient_clip: None,
             };
 
             // Optimize the display list to throw out out-of-bounds display items and so forth.
@@ -348,7 +349,9 @@ impl StackingContext {
                                   mut iterator: I)
                                   where I: Iterator<&'a DisplayItem> {
             for item in iterator {
-                if !geometry::rect_contains_point(item.base().clip_rect, point) {
+                // TODO(pcwalton): Use a precise algorithm here. This will allow us to properly hit
+                // test elements with `border-radius`, for example.
+                if !item.base().clip.might_intersect_point(&point) {
                     // Clipped out.
                     continue
                 }
@@ -477,25 +480,133 @@ pub struct BaseDisplayItem {
     /// Metadata attached to this display item.
     pub metadata: DisplayItemMetadata,
 
-    /// The rectangle to clip to.
+    /// The region to clip to.
-    ///
+    pub clip: ClippingRegion,
-    /// TODO(pcwalton): Eventually, to handle `border-radius`, this will (at least) need to grow
-    /// the ability to describe rounded rectangles.
-    pub clip_rect: Rect<Au>,
 }
 
 impl BaseDisplayItem {
     #[inline(always)]
-    pub fn new(bounds: Rect<Au>, metadata: DisplayItemMetadata, clip_rect: Rect<Au>)
+    pub fn new(bounds: Rect<Au>, metadata: DisplayItemMetadata, clip: ClippingRegion)
                -> BaseDisplayItem {
         BaseDisplayItem {
             bounds: bounds,
             metadata: metadata,
-            clip_rect: clip_rect,
+            clip: clip,
         }
     }
 }
 
+/// A clipping region for a display item. Currently, this can describe rectangles, rounded
+/// rectangles (for `border-radius`), or arbitrary intersections of the two. Arbitrary transforms
+/// are not supported because those are handled by the higher-level `StackingContext` abstraction.
+#[deriving(Clone, PartialEq, Show)]
+pub struct ClippingRegion {
+    /// The main rectangular region. This does not include any corners.
+    pub main: Rect<Au>,
+    /// Any complex regions.
+    ///
+    /// TODO(pcwalton): Atomically reference count these? Not sure if it's worth the trouble.
+    /// Measure and follow up.
+    pub complex: Vec<ComplexClippingRegion>,
+}
+
+/// A complex clipping region. These don't as easily admit arbitrary intersection operations, so
+/// they're stored in a list over to the side. Currently a complex clipping region is just a
+/// rounded rectangle, but the CSS WGs will probably make us throw more stuff in here eventually.
+#[deriving(Clone, PartialEq, Show)]
+pub struct ComplexClippingRegion {
+    /// The boundaries of the rectangle.
+    pub rect: Rect<Au>,
+    /// Border radii of this rectangle.
+    pub radii: BorderRadii<Au>,
+}
+
+impl ClippingRegion {
+    /// Returns an empty clipping region that, if set, will result in no pixels being visible.
+    #[inline]
+    pub fn empty() -> ClippingRegion {
+        ClippingRegion {
+            main: ZERO_RECT,
+            complex: Vec::new(),
+        }
+    }
+
+    /// Returns an all-encompassing clipping region that clips no pixels out.
+    #[inline]
+    pub fn max() -> ClippingRegion {
+        ClippingRegion {
+            main: MAX_RECT,
+            complex: Vec::new(),
+        }
+    }
+
+    /// Returns a clipping region that represents the given rectangle.
+    #[inline]
+    pub fn from_rect(rect: &Rect<Au>) -> ClippingRegion {
+        ClippingRegion {
+            main: *rect,
+            complex: Vec::new(),
+        }
+    }
+
+    /// Returns the intersection of this clipping region and 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(ZERO_RECT),
+            complex: self.complex,
+        }
+    }
+
+    /// Returns true if this clipping region might be nonempty. This can return false positives,
+    /// but never false negatives.
+    #[inline]
+    pub fn might_be_nonempty(&self) -> bool {
+        !self.main.is_empty()
+    }
+
+    /// Returns true if this clipping region might contain the given point and false otherwise.
+    /// This is a quick, not a precise, test; it can yield false positives.
+    #[inline]
+    pub fn might_intersect_point(&self, point: &Point2D<Au>) -> bool {
+        geometry::rect_contains_point(self.main, *point) &&
+            self.complex.iter().all(|complex| geometry::rect_contains_point(complex.rect, *point))
+    }
+
+    /// Returns true if this clipping region might intersect the given rectangle and false
+    /// otherwise. This is a quick, not a precise, test; it can yield false positives.
+    #[inline]
+    pub fn might_intersect_rect(&self, rect: &Rect<Au>) -> bool {
+        self.main.intersects(rect) &&
+            self.complex.iter().all(|complex| complex.rect.intersects(rect))
+    }
+
+
+    /// Returns a bounding rect that surrounds this entire clipping region.
+    #[inline]
+    pub fn bounding_rect(&self) -> Rect<Au> {
+        let mut rect = self.main;
+        for complex in self.complex.iter() {
+            rect = rect.union(&complex.rect)
+        }
+        rect
+    }
+
+    /// Intersects this clipping region with the given rounded rectangle.
+    #[inline]
+    pub fn intersect_with_rounded_rect(mut self, rect: &Rect<Au>, radii: &BorderRadii<Au>)
+                                       -> ClippingRegion {
+        self.complex.push(ComplexClippingRegion {
+            rect: *rect,
+            radii: *radii,
+        });
+        self
+    }
+}
+
 /// Metadata attached to each display item. This is useful for performing auxiliary tasks with
 /// the display list involving hit testing: finding the originating DOM node and determining the
 /// cursor to use when the element is hovered over.
@@ -618,6 +729,15 @@ pub struct BorderRadii<T> {
     pub bottom_left: T,
 }
 
+impl<T> BorderRadii<T> where T: PartialEq + Zero {
+    /// Returns true if all the radii are zero.
+    pub fn is_square(&self) -> bool {
+        let zero = Zero::zero();
+        self.top_left == zero && self.top_right == zero && self.bottom_right == zero &&
+            self.bottom_left == zero
+    }
+}
+
 /// Paints a line segment.
 #[deriving(Clone)]
 pub struct LineDisplayItem {
@@ -673,13 +793,12 @@ impl<'a> Iterator<&'a DisplayItem> for DisplayItemIterator<'a> {
 impl DisplayItem {
     /// Paints this display item into the given painting context.
     fn draw_into_context(&self, paint_context: &mut PaintContext) {
-        let this_clip_rect = self.base().clip_rect;
+        {
-        if paint_context.transient_clip_rect != Some(this_clip_rect) {
+            let this_clip = &self.base().clip;
-            if paint_context.transient_clip_rect.is_some() {
+            match paint_context.transient_clip {
-                paint_context.draw_pop_clip();
+                Some(ref transient_clip) if transient_clip == this_clip => {}
+                Some(_) | None => paint_context.push_transient_clip((*this_clip).clone()),
             }
-            paint_context.draw_push_clip(&this_clip_rect);
-            paint_context.transient_clip_rect = Some(this_clip_rect)
         }
 
         match *self {

components/gfx/display_list/optimizer.rs

@@ -47,7 +47,7 @@ impl DisplayListOptimizer {
                                          where I: Iterator<&'a DisplayItem> {
         for display_item in display_items {
             if self.visible_rect.intersects(&display_item.base().bounds) &&
-                    self.visible_rect.intersects(&display_item.base().clip_rect) {
+                    display_item.base().clip.might_intersect_rect(&self.visible_rect) {
                 result_list.push_back((*display_item).clone())
             }
         }

components/gfx/paint_context.rs

@@ -8,13 +8,13 @@ use azure::azure::AzIntSize;
 use azure::azure_hl::{A8, B8G8R8A8, Color, ColorPattern, ColorPatternRef, DrawOptions};
 use azure::azure_hl::{DrawSurfaceOptions, DrawTarget, ExtendClamp, GaussianBlurFilterType};
 use azure::azure_hl::{GaussianBlurInput, GradientStop, Linear, LinearGradientPattern};
-use azure::azure_hl::{LinearGradientPatternRef, Path, SourceOp, StdDeviationGaussianBlurAttribute};
+use azure::azure_hl::{LinearGradientPatternRef, Path, PathBuilder, SourceOp};
-use azure::azure_hl::{StrokeOptions};
+use azure::azure_hl::{StdDeviationGaussianBlurAttribute, StrokeOptions};
 use azure::scaled_font::ScaledFont;
 use azure::{AZ_CAP_BUTT, AzFloat, struct__AzDrawOptions, struct__AzGlyph};
 use azure::{struct__AzGlyphBuffer, struct__AzPoint, AzDrawTargetFillGlyphs};
-use display_list::{BOX_SHADOW_INFLATION_FACTOR, TextDisplayItem, BorderRadii};
 use display_list::TextOrientation::{SidewaysLeft, SidewaysRight, Upright};
+use display_list::{BOX_SHADOW_INFLATION_FACTOR, BorderRadii, ClippingRegion, TextDisplayItem};
 use font_context::FontContext;
 use geom::matrix2d::Matrix2D;
 use geom::point::Point2D;
@@ -29,6 +29,7 @@ use servo_util::geometry::{Au, MAX_RECT};
 use servo_util::opts;
 use servo_util::range::Range;
 use std::default::Default;
+use std::mem;
 use std::num::{Float, FloatMath};
 use std::ptr;
 use style::computed_values::border_style;
@@ -45,10 +46,10 @@ pub struct PaintContext<'a> {
     pub screen_rect: Rect<uint>,
     /// The clipping rect for the stacking context as a whole.
     pub clip_rect: Option<Rect<Au>>,
-    /// The current transient clipping rect, if any. A "transient clipping rect" is the clipping
+    /// The current transient clipping region, if any. A "transient clipping region" is the
-    /// rect used by the last display item. We cache the last value so that we avoid pushing and
+    /// clipping region used by the last display item. We cache the last value so that we avoid
-    /// popping clip rects unnecessarily.
+    /// pushing and popping clipping regions unnecessarily.
-    pub transient_clip_rect: Option<Rect<Au>>,
+    pub transient_clip: Option<ClippingRegion>,
 }
 
 enum Direction {
@@ -278,6 +279,12 @@ impl<'a> PaintContext<'a> {
         self.draw_target.fill(&path_builder.finish(), &ColorPattern::new(color), &draw_options);
     }
 
+    fn push_rounded_rect_clip(&self, bounds: &Rect<f32>, radii: &BorderRadii<AzFloat>) {
+        let mut path_builder = self.draw_target.create_path_builder();
+        self.create_rounded_rect_path(&mut path_builder, bounds, radii);
+        self.draw_target.push_clip(&path_builder.finish());
+    }
+
     // The following comment is wonderful, and stolen from
     // gecko:gfx/thebes/gfxContext.cpp:RoundedRectangle for reference.
     //
@@ -537,8 +544,55 @@ impl<'a> PaintContext<'a> {
         }
     }
 
-        let path = path_builder.finish();
+    /// Creates a path representing the given rounded rectangle.
-        self.draw_target.fill(&path, &ColorPattern::new(color), &draw_opts);
+    ///
+    /// TODO(pcwalton): Should we unify with the code above? It doesn't seem immediately obvious
+    /// how to do that (especially without regressing performance) unless we have some way to
+    /// efficiently intersect or union paths, since different border styles/colors can force us to
+    /// slice through the rounded corners. My first attempt to unify with the above code resulted
+    /// in making a mess of it, and the simplicity of this code path is appealing, so it may not
+    /// be worth it… In any case, revisit this decision when we support elliptical radii.
+    fn create_rounded_rect_path(&self,
+                                path_builder: &mut PathBuilder,
+                                bounds: &Rect<f32>,
+                                radii: &BorderRadii<AzFloat>) {
+        //    +----------+
+        //   / 1        2 \
+        //  + 8          3 +
+        //  |              |
+        //  + 7          4 +
+        //   \ 6        5 /
+        //    +----------+
+
+        path_builder.move_to(Point2D(bounds.origin.x + radii.top_left, bounds.origin.y));   // 1
+        path_builder.line_to(Point2D(bounds.max_x() - radii.top_right, bounds.origin.y));   // 2
+        path_builder.arc(Point2D(bounds.max_x() - radii.top_right,
+                                 bounds.origin.y + radii.top_right),
+                         radii.top_right,
+                         1.5f32 * Float::frac_pi_2(),
+                         Float::two_pi(),
+                         false);                                                            // 3
+        path_builder.line_to(Point2D(bounds.max_x(), bounds.max_y() - radii.bottom_right)); // 4
+        path_builder.arc(Point2D(bounds.max_x() - radii.bottom_right,
+                                 bounds.max_y() - radii.bottom_right),
+                         radii.bottom_right,
+                         0.0,
+                         Float::frac_pi_2(),
+                         false);                                                            // 5
+        path_builder.line_to(Point2D(bounds.origin.x + radii.bottom_left, bounds.max_y())); // 6
+        path_builder.arc(Point2D(bounds.origin.x + radii.bottom_left,
+                                 bounds.max_y() - radii.bottom_left),
+                         radii.bottom_left,
+                         Float::frac_pi_2(),
+                         Float::pi(),
+                         false);                                                            // 7
+        path_builder.line_to(Point2D(bounds.origin.x, bounds.origin.y + radii.top_left));   // 8
+        path_builder.arc(Point2D(bounds.origin.x + radii.top_left,
+                                 bounds.origin.y + radii.top_left),
+                         radii.top_left,
+                         Float::pi(),
+                         1.5f32 * Float::frac_pi_2(),
+                         false);                                                            // 1
     }
 
     fn draw_dashed_border_segment(&self,
@@ -956,11 +1010,27 @@ impl<'a> PaintContext<'a> {
     }
 
     pub fn remove_transient_clip_if_applicable(&mut self) {
-        if self.transient_clip_rect.is_some() {
+        if let Some(old_transient_clip) = mem::replace(&mut self.transient_clip, None) {
-            self.draw_pop_clip();
+            for _ in old_transient_clip.complex.iter() {
-            self.transient_clip_rect = None
+                self.draw_pop_clip()
+            }
+            self.draw_pop_clip()
         }
     }
+
+    /// Sets a new transient clipping region. Automatically calls
+    /// `remove_transient_clip_if_applicable()` first.
+    pub fn push_transient_clip(&mut self, clip_region: ClippingRegion) {
+        self.remove_transient_clip_if_applicable();
+
+        self.draw_push_clip(&clip_region.main);
+        for complex_region in clip_region.complex.iter() {
+            // FIXME(pcwalton): Actually draw a rounded rect.
+            self.push_rounded_rect_clip(&complex_region.rect.to_azure_rect(),
+                                        &complex_region.radii.to_radii_px())
+        }
+        self.transient_clip = Some(clip_region)
+    }
 }
 
 pub trait ToAzurePoint {

components/gfx/paint_task.rs

@@ -510,7 +510,7 @@ impl WorkerThread {
                 page_rect: tile.page_rect,
                 screen_rect: tile.screen_rect,
                 clip_rect: None,
-                transient_clip_rect: None,
+                transient_clip: None,
             };
 
             // Apply the translation to paint the tile we want.

components/layout/block.rs

@@ -50,10 +50,10 @@ use table::ColumnComputedInlineSize;
 use wrapper::ThreadSafeLayoutNode;
 
 use geom::Size2D;
-use gfx::display_list::DisplayList;
+use gfx::display_list::{ClippingRegion, DisplayList};
 use serialize::{Encoder, Encodable};
 use servo_msg::compositor_msg::LayerId;
-use servo_util::geometry::{Au, MAX_AU, MAX_RECT, ZERO_POINT};
+use servo_util::geometry::{Au, MAX_AU, ZERO_POINT};
 use servo_util::logical_geometry::{LogicalPoint, LogicalRect, LogicalSize};
 use servo_util::opts;
 use std::cmp::{max, min};
@@ -1699,7 +1699,7 @@ impl Flow for BlockFlow {
         let container_size = Size2D::zero();
 
         if self.is_root() {
-            self.base.clip_rect = MAX_RECT
+            self.base.clip = ClippingRegion::max()
         }
 
         if self.base.flags.contains(IS_ABSOLUTELY_POSITIONED) {
@@ -1774,8 +1774,8 @@ impl Flow for BlockFlow {
         } else {
             self.base.stacking_relative_position
         };
-        let clip_rect = self.fragment.clip_rect_for_children(&self.base.clip_rect,
+        let clip = self.fragment.clipping_region_for_children(&self.base.clip,
-                                                             &origin_for_children);
+                                                              &origin_for_children);
 
         // Process children.
         let writing_mode = self.base.writing_mode;
@@ -1789,7 +1789,7 @@ impl Flow for BlockFlow {
             }
 
             flow::mut_base(kid).absolute_position_info = absolute_position_info_for_children;
-            flow::mut_base(kid).clip_rect = clip_rect
+            flow::mut_base(kid).clip = clip.clone()
         }
     }
 

components/layout/display_list_builder.rs

@@ -23,7 +23,7 @@ use geom::approxeq::ApproxEq;
 use geom::{Point2D, Rect, Size2D, SideOffsets2D};
 use gfx::color;
 use gfx::display_list::{BOX_SHADOW_INFLATION_FACTOR, BaseDisplayItem, BorderDisplayItem};
-use gfx::display_list::{BorderRadii, BoxShadowDisplayItem};
+use gfx::display_list::{BorderRadii, BoxShadowDisplayItem, ClippingRegion};
 use gfx::display_list::{DisplayItem, DisplayList, DisplayItemMetadata};
 use gfx::display_list::{GradientDisplayItem};
 use gfx::display_list::{GradientStop, ImageDisplayItem, LineDisplayItem};
@@ -35,7 +35,7 @@ use servo_msg::compositor_msg::{FixedPosition, Scrollable};
 use servo_msg::constellation_msg::{ConstellationChan, FrameRectMsg};
 use servo_net::image::holder::ImageHolder;
 use servo_util::cursor::{DefaultCursor, TextCursor, VerticalTextCursor};
-use servo_util::geometry::{mod, Au, ZERO_POINT, ZERO_RECT};
+use servo_util::geometry::{mod, Au, ZERO_POINT};
 use servo_util::logical_geometry::{LogicalRect, WritingMode};
 use servo_util::opts;
 use std::default::Default;
@@ -82,7 +82,7 @@ pub trait FragmentDisplayListBuilding {
                                                        layout_context: &LayoutContext,
                                                        level: StackingLevel,
                                                        absolute_bounds: &Rect<Au>,
-                                                       clip_rect: &Rect<Au>);
+                                                       clip: &ClippingRegion);
 
     /// Adds the display items necessary to paint the background image of this fragment to the
     /// display list at the appropriate stacking level.
@@ -92,7 +92,7 @@ pub trait FragmentDisplayListBuilding {
                                                layout_context: &LayoutContext,
                                                level: StackingLevel,
                                                absolute_bounds: &Rect<Au>,
-                                               clip_rect: &Rect<Au>,
+                                               clip: &ClippingRegion,
                                                image_url: &Url);
 
     /// Adds the display items necessary to paint the background linear gradient of this fragment
@@ -101,7 +101,7 @@ pub trait FragmentDisplayListBuilding {
                                                          display_list: &mut DisplayList,
                                                          level: StackingLevel,
                                                          absolute_bounds: &Rect<Au>,
-                                                         clip_rect: &Rect<Au>,
+                                                         clip: &ClippingRegion,
                                                          gradient: &LinearGradient,
                                                          style: &ComputedValues);
 
@@ -112,7 +112,7 @@ pub trait FragmentDisplayListBuilding {
                                                     display_list: &mut DisplayList,
                                                     abs_bounds: &Rect<Au>,
                                                     level: StackingLevel,
-                                                    clip_rect: &Rect<Au>);
+                                                    clip: &ClippingRegion);
 
     /// Adds the display items necessary to paint the outline of this fragment to the display list
     /// if necessary.
@@ -120,7 +120,7 @@ pub trait FragmentDisplayListBuilding {
                                                     style: &ComputedValues,
                                                     display_list: &mut DisplayList,
                                                     bounds: &Rect<Au>,
-                                                    clip_rect: &Rect<Au>);
+                                                    clip: &ClippingRegion);
 
     /// Adds the display items necessary to paint the box shadow of this fragment to the display
     /// list if necessary.
@@ -130,19 +130,19 @@ pub trait FragmentDisplayListBuilding {
                                                        layout_context: &LayoutContext,
                                                        level: StackingLevel,
                                                        absolute_bounds: &Rect<Au>,
-                                                       clip_rect: &Rect<Au>);
+                                                       clip: &ClippingRegion);
 
     fn build_debug_borders_around_text_fragments(&self,
                                                  style: &ComputedValues,
                                                  display_list: &mut DisplayList,
                                                  flow_origin: Point2D<Au>,
                                                  text_fragment: &ScannedTextFragmentInfo,
-                                                 clip_rect: &Rect<Au>);
+                                                 clip: &ClippingRegion);
 
     fn build_debug_borders_around_fragment(&self,
                                            display_list: &mut DisplayList,
                                            flow_origin: Point2D<Au>,
-                                           clip_rect: &Rect<Au>);
+                                           clip: &ClippingRegion);
 
     /// Adds the display items for this fragment to the given display list.
     ///
@@ -152,13 +152,13 @@ pub trait FragmentDisplayListBuilding {
     /// * `layout_context`: The layout context.
     /// * `dirty`: The dirty rectangle in the coordinate system of the owning flow.
     /// * `flow_origin`: Position of the origin of the owning flow wrt the display list root flow.
-    /// * `clip_rect`: The rectangle to clip the display items to.
+    /// * `clip`: The region to clip the display items to.
     fn build_display_list(&mut self,
                           display_list: &mut DisplayList,
                           layout_context: &LayoutContext,
                           flow_origin: Point2D<Au>,
                           background_and_border_level: BackgroundAndBorderLevel,
-                          clip_rect: &Rect<Au>);
+                          clip: &ClippingRegion);
 
     /// Sends the size and position of this iframe fragment to the constellation. This is out of
     /// line to guide inlining.
@@ -167,13 +167,13 @@ pub trait FragmentDisplayListBuilding {
                                             offset: Point2D<Au>,
                                             layout_context: &LayoutContext);
 
-    fn clipping_region_for_children(&self, current_clip: ClippingRegion, flow_origin: Point2D<Au>)
+    fn clipping_region_for_children(&self, current_clip: &ClippingRegion, flow_origin: &Point2D<Au>)
                                     -> ClippingRegion;
 
     /// Calculates the clipping rectangle for a fragment, taking the `clip` property into account
     /// per CSS 2.1 § 11.1.2.
-    fn calculate_style_specified_clip(&self, parent_clip_rect: &Rect<Au>, origin: &Point2D<Au>)
+    fn calculate_style_specified_clip(&self, parent_clip: &ClippingRegion, origin: &Point2D<Au>)
-                                      -> Rect<Au>;
+                                      -> ClippingRegion;
 
     /// Creates the text display item for one text fragment.
     fn build_display_list_for_text_fragment(&self,
@@ -224,7 +224,14 @@ impl FragmentDisplayListBuilding for Fragment {
                                                        layout_context: &LayoutContext,
                                                        level: StackingLevel,
                                                        absolute_bounds: &Rect<Au>,
-                                                       clip_rect: &Rect<Au>) {
+                                                       clip: &ClippingRegion) {
+        // Adjust the clipping region as necessary to account for `border-radius`.
+        let border_radii = build_border_radius(absolute_bounds, style.get_border());
+        let mut clip = (*clip).clone();
+        if !border_radii.is_square() {
+            clip = clip.intersect_with_rounded_rect(absolute_bounds, &border_radii)
+        }
+
         // FIXME: This causes a lot of background colors to be displayed when they are clearly not
         // needed. We could use display list optimization to clean this up, but it still seems
         // inefficient. What we really want is something like "nearest ancestor element that
@@ -236,7 +243,7 @@ impl FragmentDisplayListBuilding for Fragment {
                                            DisplayItemMetadata::new(self.node,
                                                                     style,
                                                                     DefaultCursor),
-                                           *clip_rect),
+                                           clip.clone()),
                 color: background_color.to_gfx_color(),
             }), level);
         }
@@ -251,7 +258,7 @@ impl FragmentDisplayListBuilding for Fragment {
                 self.build_display_list_for_background_linear_gradient(display_list,
                                                                        level,
                                                                        absolute_bounds,
-                                                                       clip_rect,
+                                                                       &clip,
                                                                        gradient,
                                                                        style)
             }
@@ -261,7 +268,7 @@ impl FragmentDisplayListBuilding for Fragment {
                                                              layout_context,
                                                              level,
                                                              absolute_bounds,
-                                                             clip_rect,
+                                                             &clip,
                                                              image_url)
             }
         }
@@ -273,7 +280,7 @@ impl FragmentDisplayListBuilding for Fragment {
                                                layout_context: &LayoutContext,
                                                level: StackingLevel,
                                                absolute_bounds: &Rect<Au>,
-                                               clip_rect: &Rect<Au>,
+                                               clip: &ClippingRegion,
                                                image_url: &Url) {
         let background = style.get_background();
         let mut holder = ImageHolder::new(image_url.clone(),
@@ -297,7 +304,7 @@ impl FragmentDisplayListBuilding for Fragment {
         // Clip.
         //
         // TODO: Check the bounds to see if a clip item is actually required.
-        let clip_rect = clip_rect.intersection(&bounds).unwrap_or(ZERO_RECT);
+        let clip = clip.clone().intersect_rect(&bounds);
 
         // Use background-attachment to get the initial virtual origin
         let (virtual_origin_x, virtual_origin_y) = match background.background_attachment {
@@ -350,7 +357,7 @@ impl FragmentDisplayListBuilding for Fragment {
         display_list.push(DisplayItem::ImageClass(box ImageDisplayItem {
             base: BaseDisplayItem::new(bounds,
                                        DisplayItemMetadata::new(self.node, style, DefaultCursor),
-                                       clip_rect),
+                                       clip),
             image: image.clone(),
             stretch_size: Size2D(Au::from_px(image.width as int),
                                  Au::from_px(image.height as int)),
@@ -361,10 +368,10 @@ impl FragmentDisplayListBuilding for Fragment {
                                                          display_list: &mut DisplayList,
                                                          level: StackingLevel,
                                                          absolute_bounds: &Rect<Au>,
-                                                         clip_rect: &Rect<Au>,
+                                                         clip: &ClippingRegion,
                                                          gradient: &LinearGradient,
                                                          style: &ComputedValues) {
-        let clip_rect = clip_rect.intersection(absolute_bounds).unwrap_or(ZERO_RECT);
+        let clip = clip.clone().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.
@@ -460,7 +467,7 @@ impl FragmentDisplayListBuilding for Fragment {
         let gradient_display_item = DisplayItem::GradientClass(box GradientDisplayItem {
             base: BaseDisplayItem::new(*absolute_bounds,
                                        DisplayItemMetadata::new(self.node, style, DefaultCursor),
-                                       clip_rect),
+                                       clip),
             start_point: center - delta,
             end_point: center + delta,
             stops: stops,
@@ -475,7 +482,7 @@ impl FragmentDisplayListBuilding for Fragment {
                                                        _layout_context: &LayoutContext,
                                                        level: StackingLevel,
                                                        absolute_bounds: &Rect<Au>,
-                                                       clip_rect: &Rect<Au>) {
+                                                       clip: &ClippingRegion) {
         // NB: According to CSS-BACKGROUNDS, box shadows render in *reverse* order (front to back).
         for box_shadow in style.get_effects().box_shadow.iter().rev() {
             let inflation = box_shadow.spread_radius + box_shadow.blur_radius *
@@ -488,7 +495,7 @@ impl FragmentDisplayListBuilding for Fragment {
                                            DisplayItemMetadata::new(self.node,
                                                                     style,
                                                                     DefaultCursor),
-                                           *clip_rect),
+                                           (*clip).clone()),
                 box_bounds: *absolute_bounds,
                 color: style.resolve_color(box_shadow.color).to_gfx_color(),
                 offset: Point2D(box_shadow.offset_x, box_shadow.offset_y),
@@ -504,7 +511,7 @@ impl FragmentDisplayListBuilding for Fragment {
                                                     display_list: &mut DisplayList,
                                                     abs_bounds: &Rect<Au>,
                                                     level: StackingLevel,
-                                                    clip_rect: &Rect<Au>) {
+                                                    clip: &ClippingRegion) {
         let border = style.logical_border_width();
         if border.is_zero() {
             return
@@ -519,7 +526,7 @@ impl FragmentDisplayListBuilding for Fragment {
         display_list.push(DisplayItem::BorderClass(box BorderDisplayItem {
             base: BaseDisplayItem::new(*abs_bounds,
                                        DisplayItemMetadata::new(self.node, style, DefaultCursor),
-                                       *clip_rect),
+                                       (*clip).clone()),
             border_widths: border.to_physical(style.writing_mode),
             color: SideOffsets2D::new(top_color.to_gfx_color(),
                                       right_color.to_gfx_color(),
@@ -537,7 +544,7 @@ impl FragmentDisplayListBuilding for Fragment {
                                                     style: &ComputedValues,
                                                     display_list: &mut DisplayList,
                                                     bounds: &Rect<Au>,
-                                                    clip_rect: &Rect<Au>) {
+                                                    clip: &ClippingRegion) {
         let width = style.get_outline().outline_width;
         if width == Au(0) {
             return
@@ -561,7 +568,7 @@ impl FragmentDisplayListBuilding for Fragment {
         display_list.outlines.push_back(DisplayItem::BorderClass(box BorderDisplayItem {
             base: BaseDisplayItem::new(bounds,
                                        DisplayItemMetadata::new(self.node, style, DefaultCursor),
-                                       *clip_rect),
+                                       (*clip).clone()),
             border_widths: SideOffsets2D::new_all_same(width),
             color: SideOffsets2D::new_all_same(color),
             style: SideOffsets2D::new_all_same(outline_style),
@@ -574,7 +581,7 @@ impl FragmentDisplayListBuilding for Fragment {
                                                  display_list: &mut DisplayList,
                                                  flow_origin: Point2D<Au>,
                                                  text_fragment: &ScannedTextFragmentInfo,
-                                                 clip_rect: &Rect<Au>) {
+                                                 clip: &ClippingRegion) {
         // FIXME(#2795): Get the real container size
         let container_size = Size2D::zero();
         // Fragment position wrt to the owning flow.
@@ -587,7 +594,7 @@ impl FragmentDisplayListBuilding for Fragment {
         display_list.content.push_back(DisplayItem::BorderClass(box BorderDisplayItem {
             base: BaseDisplayItem::new(absolute_fragment_bounds,
                                        DisplayItemMetadata::new(self.node, style, DefaultCursor),
-                                       *clip_rect),
+                                       (*clip).clone()),
             border_widths: SideOffsets2D::new_all_same(Au::from_px(1)),
             color: SideOffsets2D::new_all_same(color::rgb(0, 0, 200)),
             style: SideOffsets2D::new_all_same(border_style::solid),
@@ -605,7 +612,7 @@ impl FragmentDisplayListBuilding for Fragment {
         let line_display_item = box LineDisplayItem {
             base: BaseDisplayItem::new(baseline,
                                        DisplayItemMetadata::new(self.node, style, DefaultCursor),
-                                       *clip_rect),
+                                       (*clip).clone()),
             color: color::rgb(0, 200, 0),
             style: border_style::dashed,
         };
@@ -615,7 +622,7 @@ impl FragmentDisplayListBuilding for Fragment {
     fn build_debug_borders_around_fragment(&self,
                                            display_list: &mut DisplayList,
                                            flow_origin: Point2D<Au>,
-                                           clip_rect: &Rect<Au>) {
+                                           clip: &ClippingRegion) {
         // FIXME(#2795): Get the real container size
         let container_size = Size2D::zero();
         // Fragment position wrt to the owning flow.
@@ -630,7 +637,7 @@ impl FragmentDisplayListBuilding for Fragment {
                                        DisplayItemMetadata::new(self.node,
                                                                 &*self.style,
                                                                 DefaultCursor),
-                                       *clip_rect),
+                                       (*clip).clone()),
             border_widths: SideOffsets2D::new_all_same(Au::from_px(1)),
             color: SideOffsets2D::new_all_same(color::rgb(0, 0, 200)),
             style: SideOffsets2D::new_all_same(border_style::solid),
@@ -638,22 +645,24 @@ impl FragmentDisplayListBuilding for Fragment {
         }));
     }
 
-    fn calculate_style_specified_clip(&self, parent_clip_rect: &Rect<Au>, origin: &Point2D<Au>)
+    fn calculate_style_specified_clip(&self, parent_clip: &ClippingRegion, origin: &Point2D<Au>)
-                                      -> Rect<Au> {
+                                      -> ClippingRegion {
         // 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) {
             (position::absolute, Some(style_clip_rect)) => style_clip_rect,
-            _ => return *parent_clip_rect,
+            _ => return (*parent_clip).clone(),
         };
 
         // FIXME(pcwalton, #2795): Get the real container size.
         let border_box = self.border_box.to_physical(self.style.writing_mode, Size2D::zero());
         let clip_origin = Point2D(border_box.origin.x + style_clip_rect.left,
                                   border_box.origin.y + style_clip_rect.top);
-        Rect(clip_origin + *origin,
+        let new_clip_rect =
-             Size2D(style_clip_rect.right.unwrap_or(border_box.size.width) - clip_origin.x,
+            Rect(clip_origin + *origin,
-                    style_clip_rect.bottom.unwrap_or(border_box.size.height) - clip_origin.y))
+                 Size2D(style_clip_rect.right.unwrap_or(border_box.size.width) - clip_origin.x,
+                        style_clip_rect.bottom.unwrap_or(border_box.size.height) - clip_origin.y));
+        (*parent_clip).clone().intersect_rect(&new_clip_rect)
     }
 
     fn build_display_list(&mut self,
@@ -661,7 +670,7 @@ impl FragmentDisplayListBuilding for Fragment {
                           layout_context: &LayoutContext,
                           flow_origin: Point2D<Au>,
                           background_and_border_level: BackgroundAndBorderLevel,
-                          clip_rect: &Rect<Au>) {
+                          clip: &ClippingRegion) {
         // Compute the fragment position relative to the parent stacking context. If the fragment
         // itself establishes a stacking context, then the origin of its position will be (0, 0)
         // for the purposes of this computation.
@@ -692,9 +701,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_rect = self.calculate_style_specified_clip(clip_rect,
+        let clip = self.calculate_style_specified_clip(clip, &absolute_fragment_bounds.origin);
-                                                            &absolute_fragment_bounds.origin);
+        if !clip.might_intersect_rect(&absolute_fragment_bounds) {
-        if !absolute_fragment_bounds.intersects(&clip_rect) {
             return;
         }
 
@@ -712,7 +720,7 @@ impl FragmentDisplayListBuilding for Fragment {
                                                                          layout_context,
                                                                          level,
                                                                          &absolute_fragment_bounds,
-                                                                         clip);
+                                                                         &clip);
                 }
             }
             if !self.is_scanned_text_fragment() {
@@ -721,7 +729,7 @@ impl FragmentDisplayListBuilding for Fragment {
                                                                      layout_context,
                                                                      level,
                                                                      &absolute_fragment_bounds,
-                                                                     clip);
+                                                                     &clip);
             }
 
             // Add the background to the list, if applicable.
@@ -732,7 +740,7 @@ impl FragmentDisplayListBuilding for Fragment {
                                                                          layout_context,
                                                                          level,
                                                                          &absolute_fragment_bounds,
-                                                                         clip);
+                                                                         &clip);
                 }
             }
             if !self.is_scanned_text_fragment() {
@@ -741,7 +749,7 @@ impl FragmentDisplayListBuilding for Fragment {
                                                                      layout_context,
                                                                      level,
                                                                      &absolute_fragment_bounds,
-                                                                     clip);
+                                                                     &clip);
             }
 
             // Add a border and outlines, if applicable.
@@ -751,11 +759,11 @@ impl FragmentDisplayListBuilding for Fragment {
                                                                       display_list,
                                                                       &absolute_fragment_bounds,
                                                                       level,
-                                                                      clip);
+                                                                      &clip);
                     self.build_display_list_for_outline_if_applicable(&**style,
                                                                       display_list,
                                                                       &absolute_fragment_bounds,
-                                                                      clip);
+                                                                      &clip);
                 }
             }
             if !self.is_scanned_text_fragment() {
@@ -763,19 +771,19 @@ impl FragmentDisplayListBuilding for Fragment {
                                                                   display_list,
                                                                   &absolute_fragment_bounds,
                                                                   level,
-                                                                  clip);
+                                                                  &clip);
                 self.build_display_list_for_outline_if_applicable(&*self.style,
                                                                   display_list,
                                                                   &absolute_fragment_bounds,
-                                                                  clip);
+                                                                  &clip);
             }
         }
 
         // Create special per-fragment-type display items.
-        self.build_fragment_type_specific_display_items(display_list, flow_origin, clip);
+        self.build_fragment_type_specific_display_items(display_list, flow_origin, &clip);
 
         if opts::get().show_debug_fragment_borders {
-           self.build_debug_borders_around_fragment(display_list, flow_origin, clip)
+           self.build_debug_borders_around_fragment(display_list, flow_origin, &clip)
         }
 
         // If this is an iframe, then send its position and size up to the constellation.
@@ -901,15 +909,15 @@ impl FragmentDisplayListBuilding for Fragment {
                                iframe_rect));
     }
 
-    fn clipping_region_for_children(&self, current_clip: ClippingRegion, flow_origin: Point2D<Au>)
+    fn clipping_region_for_children(&self, current_clip: &ClippingRegion, flow_origin: &Point2D<Au>)
                                     -> ClippingRegion {
         // Don't clip if we're text.
         if self.is_scanned_text_fragment() {
-            return current_clip
+            return (*current_clip).clone()
         }
 
         // Account for style-specified `clip`.
-        let current_clip_rect = self.calculate_style_specified_clip(current_clip_rect, origin);
+        let current_clip = self.calculate_style_specified_clip(current_clip, flow_origin);
 
         // Only clip if `overflow` tells us to.
         match self.style.get_box().overflow {
@@ -921,7 +929,7 @@ impl FragmentDisplayListBuilding for Fragment {
         //
         // FIXME(#2795): Get the real container size.
         let physical_rect = self.border_box.to_physical(self.style.writing_mode, Size2D::zero());
-        current_clip.intersect_rect(&Rect(physical_rect.origin + flow_origin, physical_rect.size))
+        current_clip.intersect_rect(&Rect(physical_rect.origin + *flow_origin, physical_rect.size))
     }
 
     fn build_display_list_for_text_fragment(&self,
@@ -1078,7 +1086,7 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
                                          layout_context,
                                          stacking_relative_fragment_origin,
                                          background_border_level,
-                                         &self.base.clip_rect);
+                                         &self.base.clip);
 
         for kid in self.base.children.iter_mut() {
             flow::mut_base(kid).display_list_building_result.add_to(display_list);
@@ -1192,7 +1200,7 @@ impl ListItemFlowDisplayListBuilding for ListItemFlow {
                                           layout_context,
                                           stacking_relative_fragment_origin,
                                           BackgroundAndBorderLevel::Content,
-                                          &self.block_flow.base.clip_rect);
+                                          &self.block_flow.base.clip);
             }
         }
 

components/layout/flow.rs

@@ -47,6 +47,7 @@ use table_wrapper::TableWrapperFlow;
 use wrapper::ThreadSafeLayoutNode;
 
 use geom::{Point2D, Rect, Size2D};
+use gfx::display_list::ClippingRegion;
 use serialize::{Encoder, Encodable};
 use servo_msg::compositor_msg::LayerId;
 use servo_util::geometry::Au;
@@ -762,11 +763,8 @@ pub struct BaseFlow {
     /// FIXME(pcwalton): Merge with `absolute_static_i_offset` and `fixed_static_i_offset` above?
     pub absolute_position_info: AbsolutePositionInfo,
 
-    /// The clipping rectangle for this flow and its descendants, in layer coordinates.
+    /// The clipping region for this flow and its descendants, in layer coordinates.
-    ///
+    pub clip: ClippingRegion,
-    /// TODO(pcwalton): When we have `border-radius` this will need to at least support rounded
-    /// rectangles.
-    pub clip_rect: Rect<Au>,
 
     /// The results of display list building for this flow.
     pub display_list_building_result: DisplayListBuildingResult,
@@ -909,7 +907,7 @@ impl BaseFlow {
             absolute_cb: ContainingBlockLink::new(),
             display_list_building_result: DisplayListBuildingResult::None,
             absolute_position_info: AbsolutePositionInfo::new(writing_mode),
-            clip_rect: Rect(Point2D::zero(), Size2D::zero()),
+            clip: ClippingRegion::max(),
             flags: flags,
             writing_mode: writing_mode,
         }
@@ -945,16 +943,12 @@ impl BaseFlow {
         };
 
         for item in all_items.iter() {
-            let paint_bounds = match item.base().bounds.intersection(&item.base().clip_rect) {
+            let paint_bounds = item.base().clip.clone().intersect_rect(&item.base().bounds);
-                None => continue,
+            if !paint_bounds.might_be_nonempty() {
-                Some(rect) => rect,
-            };
-
-            if paint_bounds.is_empty() {
                 continue;
             }
 
-            if bounds.union(&paint_bounds) != bounds {
+            if bounds.union(&paint_bounds.bounding_rect()) != bounds {
                 error!("DisplayList item {} outside of Flow overflow ({})", item, paint_bounds);
             }
         }

components/layout/inline.rs

@@ -1215,15 +1215,15 @@ impl Flow for InlineFlow {
                 _ => continue,
             };
 
-            let clip_rect = fragment.clip_rect_for_children(&self.base.clip_rect,
+            let clip = fragment.clipping_region_for_children(&self.base.clip,
-                                                            &stacking_relative_position);
+                                                             &stacking_relative_position);
 
             match fragment.specific {
                 SpecificFragmentInfo::InlineBlock(ref mut info) => {
-                    flow::mut_base(info.flow_ref.deref_mut()).clip_rect = clip_rect
+                    flow::mut_base(info.flow_ref.deref_mut()).clip = clip
                 }
                 SpecificFragmentInfo::InlineAbsoluteHypothetical(ref mut info) => {
-                    flow::mut_base(info.flow_ref.deref_mut()).clip_rect = clip_rect
+                    flow::mut_base(info.flow_ref.deref_mut()).clip = clip
                 }
                 _ => {}
             }
@@ -1246,7 +1246,7 @@ impl Flow for InlineFlow {
                                         layout_context,
                                         fragment_origin,
                                         BackgroundAndBorderLevel::Content,
-                                        &self.base.clip_rect);
+                                        &self.base.clip);
             match fragment.specific {
                 SpecificFragmentInfo::InlineBlock(ref mut block_flow) => {
                     let block_flow = block_flow.flow_ref.deref_mut();

components/layout/layout_task.rs

@@ -25,7 +25,8 @@ use geom::rect::Rect;
 use geom::size::Size2D;
 use geom::scale_factor::ScaleFactor;
 use gfx::color;
-use gfx::display_list::{DisplayItemMetadata, DisplayList, OpaqueNode, StackingContext};
+use gfx::display_list::{ClippingRegion, DisplayItemMetadata, DisplayList, OpaqueNode};
+use gfx::display_list::{StackingContext};
 use gfx::font_cache_task::FontCacheTask;
 use gfx::paint_task::{mod, PaintInitMsg, PaintChan, PaintLayer};
 use layout_traits::{mod, LayoutControlMsg, LayoutTaskFactory};
@@ -633,7 +634,8 @@ impl LayoutTask {
                 LogicalPoint::zero(writing_mode).to_physical(writing_mode,
                                                              rw_data.screen_size);
 
-            flow::mut_base(&mut **layout_root).clip_rect = data.page_clip_rect;
+            flow::mut_base(&mut **layout_root).clip =
+                ClippingRegion::from_rect(&data.page_clip_rect);
 
             let rw_data = rw_data.deref_mut();
             match rw_data.parallel_traversal {

tests/ref/basic.list

@@ -221,4 +221,4 @@ fragment=top != ../html/acid2.html acid2_ref.html
 == clip_a.html clip_ref.html
 != inset_blackborder.html blackborder_ref.html
 != outset_blackborder.html blackborder_ref.html
-
+== border_radius_clip_a.html border_radius_clip_ref.html

tests/ref/border_radius_clip_a.html

@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+<head>
+<!-- Tests that `border-radius` causes the background to be clipped around the corners. -->
+<style>
+main {
+    display: block;
+    width: 16px;
+    height: 16px;
+    background: green;
+    overflow: hidden;
+}
+section {
+    display: block;
+    width: 256px;
+    height: 256px;
+    background: red;
+    border-radius: 50%;
+}
+</style>
+</head>
+<body>
+<main><section></section></main>
+</body>
+</html>
+

tests/ref/border_radius_clip_ref.html

@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+<!-- Tests that `border-radius` causes the background to be clipped around the corners. -->
+<style>
+main {
+    display: block;
+    width: 16px;
+    height: 16px;
+    background: green;
+}
+</style>
+</head>
+<body>
+<main></main>
+</body>
+</html>
+