From 73d0ceac54d3af4a8eb6339eea3383383a557edc Mon Sep 17 00:00:00 2001 From: S Pradeep Kumar Date: Mon, 27 Jan 2014 15:02:56 +0900 Subject: [PATCH] Add more documentation for layout. + Annotate functions with CSS Reference sections. + Clean up comments and whitespace. --- src/components/main/layout/block.rs | 84 +++++++++++++++++++++-------- src/components/main/layout/box_.rs | 16 ++++-- src/components/main/layout/flow.rs | 17 ++++-- 3 files changed, 85 insertions(+), 32 deletions(-) diff --git a/src/components/main/layout/block.rs b/src/components/main/layout/block.rs index 36898c91cd2d..44f8c31bf0eb 100644 --- a/src/components/main/layout/block.rs +++ b/src/components/main/layout/block.rs @@ -136,8 +136,8 @@ impl BlockFlow { right_margin: MaybeAuto, available_width: Au) -> (Au, Au, Au) { - // If width is not 'auto', and width + margins > available_width, all 'auto' margins are - // treated as 0. + // If width is not 'auto', and width + margins > available_width, all + // 'auto' margins are treated as 0. let (left_margin, right_margin) = match width { Auto => (left_margin, right_margin), Specified(width) => { @@ -152,34 +152,46 @@ impl BlockFlow { } }; - //Invariant: left_margin_Au + width_Au + right_margin_Au == available_width + // Invariant: left_margin_Au + width_Au + right_margin_Au == available_width let (left_margin_Au, width_Au, right_margin_Au) = match (left_margin, width, right_margin) { - //If all have a computed value other than 'auto', the system is over-constrained and we need to discard a margin. - //if direction is ltr, ignore the specified right margin and solve for it. If it is rtl, ignore the specified - //left margin. FIXME(eatkinson): this assumes the direction is ltr - (Specified(margin_l), Specified(width), Specified(_margin_r)) => (margin_l, width, available_width - (margin_l + width )), - - //If exactly one value is 'auto', solve for it - (Auto, Specified(width), Specified(margin_r)) => (available_width - (width + margin_r), width, margin_r), - (Specified(margin_l), Auto, Specified(margin_r)) => (margin_l, available_width - (margin_l + margin_r), margin_r), - (Specified(margin_l), Specified(width), Auto) => (margin_l, width, available_width - (margin_l + width)), - - //If width is set to 'auto', any other 'auto' value becomes '0', and width is solved for - (Auto, Auto, Specified(margin_r)) => (Au::new(0), available_width - margin_r, margin_r), - (Specified(margin_l), Auto, Auto) => (margin_l, available_width - margin_l, Au::new(0)), - (Auto, Auto, Auto) => (Au::new(0), available_width, Au::new(0)), - - //If left and right margins are auto, they become equal + // If all have a computed value other than 'auto', the system is + // over-constrained and we need to discard a margin. + // If direction is ltr, ignore the specified right margin and + // solve for it. + // If it is rtl, ignore the specified left margin. + // FIXME(eatkinson): this assumes the direction is ltr + (Specified(margin_l), Specified(width), Specified(_margin_r)) => + (margin_l, width, available_width - (margin_l + width )), + + // If exactly one value is 'auto', solve for it + (Auto, Specified(width), Specified(margin_r)) => + (available_width - (width + margin_r), width, margin_r), + (Specified(margin_l), Auto, Specified(margin_r)) => + (margin_l, available_width - (margin_l + margin_r), margin_r), + (Specified(margin_l), Specified(width), Auto) => + (margin_l, width, available_width - (margin_l + width)), + + // If width is set to 'auto', any other 'auto' value becomes '0', + // and width is solved for + (Auto, Auto, Specified(margin_r)) => + (Au::new(0), available_width - margin_r, margin_r), + (Specified(margin_l), Auto, Auto) => + (margin_l, available_width - margin_l, Au::new(0)), + (Auto, Auto, Auto) => + (Au::new(0), available_width, Au::new(0)), + + // If left and right margins are auto, they become equal (Auto, Specified(width), Auto) => { let margin = (available_width - width).scale_by(0.5); (margin, width, margin) } }; - //return values in same order as params + // Return values in same order as params. (width_Au, left_margin_Au, right_margin_Au) } + // Return (content width, left margin, right, margin) fn compute_block_margins(&self, box_: &Box, remaining_width: Au, available_width: Au) -> (Au, Au, Au) { let style = box_.style(); @@ -194,6 +206,7 @@ impl BlockFlow { maybe_margin_right, available_width); + // CSS Section 10.4: Minimum and Maximum widths // If the tentative used width is greater than 'max-width', width should be recalculated, // but this time using the computed value of 'max-width' as the computed value for 'width'. let (width, margin_left, margin_right) = { @@ -223,6 +236,7 @@ impl BlockFlow { return (width, margin_left, margin_right); } + // CSS Section 10.3.5 fn compute_float_margins(&self, box_: &Box, remaining_width: Au) -> (Au, Au, Au) { let style = box_.style(); let margin_left = MaybeAuto::from_style(style.Margin.margin_left, @@ -243,6 +257,7 @@ impl BlockFlow { fn assign_height_block_base(&mut self, ctx: &mut LayoutContext, inorder: bool) { let mut cur_y = Au::new(0); let mut clearance = Au::new(0); + // Offset to content edge of box_ let mut top_offset = Au::new(0); let mut bottom_offset = Au::new(0); let mut left_offset = Au::new(0); @@ -280,7 +295,15 @@ impl BlockFlow { } } + // The amount of margin that we can potentially collapse with let mut collapsible = Au::new(0); + // How much to move up by to get to the beginning of + // current kid flow. + // Example: if previous sibling's margin-bottom is 20px and your + // margin-top is 12px, the collapsed margin will be 20px. Since cur_y + // will be at the bottom margin edge of the previous sibling, we have + // to move up by 12px to get to our top margin edge. So, `collapsing` + // will be set to 12px let mut collapsing = Au::new(0); let mut margin_top = Au::new(0); let mut margin_bottom = Au::new(0); @@ -300,7 +323,9 @@ impl BlockFlow { margin_bottom = box_.margin.get().bottom; } + // At this point, cur_y is at the content edge of the flow's box_ for kid in self.base.child_iter() { + // At this point, cur_y is at bottom margin edge of previous kid kid.collapse_margins(top_margin_collapsible, &mut first_in_flow, &mut margin_top, @@ -310,8 +335,11 @@ impl BlockFlow { let child_node = flow::mut_base(*kid); cur_y = cur_y - collapsing; + // At this point, after moving up by `collapsing`, cur_y is at the + // top margin edge of kid child_node.position.origin.y = cur_y; cur_y = cur_y + child_node.position.size.height; + // At this point, cur_y is at the bottom margin edge of kid } // The bottom margin collapses with its last in-flow block-level child's bottom margin @@ -338,6 +366,9 @@ impl BlockFlow { // infrastructure to make it scrollable. Au::max(screen_height, cur_y) } else { + // (cur_y - collapsing) will get you the bottom content edge + // top_offset will be at top content edge + // hence, height = content height cur_y - top_offset - collapsing }; @@ -347,18 +378,23 @@ impl BlockFlow { // At this point, `height` is the height of the containing block, so passing `height` // as the second argument here effectively makes percentages relative to the containing // block per CSS 2.1 ยง 10.5. + // TODO: We need to pass in the correct containing block height + // for absolutely positioned elems height = match MaybeAuto::from_style(style.Box.height, height) { Auto => height, Specified(value) => value }; } + // Here, height is content height of box_ + let mut noncontent_height = Au::new(0); for box_ in self.box_.iter() { let mut position = box_.position.get(); let mut margin = box_.margin.get(); // The associated box is the border box of this flow. + // Margin after collapse margin.top = margin_top; margin.bottom = margin_bottom; @@ -373,7 +409,7 @@ impl BlockFlow { position.origin.y = y; height = h; - if self.is_fixed { + if self.is_fixed { for kid in self.base.child_iter() { let child_node = flow::mut_base(*kid); child_node.position.origin.y = position.origin.y + top_offset; @@ -383,6 +419,7 @@ impl BlockFlow { position.size.height = if self.is_fixed { height } else { + // Border box height height + noncontent_height }; @@ -395,6 +432,7 @@ impl BlockFlow { self.base.position.size.height = if self.is_fixed { height } else { + // Height of margin box + clearance height + noncontent_height }; @@ -685,7 +723,7 @@ impl Flow for BlockFlow { margin_left)); let screen_size = ctx.screen_size; - let (x, w) = box_.get_x_coord_and_new_width_if_fixed(screen_size.width, + let (x, w) = box_.get_x_coord_and_new_width_if_fixed(screen_size.width, screen_size.height, width, box_.offset(), @@ -771,6 +809,7 @@ impl Flow for BlockFlow { } } + // CSS Section 8.3.1 - Collapsing Margins fn collapse_margins(&mut self, top_margin_collapsible: bool, first_in_flow: &mut bool, @@ -825,4 +864,3 @@ impl Flow for BlockFlow { }) } } - diff --git a/src/components/main/layout/box_.rs b/src/components/main/layout/box_.rs index 7604c95e5b52..3671f77aed8b 100644 --- a/src/components/main/layout/box_.rs +++ b/src/components/main/layout/box_.rs @@ -321,14 +321,20 @@ impl Box { } } + // CSS Section 10.6.4 + // We have to solve the constraint equation: + // top + bottom + height + (vertical border + padding) = height of + // containing block (`screen_height`) + // + // `y`: static position of the element //TODO(ibnc) take into account padding. - pub fn get_y_coord_and_new_height_if_fixed(&self, + pub fn get_y_coord_and_new_height_if_fixed(&self, screen_height: Au, mut height: Au, mut y: Au, is_fixed: bool) -> (Au, Au) { - if is_fixed { + if is_fixed { let position_offsets = self.position_offsets.get(); match (position_offsets.top, position_offsets.bottom) { (Au(0), Au(0)) => {} @@ -352,6 +358,7 @@ impl Box { return (y, height); } + // CSS Section 10.3.7 //TODO(ibnc) removing padding when width needs to be stretched. pub fn get_x_coord_and_new_width_if_fixed(&self, screen_width: Au, @@ -1461,7 +1468,7 @@ impl Box { image_box_info.dom_height, Au::new(0)); - let height = match (self.style().Box.width, + let height = match (self.style().Box.width, image_box_info.dom_width, height) { (LPA_Auto, None, Auto) => { @@ -1545,7 +1552,7 @@ impl Box { *value.right, *value.bottom, *value.left) - } + } /// Sends the size and position of this iframe box to the constellation. This is out of line to /// guide inlining. @@ -1571,4 +1578,3 @@ impl Box { layout_context.constellation_chan.send(msg) } } - diff --git a/src/components/main/layout/flow.rs b/src/components/main/layout/flow.rs index 9abd29ab9346..953a008ad0ab 100644 --- a/src/components/main/layout/flow.rs +++ b/src/components/main/layout/flow.rs @@ -6,20 +6,20 @@ //! layout constraints to obtain positions and display attributes of tree nodes. Positions are //! computed in several tree traversals driven by the fundamental data dependencies required by /// inline and block layout. -/// +/// /// Flows are interior nodes in the layout tree and correspond closely to *flow contexts* in the /// CSS specification. Flows are responsible for positioning their child flow contexts and boxes. /// Flows have purpose-specific fields, such as auxiliary line box structs, out-of-flow child /// lists, and so on. /// /// Currently, the important types of flows are: -/// +/// /// * `BlockFlow`: A flow that establishes a block context. It has several child flows, each of /// which are positioned according to block formatting context rules (CSS block boxes). Block /// flows also contain a single `GenericBox` to represent their rendered borders, padding, etc. /// The BlockFlow at the root of the tree has special behavior: it stretches to the boundaries of /// the viewport. -/// +/// /// * `InlineFlow`: A flow that establishes an inline context. It has a flat list of child /// boxes/flows that are subject to inline layout and line breaking and structs to represent /// line breaks and mapping to CSS boxes, for the purpose of handling `getClientRects()` and @@ -698,6 +698,11 @@ impl<'a> MutableFlowUtils for &'a mut Flow { mut_base(self).overflow = overflow } + /// Push display items for current flow and its children onto `list`. + /// + /// For InlineFlow, add display items for all its boxes onto list`. + /// For BlockFlow, add a ClipDisplayItemClass for itself and its children, + /// plus any other display items like border. fn build_display_lists( self, builder: &DisplayListBuilder, @@ -723,6 +728,11 @@ impl<'a> MutableFlowUtils for &'a mut Flow { } let mut child_lists = Some(child_lists.unwrap()); + // Find parent ClipDisplayItemClass and push all child display items + // under it + // FIXME: Once we have children for InlineFlow, this might lead to + // children display items being pushed under the ClipDisplayItemClass + // created by the last box of the InlineFlow. Fix the logic. lists.with_mut(|lists| { let mut child_lists = child_lists.take_unwrap(); let result = lists.lists[index].list.mut_rev_iter().position(|item| { @@ -848,4 +858,3 @@ impl FlowLeafSet { self.set.iter() } } -