From f3f96c3393edad9d576d9c4f64c57d75fa2902a2 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Sat, 7 Sep 2024 15:08:48 -0700 Subject: [PATCH] layout: Do not use orthogonal baselines in flex layout (#33347) When a baseline is orthogonal to the main flexbox axis, it should not take part in baseline alignment. This change does that for column flex. While there is no support for vertical writing modes, this change is made to be as writing mode-agnostic as possible. Signed-off-by: Martin Robinson Co-authored-by: Oriol Brufau --- components/layout_2020/flexbox/layout.rs | 63 +++++++++++-------- components/layout_2020/flexbox/mod.rs | 8 ++- tests/wpt/meta/MANIFEST.json | 13 ++++ ...box-align-self-baseline-compatability.html | 36 +++++++++++ 4 files changed, 92 insertions(+), 28 deletions(-) create mode 100644 tests/wpt/tests/css/css-flexbox/flexbox-align-self-baseline-compatability.html diff --git a/components/layout_2020/flexbox/layout.rs b/components/layout_2020/flexbox/layout.rs index 28854ecc048..3374844a32c 100644 --- a/components/layout_2020/flexbox/layout.rs +++ b/components/layout_2020/flexbox/layout.rs @@ -19,7 +19,7 @@ use style::values::computed::length::Size; use style::values::computed::Length; use style::values::generics::flex::GenericFlexBasis as FlexBasis; use style::values::generics::length::{GenericLengthPercentageOrAuto, LengthPercentageOrNormal}; -use style::values::specified::align::AlignFlags; +use style::values::specified::align::{AlignFlags, AxisDirection}; use style::Zero; use super::geom::{FlexAxis, FlexRelativeRect, FlexRelativeSides, FlexRelativeVec2}; @@ -99,9 +99,9 @@ struct FlexItemLayoutResult { } impl FlexItemLayoutResult { - fn get_or_synthesize_baseline_with_block_size(&self, block_size: Au, item: &FlexItem) -> Au { + fn get_or_synthesize_baseline_with_cross_size(&self, cross_size: Au, item: &FlexItem) -> Au { self.baseline_relative_to_margin_box - .unwrap_or_else(|| item.synthesized_baseline_relative_to_margin_box(block_size)) + .unwrap_or_else(|| item.synthesized_baseline_relative_to_margin_box(cross_size)) } fn collect_fragment( @@ -731,6 +731,7 @@ impl FlexContainer { _ => Au::zero(), }; + let inline_axis_is_main_axis = self.config.flex_axis == FlexAxis::Row; let mut baseline_alignment_participating_baselines = Baselines::default(); let mut all_baselines = Baselines::default(); let flex_item_fragments: Vec<_> = initial_line_layouts @@ -786,20 +787,27 @@ impl FlexContainer { }, }; - let line_shared_alignment_baseline = final_line_layout - .shared_alignment_baseline - .map(|baseline| baseline + flow_relative_line_position.block); + if inline_axis_is_main_axis { + let line_shared_alignment_baseline = final_line_layout + .shared_alignment_baseline + .map(|baseline| baseline + flow_relative_line_position.block); + if index == 0 { + baseline_alignment_participating_baselines.first = + line_shared_alignment_baseline; + } + if index == num_lines - 1 { + baseline_alignment_participating_baselines.last = + line_shared_alignment_baseline; + } + } + let line_all_baselines = final_line_layout .all_baselines .offset(flow_relative_line_position.block); if index == 0 { - baseline_alignment_participating_baselines.first = - line_shared_alignment_baseline; all_baselines.first = line_all_baselines.first; } if index == num_lines - 1 { - baseline_alignment_participating_baselines.last = - line_shared_alignment_baseline; all_baselines.last = line_all_baselines.last; } @@ -1389,7 +1397,7 @@ impl InitialFlexLineLayout<'_> { item.align_self.0.value(), AlignFlags::BASELINE | AlignFlags::LAST_BASELINE ) { - let baseline = item_result.get_or_synthesize_baseline_with_block_size( + let baseline = item_result.get_or_synthesize_baseline_with_cross_size( item_result.hypothetical_cross_size, item, ); @@ -1475,9 +1483,8 @@ impl InitialFlexLineLayout<'_> { item.layout(*used_main_size, flex_context, Some(used_cross_size)); } - // TODO: This also needs to check whether we have a compatible writing mode. let baseline = item_layout_result - .get_or_synthesize_baseline_with_block_size(used_cross_size, item); + .get_or_synthesize_baseline_with_cross_size(used_cross_size, item); if matches!( item.align_self.0.value(), AlignFlags::BASELINE | AlignFlags::LAST_BASELINE @@ -1758,8 +1765,23 @@ impl FlexItem<'_> { containing_block, ); - let baselines_relative_to_margin_box = - self.layout_baselines_relative_to_margin_box(&content_box_baselines); + let item_writing_mode_is_orthogonal_to_container_writing_mode = + flex_context.config.writing_mode.is_horizontal() != + non_replaced.style.effective_writing_mode().is_horizontal(); + let has_compatible_baseline = match flex_context.config.flex_axis { + FlexAxis::Row => !item_writing_mode_is_orthogonal_to_container_writing_mode, + FlexAxis::Column => item_writing_mode_is_orthogonal_to_container_writing_mode, + }; + + let baselines_relative_to_margin_box = if has_compatible_baseline { + content_box_baselines.offset( + self.margin.cross_start.auto_is(Au::zero) + + self.padding.cross_start + + self.border.cross_start, + ) + } else { + Baselines::default() + }; let baseline_relative_to_margin_box = match self.align_self.0.value() { // ‘baseline’ computes to ‘first baseline’. @@ -1793,17 +1815,6 @@ impl FlexItem<'_> { } } - fn layout_baselines_relative_to_margin_box( - &self, - baselines_relative_to_content_box: &Baselines, - ) -> Baselines { - baselines_relative_to_content_box.offset( - self.margin.cross_start.auto_is(Au::zero) + - self.padding.cross_start + - self.border.cross_start, - ) - } - fn synthesized_baseline_relative_to_margin_box(&self, content_size: Au) -> Au { // If the item does not have a baseline in the necessary axis, // then one is synthesized from the flex item’s border box. diff --git a/components/layout_2020/flexbox/mod.rs b/components/layout_2020/flexbox/mod.rs index 592d6d5e442..d7005fa357b 100644 --- a/components/layout_2020/flexbox/mod.rs +++ b/components/layout_2020/flexbox/mod.rs @@ -2,20 +2,22 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use geom::{FlexAxis, MainStartCrossStart}; +use geom::{FlexAxis, FlexRelativeVec2, MainStartCrossStart}; use serde::Serialize; use servo_arc::Arc as ServoArc; +use style::logical_geometry::WritingMode; use style::properties::longhands::align_items::computed_value::T as AlignItems; use style::properties::longhands::flex_direction::computed_value::T as FlexDirection; use style::properties::longhands::flex_wrap::computed_value::T as FlexWrap; use style::properties::ComputedValues; use style::values::computed::{AlignContent, JustifyContent}; -use style::values::specified::align::AlignFlags; +use style::values::specified::align::{AlignFlags, AxisDirection}; use crate::cell::ArcRefCell; use crate::formatting_contexts::IndependentFormattingContext; use crate::fragment_tree::BaseFragmentInfo; use crate::positioned::AbsolutelyPositionedBox; +use crate::style_ext::ComputedValuesExt; mod construct; mod geom; @@ -26,6 +28,7 @@ mod layout; #[derive(Clone, Debug, Serialize)] pub(crate) struct FlexContainerConfig { container_is_single_line: bool, + writing_mode: WritingMode, flex_axis: FlexAxis, flex_direction: FlexDirection, flex_direction_is_reversed: bool, @@ -66,6 +69,7 @@ impl FlexContainerConfig { FlexContainerConfig { container_is_single_line, + writing_mode: container_style.effective_writing_mode(), flex_axis, flex_direction, flex_direction_is_reversed, diff --git a/tests/wpt/meta/MANIFEST.json b/tests/wpt/meta/MANIFEST.json index cb73d4b1a64..8858f00ef47 100644 --- a/tests/wpt/meta/MANIFEST.json +++ b/tests/wpt/meta/MANIFEST.json @@ -169326,6 +169326,19 @@ {} ] ], + "flexbox-align-self-baseline-compatability.html": [ + "42270c1b364c14dc21ba0eb67294eb263a4732f1", + [ + null, + [ + [ + "/css/reference/ref-filled-green-100px-square.xht", + "==" + ] + ], + {} + ] + ], "flexbox-align-self-baseline-horiz-001a.xhtml": [ "0460f4162f5f424b4693c19cab688f5b5db16fa3", [ diff --git a/tests/wpt/tests/css/css-flexbox/flexbox-align-self-baseline-compatability.html b/tests/wpt/tests/css/css-flexbox/flexbox-align-self-baseline-compatability.html new file mode 100644 index 00000000000..42270c1b364 --- /dev/null +++ b/tests/wpt/tests/css/css-flexbox/flexbox-align-self-baseline-compatability.html @@ -0,0 +1,36 @@ + + + + + + + + + + +

Test passes if there is a filled green square and no red.

+ +
+
A
+ +
A
+
+