Auto merge of #25475 - servo:background-image, r=nox

Refactor display list creation, preparing for background-image

`backgound-image` itself will require more plumbing to get an image cache in `layout_thread_2020`. This can land in the mean time.

components/layout/display_list/builder.rs

@@ -800,11 +800,9 @@ impl Fragment {
                         );
                     }
                 },
-                Image::Rect(_) => {
+                Image::Rect(ref rect) => {
-                    // TODO: Implement `-moz-image-rect`
+                    // This is a (boxed) empty enum on non-Gecko
-                },
+                    match **rect {}
-                Image::Element(_) => {
-                    // TODO: Implement `-moz-element`
                 },
             }
         }

components/layout_2020/display_list.rs

@@ -21,6 +21,11 @@ type HitInfo = Option<ItemTag>;
 pub struct DisplayListBuilder {
     current_space_and_clip: wr::SpaceAndClipInfo,
     pub wr: wr::DisplayListBuilder,
+
+    /// 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,
+    /// image, non-white canvas or SVG). Used by metrics.
     pub is_contentful: bool,
 }
 
@@ -33,21 +38,13 @@ impl DisplayListBuilder {
         }
     }
 
-    fn common_properties(
+    fn common_properties(&self, clip_rect: units::LayoutRect) -> wr::CommonItemProperties {
-        &self,
-        clip_rect: units::LayoutRect,
-        hit_info: HitInfo,
-    ) -> wr::CommonItemProperties {
-        wr::CommonItemProperties {
-            clip_rect,
-            clip_id: self.current_space_and_clip.clip_id,
-            spatial_id: self.current_space_and_clip.spatial_id,
-            hit_info,
         // TODO(gw): Make use of the WR backface visibility functionality.
-            flags: wr::PrimitiveFlags::default(),
+        wr::CommonItemProperties::new(clip_rect, self.current_space_and_clip)
-        }
     }
 
+    // FIXME: use this for the `overflow` property or anything else that clips an entire subtree.
+    #[allow(unused)]
     fn clipping_and_scrolling_scope<R>(&mut self, f: impl FnOnce(&mut Self) -> R) -> R {
         let previous = self.current_space_and_clip;
         let result = f(self);
@@ -56,32 +53,27 @@ impl DisplayListBuilder {
     }
 }
 
-/// 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,
-/// image, non-white canvas or SVG). Used by metrics.
-pub struct IsContentful(pub bool);
-
 impl Fragment {
     pub(crate) fn build_display_list(
         &self,
         builder: &mut DisplayListBuilder,
-        is_contentful: &mut IsContentful,
         containing_block: &Rect<Length>,
     ) {
         match self {
-            Fragment::Box(b) => b.build_display_list(builder, is_contentful, containing_block),
+            Fragment::Box(b) => {
+                BuilderForBoxFragment::new(b, containing_block).build(builder, containing_block)
+            },
             Fragment::Anonymous(a) => {
                 let rect = a
                     .rect
                     .to_physical(a.mode, containing_block)
                     .translate(&containing_block.top_left);
                 for child in &a.children {
-                    child.build_display_list(builder, is_contentful, &rect)
+                    child.build_display_list(builder, &rect)
                 }
             },
             Fragment::Text(t) => {
-                is_contentful.0 = true;
+                builder.is_contentful = true;
                 let rect = t
                     .rect
                     .to_physical(t.parent_style.writing_mode, containing_block)
@@ -92,8 +84,8 @@ impl Fragment {
                 if glyphs.is_empty() {
                     return;
                 }
-                let hit_info = hit_info(&t.parent_style, t.tag, Cursor::Text);
+                let mut common = builder.common_properties(rect.clone().into());
-                let common = builder.common_properties(rect.clone().into(), hit_info);
+                common.hit_info = hit_info(&t.parent_style, t.tag, Cursor::Text);
                 let color = t.parent_style.clone_color();
                 builder
                     .wr
@@ -101,13 +93,12 @@ impl Fragment {
             },
             Fragment::Image(i) => {
                 use style::computed_values::image_rendering::T as ImageRendering;
-                is_contentful.0 = true;
+                builder.is_contentful = true;
                 let rect = i
                     .rect
                     .to_physical(i.style.writing_mode, containing_block)
                     .translate(&containing_block.top_left);
-                let hit_info = None;
+                let common = builder.common_properties(rect.clone().into());
-                let common = builder.common_properties(rect.clone().into(), hit_info);
                 builder.wr.push_image(
                     &common,
                     rect.into(),
@@ -125,33 +116,25 @@ impl Fragment {
     }
 }
 
-impl BoxFragment {
+struct BuilderForBoxFragment<'a> {
-    fn build_display_list(
+    fragment: &'a BoxFragment,
-        &self,
+    border_rect: units::LayoutRect,
-        builder: &mut DisplayListBuilder,
+    border_radius: wr::BorderRadius,
-        is_contentful: &mut IsContentful,
+
-        containing_block: &Rect<Length>,
+    // Outer `Option` is `None`: not initialized yet
-    ) {
+    // Inner `Option` is `None`: no border radius, no need to clip
-        let border_rect = self
+    border_edge_clip_id: Option<Option<wr::ClipId>>,
+}
+
+impl<'a> BuilderForBoxFragment<'a> {
+    fn new(fragment: &'a BoxFragment, containing_block: &Rect<Length>) -> Self {
+        let border_rect: units::LayoutRect = fragment
             .border_rect()
-            .to_physical(self.style.writing_mode, containing_block)
+            .to_physical(fragment.style.writing_mode, containing_block)
             .translate(&containing_block.top_left)
             .into();
-        let hit_info = hit_info(&self.style, self.tag, Cursor::Default);
-        let border_radius = self.border_radius(&border_rect);
 
-        self.background_display_items(builder, hit_info, border_rect, &border_radius);
+        let border_radius = {
-        self.border_display_items(builder, hit_info, border_rect, border_radius);
-        let content_rect = self
-            .content_rect
-            .to_physical(self.style.writing_mode, containing_block)
-            .translate(&containing_block.top_left);
-        for child in &self.children {
-            child.build_display_list(builder, is_contentful, &content_rect)
-        }
-    }
-
-    fn border_radius(&self, border_rect: &units::LayoutRect) -> wr::BorderRadius {
             let resolve = |radius: &LengthPercentage, box_size: f32| {
                 radius.percentage_relative_to(Length::new(box_size)).px()
             };
@@ -161,53 +144,86 @@ impl BoxFragment {
                     resolve(&corner.0.height.0, border_rect.size.height),
                 )
             };
-        let b = self.style.get_border();
+            let b = fragment.style.get_border();
             wr::BorderRadius {
                 top_left: corner(&b.border_top_left_radius),
                 top_right: corner(&b.border_top_right_radius),
                 bottom_right: corner(&b.border_bottom_right_radius),
                 bottom_left: corner(&b.border_bottom_left_radius),
             }
+        };
+
+        Self {
+            fragment,
+            border_rect,
+            border_radius,
+            border_edge_clip_id: None,
+        }
     }
 
-    fn background_display_items(
+    fn with_border_edge_clip(
-        &self,
+        &mut self,
         builder: &mut DisplayListBuilder,
-        hit_info: HitInfo,
+        common: &mut wr::CommonItemProperties,
-        border_rect: units::LayoutRect,
-        border_radius: &wr::BorderRadius,
     ) {
-        let background_color = self
+        let border_radius = &self.border_radius;
-            .style
+        let border_rect = &self.border_rect;
-            .resolve_color(self.style.clone_background_color());
+        let initialized = self.border_edge_clip_id.get_or_insert_with(|| {
-        if background_color.alpha > 0 || hit_info.is_some() {
+            if border_radius.is_zero() {
-            builder.clipping_and_scrolling_scope(|builder| {
+                None
-                if !border_radius.is_zero() {
+            } else {
-                    builder.current_space_and_clip.clip_id = builder.wr.define_clip(
+                Some(builder.wr.define_clip(
                     &builder.current_space_and_clip,
-                        border_rect,
+                    *border_rect,
                     Some(wr::ComplexClipRegion {
-                            rect: border_rect,
+                        rect: *border_rect,
                         radii: *border_radius,
                         mode: wr::ClipMode::Clip,
                     }),
                     None,
-                    );
+                ))
             }
-                let common = builder.common_properties(border_rect, hit_info);
-                builder.wr.push_rect(&common, rgba(background_color))
         });
+        if let Some(clip_id) = *initialized {
+            common.clip_id = clip_id
         }
     }
 
-    fn border_display_items(
+    fn build(&mut self, builder: &mut DisplayListBuilder, containing_block: &Rect<Length>) {
-        &self,
+        let hit_info = hit_info(&self.fragment.style, self.fragment.tag, Cursor::Default);
-        builder: &mut DisplayListBuilder,
+        if hit_info.is_some() {
-        hit_info: HitInfo,
+            let mut common = builder.common_properties(self.border_rect);
-        border_rect: units::LayoutRect,
+            common.hit_info = hit_info;
-        radius: wr::BorderRadius,
+            self.with_border_edge_clip(builder, &mut common);
-    ) {
+            builder.wr.push_hit_test(&common)
-        let b = self.style.get_border();
+        }
+
+        self.background_display_items(builder);
+        self.border_display_items(builder);
+        let content_rect = self
+            .fragment
+            .content_rect
+            .to_physical(self.fragment.style.writing_mode, containing_block)
+            .translate(&containing_block.top_left);
+        for child in &self.fragment.children {
+            child.build_display_list(builder, &content_rect)
+        }
+    }
+
+    fn background_display_items(&mut self, builder: &mut DisplayListBuilder) {
+        let background_color = self
+            .fragment
+            .style
+            .resolve_color(self.fragment.style.clone_background_color());
+        if background_color.alpha > 0 {
+            let mut common = builder.common_properties(self.border_rect);
+            self.with_border_edge_clip(builder, &mut common);
+            builder.wr.push_rect(&common, rgba(background_color))
+        }
+    }
+
+    fn border_display_items(&mut self, builder: &mut DisplayListBuilder) {
+        let b = self.fragment.style.get_border();
         let widths = SideOffsets2D::new(
             b.border_top_width.px(),
             b.border_right_width.px(),
@@ -218,7 +234,7 @@ impl BoxFragment {
             return;
         }
         let side = |style, color| wr::BorderSide {
-            color: rgba(self.style.resolve_color(color)),
+            color: rgba(self.fragment.style.resolve_color(color)),
             style: match style {
                 BorderStyle::None => wr::BorderStyle::None,
                 BorderStyle::Solid => wr::BorderStyle::Solid,
@@ -232,18 +248,18 @@ impl BoxFragment {
                 BorderStyle::Outset => wr::BorderStyle::Outset,
             },
         };
-        let common = builder.common_properties(border_rect, hit_info);
+        let common = builder.common_properties(self.border_rect);
         let details = wr::BorderDetails::Normal(wr::NormalBorder {
             top: side(b.border_top_style, b.border_top_color),
             right: side(b.border_right_style, b.border_right_color),
             bottom: side(b.border_bottom_style, b.border_bottom_color),
             left: side(b.border_left_style, b.border_left_color),
-            radius,
+            radius: self.border_radius,
             do_aa: true,
         });
         builder
             .wr
-            .push_border(&common, border_rect, widths, details)
+            .push_border(&common, self.border_rect, widths, details)
     }
 }
 

components/layout_2020/flow/root.rs

@@ -3,7 +3,6 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 use crate::context::LayoutContext;
-use crate::display_list::IsContentful;
 use crate::dom_traversal::{Contents, NodeExt};
 use crate::flow::construct::ContainsFloats;
 use crate::flow::float::FloatBox;
@@ -140,7 +139,7 @@ impl FragmentTreeRoot {
         &self,
         builder: &mut crate::display_list::DisplayListBuilder,
         viewport_size: webrender_api::units::LayoutSize,
-    ) -> IsContentful {
+    ) {
         let containing_block = geom::physical::Rect {
             top_left: geom::physical::Vec2 {
                 x: Length::zero(),
@@ -151,10 +150,8 @@ impl FragmentTreeRoot {
                 y: Length::new(viewport_size.height),
             },
         };
-        let mut is_contentful = IsContentful(false);
         for fragment in &self.0 {
-            fragment.build_display_list(builder, &mut is_contentful, &containing_block)
+            fragment.build_display_list(builder, &containing_block)
-        }
+        }
-        is_contentful
     }
 }

components/layout_thread_2020/lib.rs

@@ -1290,7 +1290,7 @@ impl LayoutThread {
             self.viewport_size.height.to_f32_px(),
         ));
         let mut display_list = DisplayListBuilder::new(self.id.to_webrender(), viewport_size);
-        let is_contentful = fragment_tree.build_display_list(&mut display_list, viewport_size);
+        fragment_tree.build_display_list(&mut display_list, viewport_size);
 
         debug!("Layout done!");
 
@@ -1302,7 +1302,7 @@ impl LayoutThread {
         // sending the display list to WebRender in order to set time related
         // Progressive Web Metrics.
         self.paint_time_metrics
-            .maybe_observe_paint_time(self, epoch, is_contentful.0);
+            .maybe_observe_paint_time(self, epoch, display_list.is_contentful);
 
         self.webrender_api.send_display_list(
             self.webrender_document,

components/style/traversal.rs

@@ -679,7 +679,7 @@ where
     element.finish_restyle(context, data, new_styles, important_rules_changed)
 }
 
-#[cfg(feature = "servo")]
+#[cfg(feature = "servo-layout-2013")]
 fn notify_paint_worklet<E>(context: &StyleContext<E>, data: &ElementData)
 where
     E: TElement,
@@ -719,7 +719,7 @@ where
     }
 }
 
-#[cfg(feature = "gecko")]
+#[cfg(not(feature = "servo-layout-2013"))]
 fn notify_paint_worklet<E>(_context: &StyleContext<E>, _data: &ElementData)
 where
     E: TElement,

components/style/values/computed/image.rs

@@ -9,10 +9,11 @@
 
 use crate::values::computed::position::Position;
 use crate::values::computed::url::ComputedImageUrl;
+#[cfg(feature = "gecko")]
+use crate::values::computed::NumberOrPercentage;
 use crate::values::computed::{Angle, Color, Context};
 use crate::values::computed::{
-    LengthPercentage, NonNegativeLength, NonNegativeLengthPercentage, NumberOrPercentage,
+    LengthPercentage, NonNegativeLength, NonNegativeLengthPercentage, ToComputedValue,
-    ToComputedValue,
 };
 use crate::values::generics::image::{self as generic, GradientCompatMode};
 use crate::values::specified::image::LineDirection as SpecifiedLineDirection;
@@ -63,8 +64,13 @@ pub type GradientItem = generic::GenericGradientItem<Color, LengthPercentage>;
 pub type ColorStop = generic::ColorStop<Color, LengthPercentage>;
 
 /// Computed values for `-moz-image-rect(...)`.
+#[cfg(feature = "gecko")]
 pub type MozImageRect = generic::MozImageRect<NumberOrPercentage, ComputedImageUrl>;
 
+/// Empty enum on non-gecko
+#[cfg(not(feature = "gecko"))]
+pub type MozImageRect = crate::values::specified::image::MozImageRect;
+
 impl generic::LineDirection for LineDirection {
     fn points_downwards(&self, compat_mode: GradientCompatMode) -> bool {
         match *self {

components/style/values/generics/image.rs

@@ -53,17 +53,24 @@ impl<I> ImageLayer<I> {
 pub enum GenericImage<Gradient, MozImageRect, ImageUrl> {
     /// A `<url()>` image.
     Url(ImageUrl),
+
     /// A `<gradient>` image.  Gradients are rather large, and not nearly as
     /// common as urls, so we box them here to keep the size of this enum sane.
     Gradient(Box<Gradient>),
+
     /// A `-moz-image-rect` image.  Also fairly large and rare.
+    // not cfg’ed out on non-Gecko to avoid `error[E0392]: parameter `MozImageRect` is never used`
+    // Instead we make MozImageRect an empty enum
     Rect(Box<MozImageRect>),
+
     /// A `-moz-element(# <element-id>)`
+    #[cfg(feature = "gecko")]
     #[css(function = "-moz-element")]
     Element(Atom),
+
     /// A paint worklet image.
     /// <https://drafts.css-houdini.org/css-paint-api/>
-    #[cfg(feature = "servo")]
+    #[cfg(feature = "servo-layout-2013")]
     PaintWorklet(PaintWorklet),
 }
 
@@ -323,8 +330,9 @@ where
             Image::Url(ref url) => url.to_css(dest),
             Image::Gradient(ref gradient) => gradient.to_css(dest),
             Image::Rect(ref rect) => rect.to_css(dest),
-            #[cfg(feature = "servo")]
+            #[cfg(feature = "servo-layout-2013")]
             Image::PaintWorklet(ref paint_worklet) => paint_worklet.to_css(dest),
+            #[cfg(feature = "gecko")]
             Image::Element(ref selector) => {
                 dest.write_str("-moz-element(#")?;
                 serialize_atom_identifier(selector, dest)?;

components/style/values/specified/image.rs

@@ -9,7 +9,6 @@
 
 use crate::custom_properties::SpecifiedValue;
 use crate::parser::{Parse, ParserContext};
-use crate::stylesheets::CorsMode;
 use crate::values::generics::image::PaintWorklet;
 use crate::values::generics::image::{
     self as generic, Circle, Ellipse, GradientCompatMode, ShapeExtent,
@@ -119,8 +118,24 @@ pub type ColorStop = generic::ColorStop<Color, LengthPercentage>;
 
 /// Specified values for `moz-image-rect`
 /// -moz-image-rect(<uri>, top, right, bottom, left);
+#[cfg(feature = "gecko")]
 pub type MozImageRect = generic::MozImageRect<NumberOrPercentage, SpecifiedImageUrl>;
 
+#[cfg(not(feature = "gecko"))]
+#[derive(
+    Clone,
+    Debug,
+    MallocSizeOf,
+    PartialEq,
+    SpecifiedValueInfo,
+    ToComputedValue,
+    ToCss,
+    ToResolvedValue,
+    ToShmem,
+)]
+/// Empty enum on non-Gecko
+pub enum MozImageRect {}
+
 impl Parse for Image {
     fn parse<'i, 't>(
         context: &ParserContext,
@@ -132,17 +147,22 @@ impl Parse for Image {
         if let Ok(gradient) = input.try(|i| Gradient::parse(context, i)) {
             return Ok(generic::Image::Gradient(Box::new(gradient)));
         }
-        #[cfg(feature = "servo")]
+        #[cfg(feature = "servo-layout-2013")]
         {
             if let Ok(paint_worklet) = input.try(|i| PaintWorklet::parse(context, i)) {
                 return Ok(generic::Image::PaintWorklet(paint_worklet));
             }
         }
+        #[cfg(feature = "gecko")]
+        {
             if let Ok(image_rect) = input.try(|input| MozImageRect::parse(context, input)) {
                 return Ok(generic::Image::Rect(Box::new(image_rect)));
             }
             Ok(generic::Image::Element(Image::parse_element(input)?))
         }
+        #[cfg(not(feature = "gecko"))]
+        Err(input.new_error_for_next_token())
+    }
 }
 
 impl Image {
@@ -155,6 +175,7 @@ impl Image {
     }
 
     /// Parses a `-moz-element(# <element-id>)`.
+    #[cfg(feature = "gecko")]
     fn parse_element<'i, 't>(input: &mut Parser<'i, 't>) -> Result<Atom, ParseError<'i>> {
         input.try(|i| i.expect_function_matching("-moz-element"))?;
         let location = input.current_source_location();
@@ -856,6 +877,15 @@ impl Parse for PaintWorklet {
 }
 
 impl Parse for MozImageRect {
+    #[cfg(not(feature = "gecko"))]
+    fn parse<'i, 't>(
+        _context: &ParserContext,
+        input: &mut Parser<'i, 't>,
+    ) -> Result<Self, ParseError<'i>> {
+        Err(input.new_error_for_next_token())
+    }
+
+    #[cfg(feature = "gecko")]
     fn parse<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
@@ -866,7 +896,7 @@ impl Parse for MozImageRect {
             let url = SpecifiedImageUrl::parse_from_string(
                 string.as_ref().to_owned(),
                 context,
-                CorsMode::None,
+                crate::stylesheets::CorsMode::None,
             );
             i.expect_comma()?;
             let top = NumberOrPercentage::parse_non_negative(context, i)?;