Skip to content

Commit

Permalink
Don't use a whole in-order traversal for computing heights.
Browse files Browse the repository at this point in the history
  • Loading branch information
Eric Atkinson committed Aug 9, 2013
1 parent 73e7b65 commit eb1b40d
Show file tree
Hide file tree
Showing 6 changed files with 198 additions and 90 deletions.
74 changes: 50 additions & 24 deletions src/components/main/layout/block.rs
View file
Expand Up @@ -10,7 +10,7 @@ use layout::display_list_builder::{DisplayListBuilder, ExtraDisplayListData};
use layout::flow::{BlockFlow, FlowContext, FlowData, InlineBlockFlow, FloatFlow};
use layout::inline::InlineLayout;
use layout::model::{MaybeAuto, Specified, Auto};
use layout::float_context::FloatContext;
use layout::float_context::{FloatContext, Invalid};

use newcss::values::{CSSClearNone, CSSClearLeft, CSSClearRight, CSSClearBoth};
use layout::float_context::{ClearLeft, ClearRight, ClearBoth};
Expand Down Expand Up @@ -187,6 +187,7 @@ impl BlockFlowData {
self.common.position.origin = Au::zero_point();
self.common.position.size.width = ctx.screen_size.size.width;
self.common.floats_in = FloatContext::new(self.common.num_floats);
self.common.is_inorder = false;
}

//position was set to the containing block by the flow's parent
Expand Down Expand Up @@ -239,22 +240,45 @@ impl BlockFlowData {
}
}

let has_inorder_children = self.common.is_inorder || self.common.num_floats > 0;
for BlockFlow(self).each_child |kid| {
assert!(kid.starts_block_flow() || kid.starts_inline_flow());

do kid.with_mut_base |child_node| {
child_node.position.origin.x = x_offset;
child_node.position.size.width = remaining_width;
child_node.is_inorder = has_inorder_children;

if !child_node.is_inorder {
child_node.floats_in = FloatContext::new(0);
}
}
}
}

pub fn assign_height_inorder_block(@mut self, ctx: &mut LayoutContext) {
debug!("assign_height_inorder_block: assigning height for block %?", self.common.id);
self.assign_height_block_base(ctx, true);
}

pub fn assign_height_block(@mut self, ctx: &mut LayoutContext) {
debug!("assign_height_block: assigning height for block %?", self.common.id);
// This is the only case in which a block flow can start an inorder
// subtraversal.
if self.is_root && self.common.num_floats > 0 {
self.assign_height_inorder_block(ctx);
return;
}
self.assign_height_block_base(ctx, false);
}

fn assign_height_block_base(@mut self, ctx: &mut LayoutContext, inorder: bool) {
let mut cur_y = Au(0);
let mut clearance = Au(0);
let mut top_offset = Au(0);
let mut bottom_offset = Au(0);
let mut left_offset = Au(0);
let mut float_ctx = Invalid;

for self.box.iter().advance |&box| {
let style = box.style();
Expand All @@ -279,27 +303,25 @@ impl BlockFlowData {
};
}

// TODO(eatkinson): the translation here is probably
// totally wrong. We need to do it right or pages
// with floats will look very strange.

// Floats for blocks work like this:
// self.floats_in -> child[0].floats_in
// visit child[0]
// child[i-1].floats_out -> child[i].floats_in
// visit child[i]
// repeat until all children are visited.
// last_child.floats_out -> self.floats_out (done at the end of this method)
let mut float_ctx = self.common.floats_in.translate(Point2D(-left_offset, -top_offset));
for BlockFlow(self).each_child |kid| {
do kid.with_mut_base |child_node| {
child_node.floats_in = float_ctx.clone();
}
kid.assign_height(ctx);
do kid.with_mut_base |child_node| {
float_ctx = child_node.floats_out.clone();
}
if inorder {
// Floats for blocks work like this:
// self.floats_in -> child[0].floats_in
// visit child[0]
// child[i-1].floats_out -> child[i].floats_in
// visit child[i]
// repeat until all children are visited.
// last_child.floats_out -> self.floats_out (done at the end of this method)
float_ctx = self.common.floats_in.translate(Point2D(-left_offset, -top_offset));
for BlockFlow(self).each_child |kid| {
do kid.with_mut_base |child_node| {
child_node.floats_in = float_ctx.clone();
}
kid.assign_height_inorder(ctx);
do kid.with_mut_base |child_node| {
float_ctx = child_node.floats_out.clone();
}

}
}
for BlockFlow(self).each_child |kid| {
do kid.with_mut_base |child_node| {
Expand All @@ -311,7 +333,7 @@ impl BlockFlowData {
let mut height = if self.is_root {
Au::max(ctx.screen_size.size.height, cur_y)
} else {
cur_y - top_offset
cur_y - top_offset
};

for self.box.iter().advance |&box| {
Expand All @@ -338,8 +360,12 @@ impl BlockFlowData {
//TODO(eatkinson): compute heights using the 'height' property.
self.common.position.size.height = height + noncontent_height;

let extra_height = height - (cur_y - top_offset) + bottom_offset;
self.common.floats_out = float_ctx.translate(Point2D(left_offset, -extra_height));
if inorder {
let extra_height = height - (cur_y - top_offset) + bottom_offset;
self.common.floats_out = float_ctx.translate(Point2D(left_offset, -extra_height));
} else {
self.common.floats_out = self.common.floats_in.clone();
}
}

pub fn build_display_list_block<E:ExtraDisplayListData>(@mut self,
Expand Down
2 changes: 1 addition & 1 deletion src/components/main/layout/box.rs
View file
Expand Up @@ -271,7 +271,7 @@ impl RenderBox {

/// Attempts to split this box so that its width is no more than `max_width`. Fails if this box
/// is an unscanned text box.
pub fn split_to_width(&self, _: &LayoutContext, max_width: Au, starts_line: bool)
pub fn split_to_width(&self, max_width: Au, starts_line: bool)
-> SplitBoxResult {
match *self {
GenericRenderBoxClass(*) | ImageRenderBoxClass(*) => CannotSplit(*self),
Expand Down
100 changes: 71 additions & 29 deletions src/components/main/layout/float.rs
View file
Expand Up @@ -35,6 +35,9 @@ pub struct FloatFlowData {
/// Index into the box list for inline floats
index: Option<uint>,

/// Number of floated children
floated_children: uint,

}

impl FloatFlowData {
Expand All @@ -46,6 +49,7 @@ impl FloatFlowData {
index: None,
float_type: float_type,
rel_pos: Point2D(Au(0), Au(0)),
floated_children: 0,
}
}

Expand All @@ -63,20 +67,20 @@ impl FloatFlowData {
pub fn bubble_widths_float(@mut self, ctx: &LayoutContext) {
let mut min_width = Au(0);
let mut pref_width = Au(0);
let mut num_floats = 1;
let mut num_floats = 0;

for FloatFlow(self).each_child |child_ctx| {
//assert!(child_ctx.starts_block_flow() || child_ctx.starts_inline_flow());

do child_ctx.with_mut_base |child_node| {
min_width = geometry::max(min_width, child_node.min_width);
pref_width = geometry::max(pref_width, child_node.pref_width);

num_floats = num_floats + child_node.num_floats;
}
}

self.common.num_floats = num_floats;
self.common.num_floats = 1;
self.floated_children = num_floats;


self.box.map(|&box| {
Expand All @@ -93,13 +97,16 @@ impl FloatFlowData {
self.common.pref_width = pref_width;
}

pub fn assign_widths_float(@mut self, _: &LayoutContext) {
pub fn assign_widths_float(@mut self) {
debug!("assign_widths_float: assigning width for flow %?", self.common.id);
// position.size.width is set by parent even though we don't know
// position.origin yet.
let mut remaining_width = self.common.position.size.width;
self.containing_width = remaining_width;
let mut x_offset = Au(0);

// Parent usually sets this, but floats are never inorder
self.common.is_inorder = false;

for self.box.iter().advance |&box| {
let style = box.style();
Expand Down Expand Up @@ -154,25 +161,77 @@ impl FloatFlowData {

self.common.position.size.width = remaining_width;

let has_inorder_children = self.common.num_floats > 0;
for FloatFlow(self).each_child |kid| {
//assert!(kid.starts_block_flow() || kid.starts_inline_flow());

do kid.with_mut_base |child_node| {
child_node.position.origin.x = x_offset;
child_node.position.size.width = remaining_width;
child_node.is_inorder = has_inorder_children;

if !child_node.is_inorder {
child_node.floats_in = FloatContext::new(0);
}
}
}
}

pub fn assign_height_float(@mut self, ctx: &mut LayoutContext) {
let mut float_ctx = FloatContext::new(self.common.num_floats);
for FloatFlow(self).each_child |kid| {
do kid.with_mut_base |child_node| {
child_node.floats_in = float_ctx.clone();
pub fn assign_height_inorder_float(@mut self) {
debug!("assign_height_inorder_float: assigning height for float %?", self.common.id);
// assign_height_float was already called by the traversal function
// so this is well-defined

let mut height = Au(0);
let mut full_noncontent_width = Au(0);
let mut full_noncontent_height = Au(0);

self.box.map(|&box| {
height = do box.with_base |base| {
base.position.size.height
};

do box.with_base |base| {

let noncontent_width = base.model.padding.left + base.model.padding.right +
base.model.border.left + base.model.border.right;
let noncontent_height = base.model.padding.top + base.model.padding.bottom +
base.model.border.top + base.model.border.bottom;

full_noncontent_width = noncontent_width + base.model.margin.left + base.model.margin.right;
full_noncontent_height = noncontent_height + base.model.margin.top + base.model.margin.bottom;

}
kid.assign_height(ctx);
do kid.with_mut_base |child_node| {
float_ctx = child_node.floats_out.clone();

});

let info = PlacementInfo {
width: self.common.position.size.width + full_noncontent_width,
height: height + full_noncontent_height,
ceiling: Au(0),
max_width: self.containing_width,
f_type: self.float_type,
};

// Place the float and return the FloatContext back to the parent flow.
// After, grab the position and use that to set our position.
self.common.floats_out = self.common.floats_in.add_float(&info);
self.rel_pos = self.common.floats_out.last_float_pos();
}

pub fn assign_height_float(@mut self, ctx: &mut LayoutContext) {
debug!("assign_height_float: assigning height for float %?", self.common.id);
let has_inorder_children = self.common.num_floats > 0;
if has_inorder_children {
let mut float_ctx = FloatContext::new(self.floated_children);
for FloatFlow(self).each_child |kid| {
do kid.with_mut_base |child_node| {
child_node.floats_in = float_ctx.clone();
}
kid.assign_height_inorder(ctx);
do kid.with_mut_base |child_node| {
float_ctx = child_node.floats_out.clone();
}
}
}

Expand All @@ -197,8 +256,6 @@ impl FloatFlowData {

let mut noncontent_width = Au(0);
let mut noncontent_height = Au(0);
let mut full_noncontent_width = Au(0);
let mut full_noncontent_height = Au(0);
self.box.map(|&box| {
do box.with_mut_base |base| {
//The associated box is the border box of this flow
Expand All @@ -210,8 +267,6 @@ impl FloatFlowData {
base.model.border.top + base.model.border.bottom;
base.position.size.height = height + noncontent_height;

full_noncontent_width = noncontent_width + base.model.margin.left + base.model.margin.right;
full_noncontent_height = noncontent_height + base.model.margin.top + base.model.margin.bottom;
}
});

Expand All @@ -229,19 +284,6 @@ impl FloatFlowData {
base.position.size.height = height;
}
}

let info = PlacementInfo {
width: self.common.position.size.width + full_noncontent_width,
height: height + full_noncontent_height,
ceiling: Au(0),
max_width: self.containing_width,
f_type: self.float_type,
};

// Place the float and return the FloatContext back to the parent flow.
// After, grab the position and use that to set our position.
self.common.floats_out = self.common.floats_in.add_float(&info);
self.rel_pos = self.common.floats_out.last_float_pos();
}

pub fn build_display_list_float<E:ExtraDisplayListData>(@mut self,
Expand Down

5 comments on commit eb1b40d

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from pcwalton
at eric93@eb1b40d

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging eric93/servo/parallel-layout-ownedtree = eb1b40d into auto

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eric93/servo/parallel-layout-ownedtree = eb1b40d merged ok, testing candidate = 490e81f

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all tests pass:
http://servo-buildbot.pub.build.mozilla.org/builders/linux/builds/226
http://servo-buildbot.pub.build.mozilla.org/builders/mac/builds/181

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = 490e81f

Please sign in to comment.