layout: Convert the FragmentTree to physical geometry (#33030)

This converts all geometry in the FragmentTree into physical geometry,
doing conversions ahead of time instead of when traversing the fragment
tree. This is necessary to properly implement BiDi in Servo as we need
to know what side borders are on in mixed RTL and LTR contexts.

In addition, fragments are laid out in a particular context and only
that context knows its writing mode. There were issues where were using
one writing mode to lay out and another to convert to phyisical
coordinates. This isn't an issue now since we only use the default
writing mode, but starts to be an issue with BiDi text.

Closes #25564.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>

components/layout_2020/display_list/mod.rs

@@ -260,7 +260,6 @@ impl Fragment {
                 if let Some(style) = positioning_fragment.style.as_ref() {
                     let rect = positioning_fragment
                         .rect
-                        .to_physical(style.writing_mode, containing_block)
                         .translate(containing_block.origin.to_vector());
                     self.maybe_push_hit_test_for_style_and_tag(
                         builder,
@@ -274,10 +273,7 @@ impl Fragment {
             Fragment::Image(i) => match i.style.get_inherited_box().visibility {
                 Visibility::Visible => {
                     builder.is_contentful = true;
-                    let rect = i
+                    let rect = i.rect.translate(containing_block.origin.to_vector());
-                        .rect
-                        .to_physical(i.style.writing_mode, containing_block)
-                        .translate(containing_block.origin.to_vector());
 
                     let common = builder.common_properties(rect.to_webrender(), &i.style);
                     builder.wr().push_image(
@@ -295,10 +291,7 @@ impl Fragment {
             Fragment::IFrame(iframe) => match iframe.style.get_inherited_box().visibility {
                 Visibility::Visible => {
                     builder.is_contentful = true;
-                    let rect = iframe
+                    let rect = iframe.rect.translate(containing_block.origin.to_vector());
-                        .rect
-                        .to_physical(iframe.style.writing_mode, containing_block)
-                        .translate(containing_block.origin.to_vector());
 
                     builder.iframe_sizes.insert(
                         iframe.browsing_context_id,
@@ -366,10 +359,7 @@ impl Fragment {
 
         builder.is_contentful = true;
 
-        let rect = fragment
+        let rect = fragment.rect.translate(containing_block.origin.to_vector());
-            .rect
-            .to_physical(fragment.parent_style.writing_mode, containing_block)
-            .translate(containing_block.origin.to_vector());
         let mut baseline_origin = rect.origin;
         baseline_origin.y += fragment.font_metrics.ascent;
         let glyphs = glyphs(
@@ -504,7 +494,6 @@ impl<'a> BuilderForBoxFragment<'a> {
     fn new(fragment: &'a BoxFragment, containing_block: &'a PhysicalRect<Au>) -> Self {
         let border_rect: units::LayoutRect = fragment
             .border_rect()
-            .to_physical(fragment.style.writing_mode, containing_block)
             .translate(containing_block.origin.to_vector())
             .to_webrender();
 
@@ -562,7 +551,6 @@ impl<'a> BuilderForBoxFragment<'a> {
         self.content_rect.get_or_init(|| {
             self.fragment
                 .content_rect
-                .to_physical(self.fragment.style.writing_mode, self.containing_block)
                 .translate(self.containing_block.origin.to_vector())
                 .to_webrender()
         })
@@ -572,7 +560,6 @@ impl<'a> BuilderForBoxFragment<'a> {
         self.padding_rect.get_or_init(|| {
             self.fragment
                 .padding_rect()
-                .to_physical(self.fragment.style.writing_mode, self.containing_block)
                 .translate(self.containing_block.origin.to_vector())
                 .to_webrender()
         })
@@ -606,13 +593,7 @@ impl<'a> BuilderForBoxFragment<'a> {
             return Some(clip);
         }
 
-        let radii = inner_radii(
+        let radii = inner_radii(self.border_radius, self.fragment.border.to_webrender());
-            self.border_radius,
-            self.fragment
-                .border
-                .to_physical(self.fragment.style.writing_mode)
-                .to_webrender(),
-        );
         let maybe_clip =
             create_clip_chain(radii, *self.padding_rect(), builder, force_clip_creation);
         *self.padding_edge_clip_chain_id.borrow_mut() = maybe_clip;
@@ -630,9 +611,7 @@ impl<'a> BuilderForBoxFragment<'a> {
 
         let radii = inner_radii(
             self.border_radius,
-            (self.fragment.border + self.fragment.padding)
+            (self.fragment.border + self.fragment.padding).to_webrender(),
-                .to_physical(self.fragment.style.writing_mode)
-                .to_webrender(),
         );
         let maybe_clip =
             create_clip_chain(radii, *self.content_rect(), builder, force_clip_creation);
@@ -731,7 +710,7 @@ impl<'a> BuilderForBoxFragment<'a> {
                     painting_area_override: None,
                     positioning_area_override: Some(
                         positioning_area
-                            .to_physical(self.fragment.style.writing_mode, self.containing_block)
+                            .to_physical(self.fragment.style.writing_mode)
                             .translate(self.containing_block.origin.to_vector())
                             .to_webrender(),
                     ),
@@ -887,11 +866,7 @@ impl<'a> BuilderForBoxFragment<'a> {
 
     fn build_border(&mut self, builder: &mut DisplayListBuilder) {
         let border = self.fragment.style.get_border();
-        let border_widths = self
+        let border_widths = self.fragment.border.to_webrender();
-            .fragment
-            .border
-            .to_physical(self.fragment.style.writing_mode)
-            .to_webrender();
 
         if border_widths == SideOffsets2D::zero() {
             return;

components/layout_2020/display_list/stacking_context.rs

@@ -1178,11 +1178,9 @@ impl BoxFragment {
 
         let padding_rect = self
             .padding_rect()
-            .to_physical(self.style.writing_mode, &containing_block.rect)
             .translate(containing_block.rect.origin.to_vector());
         let content_rect = self
             .content_rect
-            .to_physical(self.style.writing_mode, &containing_block.rect)
             .translate(containing_block.rect.origin.to_vector());
 
         let for_absolute_descendants = ContainingBlock::new(
@@ -1246,9 +1244,7 @@ impl BoxFragment {
             _ => return None,
         };
 
-        let border_rect = self
+        let border_rect = self.border_rect();
-            .border_rect()
-            .to_physical(self.style.writing_mode, containing_block_rect);
         let clip_rect = clip_rect
             .for_border_rect(border_rect)
             .translate(containing_block_rect.origin.to_vector())
@@ -1306,7 +1302,6 @@ impl BoxFragment {
 
         let padding_rect = self
             .padding_rect()
-            .to_physical(self.style.writing_mode, containing_block_rect)
             .translate(containing_block_rect.origin.to_vector())
             .to_webrender();
 
@@ -1314,8 +1309,7 @@ impl BoxFragment {
             parent_scroll_node_id,
             parent_clip_id,
             external_id,
-            self.scrollable_overflow(containing_block_rect)
+            self.scrollable_overflow().to_webrender(),
-                .to_webrender(),
             padding_rect,
             sensitivity,
         );
@@ -1380,7 +1374,6 @@ impl BoxFragment {
 
         let frame_rect = self
             .border_rect()
-            .to_physical(self.style.writing_mode, containing_block_rect)
             .translate(containing_block_rect.origin.to_vector())
             .to_webrender();
 
@@ -1426,9 +1419,7 @@ impl BoxFragment {
             return None;
         }
 
-        let relative_border_rect = self
+        let relative_border_rect = self.border_rect();
-            .border_rect()
-            .to_physical(self.style.writing_mode, containing_block_rect);
         let border_rect = relative_border_rect.translate(containing_block_rect.origin.to_vector());
         let untyped_border_rect = border_rect.to_untyped();
 
@@ -1560,7 +1551,6 @@ impl PositioningFragment {
     ) {
         let rect = self
             .rect
-            .to_physical(self.writing_mode, &containing_block.rect)
             .translate(containing_block.rect.origin.to_vector());
         let new_containing_block = containing_block.new_replacing_rect(&rect);
         let new_containing_block_info =

components/layout_2020/flexbox/layout.rs

@@ -676,8 +676,10 @@ impl FlexContainer {
                     all_baselines.last = line_all_baselines.last;
                 }
 
+                let physical_line_position =
+                    flow_relative_line_position.to_physical_size(self.style.writing_mode);
                 for (fragment, _) in &mut final_line_layout.item_fragments {
-                    fragment.content_rect.start_corner += flow_relative_line_position
+                    fragment.content_rect.origin += physical_line_position;
                 }
                 final_line_layout.item_fragments
             })
@@ -705,6 +707,7 @@ impl FlexContainer {
                     let fragment = Fragment::Box(fragment);
                     child_positioning_context.adjust_static_position_of_hoisted_fragments(
                         &fragment,
+                        self.style.writing_mode,
                         PositioningContextLength::zero(),
                     );
                     positioning_context.append(child_positioning_context);
@@ -1545,15 +1548,20 @@ impl InitialFlexLineLayout<'_> {
                 let mut fragment_info = item.box_.base_fragment_info();
                 fragment_info.flags.insert(FragmentFlags::IS_FLEX_ITEM);
 
+                let container_writing_mode = flex_context.containing_block.style.writing_mode;
                 (
                     BoxFragment::new(
                         fragment_info,
                         item.box_.style().clone(),
                         item_layout_result.fragments,
-                        content_rect,
+                        content_rect.to_physical(container_writing_mode),
-                        flex_context.sides_to_flow_relative(item.padding),
+                        flex_context
-                        flex_context.sides_to_flow_relative(item.border),
+                            .sides_to_flow_relative(item.padding)
-                        margin,
+                            .to_physical(container_writing_mode),
+                        flex_context
+                            .sides_to_flow_relative(item.border)
+                            .to_physical(container_writing_mode),
+                        margin.to_physical(container_writing_mode),
                         None, /* clearance */
                         collapsed_margin,
                     ),
@@ -1620,7 +1628,12 @@ impl FlexItem<'_> {
                             &pbm,
                         );
                         let cross_size = flex_context.vec2_to_flex_relative(size).cross;
-                        let fragments = replaced.contents.make_fragments(&replaced.style, size);
+                        let container_writing_mode =
+                            flex_context.containing_block.style.writing_mode;
+                        let fragments = replaced.contents.make_fragments(
+                            &replaced.style,
+                            size.to_physical_size(container_writing_mode),
+                        );
 
                         FlexItemLayoutResult {
                             hypothetical_cross_size: cross_size,

components/layout_2020/flow/float.rs

@@ -16,6 +16,7 @@ use euclid::num::Zero;
 use serde::Serialize;
 use servo_arc::Arc;
 use style::computed_values::float::T as FloatProperty;
+use style::logical_geometry::WritingMode;
 use style::properties::ComputedValues;
 use style::values::computed::{Clear, Length};
 use style::values::specified::text::TextDecorationLine;
@@ -25,7 +26,7 @@ use crate::dom::NodeExt;
 use crate::dom_traversal::{Contents, NodeAndStyleInfo};
 use crate::formatting_contexts::IndependentFormattingContext;
 use crate::fragment_tree::{BoxFragment, CollapsedBlockMargins, CollapsedMargin};
-use crate::geom::{LogicalRect, LogicalVec2};
+use crate::geom::{LogicalRect, LogicalVec2, ToLogical};
 use crate::positioned::PositioningContext;
 use crate::style_ext::{ComputedValuesExt, DisplayInside, PaddingBorderMargin};
 use crate::ContainingBlock;
@@ -959,26 +960,24 @@ impl FloatBox {
                                 ),
                             };
                         content_size = LogicalVec2 {
-                            inline: inline_size,
+                            inline: inline_size.into(),
-                            block: block_size,
+                            block: block_size.into(),
                         };
                         children = independent_layout.fragments;
                     },
                     IndependentFormattingContext::Replaced(ref replaced) => {
                         // https://drafts.csswg.org/css2/#float-replaced-width
                         // https://drafts.csswg.org/css2/#inline-replaced-height
-                        content_size = replaced
+                        content_size = replaced.contents.used_size_as_if_inline_element(
-                            .contents
-                            .used_size_as_if_inline_element(
                             containing_block,
                             &replaced.style,
                             None,
                             &pbm,
+                        );
+                        children = replaced.contents.make_fragments(
+                            &replaced.style,
+                            content_size.to_physical_size(containing_block.style.writing_mode),
                         )
-                            .into();
-                        children = replaced
-                            .contents
-                            .make_fragments(&replaced.style, content_size.into());
                     },
                 };
 
@@ -987,14 +986,15 @@ impl FloatBox {
                     size: content_size,
                 };
 
+                let containing_block_writing_mode = containing_block.style.writing_mode;
                 BoxFragment::new(
                     self.contents.base_fragment_info(),
                     style.clone(),
                     children,
-                    content_rect.into(),
+                    content_rect.to_physical(containing_block_writing_mode),
-                    pbm.padding,
+                    pbm.padding.to_physical(containing_block_writing_mode),
-                    pbm.border,
+                    pbm.border.to_physical(containing_block_writing_mode),
-                    margin,
+                    margin.to_physical(containing_block_writing_mode),
                     // Clearance is handled internally by the float placement logic, so there's no need
                     // to store it explicitly in the fragment.
                     None, // clearance
@@ -1194,6 +1194,7 @@ impl SequentialLayoutState {
     pub(crate) fn place_float_fragment(
         &mut self,
         box_fragment: &mut BoxFragment,
+        container_writing_mode: WritingMode,
         margins_collapsing_with_parent_containing_block: CollapsedMargin,
         block_offset_from_containing_block_top: Au,
     ) {
@@ -1208,8 +1209,9 @@ impl SequentialLayoutState {
             block_start_of_containing_block_in_bfc + block_offset_from_containing_block_top,
         );
 
-        let pbm_sums = box_fragment.padding + box_fragment.border + box_fragment.margin;
+        let pbm_sums = (box_fragment.padding + box_fragment.border + box_fragment.margin)
-        let content_rect = &box_fragment.content_rect;
+            .to_logical(container_writing_mode);
+        let content_rect = &box_fragment.content_rect.to_logical(container_writing_mode);
         let margin_box_start_corner = self.floats.add_float(&PlacementInfo {
             size: content_rect.size + pbm_sums.sum(),
             side: FloatSide::from_style(&box_fragment.style).expect("Float box wasn't floated!"),
@@ -1227,6 +1229,7 @@ impl SequentialLayoutState {
             block: new_position_in_bfc.block - block_start_of_containing_block_in_bfc,
         };
 
-        box_fragment.content_rect.start_corner = new_position_in_containing_block;
+        box_fragment.content_rect.origin =
+            new_position_in_containing_block.to_physical_point(container_writing_mode);
     }
 }

components/layout_2020/flow/inline/line.rs

@@ -29,7 +29,7 @@ use crate::context::LayoutContext;
 use crate::fragment_tree::{
     BaseFragmentInfo, BoxFragment, CollapsedBlockMargins, Fragment, TextFragment,
 };
-use crate::geom::{LogicalRect, LogicalVec2};
+use crate::geom::{LogicalRect, LogicalVec2, ToLogical};
 use crate::positioned::{
     relative_adjustement, AbsolutelyPositionedBox, PositioningContext, PositioningContextLength,
 };
@@ -360,13 +360,15 @@ impl<'a> LineItemLayout<'a> {
             },
         };
 
+        let ifc_writing_mode = self.ifc_containing_block.style.writing_mode;
         if inner_state
             .flags
             .contains(LineLayoutInlineContainerFlags::HAD_ANY_FLOATS)
         {
             for fragment in inner_state.fragments.iter_mut() {
                 if let Fragment::Float(box_fragment) = fragment {
-                    box_fragment.content_rect.start_corner -= pbm_sums.start_offset();
+                    box_fragment.content_rect.origin -=
+                        pbm_sums.start_offset().to_physical_size(ifc_writing_mode);
                 }
             }
         }
@@ -382,10 +384,10 @@ impl<'a> LineItemLayout<'a> {
             inline_box.base_fragment_info,
             style.clone(),
             inner_state.fragments,
-            content_rect,
+            content_rect.to_physical(ifc_writing_mode),
-            padding,
+            padding.to_physical(ifc_writing_mode),
-            border,
+            border.to_physical(ifc_writing_mode),
-            margin,
+            margin.to_physical(ifc_writing_mode),
             None, /* clearance */
             CollapsedBlockMargins::zero(),
         );
@@ -394,7 +396,7 @@ impl<'a> LineItemLayout<'a> {
             Either::First(mut positioning_context) => {
                 positioning_context.layout_collected_children(self.layout_context, &mut fragment);
                 positioning_context.adjust_static_position_of_hoisted_fragments_with_offset(
-                    &fragment.content_rect.start_corner,
+                    &fragment.content_rect.origin.to_logical(ifc_writing_mode),
                     PositioningContextLength::zero(),
                 );
                 self.current_positioning_context_mut()
@@ -403,7 +405,7 @@ impl<'a> LineItemLayout<'a> {
             Either::Second(start_offset) => {
                 self.current_positioning_context_mut()
                     .adjust_static_position_of_hoisted_fragments_with_offset(
-                        &fragment.content_rect.start_corner,
+                        &fragment.content_rect.origin.to_logical(ifc_writing_mode),
                         start_offset,
                     );
             },
@@ -484,7 +486,7 @@ impl<'a> LineItemLayout<'a> {
         self.state.fragments.push(Fragment::Text(TextFragment {
             base: text_item.base_fragment_info.into(),
             parent_style: text_item.parent_style,
-            rect,
+            rect: rect.to_physical(self.ifc_containing_block.style.writing_mode),
             font_metrics: text_item.font_metrics,
             font_key: text_item.font_key,
             glyphs: text_item.text,
@@ -498,18 +500,27 @@ impl<'a> LineItemLayout<'a> {
         // offset, which is the sum of the start component of the padding, border, and margin.
         // This needs to be added to the calculated block and inline positions.
         // Make the final result relative to the parent box.
-        atomic.fragment.content_rect.start_corner.inline += self.state.inline_advance;
+        let mut atomic_offset = LogicalVec2 {
-        atomic.fragment.content_rect.start_corner.block +=
+            inline: self.state.inline_advance,
-            atomic.calculate_block_start(&self.line_metrics) - self.state.parent_offset.block;
+            block: atomic.calculate_block_start(&self.line_metrics) -
+                self.state.parent_offset.block,
+        };
 
         if atomic.fragment.style.clone_position().is_relative() {
-            atomic.fragment.content_rect.start_corner +=
+            atomic_offset +=
                 relative_adjustement(&atomic.fragment.style, self.ifc_containing_block);
         }
 
+        let ifc_writing_mode = self.ifc_containing_block.style.writing_mode;
+        atomic.fragment.content_rect.origin += atomic_offset.to_physical_size(ifc_writing_mode);
+
         if let Some(mut positioning_context) = atomic.positioning_context {
             positioning_context.adjust_static_position_of_hoisted_fragments_with_offset(
-                &atomic.fragment.content_rect.start_corner,
+                &atomic
+                    .fragment
+                    .content_rect
+                    .origin
+                    .to_logical(ifc_writing_mode),
                 PositioningContextLength::zero(),
             );
             self.current_positioning_context_mut()
@@ -577,7 +588,8 @@ impl<'a> LineItemLayout<'a> {
             inline: self.state.parent_offset.inline,
             block: self.line_metrics.block_offset + self.state.parent_offset.block,
         };
-        float.fragment.content_rect.start_corner -= distance_from_parent_to_ifc;
+        float.fragment.content_rect.origin -= distance_from_parent_to_ifc
+            .to_physical_size(self.ifc_containing_block.style.writing_mode);
         self.state.fragments.push(Fragment::Float(float.fragment));
     }
 }

components/layout_2020/flow/inline/mod.rs

@@ -123,7 +123,7 @@ use crate::fragment_tree::{
     BoxFragment, CollapsedBlockMargins, CollapsedMargin, Fragment, FragmentFlags,
     PositioningFragment,
 };
-use crate::geom::{LogicalRect, LogicalVec2};
+use crate::geom::{LogicalRect, LogicalVec2, PhysicalRect, ToLogical};
 use crate::positioned::{AbsolutelyPositionedBox, PositioningContext};
 use crate::sizing::ContentSizes;
 use crate::style_ext::{ComputedValuesExt, PaddingBorderMargin};
@@ -890,11 +890,10 @@ impl<'a, 'b> InlineFormattingContextState<'a, 'b> {
                 start_positioning_context_length,
             );
 
+        let line_rect = line_rect.to_physical(self.containing_block.style.writing_mode);
         self.fragments
             .push(Fragment::Positioning(PositioningFragment::new_anonymous(
-                line_rect,
+                line_rect, fragments,
-                fragments,
-                self.containing_block.style.writing_mode,
             )));
     }
 
@@ -1014,6 +1013,7 @@ impl<'a, 'b> InlineFormattingContextState<'a, 'b> {
             state.current_containing_block_offset();
         state.place_float_fragment(
             fragment,
+            self.containing_block.style.writing_mode,
             CollapsedMargin::zero(),
             block_offset_from_containining_block_top,
         );
@@ -1034,8 +1034,8 @@ impl<'a, 'b> InlineFormattingContextState<'a, 'b> {
     ) {
         let margin_box = float_item
             .fragment
-            .border_rect()
+            .margin_rect()
-            .inflate(&float_item.fragment.margin);
+            .to_logical(self.containing_block.style.writing_mode);
         let inline_size = margin_box.size.inline.max(Au::zero());
 
         let available_inline_size = match self.current_line.placement_among_floats.get() {
@@ -1909,34 +1909,41 @@ impl IndependentFormattingContext {
         offset_in_text: usize,
     ) {
         let style = self.style();
+        let container_writing_mode = inline_formatting_context_state
+            .containing_block
+            .style
+            .writing_mode;
         let pbm = style.padding_border_margin(inline_formatting_context_state.containing_block);
         let margin = pbm.margin.auto_is(Au::zero);
         let pbm_sums = pbm.padding + pbm.border + margin;
+        let pbm_physical_origin = pbm_sums
+            .start_offset()
+            .to_physical_point(container_writing_mode);
         let mut child_positioning_context = None;
 
         // We need to know the inline size of the atomic before deciding whether to do the line break.
         let fragment = match self {
             IndependentFormattingContext::Replaced(replaced) => {
-                let size = replaced.contents.used_size_as_if_inline_element(
+                let size = replaced
+                    .contents
+                    .used_size_as_if_inline_element(
                         inline_formatting_context_state.containing_block,
                         &replaced.style,
                         None,
                         &pbm,
-                );
+                    )
+                    .to_physical_size(container_writing_mode);
                 let fragments = replaced.contents.make_fragments(&replaced.style, size);
-                let content_rect = LogicalRect {
+                let content_rect = PhysicalRect::new(pbm_physical_origin, size);
-                    start_corner: pbm_sums.start_offset(),
-                    size,
-                };
 
                 BoxFragment::new(
                     replaced.base_fragment_info,
                     replaced.style.clone(),
                     fragments,
                     content_rect,
-                    pbm.padding,
+                    pbm.padding.to_physical(container_writing_mode),
-                    pbm.border,
+                    pbm.border.to_physical(container_writing_mode),
-                    margin,
+                    margin.to_physical(container_writing_mode),
                     None, /* clearance */
                     CollapsedBlockMargins::zero(),
                 )
@@ -2015,22 +2022,23 @@ impl IndependentFormattingContext {
                         },
                     };
 
-                let content_rect = LogicalRect {
+                let content_rect = PhysicalRect::new(
-                    start_corner: pbm_sums.start_offset(),
+                    pbm_physical_origin,
-                    size: LogicalVec2 {
+                    LogicalVec2 {
                         block: block_size,
                         inline: inline_size,
-                    },
+                    }
-                };
+                    .to_physical_size(container_writing_mode),
+                );
 
                 BoxFragment::new(
                     non_replaced.base_fragment_info,
                     non_replaced.style.clone(),
                     independent_layout.fragments,
                     content_rect,
-                    pbm.padding,
+                    pbm.padding.to_physical(container_writing_mode),
-                    pbm.border,
+                    pbm.border.to_physical(container_writing_mode),
-                    margin,
+                    margin.to_physical(container_writing_mode),
                     None,
                     CollapsedBlockMargins::zero(),
                 )
@@ -2045,9 +2053,13 @@ impl IndependentFormattingContext {
             inline_formatting_context_state.process_soft_wrap_opportunity();
         }
 
-        let size = pbm_sums.sum() + fragment.content_rect.size;
+        let size = pbm_sums.sum() +
+            fragment
+                .content_rect
+                .to_logical(container_writing_mode)
+                .size;
         let baseline_offset = self
-            .pick_baseline(&fragment.baselines)
+            .pick_baseline(&fragment.baselines(container_writing_mode))
             .map(|baseline| pbm_sums.block_start + baseline)
             .unwrap_or(size.block);
 

components/layout_2020/flow/mod.rs

@@ -29,7 +29,9 @@ use crate::formatting_contexts::{
 use crate::fragment_tree::{
     BaseFragmentInfo, BoxFragment, CollapsedBlockMargins, CollapsedMargin, Fragment, FragmentFlags,
 };
-use crate::geom::{AuOrAuto, LogicalRect, LogicalSides, LogicalVec2};
+use crate::geom::{
+    AuOrAuto, LogicalRect, LogicalSides, LogicalVec2, PhysicalRect, PhysicalSides, ToLogical,
+};
 use crate::positioned::{AbsolutelyPositionedBox, PositioningContext, PositioningContextLength};
 use crate::replaced::ReplacedContent;
 use crate::sizing::{self, ContentSizes};
@@ -225,9 +227,10 @@ impl OutsideMarker {
         sequential_layout_state: Option<&mut SequentialLayoutState>,
         collapsible_with_parent_start_margin: Option<CollapsibleWithParentStartMargin>,
     ) -> Fragment {
+        let containing_block_writing_mode = containing_block.style.writing_mode;
         let content_sizes = self
             .block_container
-            .inline_content_sizes(layout_context, containing_block.style.writing_mode);
+            .inline_content_sizes(layout_context, containing_block_writing_mode);
         let containing_block_for_children = ContainingBlock {
             inline_size: content_sizes.max_content,
             block_size: AuOrAuto::auto(),
@@ -241,22 +244,29 @@ impl OutsideMarker {
             sequential_layout_state,
             collapsible_with_parent_start_margin.unwrap_or(CollapsibleWithParentStartMargin(false)),
         );
-        let max_inline_size = flow_layout.fragments.iter().fold(
+        let max_inline_size =
-            Length::zero(),
+            flow_layout
-            |current_max, fragment| match fragment {
+                .fragments
-                Fragment::Text(text) => current_max.max(text.rect.max_inline_position().into()),
+                .iter()
-                Fragment::Image(image) => current_max.max(image.rect.max_inline_position().into()),
+                .fold(Au::zero(), |current_max, fragment| {
-                Fragment::Positioning(positioning) => {
+                    current_max.max(
-                    current_max.max(positioning.rect.max_inline_position().into())
+                        match fragment {
-                },
+                            Fragment::Text(text) => text.rect,
+                            Fragment::Image(image) => image.rect,
+                            Fragment::Positioning(positioning) => positioning.rect,
                             Fragment::Box(_) |
                             Fragment::Float(_) |
                             Fragment::AbsoluteOrFixedPositioned(_) |
                             Fragment::IFrame(_) => {
-                    unreachable!("Found unexpected fragment type in outside list marker!");
+                                unreachable!(
-                },
+                                    "Found unexpected fragment type in outside list marker!"
-            },
                                 );
+                            },
+                        }
+                        .to_logical(containing_block_writing_mode)
+                        .max_inline_position(),
+                    )
+                });
 
         // Position the marker beyond the inline start of the border box list item. This needs to
         // take into account the border and padding of the item.
@@ -269,13 +279,12 @@ impl OutsideMarker {
             start_corner: LogicalVec2 {
                 inline: -max_inline_size -
                     (pbm_of_list_item.border.inline_start +
-                        pbm_of_list_item.padding.inline_start)
+                        pbm_of_list_item.padding.inline_start),
-                        .into(),
                 block: Zero::zero(),
             },
             size: LogicalVec2 {
                 inline: max_inline_size,
-                block: flow_layout.content_block_size,
+                block: flow_layout.content_block_size.into(),
             },
         };
 
@@ -286,10 +295,10 @@ impl OutsideMarker {
             base_fragment_info,
             self.marker_style.clone(),
             flow_layout.fragments,
-            content_rect.into(),
+            content_rect.to_physical(containing_block_writing_mode),
-            LogicalSides::zero(),
+            PhysicalSides::zero(),
-            LogicalSides::zero(),
+            PhysicalSides::zero(),
-            LogicalSides::zero(),
+            PhysicalSides::zero(),
             None,
             CollapsedBlockMargins::zero(),
         ))
@@ -567,6 +576,7 @@ fn layout_block_level_children_in_parallel(
             placement_state.place_fragment_and_update_baseline(&mut fragment, None);
             child_positioning_context.adjust_static_position_of_hoisted_fragments(
                 &fragment,
+                containing_block.style.writing_mode,
                 PositioningContextLength::zero(),
             );
             positioning_context.append(child_positioning_context);
@@ -604,6 +614,7 @@ fn layout_block_level_children_sequentially(
                 .place_fragment_and_update_baseline(&mut fragment, Some(sequential_layout_state));
             positioning_context.adjust_static_position_of_hoisted_fragments(
                 &fragment,
+                containing_block.style.writing_mode,
                 positioning_context_length_before_layout,
             );
 
@@ -906,14 +917,15 @@ fn layout_in_flow_non_replaced_block_level_same_formatting_context(
         },
     };
 
+    let containing_block_writing_mode = containing_block.style.writing_mode;
     BoxFragment::new(
         base_fragment_info,
         style.clone(),
         flow_layout.fragments,
-        content_rect,
+        content_rect.to_physical(containing_block_writing_mode),
-        pbm.padding,
+        pbm.padding.to_physical(containing_block_writing_mode),
-        pbm.border,
+        pbm.border.to_physical(containing_block_writing_mode),
-        margin,
+        margin.to_physical(containing_block_writing_mode),
         clearance,
         block_margins_collapsed_with_children,
     )
@@ -991,15 +1003,15 @@ impl NonReplacedFormattingContext {
         };
 
         let block_margins_collapsed_with_children = CollapsedBlockMargins::from_margin(&margin);
-
+        let containing_block_writing_mode = containing_block.style.writing_mode;
         BoxFragment::new(
             self.base_fragment_info,
             self.style.clone(),
             layout.fragments,
-            content_rect,
+            content_rect.to_physical(containing_block_writing_mode),
-            pbm.padding,
+            pbm.padding.to_physical(containing_block_writing_mode),
-            pbm.border,
+            pbm.border.to_physical(containing_block_writing_mode),
-            margin,
+            margin.to_physical(containing_block_writing_mode),
             None, /* clearance */
             block_margins_collapsed_with_children,
         )
@@ -1242,14 +1254,15 @@ impl NonReplacedFormattingContext {
         };
         let block_margins_collapsed_with_children = CollapsedBlockMargins::from_margin(&margin);
 
+        let containing_block_writing_mode = containing_block.style.writing_mode;
         BoxFragment::new(
             self.base_fragment_info,
             self.style.clone(),
             layout.fragments,
-            content_rect,
+            content_rect.to_physical(containing_block_writing_mode),
-            pbm.padding,
+            pbm.padding.to_physical(containing_block_writing_mode),
-            pbm.border,
+            pbm.border.to_physical(containing_block_writing_mode),
-            margin,
+            margin.to_physical(containing_block_writing_mode),
             clearance,
             block_margins_collapsed_with_children,
         )
@@ -1274,7 +1287,10 @@ fn layout_in_flow_replaced_block_level(
     let margin_inline_end;
     let effective_margin_inline_start;
     let (margin_block_start, margin_block_end) = solve_block_margins_for_in_flow_block_level(&pbm);
-    let fragments = replaced.make_fragments(style, content_size);
+
+    let containing_block_writing_mode = containing_block.style.writing_mode;
+    let physical_content_size = content_size.to_physical_size(containing_block_writing_mode);
+    let fragments = replaced.make_fragments(style, physical_content_size);
 
     let clearance;
     if let Some(ref mut sequential_layout_state) = sequential_layout_state {
@@ -1341,10 +1357,10 @@ fn layout_in_flow_replaced_block_level(
         inline: pbm.padding.inline_start + pbm.border.inline_start + effective_margin_inline_start,
     };
 
-    let content_rect = LogicalRect {
+    let content_rect = PhysicalRect::new(
-        start_corner,
+        start_corner.to_physical_point(containing_block_writing_mode),
-        size: content_size,
+        physical_content_size,
-    };
+    );
     let block_margins_collapsed_with_children = CollapsedBlockMargins::from_margin(&margin);
 
     BoxFragment::new(
@@ -1352,9 +1368,9 @@ fn layout_in_flow_replaced_block_level(
         style.clone(),
         fragments,
         content_rect,
-        pbm.padding,
+        pbm.padding.to_physical(containing_block_writing_mode),
-        pbm.border,
+        pbm.border.to_physical(containing_block_writing_mode),
-        margin,
+        margin.to_physical(containing_block_writing_mode),
         clearance,
         block_margins_collapsed_with_children,
     )
@@ -1618,6 +1634,9 @@ struct PlacementState {
     /// least as tall as the marker size -- even though the marker doesn't advance the block
     /// position of the placement.
     marker_block_size: Option<Au>,
+
+    /// The [`WritingMode`] of the containing fragment where these fragments are being laid out.
+    writing_mode: WritingMode,
 }
 
 impl PlacementState {
@@ -1637,6 +1656,7 @@ impl PlacementState {
             inflow_baselines: Baselines::default(),
             is_inline_block_context,
             marker_block_size: None,
+            writing_mode: containing_block_style.writing_mode,
         }
     }
 
@@ -1662,11 +1682,16 @@ impl PlacementState {
             return;
         }
 
-        let box_block_offset = box_fragment.content_rect.start_corner.block;
+        let box_block_offset = box_fragment
-        if let (None, Some(first)) = (self.inflow_baselines.first, box_fragment.baselines.first) {
+            .content_rect
+            .origin
+            .to_logical(self.writing_mode)
+            .block;
+        let box_fragment_baselines = box_fragment.baselines(self.writing_mode);
+        if let (None, Some(first)) = (self.inflow_baselines.first, box_fragment_baselines.first) {
             self.inflow_baselines.first = Some(first + box_block_offset);
         }
-        if let Some(last) = box_fragment.baselines.last {
+        if let Some(last) = box_fragment_baselines.last {
             self.inflow_baselines.last = Some(last + box_block_offset);
         }
     }
@@ -1694,14 +1719,22 @@ impl PlacementState {
                     .contains(FragmentFlags::IS_OUTSIDE_LIST_ITEM_MARKER);
                 if is_outside_marker {
                     assert!(self.marker_block_size.is_none());
-                    self.marker_block_size = Some(fragment.content_rect.size.block);
+                    self.marker_block_size = Some(
+                        fragment
+                            .content_rect
+                            .size
+                            .to_logical(self.writing_mode)
+                            .block,
+                    );
                     return;
                 }
 
                 let fragment_block_margins = &fragment.block_margins_collapsed_with_children;
-                let mut fragment_block_size = fragment.padding.block_sum() +
+                let mut fragment_block_size = fragment
-                    fragment.border.block_sum() +
+                    .border_rect()
-                    fragment.content_rect.size.block;
+                    .to_logical(self.writing_mode)
+                    .size
+                    .block;
 
                 // We use `last_in_flow_margin_collapses_with_parent_end_margin` to implement
                 // this quote from https://drafts.csswg.org/css2/#collapsing-margins
@@ -1737,8 +1770,12 @@ impl PlacementState {
                     self.current_margin
                         .adjoin_assign(&fragment_block_margins.start);
                 }
-                fragment.content_rect.start_corner.block +=
+
-                    self.current_margin.solve() + self.current_block_direction_position;
+                fragment.content_rect.origin += LogicalVec2 {
+                    inline: Au::zero(),
+                    block: self.current_margin.solve() + self.current_block_direction_position,
+                }
+                .to_physical_size(self.writing_mode);
 
                 if fragment_block_margins.collapsed_through {
                     // `fragment_block_size` is typically zero when collapsing through,
@@ -1766,6 +1803,7 @@ impl PlacementState {
                     self.current_block_direction_position + self.current_margin.solve();
                 sequential_layout_state.place_float_fragment(
                     box_fragment,
+                    self.writing_mode,
                     self.start_margin,
                     block_offset_from_containing_block_top,
                 );

components/layout_2020/flow/root.rs

@@ -353,9 +353,7 @@ impl BoxTree {
         let scrollable_overflow = root_fragments
             .iter()
             .fold(PhysicalRect::zero(), |acc, child| {
-                let child_overflow = child
+                let child_overflow = child.borrow().scrollable_overflow();
-                    .borrow()
-                    .scrollable_overflow(&physical_containing_block);
 
                 // https://drafts.csswg.org/css-overflow/#scrolling-direction
                 // We want to clip scrollable overflow on box-start and inline-start

components/layout_2020/fragment_tree/box_fragment.rs

@@ -8,6 +8,7 @@ use serde::Serialize;
 use servo_arc::Arc as ServoArc;
 use style::computed_values::overflow_x::T as ComputedOverflow;
 use style::computed_values::position::T as ComputedPosition;
+use style::logical_geometry::WritingMode;
 use style::properties::ComputedValues;
 use style::values::computed::{CSSPixelLength, Length, LengthPercentage, LengthPercentageOrAuto};
 use style::Zero;
@@ -16,7 +17,7 @@ use super::{BaseFragment, BaseFragmentInfo, CollapsedBlockMargins, Fragment};
 use crate::cell::ArcRefCell;
 use crate::formatting_contexts::Baselines;
 use crate::geom::{
-    AuOrAuto, LogicalRect, LogicalSides, PhysicalPoint, PhysicalRect, PhysicalSides, PhysicalSize,
+    AuOrAuto, LogicalRect, PhysicalPoint, PhysicalRect, PhysicalSides, PhysicalSize, ToLogical,
 };
 use crate::style_ext::ComputedValuesExt;
 
@@ -46,14 +47,13 @@ pub(crate) struct BoxFragment {
     pub style: ServoArc<ComputedValues>,
     pub children: Vec<ArcRefCell<Fragment>>,
 
-    /// From the containing block’s start corner…?
+    /// The content rect of this fragment in the parent fragment's content rectangle. This
-    /// This might be broken when the containing block is in a different writing mode:
+    /// does not include padding, border, or margin -- it only includes content.
-    /// <https://drafts.csswg.org/css-writing-modes/#orthogonal-flows>
+    pub content_rect: PhysicalRect<Au>,
-    pub content_rect: LogicalRect<Au>,
 
-    pub padding: LogicalSides<Au>,
+    pub padding: PhysicalSides<Au>,
-    pub border: LogicalSides<Au>,
+    pub border: PhysicalSides<Au>,
-    pub margin: LogicalSides<Au>,
+    pub margin: PhysicalSides<Au>,
 
     /// When the `clear` property is not set to `none`, it may introduce clearance.
     /// Clearance is some extra spacing that is added above the top margin,
@@ -66,7 +66,7 @@ pub(crate) struct BoxFragment {
     /// When this [`BoxFragment`] is for content that has a baseline, this tracks
     /// the first and last baselines of that content. This is used to propagate baselines
     /// to things such as tables and inline formatting contexts.
-    pub baselines: Baselines,
+    baselines: Baselines,
 
     pub block_margins_collapsed_with_children: CollapsedBlockMargins,
 
@@ -91,10 +91,10 @@ impl BoxFragment {
         base_fragment_info: BaseFragmentInfo,
         style: ServoArc<ComputedValues>,
         children: Vec<Fragment>,
-        content_rect: LogicalRect<Au>,
+        content_rect: PhysicalRect<Au>,
-        padding: LogicalSides<Au>,
+        padding: PhysicalSides<Au>,
-        border: LogicalSides<Au>,
+        border: PhysicalSides<Au>,
-        margin: LogicalSides<Au>,
+        margin: PhysicalSides<Au>,
         clearance: Option<Au>,
         block_margins_collapsed_with_children: CollapsedBlockMargins,
     ) -> BoxFragment {
@@ -126,37 +126,19 @@ impl BoxFragment {
         base_fragment_info: BaseFragmentInfo,
         style: ServoArc<ComputedValues>,
         children: Vec<Fragment>,
-        content_rect: LogicalRect<Au>,
+        content_rect: PhysicalRect<Au>,
-        padding: LogicalSides<Au>,
+        padding: PhysicalSides<Au>,
-        border: LogicalSides<Au>,
+        border: PhysicalSides<Au>,
-        margin: LogicalSides<Au>,
+        margin: PhysicalSides<Au>,
         clearance: Option<Au>,
         block_margins_collapsed_with_children: CollapsedBlockMargins,
         overconstrained: PhysicalSize<bool>,
     ) -> BoxFragment {
-        // FIXME(mrobinson, bug 25564): We should be using the containing block
-        // here to properly convert scrollable overflow to physical geometry.
-        let containing_block = PhysicalRect::zero();
         let scrollable_overflow_from_children =
             children.iter().fold(PhysicalRect::zero(), |acc, child| {
-                acc.union(&child.scrollable_overflow(&containing_block))
+                acc.union(&child.scrollable_overflow())
             });
 
-        // From the https://drafts.csswg.org/css-align-3/#baseline-export section on "block containers":
-        // > However, for legacy reasons if its baseline-source is auto (the initial
-        // > value) a block-level or inline-level block container that is a scroll container
-        // > always has a last baseline set, whose baselines all correspond to its block-end
-        // > margin edge.
-        //
-        // This applies even if there is no baseline set, so we unconditionally set the value here
-        // and ignore anything that is set via [`Self::with_baselines`].
-        let mut baselines = Baselines::default();
-        if style.establishes_scroll_container() {
-            baselines.last = Some(
-                content_rect.size.block + padding.block_end + border.block_end + margin.block_end,
-            )
-        }
-
         BoxFragment {
             base: base_fragment_info.into(),
             style,
@@ -166,7 +148,7 @@ impl BoxFragment {
             border,
             margin,
             clearance,
-            baselines,
+            baselines: Baselines::default(),
             block_margins_collapsed_with_children,
             scrollable_overflow_from_children,
             overconstrained,
@@ -176,16 +158,41 @@ impl BoxFragment {
     }
 
     pub fn with_baselines(mut self, baselines: Baselines) -> Self {
+        self.baselines = baselines;
+        self
+    }
+
+    /// Get the baselines for this [`BoxFragment`] if they are compatible with the given [`WritingMode`].
+    /// If they are not compatible, [`Baselines::default()`] is returned.
+    pub fn baselines(&self, writing_mode: WritingMode) -> Baselines {
+        let mut baselines =
+            if writing_mode.is_horizontal() == self.style.writing_mode.is_horizontal() {
+                self.baselines
+            } else {
+                // If the writing mode of the container requesting baselines is not
+                // compatible, ensure that the baselines established by this fragment are
+                // not used.
+                Baselines::default()
+            };
+
         // From the https://drafts.csswg.org/css-align-3/#baseline-export section on "block containers":
         // > However, for legacy reasons if its baseline-source is auto (the initial
         // > value) a block-level or inline-level block container that is a scroll container
         // > always has a last baseline set, whose baselines all correspond to its block-end
         // > margin edge.
-        if !self.style.establishes_scroll_container() {
+        //
-            self.baselines.last = baselines.last;
+        // This applies even if there is no baseline set, so we unconditionally set the value here
+        // and ignore anything that is set via [`Self::with_baselines`].
+        if self.style.establishes_scroll_container() {
+            let content_rect = self.content_rect.to_logical(writing_mode);
+            let padding = self.padding.to_logical(writing_mode);
+            let border = self.border.to_logical(writing_mode);
+            let margin = self.margin.to_logical(writing_mode);
+            baselines.last = Some(
+                content_rect.size.block + padding.block_end + border.block_end + margin.block_end,
+            )
         }
-        self.baselines.first = baselines.first;
+        baselines
-        self
     }
 
     pub fn add_extra_background(&mut self, extra_background: ExtraBackground) {
@@ -199,32 +206,26 @@ impl BoxFragment {
         self.background_mode = BackgroundMode::None;
     }
 
-    pub fn scrollable_overflow(&self, containing_block: &PhysicalRect<Au>) -> PhysicalRect<Au> {
+    pub fn scrollable_overflow(&self) -> PhysicalRect<Au> {
-        let physical_padding_rect = self
+        let physical_padding_rect = self.padding_rect();
-            .padding_rect()
+        let content_origin = self.content_rect.origin.to_vector();
-            .to_physical(self.style.writing_mode, containing_block);
-
-        let content_origin = self
-            .content_rect
-            .start_corner
-            .to_physical(self.style.writing_mode);
         physical_padding_rect.union(
             &self
                 .scrollable_overflow_from_children
-                .translate(content_origin.to_vector()),
+                .translate(content_origin),
         )
     }
 
-    pub(crate) fn padding_rect(&self) -> LogicalRect<Au> {
+    pub(crate) fn padding_rect(&self) -> PhysicalRect<Au> {
-        self.content_rect.inflate(&self.padding)
+        self.content_rect.outer_rect(self.padding)
     }
 
-    pub(crate) fn border_rect(&self) -> LogicalRect<Au> {
+    pub(crate) fn border_rect(&self) -> PhysicalRect<Au> {
-        self.padding_rect().inflate(&self.border)
+        self.padding_rect().outer_rect(self.border)
     }
 
-    pub(crate) fn margin_rect(&self) -> LogicalRect<Au> {
+    pub(crate) fn margin_rect(&self) -> PhysicalRect<Au> {
-        self.border_rect().inflate(&self.margin)
+        self.border_rect().outer_rect(self.margin)
     }
 
     pub fn print(&self, tree: &mut PrintTree) {
@@ -245,7 +246,7 @@ impl BoxFragment {
             self.border_rect(),
             self.margin,
             self.clearance,
-            self.scrollable_overflow(&PhysicalRect::zero()),
+            self.scrollable_overflow(),
             self.baselines,
             self.style.get_box().overflow_x,
             self.style.get_box().overflow_y,
@@ -257,21 +258,15 @@ impl BoxFragment {
         tree.end_level();
     }
 
-    pub fn scrollable_overflow_for_parent(
+    pub fn scrollable_overflow_for_parent(&self) -> PhysicalRect<Au> {
-        &self,
+        let mut overflow = self.border_rect();
-        containing_block: &PhysicalRect<Au>,
-    ) -> PhysicalRect<Au> {
-        let mut overflow = self
-            .border_rect()
-            .to_physical(self.style.writing_mode, containing_block);
-
         if self.style.establishes_scroll_container() {
             return overflow;
         }
 
         // https://www.w3.org/TR/css-overflow-3/#scrollable
         // Only include the scrollable overflow of a child box if it has overflow: visible.
-        let scrollable_overflow = self.scrollable_overflow(containing_block);
+        let scrollable_overflow = self.scrollable_overflow();
         let bottom_right = PhysicalPoint::new(
             overflow.max_x().max(scrollable_overflow.max_x()),
             overflow.max_y().max(scrollable_overflow.max_y()),
@@ -302,9 +297,7 @@ impl BoxFragment {
         );
 
         let (cb_width, cb_height) = (containing_block.width(), containing_block.height());
-        let content_rect = self
+        let content_rect = self.content_rect;
-            .content_rect
-            .to_physical(self.style.writing_mode, containing_block);
 
         if let Some(resolved_sticky_insets) = self.resolved_sticky_insets {
             return resolved_sticky_insets;

components/layout_2020/fragment_tree/fragment.rs

@@ -20,7 +20,7 @@ use super::{
     Tag,
 };
 use crate::cell::ArcRefCell;
-use crate::geom::{LogicalRect, LogicalSides, PhysicalRect};
+use crate::geom::{LogicalSides, PhysicalRect};
 use crate::style_ext::ComputedValuesExt;
 
 #[derive(Serialize)]
@@ -64,7 +64,7 @@ pub(crate) struct TextFragment {
     pub base: BaseFragment,
     #[serde(skip_serializing)]
     pub parent_style: ServoArc<ComputedValues>,
-    pub rect: LogicalRect<Au>,
+    pub rect: PhysicalRect<Au>,
     pub font_metrics: FontMetrics,
     #[serde(skip_serializing)]
     pub font_key: FontInstanceKey,
@@ -82,7 +82,7 @@ pub(crate) struct ImageFragment {
     pub base: BaseFragment,
     #[serde(skip_serializing)]
     pub style: ServoArc<ComputedValues>,
-    pub rect: LogicalRect<Au>,
+    pub rect: PhysicalRect<Au>,
     #[serde(skip_serializing)]
     pub image_key: ImageKey,
 }
@@ -92,7 +92,7 @@ pub(crate) struct IFrameFragment {
     pub base: BaseFragment,
     pub pipeline_id: PipelineId,
     pub browsing_context_id: BrowsingContextId,
-    pub rect: LogicalRect<Au>,
+    pub rect: PhysicalRect<Au>,
     #[serde(skip_serializing)]
     pub style: ServoArc<ComputedValues>,
 }
@@ -135,28 +135,22 @@ impl Fragment {
     pub fn scrolling_area(&self, containing_block: &PhysicalRect<Au>) -> PhysicalRect<Au> {
         match self {
             Fragment::Box(fragment) | Fragment::Float(fragment) => fragment
-                .scrollable_overflow(containing_block)
+                .scrollable_overflow()
                 .translate(containing_block.origin.to_vector()),
-            _ => self.scrollable_overflow(containing_block),
+            _ => self.scrollable_overflow(),
         }
     }
 
-    pub fn scrollable_overflow(&self, containing_block: &PhysicalRect<Au>) -> PhysicalRect<Au> {
+    pub fn scrollable_overflow(&self) -> PhysicalRect<Au> {
         match self {
             Fragment::Box(fragment) | Fragment::Float(fragment) => {
-                fragment.scrollable_overflow_for_parent(containing_block)
+                fragment.scrollable_overflow_for_parent()
             },
             Fragment::AbsoluteOrFixedPositioned(_) => PhysicalRect::zero(),
             Fragment::Positioning(fragment) => fragment.scrollable_overflow,
-            Fragment::Text(fragment) => fragment
+            Fragment::Text(fragment) => fragment.rect,
-                .rect
+            Fragment::Image(fragment) => fragment.rect,
-                .to_physical(fragment.parent_style.writing_mode, containing_block),
+            Fragment::IFrame(fragment) => fragment.rect,
-            Fragment::Image(fragment) => fragment
-                .rect
-                .to_physical(fragment.style.writing_mode, containing_block),
-            Fragment::IFrame(fragment) => fragment
-                .rect
-                .to_physical(fragment.style.writing_mode, containing_block),
         }
     }
 
@@ -175,11 +169,9 @@ impl Fragment {
             Fragment::Box(fragment) | Fragment::Float(fragment) => {
                 let content_rect = fragment
                     .content_rect
-                    .to_physical(fragment.style.writing_mode, containing_block)
                     .translate(containing_block.origin.to_vector());
                 let padding_rect = fragment
                     .padding_rect()
-                    .to_physical(fragment.style.writing_mode, containing_block)
                     .translate(containing_block.origin.to_vector());
                 let new_manager = if fragment
                     .style
@@ -201,10 +193,7 @@ impl Fragment {
                     .find_map(|child| child.borrow().find(&new_manager, level + 1, process_func))
             },
             Fragment::Positioning(fragment) => {
-                let content_rect = fragment
+                let content_rect = fragment.rect.translate(containing_block.origin.to_vector());
-                    .rect
-                    .to_physical(fragment.writing_mode, containing_block)
-                    .translate(containing_block.origin.to_vector());
                 let new_manager = manager.new_for_non_absolute_descendants(&content_rect);
                 fragment
                     .children

components/layout_2020/fragment_tree/fragment_tree.rs

@@ -110,15 +110,9 @@ impl FragmentTree {
             }
 
             let fragment_relative_rect = match fragment {
-                Fragment::Box(fragment) | Fragment::Float(fragment) => fragment
+                Fragment::Box(fragment) | Fragment::Float(fragment) => fragment.border_rect(),
-                    .border_rect()
+                Fragment::Positioning(fragment) => fragment.rect,
-                    .to_physical(fragment.style.writing_mode, containing_block),
+                Fragment::Text(fragment) => fragment.rect,
-                Fragment::Positioning(fragment) => fragment
-                    .rect
-                    .to_physical(fragment.writing_mode, containing_block),
-                Fragment::Text(fragment) => fragment
-                    .rect
-                    .to_physical(fragment.parent_style.writing_mode, containing_block),
                 Fragment::AbsoluteOrFixedPositioned(_) |
                 Fragment::Image(_) |
                 Fragment::IFrame(_) => return None,
@@ -134,7 +128,7 @@ impl FragmentTree {
 
     pub fn get_border_dimensions_for_node(&self, requested_node: OpaqueNode) -> Rect<i32> {
         let tag_to_find = Tag::new(requested_node);
-        self.find(|fragment, _, containing_block| {
+        self.find(|fragment, _, _containing_block| {
             if fragment.tag() != Some(tag_to_find) {
                 return None;
             }
@@ -151,18 +145,13 @@ impl FragmentTree {
                     }
 
                     let border = fragment.style.get_border();
-                    let padding_rect = fragment
+                    let padding_rect = fragment.padding_rect();
-                        .padding_rect()
-                        .to_physical(fragment.style.writing_mode, containing_block);
                     Rect::new(
                         Point2D::new(border.border_left_width, border.border_top_width),
                         Size2D::new(padding_rect.size.width, padding_rect.size.height),
                     )
                 },
-                Fragment::Positioning(fragment) => fragment
+                Fragment::Positioning(fragment) => fragment.rect.cast_unit(),
-                    .rect
-                    .to_physical(fragment.writing_mode, containing_block)
-                    .cast_unit(),
                 _ => return None,
             };
 

components/layout_2020/fragment_tree/positioning_fragment.rs

@@ -6,12 +6,11 @@ use app_units::Au;
 use base::print_tree::PrintTree;
 use serde::Serialize;
 use servo_arc::Arc as ServoArc;
-use style::logical_geometry::WritingMode;
 use style::properties::ComputedValues;
 
 use super::{BaseFragment, BaseFragmentInfo, Fragment};
 use crate::cell::ArcRefCell;
-use crate::geom::{LogicalRect, PhysicalRect};
+use crate::geom::PhysicalRect;
 
 /// Can contain child fragments with relative coordinates, but does not contribute to painting
 /// itself. [`PositioningFragment`]s may be completely anonymous, or just non-painting Fragments
@@ -19,10 +18,8 @@ use crate::geom::{LogicalRect, PhysicalRect};
 #[derive(Serialize)]
 pub(crate) struct PositioningFragment {
     pub base: BaseFragment,
-    pub rect: LogicalRect<Au>,
+    pub rect: PhysicalRect<Au>,
     pub children: Vec<ArcRefCell<Fragment>>,
-    pub writing_mode: WritingMode,
-
     /// The scrollable overflow of this anonymous fragment's children.
     pub scrollable_overflow: PhysicalRect<Au>,
 
@@ -32,44 +29,29 @@ pub(crate) struct PositioningFragment {
 }
 
 impl PositioningFragment {
-    pub fn new_anonymous(
+    pub fn new_anonymous(rect: PhysicalRect<Au>, children: Vec<Fragment>) -> Self {
-        rect: LogicalRect<Au>,
+        Self::new_with_base_fragment(BaseFragment::anonymous(), None, rect, children)
-        children: Vec<Fragment>,
-        mode: WritingMode,
-    ) -> Self {
-        Self::new_with_base_fragment(BaseFragment::anonymous(), None, rect, children, mode)
     }
 
     pub fn new_empty(
         base_fragment_info: BaseFragmentInfo,
-        rect: LogicalRect<Au>,
+        rect: PhysicalRect<Au>,
         style: ServoArc<ComputedValues>,
     ) -> Self {
-        let writing_mode = style.writing_mode;
+        Self::new_with_base_fragment(base_fragment_info.into(), Some(style), rect, Vec::new())
-        Self::new_with_base_fragment(
-            base_fragment_info.into(),
-            Some(style),
-            rect,
-            Vec::new(),
-            writing_mode,
-        )
     }
 
     fn new_with_base_fragment(
         base: BaseFragment,
         style: Option<ServoArc<ComputedValues>>,
-        rect: LogicalRect<Au>,
+        rect: PhysicalRect<Au>,
         children: Vec<Fragment>,
-        mode: WritingMode,
     ) -> Self {
-        // FIXME(mrobinson, bug 25564): We should be using the containing block
+        let content_origin = rect.origin;
-        // here to properly convert scrollable overflow to physical geometry.
-        let containing_block = PhysicalRect::zero();
-        let content_origin = rect.start_corner.to_physical(mode);
         let scrollable_overflow = children.iter().fold(PhysicalRect::zero(), |acc, child| {
             acc.union(
                 &child
-                    .scrollable_overflow(&containing_block)
+                    .scrollable_overflow()
                     .translate(content_origin.to_vector()),
             )
         });
@@ -78,7 +60,6 @@ impl PositioningFragment {
             style,
             rect,
             children: children.into_iter().map(ArcRefCell::new).collect(),
-            writing_mode: mode,
             scrollable_overflow,
         }
     }

components/layout_2020/geom.rs

@@ -71,6 +71,19 @@ impl<T: Clone> LogicalVec2<T> {
         }
     }
 
+    pub fn from_physical_point(physical_point: &PhysicalPoint<T>, mode: WritingMode) -> Self {
+        // https://drafts.csswg.org/css-writing-modes/#logical-to-physical
+        let (i, b) = if mode.is_horizontal() {
+            (&physical_point.x, &physical_point.y)
+        } else {
+            (&physical_point.y, &physical_point.x)
+        };
+        LogicalVec2 {
+            inline: i.clone(),
+            block: b.clone(),
+        }
+    }
+
     pub fn map<U>(&self, f: impl Fn(&T) -> U) -> LogicalVec2<U> {
         LogicalVec2 {
             inline: f(&self.inline),
@@ -195,7 +208,7 @@ impl fmt::Debug for LogicalRect<Au> {
 }
 
 impl<T: Clone> LogicalVec2<T> {
-    pub fn to_physical(&self, mode: WritingMode) -> PhysicalSize<T> {
+    pub fn to_physical_size(&self, mode: WritingMode) -> PhysicalSize<T> {
         // https://drafts.csswg.org/css-writing-modes/#logical-to-physical
         let (x, y) = if mode.is_horizontal() {
             (&self.inline, &self.block)
@@ -204,6 +217,16 @@ impl<T: Clone> LogicalVec2<T> {
         };
         PhysicalSize::new(x.clone(), y.clone())
     }
+
+    pub fn to_physical_point(&self, mode: WritingMode) -> PhysicalPoint<T> {
+        // https://drafts.csswg.org/css-writing-modes/#logical-to-physical
+        let (x, y) = if mode.is_horizontal() {
+            (&self.inline, &self.block)
+        } else {
+            (&self.block, &self.inline)
+        };
+        PhysicalPoint::new(x.clone(), y.clone())
+    }
 }
 
 impl<T: Clone> LogicalSides<T> {
@@ -464,14 +487,7 @@ impl<T> LogicalRect<T> {
         }
     }
 
-    pub fn to_physical(
+    pub fn to_physical(&self, mode: WritingMode) -> PhysicalRect<T>
-        &self,
-        mode: WritingMode,
-        // Will be needed for other writing modes
-        // FIXME: what if the containing block has a different mode?
-        // https://drafts.csswg.org/css-writing-modes/#orthogonal-flows
-        _containing_block: &PhysicalRect<T>,
-    ) -> PhysicalRect<T>
     where
         T: Clone,
     {
@@ -482,7 +498,7 @@ impl<T> LogicalRect<T> {
         };
         PhysicalRect::new(
             PhysicalPoint::new(tl_x.clone(), tl_y.clone()),
-            self.size.to_physical(mode),
+            self.size.to_physical_size(mode),
         )
     }
 }
@@ -522,3 +538,37 @@ impl From<LogicalRect<CSSPixelLength>> for LogicalRect<Au> {
         }
     }
 }
+
+pub(crate) trait ToLogical<Unit, LogicalType> {
+    fn to_logical(&self, writing_mode: WritingMode) -> LogicalType;
+}
+
+impl<Unit: Copy> ToLogical<Unit, LogicalRect<Unit>> for PhysicalRect<Unit> {
+    fn to_logical(&self, writing_mode: WritingMode) -> LogicalRect<Unit> {
+        LogicalRect {
+            start_corner: LogicalVec2::from_physical_size(
+                &PhysicalSize::new(self.origin.x, self.origin.y),
+                writing_mode,
+            ),
+            size: LogicalVec2::from_physical_size(&self.size, writing_mode),
+        }
+    }
+}
+
+impl<Unit: Copy> ToLogical<Unit, LogicalVec2<Unit>> for PhysicalSize<Unit> {
+    fn to_logical(&self, writing_mode: WritingMode) -> LogicalVec2<Unit> {
+        LogicalVec2::from_physical_size(self, writing_mode)
+    }
+}
+
+impl<Unit: Copy> ToLogical<Unit, LogicalVec2<Unit>> for PhysicalPoint<Unit> {
+    fn to_logical(&self, writing_mode: WritingMode) -> LogicalVec2<Unit> {
+        LogicalVec2::from_physical_point(self, writing_mode)
+    }
+}
+
+impl<Unit: Copy> ToLogical<Unit, LogicalSides<Unit>> for PhysicalSides<Unit> {
+    fn to_logical(&self, writing_mode: WritingMode) -> LogicalSides<Unit> {
+        LogicalSides::from_physical(self, writing_mode)
+    }
+}

components/layout_2020/positioned.rs

@@ -7,6 +7,7 @@ use rayon::iter::IntoParallelRefMutIterator;
 use rayon::prelude::{IndexedParallelIterator, ParallelIterator};
 use serde::Serialize;
 use style::computed_values::position::T as Position;
+use style::logical_geometry::WritingMode;
 use style::properties::ComputedValues;
 use style::values::computed::Length;
 use style::values::specified::text::TextDecorationLine;
@@ -23,6 +24,7 @@ use crate::fragment_tree::{
 };
 use crate::geom::{
     AuOrAuto, LengthOrAuto, LengthPercentageOrAuto, LogicalRect, LogicalSides, LogicalVec2,
+    PhysicalPoint, PhysicalRect, ToLogical,
 };
 use crate::style_ext::{ComputedValuesExt, DisplayInside};
 use crate::{ContainingBlock, DefiniteContainingBlock};
@@ -176,15 +178,17 @@ impl PositioningContext {
     pub(crate) fn adjust_static_position_of_hoisted_fragments(
         &mut self,
         parent_fragment: &Fragment,
+        parent_fragment_writing_mode: WritingMode,
         index: PositioningContextLength,
     ) {
         let start_offset = match &parent_fragment {
-            Fragment::Box(b) | Fragment::Float(b) => &b.content_rect.start_corner,
+            Fragment::Box(fragment) | Fragment::Float(fragment) => &fragment.content_rect.origin,
             Fragment::AbsoluteOrFixedPositioned(_) => return,
-            Fragment::Positioning(a) => &a.rect.start_corner,
+            Fragment::Positioning(fragment) => &fragment.rect.origin,
             _ => unreachable!(),
-        };
+        }
-        self.adjust_static_position_of_hoisted_fragments_with_offset(start_offset, index);
+        .to_logical(parent_fragment_writing_mode);
+        self.adjust_static_position_of_hoisted_fragments_with_offset(&start_offset, index);
     }
 
     /// See documentation for [PositioningContext::adjust_static_position_of_hoisted_fragments].
@@ -240,7 +244,8 @@ impl PositioningContext {
         self.append(new_context);
 
         if style.clone_position() == Position::Relative {
-            new_fragment.content_rect.start_corner += relative_adjustement(style, containing_block);
+            new_fragment.content_rect.origin += relative_adjustement(style, containing_block)
+                .to_physical_size(containing_block.style.writing_mode)
         }
 
         new_fragment
@@ -253,14 +258,16 @@ impl PositioningContext {
         layout_context: &LayoutContext,
         new_fragment: &mut BoxFragment,
     ) {
-        let padding_rect = LogicalRect {
+        let padding_rect = PhysicalRect::new(
-            size: new_fragment.content_rect.size,
             // Ignore the content rect’s position in its own containing block:
-            start_corner: LogicalVec2::zero(),
+            PhysicalPoint::origin(),
-        }
+            new_fragment.content_rect.size,
-        .inflate(&new_fragment.padding);
+        )
+        .outer_rect(new_fragment.padding);
         let containing_block = DefiniteContainingBlock {
-            size: padding_rect.size,
+            size: padding_rect
+                .size
+                .to_logical(new_fragment.style.writing_mode),
             style: &new_fragment.style,
         };
 
@@ -479,6 +486,7 @@ impl HoistedAbsolutelyPositionedBox {
         let cbis = containing_block.size.inline;
         let cbbs = containing_block.size.block;
         let mut absolutely_positioned_box = self.absolutely_positioned_box.borrow_mut();
+        let containing_block_writing_mode = containing_block.style.writing_mode;
         let pbm = absolutely_positioned_box
             .context
             .style()
@@ -543,7 +551,10 @@ impl HoistedAbsolutelyPositionedBox {
                     // https://drafts.csswg.org/css2/visudet.html#abs-replaced-height
                     let style = &replaced.style;
                     content_size = computed_size.auto_is(|| unreachable!()).into();
-                    fragments = replaced.contents.make_fragments(style, content_size);
+                    fragments = replaced.contents.make_fragments(
+                        style,
+                        content_size.to_physical_size(containing_block_writing_mode),
+                    );
                 },
                 IndependentFormattingContext::NonReplaced(non_replaced) => {
                     // https://drafts.csswg.org/css2/#min-max-widths
@@ -711,16 +722,16 @@ impl HoistedAbsolutelyPositionedBox {
             };
 
             let physical_overconstrained =
-                overconstrained.to_physical(containing_block.style.writing_mode);
+                overconstrained.to_physical_size(containing_block.style.writing_mode);
 
             BoxFragment::new_with_overconstrained(
                 absolutely_positioned_box.context.base_fragment_info(),
                 absolutely_positioned_box.context.style().clone(),
                 fragments,
-                content_rect,
+                content_rect.to_physical(containing_block_writing_mode),
-                pbm.padding,
+                pbm.padding.to_physical(containing_block_writing_mode),
-                pbm.border,
+                pbm.border.to_physical(containing_block_writing_mode),
-                margin,
+                margin.to_physical(containing_block_writing_mode),
                 None, /* clearance */
                 // We do not set the baseline offset, because absolutely positioned
                 // elements are not inflow.
@@ -736,7 +747,10 @@ impl HoistedAbsolutelyPositionedBox {
         // other elements. If any of them have a static start position though, we need to
         // adjust it to account for the start corner of this absolute.
         positioning_context.adjust_static_position_of_hoisted_fragments_with_offset(
-            &new_fragment.content_rect.start_corner,
+            &new_fragment
+                .content_rect
+                .origin
+                .to_logical(containing_block_writing_mode),
             PositioningContextLength::zero(),
         );
 

components/layout_2020/query.rs

@@ -198,21 +198,13 @@ pub fn process_resolved_style_request<'dom>(
                             _ => {},
                         }
                     }
-                    let content_rect = box_fragment
+                    let content_rect = box_fragment.content_rect;
-                        .content_rect
+                    let margins = box_fragment.margin;
-                        .to_physical(box_fragment.style.writing_mode, containing_block);
+                    let padding = box_fragment.padding;
-                    let margins = box_fragment
-                        .margin
-                        .to_physical(box_fragment.style.writing_mode);
-                    let padding = box_fragment
-                        .padding
-                        .to_physical(box_fragment.style.writing_mode);
                     (content_rect, margins, padding)
                 },
                 Fragment::Positioning(positioning_fragment) => {
-                    let content_rect = positioning_fragment
+                    let content_rect = positioning_fragment.rect;
-                        .rect
-                        .to_physical(positioning_fragment.writing_mode, containing_block);
                     (content_rect, SideOffsets2D::zero(), SideOffsets2D::zero())
                 },
                 _ => return None,
@@ -370,15 +362,9 @@ fn process_offset_parent_query_inner(
             //
             // [1]: https://github.com/w3c/csswg-drafts/issues/4541
             let fragment_relative_rect = match fragment {
-                Fragment::Box(fragment) | Fragment::Float(fragment) => fragment
+                Fragment::Box(fragment) | Fragment::Float(fragment) => fragment.border_rect(),
-                    .border_rect()
+                Fragment::Text(fragment) => fragment.rect,
-                    .to_physical(fragment.style.writing_mode, containing_block),
+                Fragment::Positioning(fragment) => fragment.rect,
-                Fragment::Text(fragment) => fragment
-                    .rect
-                    .to_physical(fragment.parent_style.writing_mode, containing_block),
-                Fragment::Positioning(fragment) => fragment
-                    .rect
-                    .to_physical(fragment.writing_mode, containing_block),
                 Fragment::AbsoluteOrFixedPositioned(_) |
                 Fragment::Image(_) |
                 Fragment::IFrame(_) => unreachable!(),
@@ -461,11 +447,8 @@ fn process_offset_parent_query_inner(
                         Fragment::Box(fragment) | Fragment::Float(fragment) => {
                             if fragment.base.tag == Some(offset_parent_node_tag) {
                                 // Again, take the *first* associated CSS layout box.
-                                let padding_box_corner = fragment
+                                let padding_box_corner =
-                                    .padding_rect()
+                                    fragment.padding_rect().origin.to_vector() +
-                                    .to_physical(fragment.style.writing_mode, containing_block)
-                                    .origin
-                                    .to_vector() +
                                         containing_block.origin.to_vector();
                                 let padding_box_corner = padding_box_corner.to_untyped();
                                 Some(padding_box_corner)

components/layout_2020/replaced.rs

@@ -27,7 +27,7 @@ use webrender_api::ImageKey;
 use crate::context::LayoutContext;
 use crate::dom::NodeExt;
 use crate::fragment_tree::{BaseFragmentInfo, Fragment, IFrameFragment, ImageFragment};
-use crate::geom::{LogicalRect, LogicalVec2, PhysicalSize};
+use crate::geom::{LogicalVec2, PhysicalPoint, PhysicalRect, PhysicalSize};
 use crate::sizing::ContentSizes;
 use crate::style_ext::{Clamp, ComputedValuesExt, PaddingBorderMargin};
 use crate::{AuOrAuto, ContainingBlock};
@@ -268,8 +268,9 @@ impl ReplacedContent {
     pub fn make_fragments(
         &self,
         style: &ServoArc<ComputedValues>,
-        size: LogicalVec2<Au>,
+        size: PhysicalSize<Au>,
     ) -> Vec<Fragment> {
+        let rect = PhysicalRect::new(PhysicalPoint::origin(), size);
         match &self.kind {
             ReplacedContentKind::Image(image) => image
                 .as_ref()
@@ -278,10 +279,7 @@ impl ReplacedContent {
                     Fragment::Image(ImageFragment {
                         base: self.base_fragment_info.into(),
                         style: style.clone(),
-                        rect: LogicalRect {
+                        rect,
-                            start_corner: LogicalVec2::zero(),
-                            size,
-                        },
                         image_key,
                     })
                 })
@@ -290,10 +288,7 @@ impl ReplacedContent {
             ReplacedContentKind::Video(video) => vec![Fragment::Image(ImageFragment {
                 base: self.base_fragment_info.into(),
                 style: style.clone(),
-                rect: LogicalRect {
+                rect,
-                    start_corner: LogicalVec2::zero(),
-                    size,
-                },
                 image_key: video.image_key,
             })],
             ReplacedContentKind::IFrame(iframe) => {
@@ -302,10 +297,7 @@ impl ReplacedContent {
                     style: style.clone(),
                     pipeline_id: iframe.pipeline_id,
                     browsing_context_id: iframe.browsing_context_id,
-                    rect: LogicalRect {
+                    rect,
-                        start_corner: LogicalVec2::zero(),
-                        size,
-                    },
                 })]
             },
             ReplacedContentKind::Canvas(canvas_info) => {
@@ -336,10 +328,7 @@ impl ReplacedContent {
                 vec![Fragment::Image(ImageFragment {
                     base: self.base_fragment_info.into(),
                     style: style.clone(),
-                    rect: LogicalRect {
+                    rect,
-                        start_corner: LogicalVec2::zero(),
-                        size,
-                    },
                     image_key,
                 })]
             },

components/layout_2020/table/layout.rs

@@ -27,7 +27,10 @@ use crate::fragment_tree::{
     BaseFragmentInfo, BoxFragment, CollapsedBlockMargins, ExtraBackground, Fragment, FragmentFlags,
     PositioningFragment,
 };
-use crate::geom::{AuOrAuto, LengthPercentageOrAuto, LogicalRect, LogicalSides, LogicalVec2};
+use crate::geom::{
+    AuOrAuto, LengthPercentageOrAuto, LogicalRect, LogicalSides, LogicalVec2, PhysicalSides,
+    ToLogical,
+};
 use crate::positioned::{relative_adjustement, PositioningContext, PositioningContextLength};
 use crate::sizing::ContentSizes;
 use crate::style_ext::{Clamp, ComputedValuesExt, PaddingBorderMargin};
@@ -1539,10 +1542,15 @@ impl<'a> TableLayout<'a> {
             None, /* sequential_layout_state */
         );
 
-        box_fragment.content_rect.start_corner.block += box_fragment
+        let margin_offset = LogicalVec2 {
+            inline: Au::zero(),
+            block: box_fragment
                 .block_margins_collapsed_with_children
                 .start
-            .solve();
+                .solve(),
+        }
+        .to_physical_size(containing_block.style.writing_mode);
+        box_fragment.content_rect.origin += margin_offset;
 
         if let Some(positioning_context) = positioning_context.take() {
             parent_positioning_context.append(positioning_context);
@@ -1593,6 +1601,7 @@ impl<'a> TableLayout<'a> {
             content_inline_size_for_table: None,
             baselines: Baselines::default(),
         };
+        let table_writing_mode = containing_block_for_children.style.writing_mode;
 
         table_layout
             .fragments
@@ -1610,14 +1619,22 @@ impl<'a> TableLayout<'a> {
                     positioning_context,
                 );
 
-                caption_fragment.content_rect.start_corner.inline +=
+                let caption_offset = LogicalVec2 {
-                    offset_from_wrapper.inline_start;
+                    inline: offset_from_wrapper.inline_start,
-                caption_fragment.content_rect.start_corner.block += current_block_offset;
+                    block: current_block_offset,
-                current_block_offset += caption_fragment.margin_rect().size.block;
+                }
+                .to_physical_size(table_writing_mode);
+                caption_fragment.content_rect.origin += caption_offset;
+                current_block_offset += caption_fragment
+                    .margin_rect()
+                    .size
+                    .to_logical(table_writing_mode)
+                    .block;
 
                 let caption_fragment = Fragment::Box(caption_fragment);
                 positioning_context.adjust_static_position_of_hoisted_fragments(
                     &caption_fragment,
+                    table_writing_mode,
                     original_positioning_context_length,
                 );
                 Some(caption_fragment)
@@ -1634,18 +1651,27 @@ impl<'a> TableLayout<'a> {
 
         // Take the baseline of the grid fragment, after adjusting it to be in the coordinate system
         // of the table wrapper.
+        let logical_grid_content_rect = grid_fragment.content_rect.to_logical(table_writing_mode);
         table_layout.baselines = grid_fragment
-            .baselines
+            .baselines(table_writing_mode)
-            .offset(current_block_offset + grid_fragment.content_rect.start_corner.block);
+            .offset(current_block_offset + logical_grid_content_rect.start_corner.block);
 
-        grid_fragment.content_rect.start_corner.inline += offset_from_wrapper.inline_start;
+        grid_fragment.content_rect.origin += LogicalVec2 {
-        grid_fragment.content_rect.start_corner.block += current_block_offset;
+            inline: offset_from_wrapper.inline_start,
-        current_block_offset += grid_fragment.border_rect().size.block;
+            block: current_block_offset,
-        table_layout.content_inline_size_for_table = Some(grid_fragment.content_rect.size.inline);
+        }
+        .to_physical_size(table_writing_mode);
+        current_block_offset += grid_fragment
+            .border_rect()
+            .size
+            .to_logical(table_writing_mode)
+            .block;
+        table_layout.content_inline_size_for_table = Some(logical_grid_content_rect.size.inline);
 
         let grid_fragment = Fragment::Box(grid_fragment);
         positioning_context.adjust_static_position_of_hoisted_fragments(
             &grid_fragment,
+            table_writing_mode,
             original_positioning_context_length,
         );
         table_layout.fragments.push(grid_fragment);
@@ -1666,14 +1692,22 @@ impl<'a> TableLayout<'a> {
                     positioning_context,
                 );
 
-                caption_fragment.content_rect.start_corner.inline +=
+                let caption_offset = LogicalVec2 {
-                    offset_from_wrapper.inline_start;
+                    inline: offset_from_wrapper.inline_start,
-                caption_fragment.content_rect.start_corner.block += current_block_offset;
+                    block: current_block_offset,
-                current_block_offset += caption_fragment.margin_rect().size.block;
+                }
+                .to_physical_size(containing_block_for_children.style.writing_mode);
+                caption_fragment.content_rect.origin += caption_offset;
+                current_block_offset += caption_fragment
+                    .margin_rect()
+                    .size
+                    .to_logical(table_writing_mode)
+                    .block;
 
                 let caption_fragment = Fragment::Box(caption_fragment);
                 positioning_context.adjust_static_position_of_hoisted_fragments(
                     &caption_fragment,
+                    table_writing_mode,
                     original_positioning_context_length,
                 );
                 Some(caption_fragment)
@@ -1710,6 +1744,7 @@ impl<'a> TableLayout<'a> {
         assert_eq!(self.table.size.height, self.row_sizes.len());
         assert_eq!(self.table.size.width, self.distributed_column_widths.len());
 
+        let table_writing_mode = containing_block_for_children.style.writing_mode;
         if self.table.size.width == 0 && self.table.size.height == 0 {
             let content_rect = LogicalRect {
                 start_corner: table_pbm.border_padding_start(),
@@ -1717,15 +1752,16 @@ impl<'a> TableLayout<'a> {
                     inline: self.table_width,
                     block: self.final_table_height,
                 },
-            };
+            }
+            .to_physical(table_writing_mode);
             return BoxFragment::new(
                 self.table.grid_base_fragment_info,
                 self.table.grid_style.clone(),
                 Vec::new(),
                 content_rect,
-                table_pbm.padding,
+                table_pbm.padding.to_physical(table_writing_mode),
-                table_pbm.border,
+                table_pbm.border.to_physical(table_writing_mode),
-                LogicalSides::zero(),
+                PhysicalSides::zero(),
                 None, /* clearance */
                 CollapsedBlockMargins::zero(),
             );
@@ -1845,15 +1881,16 @@ impl<'a> TableLayout<'a> {
                 inline: table_and_track_dimensions.table_rect.max_inline_position(),
                 block: table_and_track_dimensions.table_rect.max_block_position(),
             },
-        };
+        }
+        .to_physical(table_writing_mode);
         BoxFragment::new(
             self.table.grid_base_fragment_info,
             self.table.grid_style.clone(),
             table_fragments,
             content_rect,
-            table_pbm.padding,
+            table_pbm.padding.to_physical(table_writing_mode),
-            table_pbm.border,
+            table_pbm.border.to_physical(table_writing_mode),
-            LogicalSides::zero(),
+            PhysicalSides::zero(),
             None, /* clearance */
             CollapsedBlockMargins::zero(),
         )
@@ -1987,11 +2024,14 @@ impl<'a> TableLayout<'a> {
         dimensions: &TableAndTrackDimensions,
         fragments: &mut Vec<Fragment>,
     ) {
+        let table_writing_mode = self.table.style.writing_mode;
         for column_group in self.table.column_groups.iter() {
             if !column_group.is_empty() {
                 fragments.push(Fragment::Positioning(PositioningFragment::new_empty(
                     column_group.base_fragment_info,
-                    dimensions.get_column_group_rect(column_group),
+                    dimensions
+                        .get_column_group_rect(column_group)
+                        .to_physical(table_writing_mode),
                     column_group.style.clone(),
                 )));
             }
@@ -2000,7 +2040,9 @@ impl<'a> TableLayout<'a> {
         for (column_index, column) in self.table.columns.iter().enumerate() {
             fragments.push(Fragment::Positioning(PositioningFragment::new_empty(
                 column.base_fragment_info,
-                dimensions.get_column_rect(column_index),
+                dimensions
+                    .get_column_rect(column_index)
+                    .to_physical(table_writing_mode),
                 column.style.clone(),
             )));
         }
@@ -2105,10 +2147,10 @@ impl<'a> RowFragmentLayout<'a> {
             self.row.base_fragment_info,
             self.row.style.clone(),
             self.fragments,
-            self.rect,
+            self.rect.to_physical(containing_block.style.writing_mode),
-            LogicalSides::zero(), /* padding */
+            PhysicalSides::zero(), /* padding */
-            LogicalSides::zero(), /* border */
+            PhysicalSides::zero(), /* border */
-            LogicalSides::zero(), /* margin */
+            PhysicalSides::zero(), /* margin */
             None,                  /* clearance */
             CollapsedBlockMargins::zero(),
         );
@@ -2168,10 +2210,10 @@ impl RowGroupFragmentLayout {
             self.base_fragment_info,
             self.style,
             self.fragments,
-            self.rect,
+            self.rect.to_physical(containing_block.style.writing_mode),
-            LogicalSides::zero(), /* padding */
+            PhysicalSides::zero(), /* padding */
-            LogicalSides::zero(), /* border */
+            PhysicalSides::zero(), /* border */
-            LogicalSides::zero(), /* margin */
+            PhysicalSides::zero(), /* margin */
             None,                  /* clearance */
             CollapsedBlockMargins::zero(),
         );
@@ -2526,9 +2568,8 @@ impl TableSlotCell {
             block: vertical_align_offset,
         };
         let vertical_align_fragment = PositioningFragment::new_anonymous(
-            vertical_align_fragment_rect,
+            vertical_align_fragment_rect.to_physical(table_style.writing_mode),
             layout.layout.fragments,
-            self.style.writing_mode,
         );
 
         // Adjust the static position of all absolute children based on the
@@ -2550,10 +2591,10 @@ impl TableSlotCell {
             base_fragment_info,
             self.style.clone(),
             vec![Fragment::Positioning(vertical_align_fragment)],
-            cell_content_rect,
+            cell_content_rect.to_physical(table_style.writing_mode),
-            layout.padding,
+            layout.padding.to_physical(table_style.writing_mode),
-            layout.border,
+            layout.border.to_physical(table_style.writing_mode),
-            LogicalSides::zero(), /* margin */
+            PhysicalSides::zero(), /* margin */
             None,                  /* clearance */
             CollapsedBlockMargins::zero(),
         )