layout: Explicitly thread border box dimensions and relative offsets

through display list building.

The old `flow_origin` concept was ill-defined (sometimes the border box
plus the flow origin, sometimes including horizontal margins and
sometimes not, sometimes including relative position and sometimes not),
leading to brittleness and test failures. This commit reworks the logic
to always pass border box origins in during display list building.

components/gfx/display_list/mod.rs

@@ -609,6 +609,20 @@ impl ClippingRegion {
         });
         self
     }
+
+    /// Translates this clipping region by the given vector.
+    #[inline]
+    pub fn translate(&self, delta: &Point2D<Au>) -> ClippingRegion {
+        ClippingRegion {
+            main: self.main.translate(delta),
+            complex: self.complex.iter().map(|complex| {
+                ComplexClippingRegion {
+                    rect: complex.rect.translate(delta),
+                    radii: complex.radii,
+                }
+            }).collect(),
+        }
+    }
 }
 
 /// Metadata attached to each display item. This is useful for performing auxiliary tasks with

components/layout/block.rs

@@ -32,28 +32,27 @@ use context::LayoutContext;
 use css::node_style::StyledNode;
 use display_list_builder::{BlockFlowDisplayListBuilding, FragmentDisplayListBuilding};
 use floats::{ClearType, FloatKind, Floats, PlacementInfo};
-use flow::{AbsolutePositionInfo, BaseFlow, ForceNonfloatedFlag, FlowClass, Flow};
+use flow::{mod, AbsolutePositionInfo, BaseFlow, ForceNonfloatedFlag, FlowClass, Flow};
 use flow::{ImmutableFlowUtils, MutableFlowUtils, PreorderFlowTraversal};
 use flow::{PostorderFlowTraversal, mut_base};
 use flow::{HAS_LEFT_FLOATED_DESCENDANTS, HAS_RIGHT_FLOATED_DESCENDANTS};
 use flow::{IMPACTED_BY_LEFT_FLOATS, IMPACTED_BY_RIGHT_FLOATS};
 use flow::{LAYERS_NEEDED_FOR_DESCENDANTS, NEEDS_LAYER};
 use flow::{IS_ABSOLUTELY_POSITIONED};
 use flow::{CLEARS_LEFT, CLEARS_RIGHT};
-use flow;
-use fragment::{Fragment, FragmentOverflowIterator, SpecificFragmentInfo};
+use fragment::{CoordinateSystem, Fragment, FragmentBorderBoxIterator, SpecificFragmentInfo};
 use incremental::{REFLOW, REFLOW_OUT_OF_FLOW};
 use layout_debug;
 use model::{IntrinsicISizes, MarginCollapseInfo};
 use model::{MaybeAuto, CollapsibleMargins, specified, specified_or_none};
 use table::ColumnComputedInlineSize;
 use wrapper::ThreadSafeLayoutNode;
 
-use geom::{Rect, Size2D};
+use geom::{Point2D, Rect, Size2D};
 use gfx::display_list::{ClippingRegion, DisplayList};
 use serialize::{Encoder, Encodable};
 use servo_msg::compositor_msg::LayerId;
-use servo_util::geometry::{Au, MAX_AU, ZERO_POINT};
+use servo_util::geometry::{Au, MAX_AU};
 use servo_util::logical_geometry::{LogicalPoint, LogicalRect, LogicalSize};
 use servo_util::opts;
 use std::cmp::{max, min};
@@ -1769,23 +1768,40 @@ impl Flow for BlockFlow {
         };
 
         // Compute the origin and clipping rectangle for children.
-        let origin_for_children = if self.fragment.establishes_stacking_context() {
-            ZERO_POINT
+        let relative_offset = relative_offset.to_physical(self.base.writing_mode);
+        let origin_for_children;
+        let clip_in_child_coordinate_system;
+        if self.fragment.establishes_stacking_context() {
+            // We establish a stacking context, so the position of our children is vertically
+            // correct, but has to be adjusted to accommodate horizontal margins. (Note the
+            // calculation involving `position` below and recall that inline-direction flow
+            // positions are relative to the edges of the margin box.)
+            //
+            // FIXME(pcwalton): Is this vertical-writing-direction-safe?
+            let margin = self.fragment.margin.to_physical(self.base.writing_mode);
+            origin_for_children = Point2D(-margin.left, Au(0)) + relative_offset;
+            clip_in_child_coordinate_system =
+                self.base.clip.translate(&-self.base.stacking_relative_position)
         } else {
-            self.base.stacking_relative_position
-        };
-        let clip = self.fragment.clipping_region_for_children(&self.base.clip,
-                                                              &origin_for_children);
+            origin_for_children = self.base.stacking_relative_position + relative_offset;
+            clip_in_child_coordinate_system = self.base.clip.clone()
+        }
+        let stacking_relative_border_box =
+            self.fragment
+                .stacking_relative_border_box(&self.base.stacking_relative_position,
+                                              &self.base
+                                                   .absolute_position_info
+                                                   .relative_containing_block_size,
+                                              CoordinateSystem::Self);
+        let clip = self.fragment.clipping_region_for_children(&clip_in_child_coordinate_system,
+                                                              &stacking_relative_border_box);
 
         // Process children.
-        let writing_mode = self.base.writing_mode;
         for kid in self.base.child_iter() {
             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)
-                    + relative_offset.to_physical(writing_mode);
+                kid_base.stacking_relative_position = origin_for_children +
+                    kid_base.position.start.to_physical(kid_base.writing_mode, container_size);
             }
 
             flow::mut_base(kid).absolute_position_info = absolute_position_info_for_children;
@@ -1832,16 +1848,20 @@ impl Flow for BlockFlow {
 
     fn update_late_computed_inline_position_if_necessary(&mut self, inline_position: Au) {
         if self.base.flags.contains(IS_ABSOLUTELY_POSITIONED) &&
-                self.fragment.style().logical_position().inline_start == LengthOrPercentageOrAuto::Auto &&
-                self.fragment.style().logical_position().inline_end == LengthOrPercentageOrAuto::Auto {
+                self.fragment.style().logical_position().inline_start ==
+                    LengthOrPercentageOrAuto::Auto &&
+                self.fragment.style().logical_position().inline_end ==
+                LengthOrPercentageOrAuto::Auto {
             self.base.position.start.i = inline_position
         }
     }
 
     fn update_late_computed_block_position_if_necessary(&mut self, block_position: Au) {
         if self.base.flags.contains(IS_ABSOLUTELY_POSITIONED) &&
-                self.fragment.style().logical_position().block_start == LengthOrPercentageOrAuto::Auto &&
-                self.fragment.style().logical_position().block_end == LengthOrPercentageOrAuto::Auto {
+                self.fragment.style().logical_position().block_start ==
+                    LengthOrPercentageOrAuto::Auto &&
+                self.fragment.style().logical_position().block_end ==
+                LengthOrPercentageOrAuto::Auto {
             self.base.position.start.b = block_position
         }
     }
@@ -1861,16 +1881,32 @@ impl Flow for BlockFlow {
         self.fragment.compute_overflow()
     }
 
-    fn iterate_through_fragment_overflow(&self, iterator: &mut FragmentOverflowIterator) {
-        if iterator.should_process(&self.fragment) {
-            iterator.process(&self.fragment, self.fragment.compute_overflow());
+    fn iterate_through_fragment_border_boxes(&self,
+                                             iterator: &mut FragmentBorderBoxIterator,
+                                             stacking_context_position: &Point2D<Au>) {
+        if !iterator.should_process(&self.fragment) {
+            return
         }
+
+        iterator.process(&self.fragment,
+                         &self.fragment
+                              .stacking_relative_border_box(&self.base.stacking_relative_position,
+                                                            &self.base
+                                                                 .absolute_position_info
+                                                                 .relative_containing_block_size,
+                                                            CoordinateSystem::Parent)
+                              .translate(stacking_context_position));
     }
 }
 
 impl fmt::Show for BlockFlow {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "{} - {:x}: frag={} ({})", self.class(), self.base.debug_id(), self.fragment, self.base)
+        write!(f,
+               "{} - {:x}: frag={} ({})",
+               self.class(),
+               self.base.debug_id(),
+               self.fragment,
+               self.base)
     }
 }
 

components/layout/construct.rs

@@ -52,9 +52,9 @@ use servo_util::opts;
 use std::collections::DList;
 use std::mem;
 use std::sync::atomic::Relaxed;
-use style::ComputedValues;
 use style::computed_values::{caption_side, display, empty_cells, float, list_style_position};
 use style::computed_values::{position};
+use style::{mod, ComputedValues};
 use sync::Arc;
 use url::Url;
 
@@ -681,7 +681,7 @@ impl<'a> FlowConstructor<'a> {
     /// doesn't render its children, so this just nukes a child's fragments and creates a
     /// `Fragment`.
     fn build_fragments_for_replaced_inline_content(&mut self, node: &ThreadSafeLayoutNode)
-                                               -> ConstructionResult {
+                                                   -> ConstructionResult {
         for kid in node.children() {
             kid.set_flow_construction_result(ConstructionResult::None)
         }
@@ -697,24 +697,42 @@ impl<'a> FlowConstructor<'a> {
                 node.restyle_damage()))
         }
 
+        // If the value of `display` property is not `inline`, then we have a situation like
+        // `<div style="position:absolute">foo bar baz</div>`. The fragments for `foo`, `bar`, and
+        // `baz` had better not be absolutely positioned!
+        let mut style = (*node.style()).clone();
+        if style.get_box().display != display::inline {
+            style = Arc::new(style::make_inline(&*style))
+        }
+
         // If this is generated content, then we need to initialize the accumulator with the
         // fragment corresponding to that content. Otherwise, just initialize with the ordinary
         // fragment that needs to be generated for this inline node.
         let fragment = if node.get_pseudo_element_type() != PseudoElementType::Normal {
-            let fragment_info = SpecificFragmentInfo::UnscannedText(UnscannedTextFragmentInfo::new(node));
-            Fragment::new_from_specific_info(node, fragment_info)
+            let fragment_info =
+                SpecificFragmentInfo::UnscannedText(UnscannedTextFragmentInfo::new(node));
+            Fragment::from_opaque_node_and_style(
+                OpaqueNodeMethods::from_thread_safe_layout_node(node),
+                style,
+                node.restyle_damage(),
+                fragment_info)
         } else {
-            Fragment::new(self, node)
+            Fragment::from_opaque_node_and_style(
+                OpaqueNodeMethods::from_thread_safe_layout_node(node),
+                style,
+                node.restyle_damage(),
+                self.build_specific_fragment_info_for_node(node))
         };
 
         let mut fragments = DList::new();
         fragments.push_back(fragment);
 
-        let construction_item = ConstructionItem::InlineFragments(InlineFragmentsConstructionResult {
-            splits: DList::new(),
-            fragments: fragments,
-            abs_descendants: Descendants::new(),
-        });
+        let construction_item =
+            ConstructionItem::InlineFragments(InlineFragmentsConstructionResult {
+                splits: DList::new(),
+                fragments: fragments,
+                abs_descendants: Descendants::new(),
+            });
         ConstructionResult::ConstructionItem(construction_item)
     }
 
@@ -726,17 +744,19 @@ impl<'a> FlowConstructor<'a> {
             _ => unreachable!()
         };
 
-        let fragment_info = SpecificFragmentInfo::InlineBlock(InlineBlockFragmentInfo::new(block_flow));
+        let fragment_info = SpecificFragmentInfo::InlineBlock(InlineBlockFragmentInfo::new(
+                block_flow));
         let fragment = Fragment::new_from_specific_info(node, fragment_info);
 
         let mut fragment_accumulator = InlineFragmentsAccumulator::from_inline_node(node);
         fragment_accumulator.fragments.push_back(fragment);
 
-        let construction_item = ConstructionItem::InlineFragments(InlineFragmentsConstructionResult {
-            splits: DList::new(),
-            fragments: fragment_accumulator.to_dlist(),
-            abs_descendants: abs_descendants,
-        });
+        let construction_item =
+            ConstructionItem::InlineFragments(InlineFragmentsConstructionResult {
+                splits: DList::new(),
+                fragments: fragment_accumulator.to_dlist(),
+                abs_descendants: abs_descendants,
+            });
         ConstructionResult::ConstructionItem(construction_item)
     }
 
@@ -757,11 +777,12 @@ impl<'a> FlowConstructor<'a> {
         let mut fragment_accumulator = InlineFragmentsAccumulator::from_inline_node(node);
         fragment_accumulator.fragments.push_back(fragment);
 
-        let construction_item = ConstructionItem::InlineFragments(InlineFragmentsConstructionResult {
-            splits: DList::new(),
-            fragments: fragment_accumulator.to_dlist(),
-            abs_descendants: abs_descendants,
-        });
+        let construction_item =
+            ConstructionItem::InlineFragments(InlineFragmentsConstructionResult {
+                splits: DList::new(),
+                fragments: fragment_accumulator.to_dlist(),
+                abs_descendants: abs_descendants,
+            });
         ConstructionResult::ConstructionItem(construction_item)
     }