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

stylo: avoid traversing non element/text nodes in style and layout #13172

Merged
merged 3 commits into from Sep 22, 2016
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Prev

Filter non-element / non-text nodes in LayoutIterator.

  • Loading branch information
bholley committed Sep 21, 2016
commit 63124bab66b945dea16ece366a790a693c38a452
@@ -734,9 +734,14 @@ impl<'ln> NodeInfo for ServoThreadSafeLayoutNode<'ln> {
fn is_element(&self) -> bool {
if let Some(LayoutNodeType::Element(_)) = self.type_id() { true } else { false }
}

This comment has been minimized.

@emilio

emilio Sep 21, 2016

Member

nit: missing newline


fn is_text_node(&self) -> bool {
if let Some(LayoutNodeType::Text) = self.type_id() { true } else { false }
}

fn needs_layout(&self) -> bool {
self.pseudo != PseudoElementType::Normal || self.is_element() || self.is_text_node()
}
}

impl<'ln> ThreadSafeLayoutNode for ServoThreadSafeLayoutNode<'ln> {
@@ -71,13 +71,22 @@ pub trait TRestyleDamage : Debug + PartialEq + BitOr<Output=Self> + Copy {
pub trait NodeInfo {
fn is_element(&self) -> bool;
fn is_text_node(&self) -> bool;

// Comments, doctypes, etc are ignored by layout algorithms.
fn needs_layout(&self) -> bool { self.is_element() || self.is_text_node() }
}

pub struct LayoutIterator<T>(pub T);
impl<T, I> Iterator for LayoutIterator<T> where T: Iterator<Item=I>, I: NodeInfo {
type Item = I;
fn next(&mut self) -> Option<I> {
self.0.next()
loop {
// Filter out nodes that layout should ignore.
let n = self.0.next();

This comment has been minimized.

@emilio

emilio Sep 21, 2016

Member

nit: maybe this would look clearer as:

match self.0.next() {
    None => return None,
    Some(node) if node.needs_layout() => return Some(node),
    _ => {},
}

Though I don't have a strong preference, so feel free to leave as-is.

if n.is_none() || n.as_ref().unwrap().needs_layout() {
return n
}
}
}
}

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.