From 0634b359aae2dc55dd6ac648bc83112973aa2e06 Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Thu, 14 Jan 2016 12:15:10 +1000 Subject: [PATCH] Rely on the overflow rect for layer size. This is not currently correct for layers with transformed elements. According to the spec, the overflow rect should include transformed elements, so this will need to be added/fixed in the layout code rather than WR. Fixes #137. --- src/aabbtree.rs | 77 +++++++++++++++++----------------------------- src/frame.rs | 22 +++++++------ src/layer.rs | 14 ++++----- tests/bug_137.html | 25 +++++++++++++++ 4 files changed, 74 insertions(+), 64 deletions(-) create mode 100644 tests/bug_137.html diff --git a/src/aabbtree.rs b/src/aabbtree.rs index 7e70dbf1a3..1c354a3c44 100644 --- a/src/aabbtree.rs +++ b/src/aabbtree.rs @@ -1,7 +1,6 @@ use euclid::{Point2D, Rect, Size2D}; use internal_types::{CompiledNode, DrawListId, DrawListItemIndex, DrawListGroupId}; use resource_list::ResourceList; -use std::mem; use util; #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -23,14 +22,6 @@ enum TreeState { Finalized, } -#[derive(Debug)] -struct Item { - rect: Rect, - draw_list_group_id: DrawListGroupId, - draw_list_id: DrawListId, - item_index: DrawListItemIndex, -} - pub struct AABBTreeNode { pub split_rect: Rect, pub actual_rect: Rect, @@ -126,50 +117,27 @@ pub struct AABBTree { work_node_indices: Vec, - pub local_bounds: Rect, state: TreeState, - items: Vec, } impl AABBTree { - pub fn new(split_size: f32) -> AABBTree { - AABBTree { + pub fn new(split_size: f32, + local_bounds: &Rect) -> AABBTree { + let mut tree = AABBTree { nodes: Vec::new(), split_size: split_size, work_node_indices: Vec::new(), state: TreeState::Building, - items: Vec::new(), - local_bounds: Rect::new(Point2D::zero(), Size2D::zero()), - } + }; + + let root_node = AABBTreeNode::new(local_bounds); + tree.nodes.push(root_node); + + tree } pub fn finalize(&mut self) { debug_assert!(self.state == TreeState::Building); - - let root_node = AABBTreeNode::new(&self.local_bounds); - self.nodes.push(root_node); - - let items = mem::replace(&mut self.items, Vec::new()); - for item in items { - self.find_best_nodes(NodeIndex(0), &item.rect); - for node_index in self.work_node_indices.drain(..) { - let NodeIndex(node_index) = node_index; - let node = &mut self.nodes[node_index as usize]; - node.append_item(item.draw_list_group_id, - item.draw_list_id, - item.item_index, - &item.rect); - } - } - - // TODO(gw): This is a total hack for reftests :( - if self.nodes.len() == 1 { - let root_node = self.node_mut(NodeIndex(0)); - if root_node.split_rect.size.width > 0.0 && root_node.split_rect.size.height > 0.0 { - root_node.split_rect = root_node.split_rect.inflate(3.0, 3.0); - } - } - self.state = TreeState::Finalized; } @@ -241,13 +209,26 @@ impl AABBTree { draw_list_id: DrawListId, item_index: DrawListItemIndex) { debug_assert!(self.state == TreeState::Building); - self.local_bounds = self.local_bounds.union(&rect); - self.items.push(Item { - draw_list_group_id: draw_list_group_id, - draw_list_id: draw_list_id, - item_index: item_index, - rect: rect, - }); + + self.find_best_nodes(NodeIndex(0), &rect); + if self.work_node_indices.is_empty() { + // TODO(gw): If this happens, it it probably caused by items having + // transforms that move them outside the local overflow. According + // to the transforms spec, the stacking context overflow should + // include transformed elements, however this isn't currently + // handled by the layout code! If it's not that, this is an + // unexpected condition and should be investigated! + println!("WARNING: insert rect {:?} outside bounds, dropped.", rect); + } else { + for node_index in self.work_node_indices.drain(..) { + let NodeIndex(node_index) = node_index; + let node = &mut self.nodes[node_index as usize]; + node.append_item(draw_list_group_id, + draw_list_id, + item_index, + &rect); + } + } } fn split_if_needed(&mut self, node_index: NodeIndex) { diff --git a/src/frame.rs b/src/frame.rs index 7b0721f521..ff60b3c436 100644 --- a/src/frame.rs +++ b/src/frame.rs @@ -575,7 +575,7 @@ impl Frame { pub fn scroll(&mut self, delta: &Point2D) { // TODO: Select correct layer for scrolling! for (_, layer) in &mut self.layers { - let layer_size = layer.size(); + let layer_size = layer.layer_size; if layer_size.width > layer.viewport_size.width { layer.scroll_offset.x = layer.scroll_offset.x + delta.x; @@ -617,7 +617,10 @@ impl Frame { // Insert global position: fixed elements layer debug_assert!(self.layers.is_empty()); self.layers.insert(ScrollLayerId::fixed_layer(), - Layer::new(Point2D::zero(), root_stacking_context.stacking_context.bounds.size)); + Layer::new(root_stacking_context.stacking_context.overflow.origin, + root_stacking_context.stacking_context.overflow.size, + Size2D::new(viewport_size.width as f32, + viewport_size.height as f32))); // Work around borrow check on resource cache { @@ -780,7 +783,7 @@ impl Frame { parent_info: &FlattenInfo, context: &mut FlattenContext, target: &mut RenderTarget, - _level: i32) { + level: i32) { let _pf = util::ProfileScope::new(" flatten"); let stacking_context = match scene_item { @@ -797,6 +800,10 @@ impl Frame { } }; + let local_clip_rect = parent_info.current_clip_rect + .translate(&-stacking_context.bounds.origin) + .intersection(&stacking_context.overflow); + /* let mut indent = String::new(); for _ in 0..level { @@ -804,10 +811,6 @@ impl Frame { } */ - let local_clip_rect = parent_info.current_clip_rect - .translate(&-stacking_context.bounds.origin) - .intersection(&stacking_context.overflow); - if let Some(local_clip_rect) = local_clip_rect { let scene_items = scene_item.collect_scene_items(&context.scene); if !scene_items.is_empty() { @@ -818,6 +821,7 @@ impl Frame { Some(scroll_layer_id) => { debug_assert!(!self.layers.contains_key(&scroll_layer_id)); let layer = Layer::new(parent_info.offset_from_origin, + stacking_context.overflow.size, parent_info.viewport_size); self.layers.insert(scroll_layer_id, layer); (scroll_layer_id, scroll_layer_id, Point2D::zero()) @@ -876,7 +880,7 @@ impl Frame { &info, target, context, - _level); + level); } else { let target_size = Size2D::new(local_clip_rect.size.width as i32, local_clip_rect.size.height as i32); @@ -919,7 +923,7 @@ impl Frame { &info, &mut new_target, context, - _level); + level); target.children.push(new_target); } diff --git a/src/layer.rs b/src/layer.rs index ae4838e435..a6cc96b95f 100644 --- a/src/layer.rs +++ b/src/layer.rs @@ -7,20 +7,24 @@ pub struct Layer { // TODO: Remove pub from here if possible in the future pub aabb_tree: AABBTree, pub scroll_offset: Point2D, - pub world_origin: Point2D, pub viewport_size: Size2D, + pub layer_size: Size2D, + pub world_origin: Point2D, } impl Layer { pub fn new(world_origin: Point2D, + layer_size: Size2D, viewport_size: Size2D) -> Layer { - let aabb_tree = AABBTree::new(1024.0); + let rect = Rect::new(Point2D::zero(), layer_size); + let aabb_tree = AABBTree::new(1024.0, &rect); Layer { aabb_tree: aabb_tree, - world_origin: world_origin, scroll_offset: Point2D::zero(), viewport_size: viewport_size, + world_origin: world_origin, + layer_size: layer_size, } } @@ -48,10 +52,6 @@ impl Layer { item_index); } - pub fn size(&self) -> Size2D { - self.aabb_tree.local_bounds.size - } - pub fn finalize(&mut self, initial_scroll_offset: Point2D) { self.scroll_offset = initial_scroll_offset; diff --git a/tests/bug_137.html b/tests/bug_137.html new file mode 100644 index 0000000000..c472327baf --- /dev/null +++ b/tests/bug_137.html @@ -0,0 +1,25 @@ + + +
\ No newline at end of file