layout: Regardless of restyle damage, always reflow when viewport changes (#37099)

This fixes an issue where a viewport change did not trigger a reflow,
when the restyle damage was was otherwise REPAINT. Viewport changes
mean changes to the initial containing block, which is one of the main
inputs to layout. This should trigger a reflow always.

Testing: Unfortunately, our testing infrastructure is not good enough
yet
to test changes to layout when resizing the `WebView`, so it is quite
difficult to write tests for this change.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
This commit is contained in:
Martin Robinson 2025-05-23 15:42:45 +02:00 committed by GitHub
parent 8788248fec
commit 9c0c1cf30f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 21 additions and 34 deletions

View file

@ -5,6 +5,8 @@
use app_units::Au; use app_units::Au;
use atomic_refcell::AtomicRef; use atomic_refcell::AtomicRef;
use compositing_traits::display_list::AxesScrollSensitivity; use compositing_traits::display_list::AxesScrollSensitivity;
use euclid::Rect;
use euclid::default::Size2D as UntypedSize2D;
use malloc_size_of_derive::MallocSizeOf; use malloc_size_of_derive::MallocSizeOf;
use script::layout_dom::ServoLayoutNode; use script::layout_dom::ServoLayoutNode;
use script_layout_interface::wrapper_traits::{ use script_layout_interface::wrapper_traits::{
@ -27,7 +29,7 @@ use crate::flow::inline::InlineItem;
use crate::flow::{BlockContainer, BlockFormattingContext, BlockLevelBox}; use crate::flow::{BlockContainer, BlockFormattingContext, BlockLevelBox};
use crate::formatting_contexts::IndependentFormattingContext; use crate::formatting_contexts::IndependentFormattingContext;
use crate::fragment_tree::FragmentTree; use crate::fragment_tree::FragmentTree;
use crate::geom::{LogicalVec2, PhysicalPoint, PhysicalRect, PhysicalSize}; use crate::geom::{LogicalVec2, PhysicalRect, PhysicalSize};
use crate::positioned::{AbsolutelyPositionedBox, PositioningContext}; use crate::positioned::{AbsolutelyPositionedBox, PositioningContext};
use crate::replaced::ReplacedContents; use crate::replaced::ReplacedContents;
use crate::style_ext::{Display, DisplayGeneratingBox, DisplayInside}; use crate::style_ext::{Display, DisplayGeneratingBox, DisplayInside};
@ -348,7 +350,7 @@ impl BoxTree {
pub fn layout( pub fn layout(
&self, &self,
layout_context: &LayoutContext, layout_context: &LayoutContext,
viewport: euclid::Size2D<f32, CSSPixel>, viewport: UntypedSize2D<Au>,
) -> FragmentTree { ) -> FragmentTree {
let style = layout_context let style = layout_context
.style_context .style_context
@ -358,13 +360,8 @@ impl BoxTree {
// FIXME: use the documents mode: // FIXME: use the documents mode:
// https://drafts.csswg.org/css-writing-modes/#principal-flow // https://drafts.csswg.org/css-writing-modes/#principal-flow
let physical_containing_block = PhysicalRect::new( let physical_containing_block: Rect<Au, CSSPixel> =
PhysicalPoint::zero(), PhysicalSize::from_untyped(viewport).into();
PhysicalSize::new(
Au::from_f32_px(viewport.width),
Au::from_f32_px(viewport.height),
),
);
let initial_containing_block = DefiniteContainingBlock { let initial_containing_block = DefiniteContainingBlock {
size: LogicalVec2 { size: LogicalVec2 {
inline: physical_containing_block.size.width, inline: physical_containing_block.size.width,

View file

@ -16,7 +16,7 @@ use base::id::{PipelineId, WebViewId};
use compositing_traits::CrossProcessCompositorApi; use compositing_traits::CrossProcessCompositorApi;
use constellation_traits::ScrollState; use constellation_traits::ScrollState;
use embedder_traits::{UntrustedNodeAddress, ViewportDetails}; use embedder_traits::{UntrustedNodeAddress, ViewportDetails};
use euclid::default::{Point2D as UntypedPoint2D, Rect as UntypedRect, Size2D as UntypedSize2D}; use euclid::default::{Point2D as UntypedPoint2D, Rect as UntypedRect};
use euclid::{Point2D, Scale, Size2D, Vector2D}; use euclid::{Point2D, Scale, Size2D, Vector2D};
use fnv::FnvHashMap; use fnv::FnvHashMap;
use fonts::{FontContext, FontContextWebFontMethods}; use fonts::{FontContext, FontContextWebFontMethods};
@ -150,10 +150,6 @@ pub struct LayoutThread {
/// A counter for epoch messages /// A counter for epoch messages
epoch: Cell<Epoch>, epoch: Cell<Epoch>,
/// The size of the viewport. This may be different from the size of the screen due to viewport
/// constraints.
viewport_size: UntypedSize2D<Au>,
/// Scroll offsets of nodes that scroll. /// Scroll offsets of nodes that scroll.
scroll_offsets: RefCell<HashMap<ExternalScrollId, Vector2D<f32, LayoutPixel>>>, scroll_offsets: RefCell<HashMap<ExternalScrollId, Vector2D<f32, LayoutPixel>>>,
@ -527,10 +523,6 @@ impl LayoutThread {
stacking_context_tree: Default::default(), stacking_context_tree: Default::default(),
// Epoch starts at 1 because of the initial display list for epoch 0 that we send to WR // Epoch starts at 1 because of the initial display list for epoch 0 that we send to WR
epoch: Cell::new(Epoch(1)), epoch: Cell::new(Epoch(1)),
viewport_size: Size2D::new(
Au::from_f32_px(config.viewport_details.size.width),
Au::from_f32_px(config.viewport_details.size.height),
),
compositor_api: config.compositor_api, compositor_api: config.compositor_api,
scroll_offsets: Default::default(), scroll_offsets: Default::default(),
stylist: Stylist::new(device, QuirksMode::NoQuirks), stylist: Stylist::new(device, QuirksMode::NoQuirks),
@ -611,7 +603,8 @@ impl LayoutThread {
ua_or_user: &ua_or_user_guard, ua_or_user: &ua_or_user_guard,
}; };
if self.update_device_if_necessary(&reflow_request, &guards) { let viewport_changed = self.viewport_did_change(reflow_request.viewport_details);
if self.update_device_if_necessary(&reflow_request, viewport_changed, &guards) {
if let Some(mut data) = root_element.mutate_data() { if let Some(mut data) = root_element.mutate_data() {
data.hint.insert(RestyleHint::recascade_subtree()); data.hint.insert(RestyleHint::recascade_subtree());
} }
@ -658,6 +651,7 @@ impl LayoutThread {
root_element, root_element,
rayon_pool, rayon_pool,
&mut layout_context, &mut layout_context,
viewport_changed,
); );
self.build_stacking_context_tree(&reflow_request, did_reflow); self.build_stacking_context_tree(&reflow_request, did_reflow);
@ -683,12 +677,12 @@ impl LayoutThread {
fn update_device_if_necessary( fn update_device_if_necessary(
&mut self, &mut self,
reflow_request: &ReflowRequest, reflow_request: &ReflowRequest,
viewport_changed: bool,
guards: &StylesheetGuards, guards: &StylesheetGuards,
) -> bool { ) -> bool {
let had_used_viewport_units = self.stylist.device().used_viewport_units(); let had_used_viewport_units = self.stylist.device().used_viewport_units();
let viewport_size_changed = self.viewport_did_change(reflow_request.viewport_details);
let theme_changed = self.theme_did_change(reflow_request.theme); let theme_changed = self.theme_did_change(reflow_request.theme);
if !viewport_size_changed && !theme_changed { if !viewport_changed && !theme_changed {
return false; return false;
} }
self.update_device( self.update_device(
@ -696,7 +690,7 @@ impl LayoutThread {
reflow_request.theme, reflow_request.theme,
guards, guards,
); );
(viewport_size_changed && had_used_viewport_units) || theme_changed (viewport_changed && had_used_viewport_units) || theme_changed
} }
fn prepare_stylist_for_reflow<'dom>( fn prepare_stylist_for_reflow<'dom>(
@ -749,6 +743,7 @@ impl LayoutThread {
root_element: ServoLayoutElement<'_>, root_element: ServoLayoutElement<'_>,
rayon_pool: Option<&ThreadPool>, rayon_pool: Option<&ThreadPool>,
layout_context: &mut LayoutContext<'_>, layout_context: &mut LayoutContext<'_>,
viewport_changed: bool,
) -> bool { ) -> bool {
let dirty_root = unsafe { let dirty_root = unsafe {
ServoLayoutNode::new(&reflow_request.dirty_root.unwrap()) ServoLayoutNode::new(&reflow_request.dirty_root.unwrap())
@ -773,7 +768,7 @@ impl LayoutThread {
let root_node = root_element.as_node(); let root_node = root_element.as_node();
let damage = compute_damage_and_repair_style(layout_context.shared_context(), root_node); let damage = compute_damage_and_repair_style(layout_context.shared_context(), root_node);
if damage.is_empty() || damage == RestyleDamage::REPAINT { if !viewport_changed && (damage.is_empty() || damage == RestyleDamage::REPAINT) {
layout_context.style_context.stylist.rule_tree().maybe_gc(); layout_context.style_context.stylist.rule_tree().maybe_gc();
return false; return false;
} }
@ -794,10 +789,7 @@ impl LayoutThread {
build_box_tree() build_box_tree()
}; };
let viewport_size = Size2D::new( let viewport_size = self.stylist.device().au_viewport_size();
self.viewport_size.width.to_f32_px(),
self.viewport_size.height.to_f32_px(),
);
let run_layout = || { let run_layout = || {
box_tree box_tree
.as_ref() .as_ref()
@ -852,10 +844,11 @@ impl LayoutThread {
return; return;
} }
let viewport_size = LayoutSize::from_untyped(Size2D::new( let viewport_size = self.stylist.device().au_viewport_size();
self.viewport_size.width.to_f32_px(), let viewport_size = LayoutSize::new(
self.viewport_size.height.to_f32_px(), viewport_size.width.to_f32_px(),
)); viewport_size.height.to_f32_px(),
);
// Build the StackingContextTree. This turns the `FragmentTree` into a // Build the StackingContextTree. This turns the `FragmentTree` into a
// tree of fragments in CSS painting order and also creates all // tree of fragments in CSS painting order and also creates all
@ -949,9 +942,6 @@ impl LayoutThread {
Au::from_f32_px(viewport_details.size.height), Au::from_f32_px(viewport_details.size.height),
); );
// TODO: eliminate self.viewport_size in favour of using self.device.au_viewport_size()
self.viewport_size = new_viewport_size;
let device = self.stylist.device(); let device = self.stylist.device();
let size_did_change = device.au_viewport_size() != new_viewport_size; let size_did_change = device.au_viewport_size() != new_viewport_size;
let pixel_ratio_did_change = device.device_pixel_ratio().get() != new_pixel_ratio; let pixel_ratio_did_change = device.device_pixel_ratio().get() != new_pixel_ratio;