CSS FPS 404 page doesn't work in Servo

[jdm]

http://keithclark.co.uk/labs/css-fps/nonexistent

I get a white page in Servo when loading this, whereas it's much more interesting in Firefox.

[jdm]

First problem:

<style>
html {
 background:#000;
 position:absolute;
}
</style>

yields a thin black line in servo, and a full black page in Firefox.

[jdm]

Looking at the layout inspector for the testcase in Firefox and Chrome, we appear to be matching the layout behaviour for Chrome. The reason why we look different is that we do not treat the background for the root element as special, so it is clipped to the dimensions of the element, rather than covering the entire page. We should either add a special display item for the background of the root element, or ignore the provided clipping rect when creating display items for the background of the root element: https://github.com/servo/servo/blob/6a2a5be7641dbda2391f4fff7c102f0176899562/components/layout/display_list_builder.rs#L555

[jdm]

@glennw Any idea why this diff does not affect the output? The debugger shows that the is_root path is being taken when there's an opaque black background color, and the size of the viewport is much larger than the size of the stacking relative border box. There's no change in the actual output from what I can see, however.

diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs
index ab3636f..4196280 100644
--- a/components/layout/display_list_builder.rs
+++ b/components/layout/display_list_builder.rs
@@ -14,7 +14,8 @@ use app_units::{AU_PER_PX, Au};
 use block::{BlockFlow, BlockStackingContextType};
 use canvas_traits::{CanvasData, CanvasMsg, FromLayoutMsg};
 use context::LayoutContext;
-use euclid::{Point2D, Rect, SideOffsets2D, Size2D, TypedSize2D};
+use euclid::{Point2D, Rect, SideOffsets2D, Size2D, TypedSize2D, TypedPoint2D};
+use euclid::num::Zero;
 use flex::FlexFlow;
 use flow::{BaseFlow, Flow, IS_ABSOLUTELY_POSITIONED};
 use flow_ref::FlowRef;
@@ -127,6 +128,7 @@ pub struct DisplayListBuildState<'a> {
     pub stacking_context_children: HashMap<StackingContextId, Vec<StackingContext>>,
     pub scroll_roots: HashMap<ScrollRootId, ScrollRoot>,
     pub processing_scroll_root_element: bool,
+    pub viewport_size: Size2D<Au>,

     /// The current stacking context id, used to keep track of state when building.
     /// recursively building and processing the display list.
@@ -142,7 +144,8 @@ pub struct DisplayListBuildState<'a> {
 }

 impl<'a> DisplayListBuildState<'a> {
-    pub fn new(layout_context: &'a LayoutContext) -> DisplayListBuildState<'a> {
+    pub fn new(layout_context: &'a LayoutContext,
+               viewport_size: Size2D<Au>) -> DisplayListBuildState<'a> {
         DisplayListBuildState {
             layout_context: layout_context,
             root_stacking_context: StackingContext::root(),
@@ -153,6 +156,7 @@ impl<'a> DisplayListBuildState<'a> {
             current_stacking_context_id: StackingContextId::root(),
             current_scroll_root_id: ScrollRootId::root(),
             iframe_sizes: Vec::new(),
+            viewport_size: viewport_size,
         }
     }

@@ -360,7 +364,8 @@ pub trait FragmentDisplayListBuilding {
                                                        style: &ServoComputedValues,
                                                        display_list_section: DisplayListSection,
                                                        absolute_bounds: &Rect<Au>,
-                                                       clip: &ClippingRegion);
+                                                       clip: &ClippingRegion,
+                                                       is_root: bool);

     /// Computes the background size for an image with the given background area according to the
     /// rules in CSS-BACKGROUNDS § 3.9.
@@ -453,7 +458,8 @@ pub trait FragmentDisplayListBuilding {
                           relative_containing_block_mode: WritingMode,
                           border_painting_mode: BorderPaintingMode,
                           display_list_section: DisplayListSection,
-                          clip: &ClippingRegion);
+                          clip: &ClippingRegion,
+                          is_root: bool);

     /// Adjusts the clipping region for all descendants of this fragment as appropriate.
     fn adjust_clipping_region_for_children(&self,
@@ -557,7 +563,8 @@ impl FragmentDisplayListBuilding for Fragment {
                                                        style: &ServoComputedValues,
                                                        display_list_section: DisplayListSection,
                                                        absolute_bounds: &Rect<Au>,
-                                                       clip: &ClippingRegion) {
+                                                       clip: &ClippingRegion,
+                                                       is_root: bool) {
         // 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();
@@ -598,7 +605,10 @@ impl FragmentDisplayListBuilding for Fragment {
             }
         }

-        let base = state.create_base_display_item(&bounds,
+        let viewport_bounds = Rect::new(TypedPoint2D::zero(), state.viewport_size.clone());
+        let bounds = if is_root { &viewport_bounds } else { &bounds };
+
+        let base = state.create_base_display_item(bounds,
                                                   &clip,
                                                   self.node,
                                                   style.get_cursor(Cursor::Default),
@@ -1344,7 +1354,8 @@ impl FragmentDisplayListBuilding for Fragment {
                           relative_containing_block_mode: WritingMode,
                           border_painting_mode: BorderPaintingMode,
                           display_list_section: DisplayListSection,
-                          clip: &ClippingRegion) {
+                          clip: &ClippingRegion,
+                          is_root: bool) {
         self.restyle_damage.remove(REPAINT);
         if self.style().get_inheritedbox().visibility != visibility::T::visible {
             return
@@ -1377,7 +1388,8 @@ impl FragmentDisplayListBuilding for Fragment {
                         &*node.style,
                         display_list_section,
                         &stacking_relative_border_box,
-                        clip);
+                        clip,
+                        is_root);
                     self.build_display_list_for_box_shadow_if_applicable(
                         state,
                         &*node.style,
@@ -1411,7 +1423,8 @@ impl FragmentDisplayListBuilding for Fragment {
                                                                      &*self.style,
                                                                      display_list_section,
                                                                      &stacking_relative_border_box,
-                                                                     clip);
+                                                                     clip,
+                                                                     is_root);
                 self.build_display_list_for_box_shadow_if_applicable(state,
                                                                      &*self.style,
                                                                      display_list_section,
@@ -2022,6 +2035,7 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
         state.processing_scroll_root_element = self.has_scrolling_overflow();

         // Add the box that starts the block context.
+        let is_root = self.is_root();
         self.fragment
             .build_display_list(state,
                                 &self.base.stacking_relative_position,
@@ -2033,7 +2047,8 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
                                     .relative_containing_block_mode,
                                 border_painting_mode,
                                 background_border_section,
-                                &self.base.clip);
+                                &self.base.clip,
+                                is_root);

         self.base.build_display_items_for_debugging_tint(state, self.fragment.node);

@@ -2098,7 +2113,8 @@ impl InlineFlowDisplayListBuilding for InlineFlow {
                                         .relative_containing_block_mode,
                                     BorderPaintingMode::Separate,
                                     DisplayListSection::Content,
-                                    &self.base.clip);
+                                    &self.base.clip,
+                                    false);
     }

     fn build_display_list_for_inline(&mut self, state: &mut DisplayListBuildState) {
@@ -2155,7 +2171,8 @@ impl ListItemFlowDisplayListBuilding for ListItemFlow {
                                           .relative_containing_block_mode,
                                       BorderPaintingMode::Separate,
                                       DisplayListSection::Content,
-                                      &self.block_flow.base.clip);
+                                      &self.block_flow.base.clip,
+                                      false);
         }

         // Draw the rest of the block.
diff --git a/components/layout/sequential.rs b/components/layout/sequential.rs
index ae493b4..1f1358d 100644
--- a/components/layout/sequential.rs
+++ b/components/layout/sequential.rs
@@ -8,6 +8,7 @@ use app_units::Au;
 use context::LayoutContext;
 use display_list_builder::DisplayListBuildState;
 use euclid::point::Point2D;
+use euclid::size::Size2D;
 use floats::SpeculatedFloatPlacement;
 use flow::{self, Flow, ImmutableFlowUtils, InorderFlowTraversal, MutableFlowUtils};
 use flow::{PostorderFlowTraversal, PreorderFlowTraversal};
@@ -70,9 +71,10 @@ pub fn traverse_flow_tree_preorder(root: &mut Flow,
 }

 pub fn build_display_list_for_subtree<'a>(flow_root: &mut Flow,
-                                          layout_context: &'a LayoutContext)
+                                          layout_context: &'a LayoutContext,
+                                          viewport_size: Size2D<Au>)
                                           -> DisplayListBuildState<'a> {
-    let mut state = DisplayListBuildState::new(layout_context);
+    let mut state = DisplayListBuildState::new(layout_context, viewport_size);
     flow_root.collect_stacking_contexts(&mut state);

     let mut build_display_list = BuildDisplayList { state: state };
diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs
index 58f81a7..8b69d59 100644
--- a/components/layout_thread/lib.rs
+++ b/components/layout_thread/lib.rs
@@ -855,7 +855,8 @@ impl LayoutThread {
                     (ReflowGoal::ForDisplay, _) | (ReflowGoal::ForScriptQuery, true) => {
                         let mut build_state =
                             sequential::build_display_list_for_subtree(layout_root,
-                                                                       layout_context);
+                                                                       layout_context,
+                                                                       self.viewport_size.clone());

                         debug!("Done building display list.");

[jdm]

Next place to look might be the WR code for the background color: https://github.com/servo/webrender/blob/b9a04064c995d3bd469627cf8a6411865772a71c/webrender/src/frame.rs#L429

[mrobinson]

@jdm I imagine your change isn't working because WebRender is still clipping items to stacking context boundaries. I change that here: https://github.com/servo/webrender/pull/1002

[nox]

The page isn't all white anymore but it still doesn't work, and I can still reproduce the issue from https://github.com/servo/servo/issues/15571#issuecomment-280132823.

[dralley]

https://github.com/servo/servo/issues/15571#issuecomment-280132823 Is still reproducable

The test snippet works under layout 2020 with this PR: https://github.com/servo/servo/pull/26121

Under that PR, the original link provided also looks much better although it is not completely fixed.