From 20b8edc625bff76921628a54c8d08b45cf1bf083 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Fri, 7 Feb 2020 14:35:05 +0100 Subject: [PATCH 1/2] Clean up BoxFragment display list builder in layout_2020 This makes things a big easier to read and will make it easier to add support for position:fixed. --- components/layout_2020/display_list/mod.rs | 95 ++++++++++++---------- 1 file changed, 52 insertions(+), 43 deletions(-) diff --git a/components/layout_2020/display_list/mod.rs b/components/layout_2020/display_list/mod.rs index fa193874892..ba66a24a841 100644 --- a/components/layout_2020/display_list/mod.rs +++ b/components/layout_2020/display_list/mod.rs @@ -244,51 +244,14 @@ impl<'a> BuilderForBoxFragment<'a> { } fn build(&mut self, builder: &mut DisplayListBuilder) { - let hit_info = hit_info(&self.fragment.style, self.fragment.tag, Cursor::Default); - if hit_info.is_some() { - let mut common = builder.common_properties(self.border_rect); - common.hit_info = hit_info; - if let Some(clip_id) = self.border_edge_clip(builder) { - common.clip_id = clip_id - } - builder.wr.push_hit_test(&common) - } - - self.build_background(builder); - self.build_border(builder); - builder.clipping_and_scrolling_scope(|builder| { - let overflow_x = self.fragment.style.get_box().overflow_x; - let overflow_y = self.fragment.style.get_box().overflow_y; - let original_scroll_and_clip_info = builder.current_space_and_clip; - if overflow_x != ComputedOverflow::Visible || overflow_y != ComputedOverflow::Visible { - // TODO(mrobinson): We should use the correct fragment type, once we generate - // fragments from ::before and ::after generated content selectors. - let id = combine_id_with_fragment_type( - self.fragment.tag.id() as usize, - FragmentType::FragmentBody, - ) as u64; - let external_id = wr::ExternalScrollId(id, builder.wr.pipeline_id); + self.build_hit_test(builder); + self.build_background(builder); + self.build_border(builder); - let sensitivity = if ComputedOverflow::Hidden == overflow_x && - ComputedOverflow::Hidden == overflow_y - { - wr::ScrollSensitivity::Script - } else { - wr::ScrollSensitivity::ScriptAndInputEvents - }; - - builder.current_space_and_clip = builder.wr.define_scroll_frame( - &original_scroll_and_clip_info, - Some(external_id), - self.fragment.scrollable_overflow().to_webrender(), - *self.padding_rect(), - vec![], // complex_clips - None, // image_mask - sensitivity, - wr::units::LayoutVector2D::zero(), - ); - } + // We want to build the scroll frame after the background and border, because + // they shouldn't scroll with the rest of the box content. + self.build_scroll_frame_if_necessary(builder); let content_rect = self .fragment @@ -301,6 +264,52 @@ impl<'a> BuilderForBoxFragment<'a> { }); } + fn build_scroll_frame_if_necessary(&self, builder: &mut DisplayListBuilder) { + let overflow_x = self.fragment.style.get_box().overflow_x; + let overflow_y = self.fragment.style.get_box().overflow_y; + let original_scroll_and_clip_info = builder.current_space_and_clip; + if overflow_x != ComputedOverflow::Visible || overflow_y != ComputedOverflow::Visible { + // TODO(mrobinson): We should use the correct fragment type, once we generate + // fragments from ::before and ::after generated content selectors. + let id = combine_id_with_fragment_type( + self.fragment.tag.id() as usize, + FragmentType::FragmentBody, + ) as u64; + let external_id = wr::ExternalScrollId(id, builder.wr.pipeline_id); + + let sensitivity = if ComputedOverflow::Hidden == overflow_x && + ComputedOverflow::Hidden == overflow_y + { + wr::ScrollSensitivity::Script + } else { + wr::ScrollSensitivity::ScriptAndInputEvents + }; + + builder.current_space_and_clip = builder.wr.define_scroll_frame( + &original_scroll_and_clip_info, + Some(external_id), + self.fragment.scrollable_overflow().to_webrender(), + *self.padding_rect(), + vec![], // complex_clips + None, // image_mask + sensitivity, + wr::units::LayoutVector2D::zero(), + ); + } + } + + fn build_hit_test(&self, builder: &mut DisplayListBuilder) { + let hit_info = hit_info(&self.fragment.style, self.fragment.tag, Cursor::Default); + if hit_info.is_some() { + let mut common = builder.common_properties(self.border_rect); + common.hit_info = hit_info; + if let Some(clip_id) = self.border_edge_clip(builder) { + common.clip_id = clip_id + } + builder.wr.push_hit_test(&common) + } + } + fn build_background(&mut self, builder: &mut DisplayListBuilder) { use style::values::computed::image::{Image, ImageLayer}; let b = self.fragment.style.get_background(); From 63aae0178f3508349add959657864569ff790e05 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Mon, 10 Feb 2020 09:19:01 +0100 Subject: [PATCH 2/2] Improve position:fixed support in layout_2020 This makes it so that position:fixed elements do not scroll with the rest of the contents, but does not tackle the rest of the issues with their positioning. --- components/layout_2020/display_list/mod.rs | 15 +++++++ tests/wpt/mozilla/meta/MANIFEST.json | 40 +++++++++++++++++++ .../tests/css/position_fixed_scroll.html | 17 ++++++++ .../tests/css/position_fixed_scroll_ref.html | 12 ++++++ 4 files changed, 84 insertions(+) create mode 100644 tests/wpt/mozilla/tests/css/position_fixed_scroll.html create mode 100644 tests/wpt/mozilla/tests/css/position_fixed_scroll_ref.html diff --git a/components/layout_2020/display_list/mod.rs b/components/layout_2020/display_list/mod.rs index ba66a24a841..2e1af55c9db 100644 --- a/components/layout_2020/display_list/mod.rs +++ b/components/layout_2020/display_list/mod.rs @@ -14,8 +14,10 @@ use mitochondria::OnceCell; use net_traits::image_cache::UsePlaceholder; use std::sync::Arc; use style::computed_values::overflow_x::T as ComputedOverflow; +use style::computed_values::position::T as ComputedPosition; use style::dom::OpaqueNode; use style::properties::ComputedValues; + use style::values::computed::{BorderStyle, Length, LengthPercentage}; use style::values::specified::ui::CursorKind; use webrender_api::{self as wr, units}; @@ -245,6 +247,7 @@ impl<'a> BuilderForBoxFragment<'a> { fn build(&mut self, builder: &mut DisplayListBuilder) { builder.clipping_and_scrolling_scope(|builder| { + self.adjust_spatial_id_for_positioning(builder); self.build_hit_test(builder); self.build_background(builder); self.build_border(builder); @@ -264,6 +267,18 @@ impl<'a> BuilderForBoxFragment<'a> { }); } + fn adjust_spatial_id_for_positioning(&self, builder: &mut DisplayListBuilder) { + if self.fragment.style.get_box().position != ComputedPosition::Fixed { + return; + } + + // TODO(mrobinson): Eventually this should use the spatial id of the reference + // frame that is the parent of this one once we have full support for stacking + // contexts and transforms. + builder.current_space_and_clip.spatial_id = + wr::SpatialId::root_reference_frame(builder.wr.pipeline_id); + } + fn build_scroll_frame_if_necessary(&self, builder: &mut DisplayListBuilder) { let overflow_x = self.fragment.style.get_box().overflow_x; let overflow_y = self.fragment.style.get_box().overflow_y; diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index 145f8189f02..c71d57cb722 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -1,5 +1,14 @@ { "items": { + "conformancechecker": { + "css/position_fixed_scroll.html": [] + }, + "crashtest": { + "css/position_fixed_scroll.html": [] + }, + "manual": { + "css/position_fixed_scroll.html": [] + }, "reftest": { "css/abs-overflow-stackingcontext.html": [ [ @@ -5057,6 +5066,18 @@ {} ] ], + "css/position_fixed_scroll.html": [ + [ + "css/position_fixed_scroll.html", + [ + [ + "/_mozilla/css/position_fixed_scroll_ref.html", + "==" + ] + ], + {} + ] + ], "css/position_fixed_simple_a.html": [ [ "css/position_fixed_simple_a.html", @@ -8977,6 +8998,10 @@ "css/position_fixed_overflow_b.html": [ [] ], + "css/position_fixed_scroll.html": [], + "css/position_fixed_scroll_ref.html": [ + [] + ], "css/position_fixed_simple_b.html": [ [] ], @@ -11253,6 +11278,7 @@ {} ] ], + "css/position_fixed_scroll.html": [], "css/stylesheet_media_queries.html": [ [ "css/stylesheet_media_queries.html", @@ -12596,6 +12622,12 @@ {} ] ] + }, + "visual": { + "css/position_fixed_scroll.html": [] + }, + "wdspec": { + "css/position_fixed_scroll.html": [] } }, "paths": { @@ -16959,6 +16991,14 @@ "a8947566153c187ba3191084f89d0163bc5b666e", "support" ], + "css/position_fixed_scroll.html": [ + "712265957f2c2ff0102eab09219b44bc5bf7f032", + "reftest" + ], + "css/position_fixed_scroll_ref.html": [ + "dc2e72a96ac2d82f038605812c79b9f78c1547dd", + "support" + ], "css/position_fixed_simple_a.html": [ "d01b955091107990d014e7f7be9c1181e5b06d66", "reftest" diff --git a/tests/wpt/mozilla/tests/css/position_fixed_scroll.html b/tests/wpt/mozilla/tests/css/position_fixed_scroll.html new file mode 100644 index 00000000000..712265957f2 --- /dev/null +++ b/tests/wpt/mozilla/tests/css/position_fixed_scroll.html @@ -0,0 +1,17 @@ + + + + + + +
+
+ + + diff --git a/tests/wpt/mozilla/tests/css/position_fixed_scroll_ref.html b/tests/wpt/mozilla/tests/css/position_fixed_scroll_ref.html new file mode 100644 index 00000000000..dc2e72a96ac --- /dev/null +++ b/tests/wpt/mozilla/tests/css/position_fixed_scroll_ref.html @@ -0,0 +1,12 @@ + + + + + +
+ +