From 3bb4020ff82514b4cced945448e0542a728b1696 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Mon, 22 Sep 2014 17:34:29 -0700 Subject: [PATCH] layout: Adjust the position of block formatting contexts based on float placement. Improves Reddit considerably. Closes #3456. --- components/layout/block.rs | 43 +++++++++++++++++++-- components/util/geometry.rs | 3 ++ tests/ref/basic.list | 1 + tests/ref/block_formatting_context_a.html | 7 ++++ tests/ref/block_formatting_context_ref.html | 7 ++++ 5 files changed, 58 insertions(+), 3 deletions(-) create mode 100644 tests/ref/block_formatting_context_a.html create mode 100644 tests/ref/block_formatting_context_ref.html diff --git a/components/layout/block.rs b/components/layout/block.rs index a75a2e48274..0d8d4b2902f 100644 --- a/components/layout/block.rs +++ b/components/layout/block.rs @@ -16,7 +16,7 @@ use construct::FlowConstructor; use context::LayoutContext; -use floats::{ClearBoth, ClearLeft, ClearRight, FloatKind, Floats, PlacementInfo}; +use floats::{ClearBoth, ClearLeft, ClearRight, FloatKind, FloatLeft, Floats, PlacementInfo}; use flow::{BaseFlow, BlockFlowClass, FlowClass, Flow, ImmutableFlowUtils}; use flow::{MutableFlowUtils, PreorderFlowTraversal, PostorderFlowTraversal, mut_base}; use flow; @@ -37,7 +37,7 @@ use gfx::display_list::{FloatStackingLevel, PositionedDescendantStackingLevel}; use gfx::display_list::{RootOfStackingContextLevel}; use gfx::render_task::RenderLayer; use servo_msg::compositor_msg::{FixedPosition, LayerId, Scrollable}; -use servo_util::geometry::Au; +use servo_util::geometry::{Au, MAX_AU}; use servo_util::logical_geometry::WritingMode; use servo_util::logical_geometry::{LogicalPoint, LogicalRect, LogicalSize}; use std::cmp::{max, min}; @@ -1463,6 +1463,39 @@ impl BlockFlow { _ => NonformattingContext, } } + + /// Per CSS 2.1 ยง 9.5, block formatting contexts' inline widths and positions are affected by + /// the presence of floats. This is the part of the assign-heights traversal that computes + /// the final inline position and width for such flows. + /// + /// Note that this is part of the assign-block-sizes traversal, not the assign-inline-sizes + /// traversal as one might expect. That is because, in general, float placement cannot occur + /// until heights are assigned. To work around this unfortunate circular dependency, by the + /// time we get here we have already estimated the width of the block formatting context based + /// on the floats we could see at the time of inline-size assignment. The job of this function, + /// therefore, is not only to assign the final size but also to perform the layout again for + /// this block formatting context if our speculation was wrong. + fn assign_inline_position_for_formatting_context(&mut self) { + debug_assert!(self.formatting_context_type() != NonformattingContext); + + let info = PlacementInfo { + size: LogicalSize::new( + self.fragment.style.writing_mode, + self.base.position.size.inline + self.fragment.margin.inline_start_end() + + self.fragment.border_padding.inline_start_end(), + self.fragment.border_box.size.block), + ceiling: self.base.position.start.b, + max_inline_size: MAX_AU, + kind: FloatLeft, + }; + + // Offset our position by whatever displacement is needed to not impact the floats. + let rect = self.base.floats.place_between_floats(&info); + self.base.position.start.i = self.base.position.start.i + rect.start.i; + + // TODO(pcwalton): If the inline-size of this flow is different from the size we estimated + // earlier, lay it out again. + } } impl Flow for BlockFlow { @@ -1626,7 +1659,7 @@ impl Flow for BlockFlow { } } - // Move in from the inline-start border edge + // Move in from the inline-start border edge. let inline_start_content_edge = self.fragment.border_box.start.i + self.fragment.border_padding.inline_start; let padding_and_borders = self.fragment.border_padding.inline_start_end(); let content_inline_size = self.fragment.border_box.size.inline - padding_and_borders; @@ -1652,6 +1685,10 @@ impl Flow for BlockFlow { return true } + if self.formatting_context_type() != NonformattingContext { + self.assign_inline_position_for_formatting_context(); + } + let impacted = self.base.flags.impacted_by_floats(); if impacted { self.assign_block_size(layout_context); diff --git a/components/util/geometry.rs b/components/util/geometry.rs index bc18ce66b8b..0bf7cffe60c 100644 --- a/components/util/geometry.rs +++ b/components/util/geometry.rs @@ -9,6 +9,7 @@ use geom::size::Size2D; use serialize::{Encodable, Encoder}; use std::default::Default; +use std::i32; use std::num::{NumCast, Zero}; use std::fmt; @@ -72,6 +73,8 @@ impl Default for Au { } } +pub static MAX_AU: Au = Au(i32::MAX); + impl> Encodable for Au { fn encode(&self, e: &mut S) -> Result<(), E> { e.emit_f64(to_frac_px(*self)) diff --git a/tests/ref/basic.list b/tests/ref/basic.list index 691ef8b4d12..c67aed95d13 100644 --- a/tests/ref/basic.list +++ b/tests/ref/basic.list @@ -139,3 +139,4 @@ flaky_gpu,flaky_linux == acid2_noscroll.html acid2_ref_broken.html == img_block_maxwidth_a.html img_block_maxwidth_ref.html == img_block_maxwidth_b.html img_block_maxwidth_ref.html == float_clearance_a.html float_clearance_ref.html +== block_formatting_context_a.html block_formatting_context_ref.html diff --git a/tests/ref/block_formatting_context_a.html b/tests/ref/block_formatting_context_a.html new file mode 100644 index 00000000000..241de2e22e4 --- /dev/null +++ b/tests/ref/block_formatting_context_a.html @@ -0,0 +1,7 @@ + + + +
4913
+
RIP Richard Kiel
+ + diff --git a/tests/ref/block_formatting_context_ref.html b/tests/ref/block_formatting_context_ref.html new file mode 100644 index 00000000000..55b4e3a6fb2 --- /dev/null +++ b/tests/ref/block_formatting_context_ref.html @@ -0,0 +1,7 @@ + + + +
4913
+
RIP Richard Kiel
+ +