Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reconstruct the box tree incrementally in some case #26741

Merged
merged 1 commit into from Jun 4, 2020
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -12,6 +12,7 @@ use crate::dom_traversal::{iter_child_nodes, Contents, NodeExt};
use crate::element_data::LayoutBox;
use crate::flow::construct::ContainsFloats;
use crate::flow::float::FloatBox;
use crate::flow::inline::InlineLevelBox;
use crate::flow::{BlockContainer, BlockFormattingContext, BlockLevelBox};
use crate::formatting_contexts::IndependentFormattingContext;
use crate::fragments::Fragment;
@@ -22,9 +23,11 @@ use crate::positioned::PositioningContext;
use crate::replaced::ReplacedContent;
use crate::sizing::ContentSizesRequest;
use crate::style_ext::ComputedValuesExt;
use crate::style_ext::{Display, DisplayGeneratingBox};
use crate::style_ext::{Display, DisplayGeneratingBox, DisplayInside};
use crate::wrapper::GetStyleAndLayoutData;
use crate::DefiniteContainingBlock;
use app_units::Au;
use atomic_refcell::AtomicRef;
use euclid::default::{Point2D, Rect, Size2D};
use fxhash::FxHashSet;
use gfx_traits::print_tree::PrintTree;
@@ -88,6 +91,141 @@ impl BoxTree {
canvas_background: CanvasBackground::for_root_element(context, root_element),
}
}

/// This method attempts to incrementally update the box tree from an
/// arbitrary node that is not necessarily the document's root element.
///
/// If the node is not a valid candidate for incremental update, the method
/// loops over its parent. The only valid candidates for now are absolutely
/// positioned boxes which don't change their outside display mode (i.e. it
/// will not attempt to update from an absolutely positioned inline element
/// which became an absolutely positioned block element). The value `true`
/// is returned if an incremental update could be done, and `false`
/// otherwise.
///
/// There are various pain points that need to be taken care of to extend
/// the set of valid candidates:
/// * it is not obvious how to incrementally check whether a block
/// formatting context still contains floats or not;
/// * the propagation of text decorations towards node descendants is
/// hard to do incrementally with our current representation of boxes
/// * how intrinsic content sizes are computed eagerly makes it hard
/// to update those sizes for ancestors of the node from which we
/// made an incremental update.
pub fn update<'dom, Node>(context: &LayoutContext, mut dirty_node: Node) -> bool
where
Node: 'dom + Copy + LayoutNode<'dom> + Send + Sync,
{
enum UpdatePoint {
AbsolutelyPositionedBlockLevelBox(ArcRefCell<BlockLevelBox>),
AbsolutelyPositionedInlineLevelBox(ArcRefCell<InlineLevelBox>),
}

fn update_point<'dom, Node>(
node: Node,
) -> Option<(Arc<ComputedValues>, DisplayInside, UpdatePoint)>
where
Node: NodeExt<'dom>,
{
if !node.is_element() {
return None;
}

if node.type_id() == LayoutNodeType::Element(LayoutElementType::HTMLBodyElement) {
// This can require changes to the canvas background.
return None;
}

// Don't update unstyled nodes.
let data = node.get_style_and_layout_data()?;

// Don't update nodes that have pseudo-elements.
let element_data = data.style_data.element_data.borrow();
if !element_data.styles.pseudos.is_empty() {
return None;
}

let layout_data = data.layout_data.borrow();
if layout_data.pseudo_before_box.borrow().is_some() {
return None;
}
if layout_data.pseudo_after_box.borrow().is_some() {
return None;
}

let primary_style = element_data.styles.primary();
let box_style = primary_style.get_box();

if !box_style.position.is_absolutely_positioned() {
return None;
}

let display_inside = match Display::from(box_style.display) {
Display::GeneratingBox(DisplayGeneratingBox::OutsideInside { inside, .. }) => {
inside
},
_ => return None,
};

let update_point =
match &*AtomicRef::filter_map(layout_data.self_box.borrow(), Option::as_ref)? {
LayoutBox::DisplayContents => return None,
LayoutBox::BlockLevel(block_level_box) => match &*block_level_box.borrow() {
BlockLevelBox::OutOfFlowAbsolutelyPositionedBox(_)
if box_style.position.is_absolutely_positioned() =>
{
UpdatePoint::AbsolutelyPositionedBlockLevelBox(block_level_box.clone())
},
_ => return None,
},
LayoutBox::InlineLevel(inline_level_box) => match &*inline_level_box.borrow() {
InlineLevelBox::OutOfFlowAbsolutelyPositionedBox(_)
if box_style.position.is_absolutely_positioned() =>
{
UpdatePoint::AbsolutelyPositionedInlineLevelBox(
inline_level_box.clone(),
)
},
_ => return None,
},
};
Some((primary_style.clone(), display_inside, update_point))
}

loop {
This conversation was marked as resolved by SimonSapin

This comment has been minimized.

Copy link
@SimonSapin

SimonSapin Jun 3, 2020

Member

Nit: can this be a for loop over dirty_node.inclusive_ancestors()?

This comment has been minimized.

Copy link
@SimonSapin

SimonSapin Jun 3, 2020

Member

Or even replace the loop with dirty_node.inclusive_ancestors().find(update_point)?

This comment has been minimized.

Copy link
@SimonSapin

SimonSapin Jun 4, 2020

Member

As discussed on Matrix, this iterator is not available on wrapper types for DOM types in layout. They could be added, but no need to block this PR on that.

if let Some((primary_style, display_inside, update_point)) = update_point(dirty_node) {
let contents = ReplacedContent::for_element(dirty_node)
.map_or(Contents::OfElement, Contents::Replaced);
let out_of_flow_absolutely_positioned_box =
Arc::new(AbsolutelyPositionedBox::construct(
context,
dirty_node,
primary_style,
display_inside,
contents,
));
match update_point {
UpdatePoint::AbsolutelyPositionedBlockLevelBox(block_level_box) => {
*block_level_box.borrow_mut() =
BlockLevelBox::OutOfFlowAbsolutelyPositionedBox(
out_of_flow_absolutely_positioned_box,
);
},
UpdatePoint::AbsolutelyPositionedInlineLevelBox(inline_level_box) => {
*inline_level_box.borrow_mut() =
InlineLevelBox::OutOfFlowAbsolutelyPositionedBox(
out_of_flow_absolutely_positioned_box,
);
},
}
return true;
}
dirty_node = match dirty_node.parent_node() {
Some(parent) => parent,
None => return false,
};
}
}
}

fn construct_for_root_element<'dom>(
@@ -1081,39 +1081,43 @@ impl LayoutThread {
let rayon_pool = STYLE_THREAD_POOL.pool();
let rayon_pool = rayon_pool.as_ref();

let box_tree = if token.should_traverse() {
driver::traverse_dom(&traversal, token, rayon_pool);
if token.should_traverse() {
let dirty_root = driver::traverse_dom(&traversal, token, rayon_pool).as_node();

let root_node = root_element.as_node();
let build_box_tree = || BoxTree::construct(traversal.context(), root_node);
let box_tree = if let Some(pool) = rayon_pool {
let mut box_tree = self.box_tree.borrow_mut();
let box_tree = &mut *box_tree;
let mut build_box_tree = || {
if !BoxTree::update(traversal.context(), dirty_root) {
*box_tree = Some(Arc::new(BoxTree::construct(traversal.context(), root_node)));
}
};
if let Some(pool) = rayon_pool {
pool.install(build_box_tree)
} else {
build_box_tree()
};

Some(Arc::new(box_tree))
} else {
None
};

layout_context = traversal.destroy();

if let Some(box_tree) = box_tree {
let viewport_size = Size2D::new(
self.viewport_size.width.to_f32_px(),
self.viewport_size.height.to_f32_px(),
);
let run_layout = || box_tree.layout(&layout_context, viewport_size);
let run_layout = || {
box_tree
.as_ref()
.unwrap()
.layout(traversal.context(), viewport_size)
};
let fragment_tree = Arc::new(if let Some(pool) = rayon_pool {
pool.install(run_layout)
} else {
run_layout()
});
*self.box_tree.borrow_mut() = Some(box_tree);
*self.fragment_tree.borrow_mut() = Some(fragment_tree);
}

layout_context = traversal.destroy();

for element in elements_with_snapshot {
unsafe { element.unset_snapshot_flags() }
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.