From bf3798bbde8131a5336ad6987adb15d2dd0f5490 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Tue, 26 Mar 2024 16:00:50 +0100 Subject: [PATCH] layout: More conservatively replace Stylist's Device (#31857) Instead of replacing Stylist's device on every reflow, only replace it when the viewport changes. In addition, preserve the root font size from the previous reflow fixing an issue where `rem` units were not properly computed between reflows. This fixes a bug where fonts that are sized using `rem` units change size on reload. --- components/layout_thread/lib.rs | 67 +++++++++++------- components/layout_thread_2020/lib.rs | 70 +++++++++++++------ components/script/dom/htmlfontelement.rs | 6 +- ...omputedStyle-calc-mixed-units-002.html.ini | 3 - .../rem-root-font-size-restyle-1.html.ini | 2 - ...omputedStyle-calc-mixed-units-002.html.ini | 3 - .../rem-root-font-size-restyle-1.html.ini | 2 - .../mozilla/detached_layout.html.ini | 3 - 8 files changed, 95 insertions(+), 61 deletions(-) delete mode 100644 tests/wpt/meta-legacy-layout/css/css-values/rem-root-font-size-restyle-1.html.ini delete mode 100644 tests/wpt/meta/css/css-values/rem-root-font-size-restyle-1.html.ini delete mode 100644 tests/wpt/mozilla/meta-legacy-layout/mozilla/detached_layout.html.ini diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 34d00ff03d5..a9bfd75da72 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -379,7 +379,10 @@ impl LayoutThread { root_flow: RefCell::new(None), // Epoch starts at 1 because of the initial display list for epoch 0 that we send to WR epoch: Cell::new(Epoch(1)), - viewport_size: Size2D::new(Au(0), Au(0)), + viewport_size: Size2D::new( + Au::from_f32_px(window_size.initial_viewport.width), + Au::from_f32_px(window_size.initial_viewport.height), + ), webrender_api, stylist: Stylist::new(device, QuirksMode::NoQuirks), rw_data: Arc::new(Mutex::new(LayoutThreadData { @@ -936,16 +939,6 @@ impl LayoutThread { ); trace!("{:?}", ShowSubtree(root_element.as_node())); - let initial_viewport = data.window_size.initial_viewport; - let device_pixel_ratio = data.window_size.device_pixel_ratio; - let old_viewport_size = self.viewport_size; - let current_screen_size = Size2D::new( - Au::from_f32_px(initial_viewport.width), - Au::from_f32_px(initial_viewport.height), - ); - - let origin = data.origin.clone(); - // Calculate the actual viewport as per DEVICE-ADAPT § 6 // If the entire flow tree is invalid, then it will be reflowed anyhow. let document_shared_lock = document.style_shared_lock(); @@ -959,19 +952,7 @@ impl LayoutThread { }; let had_used_viewport_units = self.stylist.device().used_viewport_units(); - let device = Device::new( - MediaType::screen(), - self.stylist.quirks_mode(), - initial_viewport, - device_pixel_ratio, - ); - let sheet_origins_affected_by_device_change = self.stylist.set_device(device, &guards); - - self.stylist - .force_stylesheet_origins_dirty(sheet_origins_affected_by_device_change); - self.viewport_size = current_screen_size; - - let viewport_size_changed = self.viewport_size != old_viewport_size; + let viewport_size_changed = self.handle_viewport_change(data.window_size, &guards); if viewport_size_changed && had_used_viewport_units { if let Some(mut data) = root_element.mutate_data() { data.hint.insert(RestyleHint::recascade_subtree()); @@ -1063,7 +1044,7 @@ impl LayoutThread { let mut layout_context = self.build_layout_context( guards.clone(), &map, - origin, + data.origin.clone(), data.animation_timeline_value, &data.animations, data.stylesheets_changed, @@ -1518,6 +1499,42 @@ impl LayoutThread { }, }) } + + /// Update layout given a new viewport. Returns true if the viewport changed or false if it didn't. + fn handle_viewport_change( + &mut self, + window_size_data: WindowSizeData, + guards: &StylesheetGuards, + ) -> bool { + // If the viewport size and device pixel ratio has not changed, do not make any changes. + let au_viewport_size = Size2D::new( + Au::from_f32_px(window_size_data.initial_viewport.width), + Au::from_f32_px(window_size_data.initial_viewport.height), + ); + + if self.stylist.device().au_viewport_size() == au_viewport_size && + self.stylist.device().device_pixel_ratio() == window_size_data.device_pixel_ratio + { + return false; + } + + let device = Device::new( + MediaType::screen(), + self.stylist.quirks_mode(), + window_size_data.initial_viewport, + window_size_data.device_pixel_ratio, + ); + + // Preserve any previously computed root font size. + device.set_root_font_size(self.stylist.device().root_font_size()); + + let sheet_origins_affected_by_device_change = self.stylist.set_device(device, guards); + self.stylist + .force_stylesheet_origins_dirty(sheet_origins_affected_by_device_change); + + self.viewport_size = au_viewport_size; + true + } } impl ProfilerMetadataFactory for LayoutThread { diff --git a/components/layout_thread_2020/lib.rs b/components/layout_thread_2020/lib.rs index 78e6de51fec..5d17cb9fbb7 100644 --- a/components/layout_thread_2020/lib.rs +++ b/components/layout_thread_2020/lib.rs @@ -72,6 +72,7 @@ use style::dom::{TElement, TNode}; use style::driver; use style::error_reporting::RustLogReporter; use style::global_style_data::{GLOBAL_STYLE_DATA, STYLE_THREAD_POOL}; +use style::invalidation::element::restyle_hints::RestyleHint; use style::media_queries::{Device, MediaList, MediaType}; use style::properties::PropertyId; use style::selector_parser::SnapshotMap; @@ -360,7 +361,10 @@ impl LayoutThread { fragment_tree: Default::default(), // Epoch starts at 1 because of the initial display list for epoch 0 that we send to WR epoch: Cell::new(Epoch(1)), - viewport_size: Size2D::new(Au(0), Au(0)), + viewport_size: Size2D::new( + Au::from_f32_px(window_size.initial_viewport.width), + Au::from_f32_px(window_size.initial_viewport.height), + ), webrender_api: webrender_api_sender, stylist: Stylist::new(device, QuirksMode::NoQuirks), rw_data: Arc::new(Mutex::new(LayoutThreadData { @@ -628,15 +632,6 @@ impl LayoutThread { Some(x) => x, }; - let initial_viewport = data.window_size.initial_viewport; - let device_pixel_ratio = data.window_size.device_pixel_ratio; - let current_screen_size = Size2D::new( - Au::from_f32_px(initial_viewport.width), - Au::from_f32_px(initial_viewport.height), - ); - - let origin = data.origin.clone(); - // Calculate the actual viewport as per DEVICE-ADAPT § 6 // If the entire flow tree is invalid, then it will be reflowed anyhow. let document_shared_lock = document.style_shared_lock(); @@ -649,17 +644,14 @@ impl LayoutThread { ua_or_user: &ua_or_user_guard, }; - let device = Device::new( - MediaType::screen(), - self.stylist.quirks_mode(), - initial_viewport, - device_pixel_ratio, - ); - let sheet_origins_affected_by_device_change = self.stylist.set_device(device, &guards); + let had_used_viewport_units = self.stylist.device().used_viewport_units(); + let viewport_size_changed = self.handle_viewport_change(data.window_size, &guards); + if viewport_size_changed && had_used_viewport_units { + if let Some(mut data) = root_element.mutate_data() { + data.hint.insert(RestyleHint::recascade_subtree()); + } + } - self.stylist - .force_stylesheet_origins_dirty(sheet_origins_affected_by_device_change); - self.viewport_size = current_screen_size; if self.first_reflow.get() { for stylesheet in &ua_stylesheets.user_or_user_agent_stylesheets { self.stylist @@ -739,7 +731,7 @@ impl LayoutThread { let mut layout_context = self.build_layout_context( guards.clone(), &map, - origin, + data.origin.clone(), data.animation_timeline_value, &data.animations, data.stylesheets_changed, @@ -1158,6 +1150,42 @@ impl LayoutThread { } } } + + /// Update layout given a new viewport. Returns true if the viewport changed or false if it didn't. + fn handle_viewport_change( + &mut self, + window_size_data: WindowSizeData, + guards: &StylesheetGuards, + ) -> bool { + // If the viewport size and device pixel ratio has not changed, do not make any changes. + let au_viewport_size = Size2D::new( + Au::from_f32_px(window_size_data.initial_viewport.width), + Au::from_f32_px(window_size_data.initial_viewport.height), + ); + + if self.stylist.device().au_viewport_size() == au_viewport_size && + self.stylist.device().device_pixel_ratio() == window_size_data.device_pixel_ratio + { + return false; + } + + let device = Device::new( + MediaType::screen(), + self.stylist.quirks_mode(), + window_size_data.initial_viewport, + window_size_data.device_pixel_ratio, + ); + + // Preserve any previously computed root font size. + device.set_root_font_size(self.stylist.device().root_font_size()); + + let sheet_origins_affected_by_device_change = self.stylist.set_device(device, guards); + self.stylist + .force_stylesheet_origins_dirty(sheet_origins_affected_by_device_change); + + self.viewport_size = au_viewport_size; + true + } } impl ProfilerMetadataFactory for LayoutThread { diff --git a/components/script/dom/htmlfontelement.rs b/components/script/dom/htmlfontelement.rs index 641dd8a92f3..1f8014f04dd 100644 --- a/components/script/dom/htmlfontelement.rs +++ b/components/script/dom/htmlfontelement.rs @@ -81,11 +81,13 @@ impl VirtualMethods for HTMLFontElement { } fn attribute_affects_presentational_hints(&self, attr: &Attr) -> bool { - if attr.local_name() == &local_name!("color") { + if attr.local_name() == &local_name!("color") || + attr.local_name() == &local_name!("size") || + attr.local_name() == &local_name!("face") + { return true; } - // FIXME: Should also return true for `size` and `face` changes! self.super_type() .unwrap() .attribute_affects_presentational_hints(attr) diff --git a/tests/wpt/meta-legacy-layout/css/css-values/getComputedStyle-calc-mixed-units-002.html.ini b/tests/wpt/meta-legacy-layout/css/css-values/getComputedStyle-calc-mixed-units-002.html.ini index 3b65d5237ab..5be031b9d28 100644 --- a/tests/wpt/meta-legacy-layout/css/css-values/getComputedStyle-calc-mixed-units-002.html.ini +++ b/tests/wpt/meta-legacy-layout/css/css-values/getComputedStyle-calc-mixed-units-002.html.ini @@ -1,6 +1,3 @@ [getComputedStyle-calc-mixed-units-002.html] - [testing width: calc(5% + 4rem)] - expected: FAIL - [testing width: calc(8lh + 7px)] expected: FAIL diff --git a/tests/wpt/meta-legacy-layout/css/css-values/rem-root-font-size-restyle-1.html.ini b/tests/wpt/meta-legacy-layout/css/css-values/rem-root-font-size-restyle-1.html.ini deleted file mode 100644 index 69d581abf60..00000000000 --- a/tests/wpt/meta-legacy-layout/css/css-values/rem-root-font-size-restyle-1.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[rem-root-font-size-restyle-1.html] - expected: FAIL diff --git a/tests/wpt/meta/css/css-values/getComputedStyle-calc-mixed-units-002.html.ini b/tests/wpt/meta/css/css-values/getComputedStyle-calc-mixed-units-002.html.ini index 3b65d5237ab..5be031b9d28 100644 --- a/tests/wpt/meta/css/css-values/getComputedStyle-calc-mixed-units-002.html.ini +++ b/tests/wpt/meta/css/css-values/getComputedStyle-calc-mixed-units-002.html.ini @@ -1,6 +1,3 @@ [getComputedStyle-calc-mixed-units-002.html] - [testing width: calc(5% + 4rem)] - expected: FAIL - [testing width: calc(8lh + 7px)] expected: FAIL diff --git a/tests/wpt/meta/css/css-values/rem-root-font-size-restyle-1.html.ini b/tests/wpt/meta/css/css-values/rem-root-font-size-restyle-1.html.ini deleted file mode 100644 index 69d581abf60..00000000000 --- a/tests/wpt/meta/css/css-values/rem-root-font-size-restyle-1.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[rem-root-font-size-restyle-1.html] - expected: FAIL diff --git a/tests/wpt/mozilla/meta-legacy-layout/mozilla/detached_layout.html.ini b/tests/wpt/mozilla/meta-legacy-layout/mozilla/detached_layout.html.ini deleted file mode 100644 index bc10807d4c2..00000000000 --- a/tests/wpt/mozilla/meta-legacy-layout/mozilla/detached_layout.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[detached_layout.html] - [Detached layout doesn't panic] - expected: FAIL