From 8221bfc3ef854d90ecb9a0df3aa490310cbe8469 Mon Sep 17 00:00:00 2001 From: Matt Brubeck Date: Tue, 24 Feb 2015 16:43:43 -0800 Subject: [PATCH] Layout fixes for RTL child flows in LTR parents ...and vice-versa. This is not a complete fix for all mixed-direction layout cases, but it fixes enough problems to make some simple test cases pass, like tha attached reftest. There are FIXME comments for many of the remaining issues. In particular, this does not yet handle RTL layout of fixed/absolute elements. --- components/layout/block.rs | 104 +++++++++++++++++++++++++++------- components/layout/flow.rs | 8 +++ components/layout/fragment.rs | 5 +- components/layout/inline.rs | 2 + tests/ref/basic.list | 3 +- tests/ref/rtl_simple.html | 18 ++++++ tests/ref/rtl_simple_ref.html | 16 ++++++ 7 files changed, 131 insertions(+), 25 deletions(-) create mode 100644 tests/ref/rtl_simple.html create mode 100644 tests/ref/rtl_simple_ref.html diff --git a/components/layout/block.rs b/components/layout/block.rs index 81edba159b7..356232f90f8 100644 --- a/components/layout/block.rs +++ b/components/layout/block.rs @@ -54,7 +54,7 @@ use rustc_serialize::{Encoder, Encodable}; use msg::compositor_msg::LayerId; use msg::constellation_msg::ConstellationChan; use util::geometry::{Au, MAX_AU}; -use util::logical_geometry::{LogicalPoint, LogicalRect, LogicalSize}; +use util::logical_geometry::{LogicalPoint, LogicalRect, LogicalSize, WritingMode}; use util::opts; use std::cmp::{max, min}; use std::fmt; @@ -1329,6 +1329,8 @@ impl BlockFlow { } else { content_inline_size }; + // FIXME (mbrubeck): Get correct mode for absolute containing block + let containing_block_mode = self.base.writing_mode; for (i, kid) in self.base.child_iter().enumerate() { { @@ -1352,16 +1354,30 @@ impl BlockFlow { // The inline-start margin edge of the child flow is at our inline-start content edge, // and its inline-size is our content inline-size. + let kid_mode = flow::base(kid).writing_mode; { let kid_base = flow::mut_base(kid); if !kid_base.flags.contains(IS_ABSOLUTELY_POSITIONED) && !kid_base.flags.is_float() { - kid_base.position.start.i = inline_start_content_edge + kid_base.position.start.i = + if kid_mode.is_bidi_ltr() == containing_block_mode.is_bidi_ltr() { + inline_start_content_edge + } else { + // The kid's inline 'start' is at the parent's 'end' + inline_start_content_edge + content_inline_size + } } kid_base.block_container_inline_size = content_inline_size; + kid_base.block_container_writing_mode = containing_block_mode; } if kid.is_block_like() { - kid.as_block().hypothetical_position.i = inline_start_content_edge + kid.as_block().hypothetical_position.i = + if kid_mode.is_bidi_ltr() == containing_block_mode.is_bidi_ltr() { + inline_start_content_edge + } else { + // The kid's inline 'start' is at the parent's 'end' + inline_start_content_edge + content_inline_size + } } // Determine float impaction. @@ -1398,6 +1414,7 @@ impl BlockFlow { propagate_column_inline_sizes_to_child(kid, i, content_inline_size, + containing_block_mode, *column_computed_inline_sizes, &mut inline_start_margin_edge) } @@ -1610,6 +1627,7 @@ impl Flow for BlockFlow { self.base.position.start = LogicalPoint::zero(self.base.writing_mode); self.base.block_container_inline_size = LogicalSize::from_physical( self.base.writing_mode, layout_context.shared.screen_size).inline; + self.base.block_container_writing_mode = self.base.writing_mode; // The root element is never impacted by floats. self.base.flags.remove(IMPACTED_BY_LEFT_FLOATS); @@ -1717,8 +1735,9 @@ impl Flow for BlockFlow { } fn compute_absolute_position(&mut self) { - // FIXME(#2795): Get the real container size - let container_size = Size2D::zero(); + // FIXME (mbrubeck): Get the real container size, taking the container writing mode into + // account. Must handle vertical writing modes. + let container_size = Size2D(self.base.block_container_inline_size, Au(0)); if self.is_root() { self.base.clip = ClippingRegion::max() @@ -1789,6 +1808,8 @@ impl Flow for BlockFlow { .flags .contains(LAYERS_NEEDED_FOR_DESCENDANTS), }; + let container_size_for_children = + self.fragment.content_box().size.to_physical(self.base.writing_mode); // Compute the origin and clipping rectangle for children. let relative_offset = relative_offset.to_physical(self.base.writing_mode); @@ -1824,7 +1845,8 @@ impl Flow for BlockFlow { if !flow::base(kid).flags.contains(IS_ABSOLUTELY_POSITIONED) { let kid_base = flow::mut_base(kid); kid_base.stacking_relative_position = origin_for_children + - kid_base.position.start.to_physical(kid_base.writing_mode, container_size); + kid_base.position.start.to_physical(kid_base.writing_mode, + container_size_for_children); } flow::mut_base(kid).absolute_position_info = absolute_position_info_for_children; @@ -2074,17 +2096,30 @@ pub trait ISizeAndMarginsComputer { let inline_size; let extra_inline_size_from_margin; { + let block_mode = block.base.writing_mode; + + // FIXME (mbrubeck): Get correct containing block for positioned blocks? + let container_mode = block.base.block_container_writing_mode; + let container_size = block.base.block_container_inline_size; + let fragment = block.fragment(); fragment.margin.inline_start = solution.margin_inline_start; fragment.margin.inline_end = solution.margin_inline_end; - // Left border edge. - fragment.border_box.start.i = fragment.margin.inline_start; - // The associated fragment has the border box of this flow. inline_size = solution.inline_size + fragment.border_padding.inline_start_end(); fragment.border_box.size.inline = inline_size; + // Start border edge. + // FIXME (mbrubeck): Handle vertical writing modes. + fragment.border_box.start.i = + if container_mode.is_bidi_ltr() == block_mode.is_bidi_ltr() { + fragment.margin.inline_start + } else { + // The parent's "start" direction is the child's "end" direction. + container_size - inline_size - fragment.margin.inline_end + }; + // To calculate the total size of this block, we also need to account for any additional // size contribution from positive margins. Negative margins means the block isn't made // larger at all by the margin. @@ -2186,6 +2221,13 @@ pub trait ISizeAndMarginsComputer { input.inline_end_margin, input.available_inline_size); + // Check for direction of parent flow (NOT Containing Block) + let block_mode = block.base.writing_mode; + let container_mode = block.base.block_container_writing_mode; + + // FIXME (mbrubeck): Handle vertical writing modes. + let parent_has_same_direction = container_mode.is_bidi_ltr() == block_mode.is_bidi_ltr(); + // If inline-size is not 'auto', and inline-size + margins > available_inline-size, all // 'auto' margins are treated as 0. let (inline_start_margin, inline_end_margin) = match computed_inline_size { @@ -2208,10 +2250,14 @@ pub trait ISizeAndMarginsComputer { match (inline_start_margin, computed_inline_size, inline_end_margin) { // If all have a computed value other than 'auto', the system is // over-constrained so we discard the end margin. - (MaybeAuto::Specified(margin_start), MaybeAuto::Specified(inline_size), MaybeAuto::Specified(_margin_end)) => - (margin_start, inline_size, available_inline_size - - (margin_start + inline_size)), - + (MaybeAuto::Specified(margin_start), MaybeAuto::Specified(inline_size), MaybeAuto::Specified(margin_end)) => { + if parent_has_same_direction { + (margin_start, inline_size, available_inline_size - + (margin_start + inline_size)) + } else { + (available_inline_size - (margin_end + inline_size), inline_size, margin_end) + } + } // If exactly one value is 'auto', solve for it (MaybeAuto::Auto, MaybeAuto::Specified(inline_size), MaybeAuto::Specified(margin_end)) => (available_inline_size - (inline_size + margin_end), inline_size, margin_end), @@ -2284,9 +2330,12 @@ impl ISizeAndMarginsComputer for AbsoluteNonReplaced { .. } = input; - // TODO: Check for direction of parent flow (NOT Containing Block) - // when right-to-left is implemented. - // Assume direction is 'ltr' for now + // Check for direction of parent flow (NOT Containing Block) + let block_mode = block.base.writing_mode; + let container_mode = block.base.block_container_writing_mode; + + // FIXME (mbrubeck): Handle vertical writing modes. + let parent_has_same_direction = container_mode.is_bidi_ltr() == block_mode.is_bidi_ltr(); // Distance from the inline-start edge of the Absolute Containing Block to the // inline-start margin edge of a hypothetical box that would have been the @@ -2312,9 +2361,14 @@ impl ISizeAndMarginsComputer for AbsoluteNonReplaced { (MaybeAuto::Auto, MaybeAuto::Auto) => { let total_margin_val = available_inline_size - inline_start - inline_end - inline_size; if total_margin_val < Au(0) { - // margin-inline-start becomes 0 because direction is 'ltr'. - // TODO: Handle 'rtl' when it is implemented. - (inline_start, inline_end, inline_size, Au(0), total_margin_val) + if parent_has_same_direction { + // margin-inline-start becomes 0 + (inline_start, inline_end, inline_size, Au(0), total_margin_val) + } else { + // margin-inline-end becomes 0, because it's toward the parent's + // inline-start edge. + (inline_start, inline_end, inline_size, total_margin_val, Au(0)) + } } else { // Equal margins (inline_start, inline_end, inline_size, @@ -2332,10 +2386,14 @@ impl ISizeAndMarginsComputer for AbsoluteNonReplaced { } (MaybeAuto::Specified(margin_start), MaybeAuto::Specified(margin_end)) => { // Values are over-constrained. - // Ignore value for 'inline-end' cos direction is 'ltr'. - // TODO: Handle 'rtl' when it is implemented. let sum = inline_start + inline_size + margin_start + margin_end; - (inline_start, available_inline_size - sum, inline_size, margin_start, margin_end) + if parent_has_same_direction { + // Ignore value for 'inline-end' + (inline_start, available_inline_size - sum, inline_size, margin_start, margin_end) + } else { + // Ignore value for 'inline-start' + (available_inline_size - sum, inline_end, inline_size, margin_start, margin_end) + } } } } @@ -2633,6 +2691,7 @@ fn propagate_column_inline_sizes_to_child( kid: &mut Flow, child_index: uint, content_inline_size: Au, + writing_mode: WritingMode, column_computed_inline_sizes: &[ColumnComputedInlineSize], inline_start_margin_edge: &mut Au) { // If kid is table_rowgroup or table_row, the column inline-sizes info should be copied from @@ -2656,6 +2715,7 @@ fn propagate_column_inline_sizes_to_child( let kid_base = flow::mut_base(kid); kid_base.position.start.i = *inline_start_margin_edge; kid_base.block_container_inline_size = inline_size; + kid_base.block_container_writing_mode = writing_mode; } if kid.is_table_cell() { diff --git a/components/layout/flow.rs b/components/layout/flow.rs index 90cdc8a8030..0633ee43b7d 100644 --- a/components/layout/flow.rs +++ b/components/layout/flow.rs @@ -758,6 +758,13 @@ pub struct BaseFlow { /// automatic values for `width`. pub block_container_inline_size: Au, + /// The writing mode of the block container of this flow. + /// + /// FIXME (mbrubeck): Combine this and block_container_inline_size and maybe + /// block_container_explicit_block_size into a struct, to guarantee they are set at the same + /// time? Or just store a link to the containing block flow. + pub block_container_writing_mode: WritingMode, + /// The block-size of the block container of this flow, if it is an explicit size (does not /// depend on content heights). Used for computing percentage values for `height`. pub block_container_explicit_block_size: Option, @@ -924,6 +931,7 @@ impl BaseFlow { absolute_static_i_offset: Au(0), fixed_static_i_offset: Au(0), block_container_inline_size: Au(0), + block_container_writing_mode: writing_mode, block_container_explicit_block_size: None, absolute_cb: ContainingBlockLink::new(), display_list_building_result: DisplayListBuildingResult::None, diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs index 4d66a76b582..1a32b813488 100644 --- a/components/layout/fragment.rs +++ b/components/layout/fragment.rs @@ -1725,6 +1725,7 @@ impl Fragment { max(block_flow.base.intrinsic_inline_sizes.minimum_inline_size, block_flow.base.intrinsic_inline_sizes.preferred_inline_size); block_flow.base.block_container_inline_size = self.border_box.size.inline; + block_flow.base.block_container_writing_mode = self.style.writing_mode; } SpecificFragmentInfo::ScannedText(ref info) => { // Scanned text fragments will have already had their content inline-sizes assigned @@ -1974,8 +1975,8 @@ impl Fragment { relative_containing_block_size: &LogicalSize, coordinate_system: CoordinateSystem) -> Rect { - // FIXME(pcwalton, #2795): Get the real container size. - let container_size = Size2D::zero(); + // FIXME (mbrubeck): Get the real container size, taking vertical writing modes into account. + let container_size = Size2D(relative_containing_block_size.inline, relative_containing_block_size.block); let border_box = self.border_box.to_physical(self.style.writing_mode, container_size); if coordinate_system == CoordinateSystem::Self && self.establishes_stacking_context() { return Rect(ZERO_POINT, border_box.size) diff --git a/components/layout/inline.rs b/components/layout/inline.rs index a1bc2af0ae2..e5087c4d6a8 100644 --- a/components/layout/inline.rs +++ b/components/layout/inline.rs @@ -1118,6 +1118,7 @@ impl Flow for InlineFlow { debug!("InlineFlow::assign_inline_sizes: floats in: {:?}", self.base.floats); let inline_size = self.base.block_container_inline_size; + let container_mode = self.base.block_container_writing_mode; self.base.position.size.inline = inline_size; { @@ -1137,6 +1138,7 @@ impl Flow for InlineFlow { let kid_base = flow::mut_base(kid); kid_base.block_container_inline_size = inline_size; + kid_base.block_container_writing_mode = container_mode; kid_base.block_container_explicit_block_size = block_container_explicit_block_size; } } diff --git a/tests/ref/basic.list b/tests/ref/basic.list index 9000da9b778..d26f5656428 100644 --- a/tests/ref/basic.list +++ b/tests/ref/basic.list @@ -187,7 +187,7 @@ flaky_cpu == linebreak_simple_a.html linebreak_simple_b.html == outset.html outset_ref.html != outset_blackborder.html blackborder_ref.html # Should be == with expected failure. See #2797 -!= overconstrained_block.html overconstrained_block_ref.html +experimental != overconstrained_block.html overconstrained_block_ref.html == overflow_auto.html overflow_simple_b.html == overflow_scroll.html overflow_simple_b.html == overflow_simple_a.html overflow_simple_b.html @@ -223,6 +223,7 @@ flaky_cpu == linebreak_simple_a.html linebreak_simple_b.html == root_height_a.html root_height_b.html == root_margin_collapse_a.html root_margin_collapse_b.html == root_pseudo_a.html root_pseudo_b.html +experimental == rtl_simple.html rtl_simple_ref.html == setattribute_id_restyle_a.html setattribute_id_restyle_b.html == stacking_context_overflow_a.html stacking_context_overflow_ref.html == stacking_context_overflow_relative_outline_a.html stacking_context_overflow_relative_outline_ref.html diff --git a/tests/ref/rtl_simple.html b/tests/ref/rtl_simple.html new file mode 100644 index 00000000000..eb1d524e2ec --- /dev/null +++ b/tests/ref/rtl_simple.html @@ -0,0 +1,18 @@ + + + + +
+ diff --git a/tests/ref/rtl_simple_ref.html b/tests/ref/rtl_simple_ref.html new file mode 100644 index 00000000000..371e7b94da4 --- /dev/null +++ b/tests/ref/rtl_simple_ref.html @@ -0,0 +1,16 @@ + + + + +
+