From 2a199a49e20b962503ab065f29cfcb9972f4871e Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Fri, 2 Dec 2016 11:52:36 -0800 Subject: [PATCH 1/6] layout: Replace `ConstructionResult::swap_out()` with `ConstructionResult::get()`. It only actually swaps out in nonincremental mode (which isn't suitable for real world use), so the name is confusing. --- components/layout/construct.rs | 25 ++++++++++--------------- components/layout_thread/lib.rs | 2 +- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/components/layout/construct.rs b/components/layout/construct.rs index f3309b97b757..2fb7034e990c 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -85,11 +85,7 @@ pub enum ConstructionResult { } impl ConstructionResult { - pub fn swap_out(&mut self) -> ConstructionResult { - if opts::get().nonincremental_layout { - return mem::replace(self, ConstructionResult::None) - } - + pub fn get(&mut self) -> ConstructionResult { // FIXME(pcwalton): Stop doing this with inline fragments. Cloning fragments is very // inefficient! (*self).clone() @@ -478,7 +474,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> inline_fragment_accumulator: &mut InlineFragmentsAccumulator, abs_descendants: &mut AbsoluteDescendants, legalizer: &mut Legalizer) { - match kid.swap_out_construction_result() { + match kid.get_construction_result() { ConstructionResult::None => {} ConstructionResult::Flow(kid_flow, kid_abs_descendants) => { // If kid_flow is TableCaptionFlow, kid_flow should be added under @@ -776,7 +772,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> if kid.get_pseudo_element_type() != PseudoElementType::Normal { self.process(&kid); } - match kid.swap_out_construction_result() { + match kid.get_construction_result() { ConstructionResult::None => {} ConstructionResult::Flow(flow, kid_abs_descendants) => { if !flow::base(&*flow).flags.contains(IS_ABSOLUTELY_POSITIONED) { @@ -1022,7 +1018,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> side: caption_side::T) { // Only flows that are table captions are matched here. for kid in node.children() { - match kid.swap_out_construction_result() { + match kid.get_construction_result() { ConstructionResult::Flow(kid_flow, _) => { if kid_flow.is_table_caption() && kid_flow.as_block() @@ -1288,8 +1284,8 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> for kid in node.children() { // CSS 2.1 § 17.2.1. Treat all non-column child fragments of `table-column-group` // as `display: none`. - if let ConstructionResult::ConstructionItem(ConstructionItem::TableColumnFragment(fragment)) - = kid.swap_out_construction_result() { + if let ConstructionResult::ConstructionItem(ConstructionItem::TableColumnFragment( + fragment)) = kid.get_construction_result() { col_fragments.push(fragment) } } @@ -1619,9 +1615,8 @@ trait NodeUtils { /// Sets the construction result of a flow. fn set_flow_construction_result(self, result: ConstructionResult); - /// Replaces the flow construction result in a node with `ConstructionResult::None` and returns - /// the old value. - fn swap_out_construction_result(self) -> ConstructionResult; + /// Returns the construction result for this node. + fn get_construction_result(self) -> ConstructionResult; } impl NodeUtils for ConcreteThreadSafeLayoutNode @@ -1664,9 +1659,9 @@ impl NodeUtils for ConcreteThreadSafeLayoutNode } #[inline(always)] - fn swap_out_construction_result(self) -> ConstructionResult { + fn get_construction_result(self) -> ConstructionResult { let mut layout_data = self.mutate_layout_data().unwrap(); - self.construction_result_mut(&mut *layout_data).swap_out() + self.construction_result_mut(&mut *layout_data).get() } } diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index b62e25752bfc..768cb9496773 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -815,7 +815,7 @@ impl LayoutThread { Some(x) => x, None => return None, }; - let result = data.flow_construction_result.swap_out(); + let result = data.flow_construction_result.get(); let mut flow = match result { ConstructionResult::Flow(mut flow, abs_descendants) => { From ba274c9a0f064f7ca2cbe08d13a898254378a2d3 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Fri, 2 Dec 2016 11:54:07 -0800 Subject: [PATCH 2/6] script: Detect when the page is using `requestAnimationFrame()` for non-animation purposes and back off to a timer if so. After this patch, when the page calls `requestAnimationFrame()` too many times in the row without actually mutating the DOM, further calls to `rAF` will actually perform the moral equivalent of `setTimeout(16)`. This saves a lot of CPU time compared to actually waiting for the vertical blanking interval, because waiting for that requires us to draw the page. Reduces CPU usage drastically on nytimes.com, which has a perpetual `requestAnimationFrame()` loop that checks whether ads are in view. --- components/script/dom/document.rs | 98 ++++++++++++++++++++++++++----- components/script/timers.rs | 3 + 2 files changed, 87 insertions(+), 14 deletions(-) diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 8b66d2db75d3..83299f23ff83 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -104,7 +104,7 @@ use origin::Origin; use script_layout_interface::message::{Msg, ReflowQueryType}; use script_thread::{MainThreadScriptMsg, Runnable}; use script_traits::{AnimationState, CompositorEvent, MouseButton, MouseEventType, MozBrowserEvent}; -use script_traits::{ScriptMsg as ConstellationMsg, TouchpadPressurePhase}; +use script_traits::{MsDuration, ScriptMsg as ConstellationMsg, TouchpadPressurePhase}; use script_traits::{TouchEventType, TouchId}; use script_traits::UntrustedNodeAddress; use servo_atoms::Atom; @@ -128,9 +128,19 @@ use style::selector_parser::{RestyleDamage, Snapshot}; use style::str::{split_html_space_chars, str_join}; use style::stylesheets::Stylesheet; use time; +use timers::OneshotTimerCallback; use url::percent_encoding::percent_decode; use util::prefs::PREFS; +/// The number of times we are allowed to see spurious `requestAnimationFrame()` calls before +/// falling back to fake ones. +/// +/// A spurious `requestAnimationFrame()` call is defined as one that does not change the DOM. +const SPURIOUS_ANIMATION_FRAME_THRESHOLD: u8 = 5; + +/// The amount of time between fake `requestAnimationFrame()`s. +const FAKE_REQUEST_ANIMATION_FRAME_DELAY: u64 = 16; + pub enum TouchEventResult { Processed(bool), Forwarded, @@ -283,6 +293,11 @@ pub struct Document { last_click_info: DOMRefCell)>>, /// https://html.spec.whatwg.org/multipage/#ignore-destructive-writes-counter ignore_destructive_writes_counter: Cell, + /// The number of spurious `requestAnimationFrame()` requests we've received. + /// + /// A rAF request is considered spurious if nothing was actually reflowed. + spurious_animation_frames: Cell, + } #[derive(JSTraceable, HeapSizeOf)] @@ -1471,11 +1486,20 @@ impl Document { // // TODO: Should tick animation only when document is visible if !self.running_animation_callbacks.get() { - let global_scope = self.window.upcast::(); - let event = ConstellationMsg::ChangeRunningAnimationsState( - global_scope.pipeline_id(), - AnimationState::AnimationCallbacksPresent); - global_scope.constellation_chan().send(event).unwrap(); + if !self.is_faking_animation_frames() { + let global_scope = self.window.upcast::(); + let event = ConstellationMsg::ChangeRunningAnimationsState( + global_scope.pipeline_id(), + AnimationState::AnimationCallbacksPresent); + global_scope.constellation_chan().send(event).unwrap(); + } else { + let callback = FakeRequestAnimationFrameCallback { + document: Trusted::new(self), + }; + self.global() + .schedule_callback(OneshotTimerCallback::FakeRequestAnimationFrame(callback), + MsDuration::new(FAKE_REQUEST_ANIMATION_FRAME_DELAY)); + } } ident @@ -1494,6 +1518,7 @@ impl Document { let mut animation_frame_list = mem::replace(&mut *self.animation_frame_list.borrow_mut(), vec![]); self.running_animation_callbacks.set(true); + let was_faking_animation_frames = self.is_faking_animation_frames(); let timing = self.window.Performance().Now(); for (_, callback) in animation_frame_list.drain(..) { @@ -1502,24 +1527,40 @@ impl Document { } } + self.running_animation_callbacks.set(false); + + let spurious = !self.window.reflow(ReflowGoal::ForDisplay, + ReflowQueryType::NoQuery, + ReflowReason::RequestAnimationFrame); + // Only send the animation change state message after running any callbacks. // This means that if the animation callback adds a new callback for // the next frame (which is the common case), we won't send a NoAnimationCallbacksPresent // message quickly followed by an AnimationCallbacksPresent message. - if self.animation_frame_list.borrow().is_empty() { + // + // If this frame was spurious and we've seen too many spurious frames in a row, tell the + // constellation to stop giving us video refresh callbacks, to save energy. (A spurious + // animation frame is one in which the callback did not mutate the DOM—that is, an + // animation frame that wasn't actually used for animation.) + if self.animation_frame_list.borrow().is_empty() || + (!was_faking_animation_frames && self.is_faking_animation_frames()) { mem::swap(&mut *self.animation_frame_list.borrow_mut(), &mut animation_frame_list); let global_scope = self.window.upcast::(); - let event = ConstellationMsg::ChangeRunningAnimationsState(global_scope.pipeline_id(), - AnimationState::NoAnimationCallbacksPresent); + let event = ConstellationMsg::ChangeRunningAnimationsState( + global_scope.pipeline_id(), + AnimationState::NoAnimationCallbacksPresent); global_scope.constellation_chan().send(event).unwrap(); } - self.running_animation_callbacks.set(false); - - self.window.reflow(ReflowGoal::ForDisplay, - ReflowQueryType::NoQuery, - ReflowReason::RequestAnimationFrame); + // Update the counter of spurious animation frames. + if spurious { + if self.spurious_animation_frames.get() < SPURIOUS_ANIMATION_FRAME_THRESHOLD { + self.spurious_animation_frames.set(self.spurious_animation_frames.get() + 1) + } + } else { + self.spurious_animation_frames.set(0) + } } pub fn fetch_async(&self, load: LoadType, @@ -1880,6 +1921,7 @@ impl Document { target_element: MutNullableHeap::new(None), last_click_info: DOMRefCell::new(None), ignore_destructive_writes_counter: Default::default(), + spurious_animation_frames: Cell::new(0), } } @@ -2065,6 +2107,12 @@ impl Document { self.ignore_destructive_writes_counter.set( self.ignore_destructive_writes_counter.get() - 1); } + + /// Whether we've seen so many spurious animation frames (i.e. animation frames that didn't + /// mutate the DOM) that we've decided to fall back to fake ones. + fn is_faking_animation_frames(&self) -> bool { + self.spurious_animation_frames.get() >= SPURIOUS_ANIMATION_FRAME_THRESHOLD + } } @@ -3178,3 +3226,25 @@ pub enum FocusEventType { Focus, // Element gained focus. Doesn't bubble. Blur, // Element lost focus. Doesn't bubble. } + +/// A fake `requestAnimationFrame()` callback—"fake" because it is not triggered by the video +/// refresh but rather a simple timer. +/// +/// If the page is observed to be using `requestAnimationFrame()` for non-animation purposes (i.e. +/// without mutating the DOM), then we fall back to simple timeouts to save energy over video +/// refresh. +#[derive(JSTraceable, HeapSizeOf)] +pub struct FakeRequestAnimationFrameCallback { + /// The document. + #[ignore_heap_size_of = "non-owning"] + document: Trusted, +} + +impl FakeRequestAnimationFrameCallback { + #[allow(unrooted_must_root)] + pub fn invoke(self) { + let document = self.document.root(); + document.run_the_animation_frame_callbacks(); + } +} + diff --git a/components/script/timers.rs b/components/script/timers.rs index 6b67d6959e3b..a709e5966629 100644 --- a/components/script/timers.rs +++ b/components/script/timers.rs @@ -7,6 +7,7 @@ use dom::bindings::cell::DOMRefCell; use dom::bindings::codegen::Bindings::FunctionBinding::Function; use dom::bindings::reflector::Reflectable; use dom::bindings::str::DOMString; +use dom::document::FakeRequestAnimationFrameCallback; use dom::eventsource::EventSourceTimeoutCallback; use dom::globalscope::GlobalScope; use dom::testbinding::TestBindingCallback; @@ -71,6 +72,7 @@ pub enum OneshotTimerCallback { EventSourceTimeout(EventSourceTimeoutCallback), JsTimer(JsTimerTask), TestBindingCallback(TestBindingCallback), + FakeRequestAnimationFrame(FakeRequestAnimationFrameCallback), } impl OneshotTimerCallback { @@ -80,6 +82,7 @@ impl OneshotTimerCallback { OneshotTimerCallback::EventSourceTimeout(callback) => callback.invoke(), OneshotTimerCallback::JsTimer(task) => task.invoke(this, js_timers), OneshotTimerCallback::TestBindingCallback(callback) => callback.invoke(), + OneshotTimerCallback::FakeRequestAnimationFrame(callback) => callback.invoke(), } } } From 36246252936542e6060f45e8a9b041c46e20a63a Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Fri, 2 Dec 2016 11:57:45 -0800 Subject: [PATCH 3/6] query: Stop searching the entire flow tree to resolve script-to-layout queries. Instead of searching the entire tree to find the applicable fragment, we use the flow construction result of the nearest DOM node to find the flow we're looking for directly. We then recursively do this with ancestor DOM nodes in order to find the entire flow tree path, performing breadth first search to deal with anonymous flows in between. To fully eliminate the search, we will probably want some combination of (a) parent pointers in the flow tree and (b) rewritten script-to-layout queries that don't depend on visiting all ancestor flows of the query node. But this is a quick and dirty solution that dramatically improves the performance of layout queries. This is a large responsiveness increase on nytimes.com. --- components/layout/flow.rs | 84 ++++++++++++++++++++++++++++++++- components/layout/query.rs | 35 +++++++++----- components/layout/sequential.rs | 79 ++++++++++++++++++++++++------- 3 files changed, 168 insertions(+), 30 deletions(-) diff --git a/components/layout/flow.rs b/components/layout/flow.rs index 6f441d737c32..33c77247be0a 100644 --- a/components/layout/flow.rs +++ b/components/layout/flow.rs @@ -27,6 +27,7 @@ use app_units::Au; use block::{BlockFlow, FormattingContextType}; +use construct::ConstructionResult; use context::{LayoutContext, SharedLayoutContext}; use display_list_builder::DisplayListBuildState; use euclid::{Point2D, Size2D}; @@ -42,8 +43,10 @@ use inline::InlineFlow; use model::{CollapsibleMargins, IntrinsicISizes, MarginCollapseInfo}; use multicol::MulticolFlow; use parallel::FlowParallelInfo; +use script_layout_interface::wrapper_traits::LayoutNode; use serde::{Serialize, Serializer}; use std::{fmt, mem, raw}; +use std::collections::HashMap; use std::iter::Zip; use std::slice::IterMut; use std::sync::Arc; @@ -54,7 +57,8 @@ use style::dom::TRestyleDamage; use style::logical_geometry::{LogicalRect, LogicalSize, WritingMode}; use style::properties::ServoComputedValues; use style::selector_parser::RestyleDamage; -use style::servo::restyle_damage::{RECONSTRUCT_FLOW, REFLOW, REFLOW_OUT_OF_FLOW, REPAINT, REPOSITION}; +use style::servo::restyle_damage::{RECONSTRUCT_FLOW, REFLOW, REFLOW_OUT_OF_FLOW}; +use style::servo::restyle_damage::{REPAINT, REPOSITION}; use style::values::computed::LengthOrPercentageOrAuto; use table::TableFlow; use table_caption::TableCaptionFlow; @@ -63,6 +67,7 @@ use table_colgroup::TableColGroupFlow; use table_row::TableRowFlow; use table_rowgroup::TableRowGroupFlow; use table_wrapper::TableWrapperFlow; +use wrapper::LayoutNodeLayoutData; /// Virtual methods that make up a float context. /// @@ -499,6 +504,20 @@ pub trait ImmutableFlowUtils { fn floats_might_flow_through(self) -> bool; fn baseline_offset_of_last_line_box_in_flow(self) -> Option; + + /// Pushes the flows in between this flow and the given ancestor onto the supplied list. + /// + /// Returns true if there is a path from the ancestor to this flow and false otherwise. If + /// false is returned, `path` is unchanged. + /// + /// The flow itself will not be pushed onto the list, but the supplied ancestor will. + /// + /// This uses a breadth-first search, so you should try to ensure the flow and the ancestor are + /// not too far apart. + /// + /// FIXME(pcwalton): The breadth-first search is inefficient. Probably we'll want to add parent + /// pointers to flows after carefully thinking through the memory management implications. + fn find_path_to_ancestor(&self, ancestor: &Flow, path: &mut Vec) -> bool; } pub trait MutableFlowUtils { @@ -1303,6 +1322,34 @@ impl<'a> ImmutableFlowUtils for &'a Flow { } None } + + fn find_path_to_ancestor(&self, ancestor: &Flow, path: &mut Vec) -> bool { + let (mut queue, mut index, mut parents) = (vec![ancestor], 0, HashMap::new()); + loop { + let parent = queue[index].clone(); + index += 1; + if OpaqueFlow::from_flow(parent) == OpaqueFlow::from_flow(*self) { + break + } + for kid in base(parent).children.iter() { + parents.insert(OpaqueFlow::from_flow(kid), OpaqueFlow::from_flow(parent)); + queue.push(kid) + } + if index == queue.len() { + return false + } + } + + let mut flow = OpaqueFlow::from_flow(*self); + loop { + flow = *parents.get(&flow).expect("Didn't find parent during BFS?!"); + path.push(flow); + if flow == OpaqueFlow::from_flow(ancestor) { + break + } + } + true + } } impl<'a> MutableFlowUtils for &'a mut Flow { @@ -1476,7 +1523,7 @@ impl ContainingBlockLink { /// A wrapper for the pointer address of a flow. These pointer addresses may only be compared for /// equality with other such pointer addresses, never dereferenced. -#[derive(Copy, Clone, PartialEq, Eq, Debug)] +#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)] pub struct OpaqueFlow(pub usize); impl OpaqueFlow { @@ -1488,3 +1535,36 @@ impl OpaqueFlow { } } } + +/// Returns the path from the the root flow to the deepest flow that contains fragments belonging +/// to the given node. +/// +/// The returned path goes from bottom to top; thus the last element in it will always be the root. +/// The path begins with the deepest flow containing fragments for the node. +pub fn flow_tree_path_for_node(node: N) -> Vec where N: LayoutNode { + let mut result = vec![]; + trace(node, None, &mut result); + return result; + + fn trace(node: N, + mut last: Option, + path: &mut Vec) + where N: LayoutNode { + if let Some(ref layout_data) = node.borrow_layout_data() { + if let ConstructionResult::Flow(ref flow, _) = layout_data.flow_construction_result { + match last { + Some(ref descendant) => { + let succeeded = (&**descendant).find_path_to_ancestor(&**flow, path); + debug_assert!(succeeded) + } + None => path.push(OpaqueFlow::from_flow(&**flow)), + } + last = Some((*flow).clone()) + } + } + if let Some(parent) = node.parent_node() { + trace(parent, last, path) + } + } +} + diff --git a/components/layout/query.rs b/components/layout/query.rs index d34dbc59f070..43122ed7f4f0 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -369,7 +369,9 @@ pub fn process_content_box_request( // FIXME(pcwalton): This has not been updated to handle the stacking context relative // stuff. So the position is wrong in most cases. let mut iterator = UnioningFragmentBorderBoxIterator::new(requested_node.opaque()); - sequential::iterate_through_flow_tree_fragment_border_boxes(layout_root, &mut iterator); + sequential::for_each_fragment_of_node_and_ancestors(layout_root, + requested_node, + &mut iterator); match iterator.rect { Some(rect) => rect, None => Rect::zero() @@ -381,7 +383,9 @@ pub fn process_content_boxes_request(requested_node: N, layout_ro // FIXME(pcwalton): This has not been updated to handle the stacking context relative // stuff. So the position is wrong in most cases. let mut iterator = CollectingFragmentBorderBoxIterator::new(requested_node.opaque()); - sequential::iterate_through_flow_tree_fragment_border_boxes(layout_root, &mut iterator); + sequential::for_each_fragment_of_node_and_ancestors(layout_root, + requested_node, + &mut iterator); iterator.rects } @@ -574,14 +578,18 @@ impl FragmentBorderBoxIterator for ParentOffsetBorderBoxIterator { pub fn process_node_geometry_request(requested_node: N, layout_root: &mut Flow) -> Rect { let mut iterator = FragmentLocatingFragmentIterator::new(requested_node.opaque()); - sequential::iterate_through_flow_tree_fragment_border_boxes(layout_root, &mut iterator); + sequential::for_each_fragment_of_node_and_ancestors(layout_root, + requested_node, + &mut iterator); iterator.client_rect } pub fn process_node_scroll_area_request< N: LayoutNode>(requested_node: N, layout_root: &mut Flow) -> Rect { let mut iterator = UnioningFragmentScrollAreaIterator::new(requested_node.opaque()); - sequential::iterate_through_flow_tree_fragment_border_boxes(layout_root, &mut iterator); + sequential::for_each_fragment_of_node_and_ancestors(layout_root, + requested_node, + &mut iterator); match iterator.overflow_direction { OverflowDirection::RightAndDown => { let right = max(iterator.union_rect.size.width, iterator.origin_rect.size.width); @@ -679,7 +687,8 @@ pub fn process_resolved_style_request<'a, N, C>(requested_node: N, let applies = true; fn used_value_for_position_property( - layout_el: ::ConcreteThreadSafeLayoutElement, + layout_el: ::ConcreteThreadSafeLayoutElement, layout_root: &mut Flow, requested_node: N, property: &Atom) -> Option { @@ -706,8 +715,9 @@ pub fn process_resolved_style_request<'a, N, C>(requested_node: N, PositionRetrievingFragmentBorderBoxIterator::new(requested_node.opaque(), property, position); - sequential::iterate_through_flow_tree_fragment_border_boxes(layout_root, - &mut iterator); + sequential::for_each_fragment_of_node_and_ancestors(layout_root, + requested_node, + &mut iterator); iterator.result.map(|r| r.to_css_string()) } @@ -736,8 +746,9 @@ pub fn process_resolved_style_request<'a, N, C>(requested_node: N, side, margin_padding, style.writing_mode); - sequential::iterate_through_flow_tree_fragment_border_boxes(layout_root, - &mut iterator); + sequential::for_each_fragment_of_node_and_ancestors(layout_root, + requested_node, + &mut iterator); iterator.result.map(|r| r.to_css_string()) }, @@ -760,9 +771,11 @@ pub fn process_resolved_style_request<'a, N, C>(requested_node: N, } pub fn process_offset_parent_query(requested_node: N, layout_root: &mut Flow) - -> OffsetParentResponse { + -> OffsetParentResponse { let mut iterator = ParentOffsetBorderBoxIterator::new(requested_node.opaque()); - sequential::iterate_through_flow_tree_fragment_border_boxes(layout_root, &mut iterator); + sequential::for_each_fragment_of_node_and_ancestors(layout_root, + requested_node, + &mut iterator); let parent_info_index = iterator.parent_nodes.iter().rposition(|info| info.is_some()); match parent_info_index { Some(parent_info_index) => { diff --git a/components/layout/sequential.rs b/components/layout/sequential.rs index 716262432065..e727f6a8dbdb 100644 --- a/components/layout/sequential.rs +++ b/components/layout/sequential.rs @@ -9,12 +9,13 @@ use context::{LayoutContext, SharedLayoutContext}; use display_list_builder::DisplayListBuildState; use euclid::point::Point2D; use floats::SpeculatedFloatPlacement; -use flow::{self, Flow, ImmutableFlowUtils, InorderFlowTraversal, MutableFlowUtils}; +use flow::{self, Flow, ImmutableFlowUtils, InorderFlowTraversal, MutableFlowUtils, OpaqueFlow}; use flow::{PostorderFlowTraversal, PreorderFlowTraversal}; use flow::IS_ABSOLUTELY_POSITIONED; use fragment::FragmentBorderBoxIterator; use generated_content::ResolveGeneratedContent; use gfx_traits::ScrollRootId; +use script_layout_interface::wrapper_traits::LayoutNode; use style::context::StyleContext; use style::servo::restyle_damage::{REFLOW, STORE_OVERFLOW}; use traversal::{AssignBSizes, AssignISizes, BubbleISizes, BuildDisplayList}; @@ -86,29 +87,73 @@ pub fn build_display_list_for_subtree<'a>(flow_root: &mut Flow, build_display_list.state } -pub fn iterate_through_flow_tree_fragment_border_boxes(root: &mut Flow, - iterator: &mut FragmentBorderBoxIterator) { - fn doit(flow: &mut Flow, - level: i32, - iterator: &mut FragmentBorderBoxIterator, - stacking_context_position: &Point2D) { +/// Invokes a callback for each fragment belonging to the given node as well as all fragments +/// along the path leading to it. +/// +/// The callback receives the stacking-relative border box of each fragment, for convenience. +/// +/// FIXME(pcwalton): At some point, this should change to not iterate over fragments unless they +/// actually belong to the node. +pub fn for_each_fragment_of_node_and_ancestors(root_flow: &mut Flow, + node: N, + iterator: &mut FragmentBorderBoxIterator) + where N: LayoutNode { + // Find the path from the root to the node. + // + // FIXME(pcwalton): This would be unnecessary if we had parent pointers in the flow tree. I + // think they're going to be inevitable. + let mut path = flow::flow_tree_path_for_node(node); + + // If it's present, remove the root flow from the path, as we start there. + loop { + if let Some(&target) = path.last() { + if OpaqueFlow::from_flow(root_flow) == target { + path.pop(); + continue + } + } + break + } + + iterate(root_flow, 0, iterator, &mut path, &Point2D::zero()); + + fn iterate(flow: &mut Flow, + level: i32, + iterator: &mut FragmentBorderBoxIterator, + path: &mut Vec, + stacking_context_position: &Point2D) { + // Iterate through the border boxes of this flow. flow.iterate_through_fragment_border_boxes(iterator, level, stacking_context_position); + // Are we done? + if path.is_empty() { + return + } + + // Find the next child in the path. + let target_child = *path.last().expect("Path ended prematurely!"); for kid in flow::mut_base(flow).child_iter_mut() { - let stacking_context_position = - if kid.is_block_flow() && kid.as_block().fragment.establishes_stacking_context() { - let margin = Point2D::new(kid.as_block().fragment.margin.inline_start, Au(0)); - *stacking_context_position + flow::base(kid).stacking_relative_position + margin - } else { - *stacking_context_position - }; + if OpaqueFlow::from_flow(kid) == target_child { + path.pop(); + let stacking_context_position = + stacking_context_position_for_child(kid, stacking_context_position); + return iterate(kid, level + 1, iterator, path, &stacking_context_position) + } + } + + unreachable!("Didn't find next flow in path!") + } + fn stacking_context_position_for_child(child: &Flow, parent_position: &Point2D) + -> Point2D { + if child.is_block_flow() && child.as_block().fragment.establishes_stacking_context() { // FIXME(#2795): Get the real container size. - doit(kid, level + 1, iterator, &stacking_context_position); + let margin = Point2D::new(child.as_block().fragment.margin.inline_start, Au(0)); + *parent_position + flow::base(child).stacking_relative_position + margin + } else { + *parent_position } } - - doit(root, 0, iterator, &Point2D::zero()); } pub fn store_overflow(layout_context: &LayoutContext, flow: &mut Flow) { From 39a7ca274fbddabe7c74c0623b9b7138641595bd Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Fri, 2 Dec 2016 12:03:53 -0800 Subject: [PATCH 4/6] layout_thread: Don't propagate dirty bits around the tree if no styles changed and no flows were reconstructed. This saves a tree traversal. It also has the side effect of making incremental reflow bugs of the form "a dirty bit is never cleared" less likely to ruin the performance of layout queries. --- components/layout_thread/lib.rs | 70 +++++++++++++++++++++++---------- 1 file changed, 49 insertions(+), 21 deletions(-) diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 768cb9496773..0d0408a3191f 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -616,6 +616,7 @@ impl LayoutThread { self.perform_post_style_recalc_layout_passes(&reflow_info, None, None, + FlowTreeDamage::Dirty, &mut *rw_data, &mut layout_context); @@ -946,8 +947,6 @@ impl LayoutThread { println!("{}", serde_json::to_string_pretty(&display_list).unwrap()); } - debug!("Layout done!"); - // TODO: Avoid the temporary conversion and build webrender sc/dl directly! let builder = rw_data.display_list.as_ref().unwrap().convert_to_webrender(self.id); @@ -962,6 +961,8 @@ impl LayoutThread { webrender_traits::Epoch(epoch_number), viewport_size, builder); + + debug!("Layout done!"); }); } @@ -1128,8 +1129,14 @@ impl LayoutThread { viewport_size_changed, data.reflow_info.goal); + let flow_tree_damage = if element.styling_mode() == StylingMode::Stop { + FlowTreeDamage::Clean + } else { + FlowTreeDamage::Dirty + }; + let dom_depth = Some(0); // This is always the root node. - if element.styling_mode() != StylingMode::Stop { + if flow_tree_damage == FlowTreeDamage::Dirty { // Recalculate CSS styles and rebuild flows and fragments. profile(time::ProfilerCategory::LayoutStyleRecalc, self.profiler_metadata(), @@ -1179,6 +1186,7 @@ impl LayoutThread { self.perform_post_style_recalc_layout_passes(&data.reflow_info, Some(&data.query_type), Some(&document), + flow_tree_damage, &mut rw_data, &mut shared_layout_context); @@ -1313,6 +1321,7 @@ impl LayoutThread { self.perform_post_style_recalc_layout_passes(&reflow_info, None, None, + FlowTreeDamage::Dirty, &mut *rw_data, &mut layout_context); } @@ -1337,6 +1346,7 @@ impl LayoutThread { self.perform_post_style_recalc_layout_passes(&reflow_info, None, None, + FlowTreeDamage::Dirty, &mut *rw_data, &mut layout_context); } @@ -1345,27 +1355,34 @@ impl LayoutThread { data: &Reflow, query_type: Option<&ReflowQueryType>, document: Option<&ServoLayoutDocument>, + flow_tree_damage: FlowTreeDamage, rw_data: &mut LayoutThreadData, layout_context: &mut SharedLayoutContext) { - if let Some(mut root_flow) = self.root_flow.clone() { - // Kick off animations if any were triggered, expire completed ones. - animation::update_animation_state(&self.constellation_chan, - &self.script_chan, - &mut *self.running_animations.write(), - &mut *self.expired_animations.write(), - &self.new_animations_receiver, - self.id, - &self.timer); + let mut root_flow = match self.root_flow.clone() { + Some(root_flow) => root_flow, + None => return, + }; + // Kick off animations if any were triggered, expire completed ones. + animation::update_animation_state(&self.constellation_chan, + &self.script_chan, + &mut *self.running_animations.write(), + &mut *self.expired_animations.write(), + &self.new_animations_receiver, + self.id, + &self.timer); + + if flow_tree_damage == FlowTreeDamage::Dirty { profile(time::ProfilerCategory::LayoutRestyleDamagePropagation, self.profiler_metadata(), self.time_profiler_chan.clone(), || { - // Call `compute_layout_damage` even in non-incremental mode, because it sets flags - // that are needed in both incremental and non-incremental traversals. + // Call `compute_layout_damage` even in non-incremental mode, because it sets + // flags that are needed in both incremental and non-incremental traversals. let damage = FlowRef::deref_mut(&mut root_flow).compute_layout_damage(); - if opts::get().nonincremental_layout || damage.contains(REFLOW_ENTIRE_DOCUMENT) { + if opts::get().nonincremental_layout || + damage.contains(REFLOW_ENTIRE_DOCUMENT) { FlowRef::deref_mut(&mut root_flow).reflow_entire_document() } }); @@ -1419,13 +1436,13 @@ impl LayoutThread { sequential::store_overflow(&layout_context, FlowRef::deref_mut(&mut root_flow) as &mut Flow); }); - - self.perform_post_main_layout_passes(data, - query_type, - document, - rw_data, - layout_context); } + + self.perform_post_main_layout_passes(data, + query_type, + document, + rw_data, + layout_context); } fn perform_post_main_layout_passes(&mut self, @@ -1573,3 +1590,14 @@ lazy_static! { } }; } + +/// Whether the flow tree has been modified *at all* during restyling. +/// +/// Tracking this allows us to skip running passes entirely if they are trivially proven to be +/// no-ops. +#[derive(Clone, Copy, PartialEq)] +enum FlowTreeDamage { + Clean, + Dirty, +} + From 4139fd0de1689fcb3228d1109e8ebdaca5ae7653 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Fri, 2 Dec 2016 12:13:34 -0800 Subject: [PATCH 5/6] layout_thread: Assert that we removed the `REPAINT` flag from the root after building display lists. --- components/layout_thread/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 0d0408a3191f..f7d255a5089e 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -921,6 +921,8 @@ impl LayoutThread { build_state.root_stacking_context.bounds = origin; build_state.root_stacking_context.overflow = origin; rw_data.display_list = Some(Arc::new(build_state.to_display_list())); + + debug_assert!(!flow::base(layout_root).restyle_damage.contains(REPAINT)); } (ReflowGoal::ForScriptQuery, false) => {} } From 89895e331683466505119352b8977c39d20c75d0 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Fri, 2 Dec 2016 12:15:35 -0800 Subject: [PATCH 6/6] layout_thread: Minor style cleanup. --- components/layout_thread/lib.rs | 22 ++++++++++++------- .../script_layout_interface/wrapper_traits.rs | 7 ++++-- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index f7d255a5089e..2da02f5c6b35 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -1328,7 +1328,8 @@ impl LayoutThread { &mut layout_context); } - fn reflow_with_newly_loaded_web_font<'a, 'b>(&mut self, possibly_locked_rw_data: &mut RwData<'a, 'b>) { + fn reflow_with_newly_loaded_web_font<'a, 'b>(&mut self, + possibly_locked_rw_data: &mut RwData<'a, 'b>) { let mut rw_data = possibly_locked_rw_data.lock(); font_context::invalidate_font_caches(); @@ -1397,7 +1398,10 @@ impl LayoutThread { profile(time::ProfilerCategory::LayoutGeneratedContent, self.profiler_metadata(), self.time_profiler_chan.clone(), - || sequential::resolve_generated_content(FlowRef::deref_mut(&mut root_flow), &layout_context)); + || { + sequential::resolve_generated_content(FlowRef::deref_mut(&mut root_flow), + &layout_context) + }); // Guess float placement. profile(time::ProfilerCategory::LayoutFloatPlacementSpeculation, @@ -1416,15 +1420,17 @@ impl LayoutThread { match self.parallel_traversal { None => { // Sequential mode. - LayoutThread::solve_constraints(FlowRef::deref_mut(&mut root_flow), &layout_context) + LayoutThread::solve_constraints(FlowRef::deref_mut(&mut root_flow), + &layout_context) } Some(ref mut parallel) => { // Parallel mode. - LayoutThread::solve_constraints_parallel(parallel, - FlowRef::deref_mut(&mut root_flow), - profiler_metadata, - self.time_profiler_chan.clone(), - &*layout_context); + LayoutThread::solve_constraints_parallel( + parallel, + FlowRef::deref_mut(&mut root_flow), + profiler_metadata, + self.time_profiler_chan.clone(), + &*layout_context); } } }); diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs index 428642ce93b9..7ee73da5de07 100644 --- a/components/script_layout_interface/wrapper_traits.rs +++ b/components/script_layout_interface/wrapper_traits.rs @@ -60,7 +60,9 @@ impl PseudoElementType { pub fn style_pseudo_element(&self) -> PseudoElement { match *self { - PseudoElementType::Normal => unreachable!("style_pseudo_element called with PseudoElementType::Normal"), + PseudoElementType::Normal => { + unreachable!("style_pseudo_element called with PseudoElementType::Normal") + } PseudoElementType::Before(_) => PseudoElement::Before, PseudoElementType::After(_) => PseudoElement::After, PseudoElementType::DetailsSummary(_) => PseudoElement::DetailsSummary, @@ -150,7 +152,8 @@ impl Iterator for TreeIterator /// A thread-safe version of `LayoutNode`, used during flow construction. This type of layout /// node does not allow any parents or siblings of nodes to be accessed, to avoid races. -pub trait ThreadSafeLayoutNode: Clone + Copy + Debug + GetLayoutData + NodeInfo + PartialEq + Sized { +pub trait ThreadSafeLayoutNode: Clone + Copy + Debug + GetLayoutData + NodeInfo + PartialEq + + Sized { type ConcreteThreadSafeLayoutElement: ThreadSafeLayoutElement + ::selectors::Element;