From b29719e36bdf2867b1391bbc3b017675975e27ae Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Fri, 18 Mar 2016 15:23:08 -0700 Subject: [PATCH] layout: Rewrite the block formatting context/float inline-size speculation code. The old code tried to do the speculation as a single bottom-up pass after intrinsic inline-size calculation, which was unable to handle cases like this:
Foo
Bar
No single bottom-up pass could possibly handle this case, because the inline-size of the float flowing out of the "Foo" block could never make it down to the "Bar" block, where it is needed for speculation. On the pages I tried, this regresses layout performance by 1%-2%. I first noticed this breaking some pages, like the Google SERPs, several months ago. --- components/layout/block.rs | 114 ++---------------- components/layout/flex.rs | 18 +-- components/layout/floats.rs | 110 +++++++++++++++++ components/layout/flow.rs | 107 +++++++++------- components/layout/fragment.rs | 28 +++++ components/layout/layout_thread.rs | 6 + components/layout/sequential.rs | 26 +++- components/layout/table.rs | 8 +- components/layout/table_wrapper.rs | 8 +- components/layout/traversal.rs | 10 +- components/profile/time.rs | 2 + components/profile_traits/time.rs | 1 + tests/wpt/mozilla/meta/MANIFEST.json | 24 ++++ ...g_context_float_inorder_interaction_a.html | 36 ++++++ ...context_float_inorder_interaction_ref.html | 32 +++++ 15 files changed, 346 insertions(+), 184 deletions(-) create mode 100644 tests/wpt/mozilla/tests/css/block_formatting_context_float_inorder_interaction_a.html create mode 100644 tests/wpt/mozilla/tests/css/block_formatting_context_float_inorder_interaction_ref.html diff --git a/components/layout/block.rs b/components/layout/block.rs index 06f41861425..b987313b21f 100644 --- a/components/layout/block.rs +++ b/components/layout/block.rs @@ -33,10 +33,7 @@ use display_list_builder::BlockFlowDisplayListBuilding; use display_list_builder::{BorderPaintingMode, DisplayListBuildState, FragmentDisplayListBuilding}; use euclid::{Point2D, Rect, Size2D}; use floats::{ClearType, FloatKind, Floats, PlacementInfo}; -use flow::{BLOCK_POSITION_IS_STATIC}; -use flow::{CLEARS_LEFT, CLEARS_RIGHT}; -use flow::{HAS_LEFT_FLOATED_DESCENDANTS, HAS_RIGHT_FLOATED_DESCENDANTS}; -use flow::{IMPACTED_BY_LEFT_FLOATS, IMPACTED_BY_RIGHT_FLOATS, INLINE_POSITION_IS_STATIC}; +use flow::{BLOCK_POSITION_IS_STATIC, CLEARS_LEFT, CLEARS_RIGHT, INLINE_POSITION_IS_STATIC}; use flow::{IS_ABSOLUTELY_POSITIONED}; use flow::{ImmutableFlowUtils, LateAbsolutePositionInfo, MutableFlowUtils, OpaqueFlow}; use flow::{NEEDS_LAYER, PostorderFlowTraversal, PreorderFlowTraversal, FragmentationContext}; @@ -497,7 +494,7 @@ pub enum MarginsMayCollapseFlag { } #[derive(PartialEq)] -enum FormattingContextType { +pub enum FormattingContextType { None, Block, Other, @@ -844,7 +841,7 @@ impl BlockFlow { } } - // Assign block-size now for the child if it was impacted by floats and we couldn't + // Assign block-size now for the child if it might have floats in and we couldn't // before. flow::mut_base(kid).floats = floats.clone(); if flow::base(kid).flags.is_float() { @@ -1289,9 +1286,8 @@ impl BlockFlow { } /// Assigns the computed inline-start content edge and inline-size to all the children of this - /// block flow. Also computes whether each child will be impacted by floats. The given - /// `callback`, if supplied, will be called once per child; it is currently used to push down - /// column sizes for tables. + /// block flow. The given `callback`, if supplied, will be called once per child; it is + /// currently used to push down column sizes for tables. /// /// `#[inline(always)]` because this is called only from block or table inline-size assignment /// and the code for block layout is significantly simpler. @@ -1308,34 +1304,8 @@ impl BlockFlow { WritingMode, &mut Au, &mut Au) { - // Keep track of whether floats could impact each child. - let mut inline_start_floats_impact_child = - self.base.flags.contains(IMPACTED_BY_LEFT_FLOATS); - let mut inline_end_floats_impact_child = - self.base.flags.contains(IMPACTED_BY_RIGHT_FLOATS); - let flags = self.base.flags.clone(); - // Remember the inline-sizes of the last left and right floats, if there were any. These - // are used for estimating the inline-sizes of block formatting contexts. (We estimate that - // the inline-size of any block formatting context that we see will be based on the - // inline-size of the containing block as well as the last float seen before it in each - // direction.) - let mut inline_size_of_preceding_left_floats = Au(0); - let mut inline_size_of_preceding_right_floats = Au(0); - if self.formatting_context_type() == FormattingContextType::None { - if inline_start_content_edge > Au(0) { - inline_size_of_preceding_left_floats = - max(self.inline_size_of_preceding_left_floats - inline_start_content_edge, - Au(0)); - } - if inline_end_content_edge > Au(0) { - inline_size_of_preceding_right_floats = - max(self.inline_size_of_preceding_right_floats - inline_end_content_edge, - Au(0)); - } - } - let opaque_self = OpaqueFlow::from_flow(self); // Calculate non-auto block size to pass to children. @@ -1358,29 +1328,6 @@ impl BlockFlow { while let Some((i, kid)) = iterator.next() { flow::mut_base(kid).block_container_explicit_block_size = explicit_content_size; - // Determine float impaction, and update the inline size speculations if necessary. - if flow::base(kid).flags.contains(CLEARS_LEFT) { - inline_start_floats_impact_child = false; - inline_size_of_preceding_left_floats = Au(0); - } - if flow::base(kid).flags.contains(CLEARS_RIGHT) { - inline_end_floats_impact_child = false; - inline_size_of_preceding_right_floats = Au(0); - } - - // Update the speculated inline size if this child is floated. - match flow::base(kid).flags.float_kind() { - float::T::none => {} - float::T::left => { - inline_size_of_preceding_left_floats = inline_size_of_preceding_left_floats + - flow::base(kid).intrinsic_inline_sizes.preferred_inline_size; - } - float::T::right => { - inline_size_of_preceding_right_floats = inline_size_of_preceding_right_floats + - flow::base(kid).intrinsic_inline_sizes.preferred_inline_size; - } - } - // 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; @@ -1399,24 +1346,6 @@ impl BlockFlow { kid_base.block_container_writing_mode = containing_block_mode; } - { - let kid_base = flow::mut_base(kid); - inline_start_floats_impact_child = inline_start_floats_impact_child || - kid_base.flags.contains(HAS_LEFT_FLOATED_DESCENDANTS); - inline_end_floats_impact_child = inline_end_floats_impact_child || - kid_base.flags.contains(HAS_RIGHT_FLOATED_DESCENDANTS); - kid_base.flags.set(IMPACTED_BY_LEFT_FLOATS, inline_start_floats_impact_child); - kid_base.flags.set(IMPACTED_BY_RIGHT_FLOATS, inline_end_floats_impact_child); - } - - if kid.is_block_flow() { - let kid_block = kid.as_mut_block(); - kid_block.inline_size_of_preceding_left_floats = - inline_size_of_preceding_left_floats; - kid_block.inline_size_of_preceding_right_floats = - inline_size_of_preceding_right_floats; - } - // Call the callback to propagate extra inline size information down to the child. This // is currently used for tables. callback(kid, @@ -1445,7 +1374,7 @@ impl BlockFlow { /// Determines the type of formatting context this is. See the definition of /// `FormattingContextType`. - fn formatting_context_type(&self) -> FormattingContextType { + pub fn formatting_context_type(&self) -> FormattingContextType { let style = self.fragment.style(); if style.get_box().float != float::T::none { return FormattingContextType::Other @@ -1528,8 +1457,6 @@ impl BlockFlow { let _scope = layout_debug_scope!("block::bubble_inline_sizes {:x}", self.base.debug_id()); let mut flags = self.base.flags; - flags.remove(HAS_LEFT_FLOATED_DESCENDANTS); - flags.remove(HAS_RIGHT_FLOATED_DESCENDANTS); // Find the maximum inline-size from children. let mut computation = self.fragment.compute_intrinsic_inline_sizes(); @@ -1584,12 +1511,6 @@ impl BlockFlow { left_float_width + right_float_width); self.base.intrinsic_inline_sizes = computation.finish(); - - match self.fragment.style().get_box().float { - float::T::none => {} - float::T::left => flags.insert(HAS_LEFT_FLOATED_DESCENDANTS), - float::T::right => flags.insert(HAS_RIGHT_FLOATED_DESCENDANTS), - } self.base.flags = flags } @@ -1668,23 +1589,15 @@ impl BlockFlow { self.base.block_container_inline_size = LogicalSize::from_physical( self.base.writing_mode, layout_context.shared_context().viewport_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); - self.base.flags.remove(IMPACTED_BY_RIGHT_FLOATS); } // Our inline-size was set to the inline-size of the containing block by the flow's parent. // Now compute the real value. self.propagate_and_compute_used_inline_size(layout_context); - // Formatting contexts are never impacted by floats. + // Now for some speculation. match self.formatting_context_type() { - FormattingContextType::None => {} FormattingContextType::Block => { - self.base.flags.remove(IMPACTED_BY_LEFT_FLOATS); - self.base.flags.remove(IMPACTED_BY_RIGHT_FLOATS); - // We can't actually compute the inline-size of this block now, because floats // might affect it. Speculate that its inline-size is equal to the inline-size // computed above minus the inline-size of the previous left and/or right floats. @@ -1695,14 +1608,11 @@ impl BlockFlow { if self.fragment.style.max_inline_size() == LengthOrPercentageOrNone::None { self.fragment.border_box.size.inline = self.fragment.border_box.size.inline - - self.inline_size_of_preceding_left_floats - - self.inline_size_of_preceding_right_floats; + self.base.speculated_float_placement_in.left - + self.base.speculated_float_placement_in.right; } } - FormattingContextType::Other => { - self.base.flags.remove(IMPACTED_BY_LEFT_FLOATS); - self.base.flags.remove(IMPACTED_BY_RIGHT_FLOATS); - } + FormattingContextType::None | FormattingContextType::Other => {} } } } @@ -1786,7 +1696,7 @@ impl Flow for BlockFlow { self.assign_inline_position_for_formatting_context(); } - if self.base.flags.impacted_by_floats() { + if (self as &Flow).floats_might_flow_through() { self.base.thread_id = parent_thread_id; if self.base.restyle_damage.intersects(REFLOW_OUT_OF_FLOW | REFLOW) { self.assign_block_size(layout_context); @@ -1797,7 +1707,7 @@ impl Flow for BlockFlow { } if is_formatting_context { - // If this is a formatting context and was *not* impacted by floats, then we must + // If this is a formatting context and definitely did not have floats in, then we must // translate the floats past us. let writing_mode = self.base.floats.writing_mode; let delta = self.base.position.size.block; diff --git a/components/layout/flex.rs b/components/layout/flex.rs index dbc01b58178..29df5d4f43f 100644 --- a/components/layout/flex.rs +++ b/components/layout/flex.rs @@ -17,7 +17,6 @@ use flow::INLINE_POSITION_IS_STATIC; use flow::IS_ABSOLUTELY_POSITIONED; use flow::ImmutableFlowUtils; use flow::{Flow, FlowClass, OpaqueFlow}; -use flow::{HAS_LEFT_FLOATED_DESCENDANTS, HAS_RIGHT_FLOATED_DESCENDANTS}; use fragment::{Fragment, FragmentBorderBoxIterator, Overflow}; use gfx::display_list::{StackingContext, StackingContextId}; use incremental::{REFLOW, REFLOW_OUT_OF_FLOW}; @@ -26,7 +25,7 @@ use model::MaybeAuto; use model::{IntrinsicISizes}; use std::cmp::max; use std::sync::Arc; -use style::computed_values::{flex_direction, float}; +use style::computed_values::flex_direction; use style::logical_geometry::LogicalSize; use style::properties::style_structs; use style::properties::{ComputedValues, TComputedValues}; @@ -324,25 +323,10 @@ impl Flow for FlexFlow { // TODO(zentner): We need to re-order the items at some point. However, all the operations // here ignore order, so we can afford to do it later, if necessary. - // `flex item`s (our children) cannot be floated. Furthermore, they all establish BFC's. - // Therefore, we do not have to handle any floats here. - - let mut flags = self.block_flow.base.flags; - flags.remove(HAS_LEFT_FLOATED_DESCENDANTS); - flags.remove(HAS_RIGHT_FLOATED_DESCENDANTS); - match self.main_mode { Mode::Inline => self.inline_mode_bubble_inline_sizes(), Mode::Block => self.block_mode_bubble_inline_sizes() } - - // Although our children can't be floated, we can. - match self.block_flow.fragment.style().get_box().float { - float::T::none => {} - float::T::left => flags.insert(HAS_LEFT_FLOATED_DESCENDANTS), - float::T::right => flags.insert(HAS_RIGHT_FLOATED_DESCENDANTS), - } - self.block_flow.base.flags = flags } fn assign_inline_sizes(&mut self, layout_context: &LayoutContext) { diff --git a/components/layout/floats.rs b/components/layout/floats.rs index 4ef48fe8a7a..e3ae4716e53 100644 --- a/components/layout/floats.rs +++ b/components/layout/floats.rs @@ -3,6 +3,8 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use app_units::Au; +use block::FormattingContextType; +use flow::{self, CLEARS_LEFT, CLEARS_RIGHT, Flow, ImmutableFlowUtils}; use persistent_list::PersistentList; use std::cmp::{max, min}; use std::fmt; @@ -412,3 +414,111 @@ impl Floats { clearance } } + +/// The speculated inline sizes of floats flowing through or around a flow (depending on whether +/// the flow is a block formatting context). These speculations are always *upper bounds*; the +/// actual inline sizes might be less. Note that this implies that a speculated value of zero is a +/// guarantee that there will be no floats on that side. +/// +/// This is used for two purposes: (a) determining whether we can lay out blocks in parallel; (b) +/// guessing the inline-sizes of block formatting contexts in an effort to lay them out in +/// parallel. +#[derive(Copy, Clone)] +pub struct SpeculatedFloatPlacement { + /// The estimated inline size (an upper bound) of the left floats flowing through this flow. + pub left: Au, + /// The estimated inline size (an upper bound) of the right floats flowing through this flow. + pub right: Au, +} + +impl fmt::Debug for SpeculatedFloatPlacement { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "L {:?} R {:?}", self.left, self.right) + } +} + +impl SpeculatedFloatPlacement { + /// Returns a `SpeculatedFloatPlacement` objects with both left and right speculated inline + /// sizes initialized to zero. + pub fn zero() -> SpeculatedFloatPlacement { + SpeculatedFloatPlacement { + left: Au(0), + right: Au(0), + } + } + + /// Given the speculated inline size of the floats out for the inorder predecessor of this + /// flow, computes the speculated inline size of the floats flowing in. + pub fn compute_floats_in(&mut self, flow: &mut Flow) { + let base_flow = flow::base(flow); + if base_flow.flags.contains(CLEARS_LEFT) { + self.left = Au(0) + } + if base_flow.flags.contains(CLEARS_RIGHT) { + self.right = Au(0) + } + } + + /// Given the speculated inline size of the floats in for this flow, computes the speculated + /// inline size of the floats out for this flow. + pub fn compute_floats_out(&mut self, flow: &mut Flow) { + if flow.is_block_like() { + let block_flow = flow.as_block(); + if block_flow.formatting_context_type() != FormattingContextType::None { + *self = block_flow.base.speculated_float_placement_in; + + if self.left > Au(0) || self.right > Au(0) { + let speculated_inline_content_edge_offsets = + block_flow.fragment.guess_inline_content_edge_offsets(); + if self.left > Au(0) { + self.left = self.left + speculated_inline_content_edge_offsets.start + } + if self.right > Au(0) { + self.right = self.right + speculated_inline_content_edge_offsets.end + } + } + } + } + + let base_flow = flow::base(flow); + match base_flow.flags.float_kind() { + float::T::none => {} + float::T::left => { + self.left = self.left + base_flow.intrinsic_inline_sizes.preferred_inline_size + } + float::T::right => { + self.right = self.right + base_flow.intrinsic_inline_sizes.preferred_inline_size + } + } + } + + /// Given a flow, computes the speculated inline size of the floats in of its first child. + pub fn compute_floats_in_for_first_child(parent_flow: &mut Flow) -> SpeculatedFloatPlacement { + if !parent_flow.is_block_like() { + return flow::base(parent_flow).speculated_float_placement_in + } + + let parent_block_flow = parent_flow.as_block(); + if parent_block_flow.formatting_context_type() != FormattingContextType::None { + return SpeculatedFloatPlacement::zero() + } + + let mut placement = parent_block_flow.base.speculated_float_placement_in; + let speculated_inline_content_edge_offsets = + parent_block_flow.fragment.guess_inline_content_edge_offsets(); + + if placement.left > speculated_inline_content_edge_offsets.start { + placement.left = placement.left - speculated_inline_content_edge_offsets.start + } else { + placement.left = Au(0) + }; + if placement.right > speculated_inline_content_edge_offsets.end { + placement.right = placement.right - speculated_inline_content_edge_offsets.end + } else { + placement.right = Au(0) + }; + + placement + } +} + diff --git a/components/layout/flow.rs b/components/layout/flow.rs index dc434ce03f2..591db584233 100644 --- a/components/layout/flow.rs +++ b/components/layout/flow.rs @@ -26,11 +26,11 @@ //! similar methods. use app_units::Au; -use block::BlockFlow; +use block::{BlockFlow, FormattingContextType}; use context::LayoutContext; use display_list_builder::DisplayListBuildState; use euclid::{Point2D, Rect, Size2D}; -use floats::Floats; +use floats::{Floats, SpeculatedFloatPlacement}; use flow_list::{FlowList, FlowListIterator, MutFlowListIterator}; use flow_ref::{self, FlowRef, WeakFlowRef}; use fragment::{Fragment, FragmentBorderBoxIterator, Overflow, SpecificFragmentInfo}; @@ -231,8 +231,8 @@ pub trait Flow: fmt::Debug + Sync + Send + 'static { fn place_float_if_applicable<'a>(&mut self, _: &'a LayoutContext<'a>) {} /// Assigns block-sizes in-order; or, if this is a float, places the float. The default - /// implementation simply assigns block-sizes if this flow is impacted by floats. Returns true - /// if this child was impacted by floats or false otherwise. + /// implementation simply assigns block-sizes if this flow might have floats in. Returns true + /// if it was determined that this child might have had floats in or false otherwise. /// /// `parent_thread_id` is the thread ID of the parent. This is used for the layout tinting /// debug mode; if the block size of this flow was determined by its parent, we should treat @@ -241,16 +241,16 @@ pub trait Flow: fmt::Debug + Sync + Send + 'static { layout_context: &'a LayoutContext<'a>, parent_thread_id: u8) -> bool { - let impacted = base(self).flags.impacted_by_floats(); - if impacted { + let might_have_floats_in_or_out = base(self).might_have_floats_in() || + base(self).might_have_floats_out(); + if might_have_floats_in_or_out { mut_base(self).thread_id = parent_thread_id; self.assign_block_size(layout_context); mut_base(self).restyle_damage.remove(REFLOW_OUT_OF_FLOW | REFLOW); } - impacted + might_have_floats_in_or_out } - /// Calculate and set overflow for current flow. /// /// CSS Section 11.1 /// This is the union of rectangles of the flows for which we define the @@ -508,6 +508,10 @@ pub trait ImmutableFlowUtils { /// Dumps the flow tree for debugging into the given PrintTree. fn print_with_tree(self, print_tree: &mut PrintTree); + + /// Returns true if floats might flow through this flow, as determined by the float placement + /// speculation pass. + fn floats_might_flow_through(self) -> bool; } pub trait MutableFlowUtils { @@ -559,7 +563,7 @@ pub trait MutableOwnedFlowUtils { absolute_descendants: &mut AbsoluteDescendants); } -#[derive(RustcEncodable, PartialEq, Debug)] +#[derive(Copy, Clone, RustcEncodable, PartialEq, Debug)] pub enum FlowClass { Block, Inline, @@ -576,6 +580,17 @@ pub enum FlowClass { Flex, } +impl FlowClass { + fn is_block_like(self) -> bool { + match self { + FlowClass::Block | FlowClass::ListItem | FlowClass::Table | FlowClass::TableRowGroup | + FlowClass::TableRow | FlowClass::TableCaption | FlowClass::TableCell | + FlowClass::TableWrapper => true, + _ => false, + } + } +} + /// A top-down traversal. pub trait PreorderFlowTraversal { /// The operation to perform. Return true to continue or false to stop. @@ -615,21 +630,6 @@ pub trait InorderFlowTraversal { bitflags! { #[doc = "Flags used in flows."] flags FlowFlags: u32 { - // floated descendants flags - #[doc = "Whether this flow has descendants that float left in the same block formatting"] - #[doc = "context."] - const HAS_LEFT_FLOATED_DESCENDANTS = 0b0000_0000_0000_0000_0001, - #[doc = "Whether this flow has descendants that float right in the same block formatting"] - #[doc = "context."] - const HAS_RIGHT_FLOATED_DESCENDANTS = 0b0000_0000_0000_0000_0010, - #[doc = "Whether this flow is impacted by floats to the left in the same block formatting"] - #[doc = "context (i.e. its height depends on some prior flows with `float: left`)."] - const IMPACTED_BY_LEFT_FLOATS = 0b0000_0000_0000_0000_0100, - #[doc = "Whether this flow is impacted by floats to the right in the same block"] - #[doc = "formatting context (i.e. its height depends on some prior flows with `float:"] - #[doc = "right`)."] - const IMPACTED_BY_RIGHT_FLOATS = 0b0000_0000_0000_0000_1000, - // text align flags #[doc = "Whether this flow must have its own layer. Even if this flag is not set, it might"] #[doc = "get its own layer if it's deemed to be likely to overlap flows with their own"] @@ -704,20 +704,6 @@ impl FlowFlags { self.insert(other & HAS_FLOATED_DESCENDANTS_BITMASK); } - #[inline] - pub fn impacted_by_floats(&self) -> bool { - self.contains(IMPACTED_BY_LEFT_FLOATS) || self.contains(IMPACTED_BY_RIGHT_FLOATS) - } - - #[inline] - pub fn set(&mut self, flags: FlowFlags, value: bool) { - if value { - self.insert(flags); - } else { - self.remove(flags); - } - } - #[inline] pub fn float_kind(&self) -> float::T { if self.contains(FLOATS_LEFT) { @@ -910,6 +896,12 @@ pub struct BaseFlow { /// The floats next to this flow. pub floats: Floats, + /// Metrics for floats in computed during the float metrics speculation phase. + pub speculated_float_placement_in: SpeculatedFloatPlacement, + + /// Metrics for floats out computed during the float metrics speculation phase. + pub speculated_float_placement_out: SpeculatedFloatPlacement, + /// The collapsible margins for this flow, if any. pub collapsible_margins: CollapsibleMargins, @@ -994,9 +986,11 @@ impl fmt::Debug for BaseFlow { }; write!(f, - "sc={:?} pos={:?}, overflow={:?}{}{}{}", + "sc={:?} pos={:?}, floatspec-in={:?}, floatspec-out={:?}, overflow={:?}{}{}{}", self.stacking_context_id, self.position, + self.speculated_float_placement_in, + self.speculated_float_placement_out, self.overflow, child_count_string, absolute_descendants_string, @@ -1122,6 +1116,8 @@ impl BaseFlow { collapsible_margins: CollapsibleMargins::new(), stacking_relative_position: Point2D::zero(), abs_descendants: AbsoluteDescendants::new(), + speculated_float_placement_in: SpeculatedFloatPlacement::zero(), + speculated_float_placement_out: SpeculatedFloatPlacement::zero(), block_container_inline_size: Au(0), block_container_writing_mode: writing_mode, block_container_explicit_block_size: None, @@ -1172,17 +1168,24 @@ impl BaseFlow { kid.collect_stacking_contexts(parent_id, contexts); } } + + #[inline] + pub fn might_have_floats_in(&self) -> bool { + self.speculated_float_placement_in.left > Au(0) || + self.speculated_float_placement_in.right > Au(0) + } + + #[inline] + pub fn might_have_floats_out(&self) -> bool { + self.speculated_float_placement_out.left > Au(0) || + self.speculated_float_placement_out.right > Au(0) + } } impl<'a> ImmutableFlowUtils for &'a Flow { /// Returns true if this flow is a block flow or subclass thereof. fn is_block_like(self) -> bool { - match self.class() { - FlowClass::Block | FlowClass::ListItem | FlowClass::Table | FlowClass::TableRowGroup | - FlowClass::TableRow | FlowClass::TableCaption | FlowClass::TableCell | - FlowClass::TableWrapper => true, - _ => false, - } + self.class().is_block_like() } /// Returns true if this flow is a proper table child. @@ -1378,6 +1381,19 @@ impl<'a> ImmutableFlowUtils for &'a Flow { } print_tree.end_level(); } + + fn floats_might_flow_through(self) -> bool { + if !base(self).might_have_floats_in() && !base(self).might_have_floats_out() { + return false + } + if self.is_root() { + return false + } + if !self.is_block_like() { + return true + } + self.as_block().formatting_context_type() == FormattingContextType::None + } } impl<'a> MutableFlowUtils for &'a mut Flow { @@ -1563,3 +1579,4 @@ impl OpaqueFlow { } } } + diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs index 227b492b038..5bc8c506d55 100644 --- a/components/layout/fragment.rs +++ b/components/layout/fragment.rs @@ -1027,6 +1027,24 @@ impl Fragment { } } + /// Returns a guess as to the distances from the margin edge of this fragment to its content + /// in the inline direction. This will generally be correct unless percentages are involved. + /// + /// This is used for the float placement speculation logic. + pub fn guess_inline_content_edge_offsets(&self) -> SpeculatedInlineContentEdgeOffsets { + let logical_margin = self.style.logical_margin(); + let logical_padding = self.style.logical_padding(); + let border_width = self.border_width(); + SpeculatedInlineContentEdgeOffsets { + start: MaybeAuto::from_style(logical_margin.inline_start, Au(0)).specified_or_zero() + + model::specified(logical_padding.inline_start, Au(0)) + + border_width.inline_start, + end: MaybeAuto::from_style(logical_margin.inline_end, Au(0)).specified_or_zero() + + model::specified(logical_padding.inline_end, Au(0)) + + border_width.inline_end, + } + } + pub fn calculate_line_height(&self, layout_context: &LayoutContext) -> Au { let font_style = self.style.get_font_arc(); let font_metrics = text::font_metrics_for_style(&mut layout_context.font_context(), font_style); @@ -2634,3 +2652,13 @@ bitflags! { const HAS_LAYER = 0x01, } } + +/// Specified distances from the margin edge of a block to its content in the inline direction. +/// These are returned by `guess_inline_content_edge_offsets()` and are used in the float placement +/// speculation logic. +#[derive(Copy, Clone, Debug)] +pub struct SpeculatedInlineContentEdgeOffsets { + pub start: Au, + pub end: Au, +} + diff --git a/components/layout/layout_thread.rs b/components/layout/layout_thread.rs index df555b1d485..e6412348e6c 100644 --- a/components/layout/layout_thread.rs +++ b/components/layout/layout_thread.rs @@ -1345,6 +1345,12 @@ impl LayoutThread { self.time_profiler_chan.clone(), || sequential::resolve_generated_content(&mut root_flow, &layout_context)); + // Guess float placement. + profile(time::ProfilerCategory::LayoutFloatPlacementSpeculation, + self.profiler_metadata(), + self.time_profiler_chan.clone(), + || sequential::guess_float_placement(flow_ref::deref_mut(&mut root_flow))); + // Perform the primary layout passes over the flow tree to compute the locations of all // the boxes. if flow::base(&*root_flow).restyle_damage.intersects(REFLOW | REFLOW_OUT_OF_FLOW) { diff --git a/components/layout/sequential.rs b/components/layout/sequential.rs index dbf469db082..f63e2a8bfcd 100644 --- a/components/layout/sequential.rs +++ b/components/layout/sequential.rs @@ -8,17 +8,18 @@ use app_units::Au; use context::{LayoutContext, SharedLayoutContext}; use display_list_builder::DisplayListBuildState; use euclid::point::Point2D; +use floats::SpeculatedFloatPlacement; use flow::{PostorderFlowTraversal, PreorderFlowTraversal}; use flow::{self, Flow, ImmutableFlowUtils, InorderFlowTraversal, MutableFlowUtils}; use flow_ref::{self, FlowRef}; use fragment::FragmentBorderBoxIterator; use generated_content::ResolveGeneratedContent; use gfx::display_list::{DisplayListEntry, StackingContext}; -use incremental::STORE_OVERFLOW; +use incremental::{REFLOW, STORE_OVERFLOW}; use style::dom::TNode; use style::traversal::DomTraversalContext; -use traversal::{AssignBSizes, AssignISizes}; -use traversal::{BubbleISizes, BuildDisplayList, ComputeAbsolutePositions, PostorderNodeMutTraversal}; +use traversal::{AssignBSizes, AssignISizes, BubbleISizes, BuildDisplayList}; +use traversal::{ComputeAbsolutePositions, PostorderNodeMutTraversal}; use util::opts; pub use style::sequential::traverse_dom; @@ -133,3 +134,22 @@ pub fn store_overflow(layout_context: &LayoutContext, flow: &mut Flow) { flow::mut_base(flow).restyle_damage.remove(STORE_OVERFLOW); } +/// Guesses how much inline size will be taken up by floats on the left and right sides of the +/// given flow. This is needed to speculatively calculate the inline sizes of block formatting +/// contexts. The speculation typically succeeds, but if it doesn't we have to lay it out again. +pub fn guess_float_placement(flow: &mut Flow) { + if !flow::base(flow).restyle_damage.intersects(REFLOW) { + return + } + + let mut floats_in = SpeculatedFloatPlacement::compute_floats_in_for_first_child(flow); + for kid in flow::mut_base(flow).child_iter() { + floats_in.compute_floats_in(kid); + flow::mut_base(kid).speculated_float_placement_in = floats_in; + guess_float_placement(kid); + floats_in = flow::base(kid).speculated_float_placement_out; + } + floats_in.compute_floats_out(flow); + flow::mut_base(flow).speculated_float_placement_out = floats_in +} + diff --git a/components/layout/table.rs b/components/layout/table.rs index bd6b6bee6ed..7efea169a32 100644 --- a/components/layout/table.rs +++ b/components/layout/table.rs @@ -12,8 +12,8 @@ use block::{ISizeConstraintInput, ISizeConstraintSolution}; use context::LayoutContext; use display_list_builder::{BlockFlowDisplayListBuilding, BorderPaintingMode, DisplayListBuildState}; use euclid::Point2D; -use flow::{BaseFlow, IMPACTED_BY_RIGHT_FLOATS, ImmutableFlowUtils, OpaqueFlow}; -use flow::{self, EarlyAbsolutePositionInfo, Flow, FlowClass, IMPACTED_BY_LEFT_FLOATS}; +use flow::{BaseFlow, EarlyAbsolutePositionInfo, Flow, FlowClass, ImmutableFlowUtils, OpaqueFlow}; +use flow::{self}; use flow_list::MutFlowListIterator; use fragment::{Fragment, FragmentBorderBoxIterator, Overflow}; use gfx::display_list::{StackingContext, StackingContextId}; @@ -392,10 +392,6 @@ impl Flow for TableFlow { } } - // As tables are always wrapped inside a table wrapper, they are never impacted by floats. - self.block_flow.base.flags.remove(IMPACTED_BY_LEFT_FLOATS); - self.block_flow.base.flags.remove(IMPACTED_BY_RIGHT_FLOATS); - let column_computed_inline_sizes = &self.column_computed_inline_sizes; let collapsed_inline_direction_border_widths_for_table = &self.collapsed_inline_direction_border_widths_for_table; diff --git a/components/layout/table_wrapper.rs b/components/layout/table_wrapper.rs index e543f981127..cdb88ccb428 100644 --- a/components/layout/table_wrapper.rs +++ b/components/layout/table_wrapper.rs @@ -20,8 +20,7 @@ use context::LayoutContext; use display_list_builder::DisplayListBuildState; use euclid::Point2D; use floats::FloatKind; -use flow::{Flow, FlowClass, ImmutableFlowUtils}; -use flow::{IMPACTED_BY_LEFT_FLOATS, IMPACTED_BY_RIGHT_FLOATS, INLINE_POSITION_IS_STATIC, OpaqueFlow}; +use flow::{Flow, FlowClass, ImmutableFlowUtils, INLINE_POSITION_IS_STATIC, OpaqueFlow}; use fragment::{Fragment, FragmentBorderBoxIterator, Overflow}; use gfx::display_list::{StackingContext, StackingContextId}; use model::MaybeAuto; @@ -336,11 +335,6 @@ impl Flow for TableWrapperFlow { } }).collect::>(); - // Table wrappers are essentially block formatting contexts and are therefore never - // impacted by floats. - self.block_flow.base.flags.remove(IMPACTED_BY_LEFT_FLOATS); - self.block_flow.base.flags.remove(IMPACTED_BY_RIGHT_FLOATS); - // Our inline-size was set to the inline-size of the containing block by the flow's parent. // Now compute the real value. let containing_block_inline_size = self.block_flow.base.block_container_inline_size; diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index 2413348148a..586a71029e0 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -7,8 +7,8 @@ use construct::FlowConstructor; use context::{LayoutContext, SharedLayoutContext}; use display_list_builder::DisplayListBuildState; -use flow::{PostorderFlowTraversal, PreorderFlowTraversal}; -use flow::{self, Flow, CAN_BE_FRAGMENTED}; +use flow::{CAN_BE_FRAGMENTED, Flow, ImmutableFlowUtils, PostorderFlowTraversal}; +use flow::{PreorderFlowTraversal, self}; use gfx::display_list::OpaqueNode; use incremental::{BUBBLE_ISIZES, REFLOW, REFLOW_OUT_OF_FLOW, REPAINT, RestyleDamage}; use std::mem; @@ -179,10 +179,12 @@ pub struct AssignBSizes<'a> { impl<'a> PostorderFlowTraversal for AssignBSizes<'a> { #[inline] fn process(&self, flow: &mut Flow) { - // Can't do anything with flows impacted by floats until we reach their inorder parent. + // Can't do anything with anything that floats might flow through until we reach their + // inorder parent. + // // NB: We must return without resetting the restyle bits for these, as we haven't actually // reflowed anything! - if flow::base(flow).flags.impacted_by_floats() { + if flow.floats_might_flow_through() { return } diff --git a/components/profile/time.rs b/components/profile/time.rs index 9b4de01cf57..22b917454d6 100644 --- a/components/profile/time.rs +++ b/components/profile/time.rs @@ -59,6 +59,7 @@ impl Formattable for ProfilerCategory { ProfilerCategory::LayoutNonIncrementalReset | ProfilerCategory::LayoutGeneratedContent | ProfilerCategory::LayoutDisplayListSorting | + ProfilerCategory::LayoutFloatPlacementSpeculation | ProfilerCategory::LayoutMain | ProfilerCategory::LayoutStoreOverflow | ProfilerCategory::LayoutDispListBuild | @@ -83,6 +84,7 @@ impl Formattable for ProfilerCategory { ProfilerCategory::LayoutDamagePropagate => "Damage Propagation", ProfilerCategory::LayoutDisplayListSorting => "Sorting Display List", ProfilerCategory::LayoutGeneratedContent => "Generated Content Resolution", + ProfilerCategory::LayoutFloatPlacementSpeculation => "Float Placement Speculation", ProfilerCategory::LayoutMain => "Primary Layout Pass", ProfilerCategory::LayoutStoreOverflow => "Store Overflow", ProfilerCategory::LayoutParallelWarmup => "Parallel Warmup", diff --git a/components/profile_traits/time.rs b/components/profile_traits/time.rs index 19deca44e01..8a673d4db0d 100644 --- a/components/profile_traits/time.rs +++ b/components/profile_traits/time.rs @@ -49,6 +49,7 @@ pub enum ProfilerCategory { LayoutDamagePropagate, LayoutGeneratedContent, LayoutDisplayListSorting, + LayoutFloatPlacementSpeculation, LayoutMain, LayoutStoreOverflow, LayoutParallelWarmup, diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index 73d57a5c311..5f653e5a0e1 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -564,6 +564,18 @@ "url": "/_mozilla/css/block_formatting_context_containing_floats_a.html" } ], + "css/block_formatting_context_float_inorder_interaction_a.html": [ + { + "path": "css/block_formatting_context_float_inorder_interaction_a.html", + "references": [ + [ + "/_mozilla/css/block_formatting_context_float_inorder_interaction_ref.html", + "==" + ] + ], + "url": "/_mozilla/css/block_formatting_context_float_inorder_interaction_a.html" + } + ], "css/block_formatting_context_float_placement_a.html": [ { "path": "css/block_formatting_context_float_placement_a.html", @@ -6854,6 +6866,18 @@ "url": "/_mozilla/css/block_formatting_context_containing_floats_a.html" } ], + "css/block_formatting_context_float_inorder_interaction_a.html": [ + { + "path": "css/block_formatting_context_float_inorder_interaction_a.html", + "references": [ + [ + "/_mozilla/css/block_formatting_context_float_inorder_interaction_ref.html", + "==" + ] + ], + "url": "/_mozilla/css/block_formatting_context_float_inorder_interaction_a.html" + } + ], "css/block_formatting_context_float_placement_a.html": [ { "path": "css/block_formatting_context_float_placement_a.html", diff --git a/tests/wpt/mozilla/tests/css/block_formatting_context_float_inorder_interaction_a.html b/tests/wpt/mozilla/tests/css/block_formatting_context_float_inorder_interaction_a.html new file mode 100644 index 00000000000..52e884598ff --- /dev/null +++ b/tests/wpt/mozilla/tests/css/block_formatting_context_float_inorder_interaction_a.html @@ -0,0 +1,36 @@ + + + + + + + +
+
Mimi
+
+
+
Mimi
+
+ + + + diff --git a/tests/wpt/mozilla/tests/css/block_formatting_context_float_inorder_interaction_ref.html b/tests/wpt/mozilla/tests/css/block_formatting_context_float_inorder_interaction_ref.html new file mode 100644 index 00000000000..d03f614f193 --- /dev/null +++ b/tests/wpt/mozilla/tests/css/block_formatting_context_float_inorder_interaction_ref.html @@ -0,0 +1,32 @@ + + + + + + +
Mimi
+
Mimi
+ + + + +