diff options
author | bors-servo <release+servo@mozilla.com> | 2014-02-05 22:19:43 -0500 |
---|---|---|
committer | bors-servo <release+servo@mozilla.com> | 2014-02-05 22:19:43 -0500 |
commit | 17bc467b834f72612a9b8ae4d76f5b67a567fd46 (patch) | |
tree | 25979ee52113f54c2f2da952573b8b6d1ff8cd57 /src | |
parent | 1dbc73ea1c5ab395d03934024e3f1a773b357032 (diff) | |
parent | 78d2118f4728cd194b90b8b5d499bd6486ac71a9 (diff) | |
download | servo-17bc467b834f72612a9b8ae4d76f5b67a567fd46.tar.gz servo-17bc467b834f72612a9b8ae4d76f5b67a567fd46.zip |
auto merge of #1626 : kmcallister/servo/flowlist, r=pcwalton
r? @pcwalton
Some points of concern:
* Is this really memory-safe? Is there a way to access a stale `Rawlink` that doesn't exist in the normal `DList` code?
* Is it okay to coerce both `~Flow` and `&Flow` to `(uint, uint)` (or the moral equivalent) and compare them? Will a given `Flow` always have the same (obj, vtable) regardless of whether it's obtained through `~Flow` or `&Flow`?
Diffstat (limited to 'src')
-rw-r--r-- | src/components/main/layout/block.rs | 22 | ||||
-rw-r--r-- | src/components/main/layout/flow.rs | 80 | ||||
-rw-r--r-- | src/components/main/layout/flow_list.rs | 312 | ||||
-rw-r--r-- | src/components/main/layout/inline.rs | 4 | ||||
-rw-r--r-- | src/components/main/layout/layout_task.rs | 4 | ||||
-rw-r--r-- | src/components/main/layout/parallel.rs | 12 | ||||
-rwxr-xr-x | src/components/main/servo.rc | 1 |
7 files changed, 386 insertions, 49 deletions
diff --git a/src/components/main/layout/block.rs b/src/components/main/layout/block.rs index cc55ed3bb2c..89415f45745 100644 --- a/src/components/main/layout/block.rs +++ b/src/components/main/layout/block.rs @@ -285,9 +285,9 @@ impl BlockFlow { // last_child.floats_out -> self.floats_out (done at the end of this method) float_ctx = self.base.floats_in.translate(Point2D(-left_offset, -top_offset)); for kid in self.base.child_iter() { - flow::mut_base(*kid).floats_in = float_ctx.clone(); + flow::mut_base(kid).floats_in = float_ctx.clone(); kid.assign_height_inorder(ctx); - float_ctx = flow::mut_base(*kid).floats_out.clone(); + float_ctx = flow::mut_base(kid).floats_out.clone(); } } @@ -329,7 +329,7 @@ impl BlockFlow { &mut collapsing, &mut collapsible); - let child_node = flow::mut_base(*kid); + 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 @@ -407,7 +407,7 @@ impl BlockFlow { if self.is_fixed { for kid in self.base.child_iter() { - let child_node = flow::mut_base(*kid); + let child_node = flow::mut_base(kid); child_node.position.origin.y = position.origin.y + top_offset; } } @@ -484,9 +484,9 @@ impl BlockFlow { if has_inorder_children { let mut float_ctx = FloatContext::new(self.float.get_ref().floated_children); for kid in self.base.child_iter() { - flow::mut_base(*kid).floats_in = float_ctx.clone(); + flow::mut_base(kid).floats_in = float_ctx.clone(); kid.assign_height_inorder(ctx); - float_ctx = flow::mut_base(*kid).floats_out.clone(); + float_ctx = flow::mut_base(kid).floats_out.clone(); } } let mut cur_y = Au(0); @@ -498,7 +498,7 @@ impl BlockFlow { } for kid in self.base.child_iter() { - let child_base = flow::mut_base(*kid); + let child_base = flow::mut_base(kid); child_base.position.origin.y = cur_y; cur_y = cur_y + child_base.position.size.height; } @@ -573,7 +573,7 @@ impl BlockFlow { let this_position = self.base.abs_position; for child in self.base.child_iter() { - let child_base = flow::mut_base(*child); + let child_base = flow::mut_base(child); child_base.abs_position = this_position + child_base.position.origin + rel_offset; } @@ -618,7 +618,7 @@ impl BlockFlow { // go deeper into the flow tree for child in self.base.child_iter() { - let child_base = flow::mut_base(*child); + let child_base = flow::mut_base(child); child_base.abs_position = offset + child_base.position.origin + rel_offset; } @@ -652,7 +652,7 @@ impl Flow for BlockFlow { for child_ctx in self.base.child_iter() { assert!(child_ctx.starts_block_flow() || child_ctx.starts_inline_flow()); - let child_base = flow::mut_base(*child_ctx); + let child_base = flow::mut_base(child_ctx); min_width = geometry::max(min_width, child_base.min_width); pref_width = geometry::max(pref_width, child_base.pref_width); num_floats = num_floats + child_base.num_floats; @@ -783,7 +783,7 @@ impl Flow for BlockFlow { for kid in self.base.child_iter() { assert!(kid.starts_block_flow() || kid.starts_inline_flow()); - let child_base = flow::mut_base(*kid); + let child_base = flow::mut_base(kid); child_base.position.origin.x = x_offset; child_base.position.size.width = remaining_width; child_base.flags_info.flags.set_inorder(has_inorder_children); diff --git a/src/components/main/layout/flow.rs b/src/components/main/layout/flow.rs index 6549a425d36..db38b3327ce 100644 --- a/src/components/main/layout/flow.rs +++ b/src/components/main/layout/flow.rs @@ -36,8 +36,8 @@ use layout::inline::InlineFlow; use layout::parallel::{FlowParallelInfo, UnsafeFlow}; use layout::parallel; use layout::wrapper::ThreadSafeLayoutNode; +use layout::flow_list::{FlowList, Link, Rawlink, FlowListIterator, MutFlowListIterator}; -use extra::dlist::{DList, DListIterator, MutDListIterator}; use extra::container::Deque; use geom::point::Point2D; use geom::Size2D; @@ -134,7 +134,7 @@ pub fn base<'a>(this: &'a Flow) -> &'a BaseFlow { } /// Iterates over the children of this immutable flow. -pub fn imm_child_iter<'a>(flow: &'a Flow) -> DListIterator<'a,~Flow> { +pub fn imm_child_iter<'a>(flow: &'a Flow) -> FlowListIterator<'a> { base(flow).children.iter() } @@ -147,12 +147,12 @@ pub fn mut_base<'a>(this: &'a mut Flow) -> &'a mut BaseFlow { } /// Returns the last child of this flow. -pub fn last_child<'a>(flow: &'a mut Flow) -> Option<&'a mut ~Flow> { +pub fn last_child<'a>(flow: &'a mut Flow) -> Option<&'a mut Flow> { mut_base(flow).children.back_mut() } /// Iterates over the children of this flow. -pub fn child_iter<'a>(flow: &'a mut Flow) -> MutDListIterator<'a,~Flow> { +pub fn child_iter<'a>(flow: &'a mut Flow) -> MutFlowListIterator<'a> { mut_base(flow).children.mut_iter() } @@ -196,10 +196,10 @@ pub trait MutableFlowUtils { // Mutators /// Invokes a closure with the first child of this flow. - fn with_first_child<R>(self, f: |Option<&mut ~Flow>| -> R) -> R; + fn with_first_child<R>(self, f: |Option<&mut Flow>| -> R) -> R; /// Invokes a closure with the last child of this flow. - fn with_last_child<R>(self, f: |Option<&mut ~Flow>| -> R) -> R; + fn with_last_child<R>(self, f: |Option<&mut Flow>| -> R) -> R; /// Computes the overflow region for this flow. fn store_overflow(self, _: &mut LayoutContext); @@ -213,6 +213,9 @@ pub trait MutableFlowUtils { index: uint, mut list: &RefCell<DisplayListCollection<E>>) -> bool; + + /// Destroys the flow. + fn destroy(self, leaf_set: &FlowLeafSet); } pub trait MutableOwnedFlowUtils { @@ -492,7 +495,9 @@ pub struct BaseFlow { restyle_damage: RestyleDamage, /// The children of this flow. - children: DList<~Flow>, + children: FlowList, + next_sibling: Link, + prev_sibling: Rawlink, /* TODO (Issue #87): debug only */ id: int, @@ -563,7 +568,9 @@ impl BaseFlow { BaseFlow { restyle_damage: node.restyle_damage(), - children: DList::new(), + children: FlowList::new(), + next_sibling: None, + prev_sibling: Rawlink::none(), id: id, @@ -585,7 +592,7 @@ impl BaseFlow { } } - pub fn child_iter<'a>(&'a mut self) -> MutDListIterator<'a,~Flow> { + pub fn child_iter<'a>(&'a mut self) -> MutFlowListIterator<'a> { self.children.mut_iter() } } @@ -700,12 +707,12 @@ impl<'a> MutableFlowUtils for &'a mut Flow { } /// Invokes a closure with the first child of this flow. - fn with_first_child<R>(self, f: |Option<&mut ~Flow>| -> R) -> R { + fn with_first_child<R>(self, f: |Option<&mut Flow>| -> R) -> R { f(mut_base(self).children.front_mut()) } /// Invokes a closure with the last child of this flow. - fn with_last_child<R>(self, f: |Option<&mut ~Flow>| -> R) -> R { + fn with_last_child<R>(self, f: |Option<&mut Flow>| -> R) -> R { f(mut_base(self).children.back_mut()) } @@ -713,7 +720,7 @@ impl<'a> MutableFlowUtils for &'a mut Flow { let my_position = mut_base(self).position; let mut overflow = my_position; for kid in mut_base(self).child_iter() { - let mut kid_overflow = base(*kid).overflow; + let mut kid_overflow = base(kid).overflow; kid_overflow = kid_overflow.translate(&my_position.origin); overflow = overflow.union(&kid_overflow) } @@ -789,6 +796,23 @@ impl<'a> MutableFlowUtils for &'a mut Flow { true } + /// Destroys the flow. + fn destroy(self, leaf_set: &FlowLeafSet) { + let is_leaf = { + let base = mut_base(self); + base.children.len() == 0 + }; + + if is_leaf { + leaf_set.remove(self); + } else { + for kid in child_iter(self) { + kid.destroy(leaf_set) + } + } + + mut_base(self).destroyed = true + } } impl MutableOwnedFlowUtils for ~Flow { @@ -818,7 +842,8 @@ impl MutableOwnedFlowUtils for ~Flow { } base.flags_info.flags.set_is_leaf(true) } - leaf_set.insert(self) + let self_borrowed: &Flow = *self; + leaf_set.insert(self_borrowed); } /// Marks the flow as a nonleaf. The flow must not be marked as a leaf. @@ -833,21 +858,8 @@ impl MutableOwnedFlowUtils for ~Flow { /// Destroys the flow. fn destroy(&mut self, leaf_set: &FlowLeafSet) { - let is_leaf = { - let base = mut_base(*self); - base.children.len() == 0 - }; - - if is_leaf { - leaf_set.remove(self); - } else { - for kid in child_iter(*self) { - kid.destroy(leaf_set) - } - } - - let base = mut_base(*self); - base.destroyed = true + let self_borrowed: &mut Flow = *self; + self_borrowed.destroy(leaf_set); } } @@ -866,22 +878,22 @@ impl FlowLeafSet { } /// Inserts a newly-created flow into the leaf set. - fn insert(&self, flow: &~Flow) { - self.set.insert(parallel::owned_flow_to_unsafe_flow(flow), ()); + fn insert(&self, flow: &Flow) { + self.set.insert(parallel::borrowed_flow_to_unsafe_flow(flow), ()); } /// Removes a flow from the leaf set. Asserts that the flow was indeed in the leaf set. (This /// invariant is needed for memory safety, as there must always be exactly one leaf set.) - fn remove(&self, flow: &~Flow) { + fn remove(&self, flow: &Flow) { if !self.contains(flow) { fail!("attempted to remove a flow from the leaf set that wasn't in the set!") } - let flow = parallel::owned_flow_to_unsafe_flow(flow); + let flow = parallel::borrowed_flow_to_unsafe_flow(flow); self.set.remove(&flow); } - pub fn contains(&self, flow: &~Flow) -> bool { - let flow = parallel::owned_flow_to_unsafe_flow(flow); + pub fn contains(&self, flow: &Flow) -> bool { + let flow = parallel::borrowed_flow_to_unsafe_flow(flow); self.set.contains_key(&flow) } diff --git a/src/components/main/layout/flow_list.rs b/src/components/main/layout/flow_list.rs new file mode 100644 index 00000000000..232b718fba8 --- /dev/null +++ b/src/components/main/layout/flow_list.rs @@ -0,0 +1,312 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +//! A variant of `DList` specialized to store `Flow`s without an extra +//! indirection. + +use std::cast; +use std::ptr; +use std::util; + +use layout::flow::{Flow, base, mut_base}; + +pub type Link = Option<~Flow>; + +#[deriving(Clone)] +pub struct Rawlink { + priv vtable: *(), + priv obj: *mut (), +} + +pub struct FlowList { + priv length: uint, + priv list_head: Link, + priv list_tail: Rawlink, +} + +/// Double-ended FlowList iterator +#[deriving(Clone)] +pub struct FlowListIterator<'a> { + priv head: &'a Link, + priv tail: Rawlink, + priv nelem: uint, +} + +/// Double-ended mutable FlowList iterator +pub struct MutFlowListIterator<'a> { + priv list: &'a mut FlowList, + priv head: Rawlink, + priv tail: Rawlink, + priv nelem: uint, +} + +impl Rawlink { + /// Like Option::None for Rawlink + pub fn none() -> Rawlink { + Rawlink { + vtable: ptr::null(), + obj: ptr::mut_null(), + } + } + + /// Like Option::Some for Rawlink + fn some(n: &mut Flow) -> Rawlink { + unsafe { cast::transmute(n) } + } + + /// Convert the `Rawlink` into an Option value + fn resolve_immut(&self) -> Option<&Flow> { + if self.obj.is_null() { + None + } else { + let me: &Flow = unsafe { cast::transmute_copy(self) }; + Some(me) + } + } + + fn resolve(&mut self) -> Option<&mut Flow> { + if self.obj.is_null() { + None + } else { + let me: &mut Flow = unsafe { cast::transmute_copy(self) }; + Some(me) + } + } + + fn is_none(&self) -> bool { + self.obj.is_null() + } + + unsafe fn get<'a>(&'a mut self) -> &'a mut Flow { + assert!(self.obj.is_not_null()); + cast::transmute_copy(self) + } +} + +/// Set the .prev field on `next`, then return `Some(next)` +fn link_with_prev(mut next: ~Flow, prev: Rawlink) -> Link { + mut_base(next).prev_sibling = prev; + Some(next) +} + +impl Container for FlowList { + /// O(1) + #[inline] + fn is_empty(&self) -> bool { + self.list_head.is_none() + } + /// O(1) + #[inline] + fn len(&self) -> uint { + self.length + } +} + +// This doesn't quite fit the Deque trait because of the need to switch between +// &Flow and ~Flow. +impl FlowList { + /// Provide a reference to the front element, or None if the list is empty + #[inline] + pub fn front<'a>(&'a self) -> Option<&'a Flow> { + self.list_head.as_ref().map(|head| { let x: &Flow = *head; x }) + } + + /// Provide a mutable reference to the front element, or None if the list is empty + #[inline] + pub fn front_mut<'a>(&'a mut self) -> Option<&'a mut Flow> { + self.list_head.as_mut().map(|head| { let x: &mut Flow = *head; x }) + } + + /// Provide a reference to the back element, or None if the list is empty + #[inline] + pub fn back<'a>(&'a self) -> Option<&'a Flow> { + let tmp = self.list_tail.resolve_immut(); + tmp.as_ref().map(|tail| { let x: &Flow = *tail; x }) + } + + /// Provide a mutable reference to the back element, or None if the list is empty + #[inline] + pub fn back_mut<'a>(&'a mut self) -> Option<&'a mut Flow> { + // Can't use map() due to error: + // lifetime of `tail` is too short to guarantee its contents can be safely reborrowed + let tmp = self.list_tail.resolve(); + match tmp { + None => None, + Some(tail) => { + let x: &mut Flow = tail; + Some(x) + } + } + } + + /// Add an element first in the list + /// + /// O(1) + pub fn push_front(&mut self, mut new_head: ~Flow) { + match self.list_head { + None => { + self.list_tail = Rawlink::some(new_head); + self.list_head = link_with_prev(new_head, Rawlink::none()); + } + Some(ref mut head) => { + mut_base(new_head).prev_sibling = Rawlink::none(); + mut_base(*head).prev_sibling = Rawlink::some(new_head); + util::swap(head, &mut new_head); + mut_base(*head).next_sibling = Some(new_head); + } + } + self.length += 1; + } + + /// Remove the first element and return it, or None if the list is empty + /// + /// O(1) + pub fn pop_front(&mut self) -> Option<~Flow> { + self.list_head.take().map(|mut front_node| { + self.length -= 1; + match mut_base(front_node).next_sibling.take() { + Some(node) => self.list_head = link_with_prev(node, Rawlink::none()), + None => self.list_tail = Rawlink::none() + } + front_node + }) + } + + /// Add an element last in the list + /// + /// O(1) + pub fn push_back(&mut self, mut new_tail: ~Flow) { + if self.list_tail.is_none() { + return self.push_front(new_tail); + } else { + let mut old_tail = self.list_tail; + self.list_tail = Rawlink::some(new_tail); + let tail = unsafe { old_tail.get() }; + mut_base(tail).next_sibling = link_with_prev(new_tail, Rawlink::some(tail)); + } + self.length += 1; + } + + /// Remove the last element and return it, or None if the list is empty + /// + /// O(1) + pub fn pop_back(&mut self) -> Option<~Flow> { + if self.list_tail.is_none() { + None + } else { + self.length -= 1; + + self.list_tail = base(unsafe { self.list_tail.get() }).prev_sibling; + if self.list_tail.is_none() { + self.list_head.take() + } else { + mut_base(unsafe { self.list_tail.get() }).next_sibling.take() + } + } + } + + /// Create an empty list + #[inline] + pub fn new() -> FlowList { + FlowList { + list_head: None, + list_tail: Rawlink::none(), + length: 0, + } + } + + /// Provide a forward iterator + #[inline] + pub fn iter<'a>(&'a self) -> FlowListIterator<'a> { + FlowListIterator { + nelem: self.len(), + head: &self.list_head, + tail: self.list_tail + } + } + + /// Provide a forward iterator with mutable references + #[inline] + pub fn mut_iter<'a>(&'a mut self) -> MutFlowListIterator<'a> { + let head_raw = match self.list_head { + Some(ref mut h) => Rawlink::some(*h), + None => Rawlink::none(), + }; + MutFlowListIterator { + nelem: self.len(), + head: head_raw, + tail: self.list_tail, + list: self + } + } +} + +#[unsafe_destructor] +impl Drop for FlowList { + fn drop(&mut self) { + // Dissolve the list in backwards direction + // Just dropping the list_head can lead to stack exhaustion + // when length is >> 1_000_000 + let mut tail = self.list_tail; + loop { + match tail.resolve() { + None => break, + Some(prev) => { + let prev_base = mut_base(prev); + prev_base.next_sibling.take(); + tail = prev_base.prev_sibling; + } + } + } + self.length = 0; + self.list_head = None; + self.list_tail = Rawlink::none(); + } +} + +impl<'a> Iterator<&'a Flow> for FlowListIterator<'a> { + #[inline] + fn next(&mut self) -> Option<&'a Flow> { + if self.nelem == 0 { + return None; + } + self.head.as_ref().map(|head| { + let head_base = base(*head); + self.nelem -= 1; + self.head = &head_base.next_sibling; + let ret: &Flow = *head; + ret + }) + } + + #[inline] + fn size_hint(&self) -> (uint, Option<uint>) { + (self.nelem, Some(self.nelem)) + } +} + +impl<'a> Iterator<&'a mut Flow> for MutFlowListIterator<'a> { + #[inline] + fn next(&mut self) -> Option<&'a mut Flow> { + if self.nelem == 0 { + return None; + } + self.head.resolve().map(|next| { + self.nelem -= 1; + self.head = match mut_base(next).next_sibling { + Some(ref mut node) => { + let x: &mut Flow = *node; + Rawlink::some(x) + } + None => Rawlink::none(), + }; + next + }) + } + + #[inline] + fn size_hint(&self) -> (uint, Option<uint>) { + (self.nelem, Some(self.nelem)) + } +} diff --git a/src/components/main/layout/inline.rs b/src/components/main/layout/inline.rs index 2135e104cc4..b14c3dc6be6 100644 --- a/src/components/main/layout/inline.rs +++ b/src/components/main/layout/inline.rs @@ -627,7 +627,7 @@ impl Flow for InlineFlow { let mut num_floats = 0; for kid in self.base.child_iter() { - let child_base = flow::mut_base(*kid); + let child_base = flow::mut_base(kid); num_floats += child_base.num_floats; child_base.floats_in = FloatContext::new(child_base.num_floats); } @@ -668,7 +668,7 @@ impl Flow for InlineFlow { // FIXME(ksh8281) avoid copy let flags_info = self.base.flags_info.clone(); for kid in self.base.child_iter() { - let child_base = flow::mut_base(*kid); + let child_base = flow::mut_base(kid); child_base.position.size.width = self.base.position.size.width; child_base.flags_info.flags.set_inorder(self.base.flags_info.flags.inorder()); child_base.flags_info.propagate_text_alignment_from_parent(&flags_info) diff --git a/src/components/main/layout/layout_task.rs b/src/components/main/layout/layout_task.rs index cc8eae3e3db..d6aa0473d76 100644 --- a/src/components/main/layout/layout_task.rs +++ b/src/components/main/layout/layout_task.rs @@ -114,7 +114,7 @@ impl PostorderFlowTraversal for ComputeDamageTraversal { fn process(&mut self, flow: &mut Flow) -> bool { let mut damage = flow::base(flow).restyle_damage; for child in flow::child_iter(flow) { - damage.union_in_place(flow::base(*child).restyle_damage.propagate_up()) + damage.union_in_place(flow::base(child).restyle_damage.propagate_up()) } flow::mut_base(flow).restyle_damage = damage; true @@ -139,7 +139,7 @@ impl PreorderFlowTraversal for PropagateDamageTraversal { let prop = flow::base(flow).restyle_damage.propagate_down(); if prop.is_nonempty() { for kid_ctx in flow::child_iter(flow) { - flow::mut_base(*kid_ctx).restyle_damage.union_in_place(prop) + flow::mut_base(kid_ctx).restyle_damage.union_in_place(prop) } } true diff --git a/src/components/main/layout/parallel.rs b/src/components/main/layout/parallel.rs index 74c4a5c0073..589224bae4c 100644 --- a/src/components/main/layout/parallel.rs +++ b/src/components/main/layout/parallel.rs @@ -47,6 +47,18 @@ pub fn mut_owned_flow_to_unsafe_flow(flow: *mut ~Flow) -> UnsafeFlow { } } +pub fn borrowed_flow_to_unsafe_flow(flow: &Flow) -> UnsafeFlow { + unsafe { + cast::transmute_copy(&flow) + } +} + +pub fn mut_borrowed_flow_to_unsafe_flow(flow: &mut Flow) -> UnsafeFlow { + unsafe { + cast::transmute_copy(&flow) + } +} + /// Information that we need stored in each DOM node. pub struct DomParallelInfo { /// The number of children that still need work done. diff --git a/src/components/main/servo.rc b/src/components/main/servo.rc index 8b717f181f9..f45b83e4a53 100755 --- a/src/components/main/servo.rc +++ b/src/components/main/servo.rc @@ -92,6 +92,7 @@ pub mod layout { pub mod display_list_builder; pub mod float_context; pub mod flow; + pub mod flow_list; pub mod layout_task; pub mod inline; pub mod model; |