From 41dfd07d16340d437182c83517484b16303a83c9 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Fri, 30 Oct 2015 16:08:09 -0700 Subject: [PATCH 1/3] Rev rust-selectors. --- components/servo/Cargo.lock | 2 +- ports/cef/Cargo.lock | 2 +- ports/gonk/Cargo.lock | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/components/servo/Cargo.lock b/components/servo/Cargo.lock index 5c4478bfb36..d62b48cec42 100644 --- a/components/servo/Cargo.lock +++ b/components/servo/Cargo.lock @@ -1594,7 +1594,7 @@ dependencies = [ [[package]] name = "selectors" version = "0.2.0" -source = "git+https://github.com/servo/rust-selectors#d3bd9b7c665c838f703ef0bbc7fd67b89847550d" +source = "git+https://github.com/servo/rust-selectors#53f5e09a37684f6a42eb894d7a6fd0b14380a1c6" dependencies = [ "bitflags 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)", "cssparser 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/ports/cef/Cargo.lock b/ports/cef/Cargo.lock index 051695a4b02..1fa03c0f628 100644 --- a/ports/cef/Cargo.lock +++ b/ports/cef/Cargo.lock @@ -1518,7 +1518,7 @@ dependencies = [ [[package]] name = "selectors" version = "0.2.0" -source = "git+https://github.com/servo/rust-selectors#d3bd9b7c665c838f703ef0bbc7fd67b89847550d" +source = "git+https://github.com/servo/rust-selectors#53f5e09a37684f6a42eb894d7a6fd0b14380a1c6" dependencies = [ "bitflags 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)", "cssparser 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/ports/gonk/Cargo.lock b/ports/gonk/Cargo.lock index 07d85f98b20..4004400466a 100644 --- a/ports/gonk/Cargo.lock +++ b/ports/gonk/Cargo.lock @@ -1467,7 +1467,7 @@ dependencies = [ [[package]] name = "selectors" version = "0.2.0" -source = "git+https://github.com/servo/rust-selectors#d3bd9b7c665c838f703ef0bbc7fd67b89847550d" +source = "git+https://github.com/servo/rust-selectors#53f5e09a37684f6a42eb894d7a6fd0b14380a1c6" dependencies = [ "bitflags 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)", "cssparser 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)", From 45fc7f2930682fc52822db0e5aaaeab0f732229a Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Tue, 20 Oct 2015 21:58:50 -0700 Subject: [PATCH 2/3] Implement restyle hints for state changes. --- components/layout/layout_task.rs | 7 +- components/layout/wrapper.rs | 68 ++++--- components/style/lib.rs | 3 +- components/style/restyle_hints.rs | 247 ++++++++++++++++++++++++++ components/style/selector_matching.rs | 18 ++ 5 files changed, 315 insertions(+), 28 deletions(-) create mode 100644 components/style/restyle_hints.rs diff --git a/components/layout/layout_task.rs b/components/layout/layout_task.rs index 4ebe8363fbc..273d56bca80 100644 --- a/components/layout/layout_task.rs +++ b/components/layout/layout_task.rs @@ -1182,9 +1182,10 @@ impl LayoutTask { let state_changes = document.drain_element_state_changes(); if !needs_dirtying { - for &(el, state) in state_changes.iter() { - assert!(!state.is_empty()); - el.note_state_change(); + for &(el, state_change) in state_changes.iter() { + debug_assert!(!state_change.is_empty()); + let hint = rw_data.stylist.restyle_hint_for_state_change(&el, el.get_state(), state_change); + el.note_restyle_hint(hint); } } diff --git a/components/layout/wrapper.rs b/components/layout/wrapper.rs index 46a2caed506..a9bb80ee49a 100644 --- a/components/layout/wrapper.rs +++ b/components/layout/wrapper.rs @@ -70,6 +70,7 @@ use style::legacy::UnsignedIntegerAttribute; use style::node::TElementAttributes; use style::properties::ComputedValues; use style::properties::{PropertyDeclaration, PropertyDeclarationBlock}; +use style::restyle_hints::{RESTYLE_DESCENDANTS, RESTYLE_LATER_SIBLINGS, RESTYLE_SELF, RestyleHint}; use url::Url; use util::str::{is_whitespace, search_index}; @@ -408,39 +409,58 @@ impl<'le> LayoutElement<'le> { } } - /// Properly marks nodes as dirty in response to state changes. - /// - /// Currently this implementation is very conservative, and basically mirrors node::dirty_impl. - /// With restyle hints, we can do less work here. - pub fn note_state_change(&self) { - let node = self.as_node(); + pub fn get_state(&self) -> ElementState { + self.element.get_state_for_layout() + } - // Bail out if we're already dirty. This won't be valid when we start doing more targeted - // dirtying with restyle hints. - if node.is_dirty() { return } - - // Dirty descendants. - fn dirty_subtree(node: LayoutNode) { - // Stop if this subtree is already dirty. This won't be valid with restyle hints, see above. - if node.is_dirty() { return } - - unsafe { - node.set_dirty(true); - node.set_dirty_descendants(true); - } - - for kid in node.children() { - dirty_subtree(kid); - } + /// Properly marks nodes as dirty in response to restyle hints. + pub fn note_restyle_hint(&self, hint: RestyleHint) { + // Bail early if there's no restyling to do. + if hint.is_empty() { + return; } - dirty_subtree(node); + // If the restyle hint is non-empty, we need to restyle either this element + // or one of its siblings. Mark our ancestor chain as having dirty descendants. + let node = self.as_node(); let mut curr = node; while let Some(parent) = curr.parent_node() { if parent.has_dirty_descendants() { break } unsafe { parent.set_dirty_descendants(true); } curr = parent; } + + // Set up our helpers. + fn dirty_node(node: &LayoutNode) { + unsafe { + node.set_dirty(true); + node.set_dirty_descendants(true); + } + } + fn dirty_descendants(node: &LayoutNode) { + for ref child in node.children() { + dirty_node(child); + dirty_descendants(child); + } + } + + // Process hints. + if hint.contains(RESTYLE_SELF) { + dirty_node(&node); + } + if hint.contains(RESTYLE_DESCENDANTS) { + unsafe { node.set_dirty_descendants(true); } + dirty_descendants(&node); + } + if hint.contains(RESTYLE_LATER_SIBLINGS) { + let mut next = ::selectors::Element::next_sibling_element(self); + while let Some(sib) = next { + let sib_node = sib.as_node(); + dirty_node(&sib_node); + dirty_descendants(&sib_node); + next = ::selectors::Element::next_sibling_element(&sib); + } + } } } diff --git a/components/style/lib.rs b/components/style/lib.rs index ed898ff2f60..997cd90b79c 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -34,7 +34,7 @@ extern crate euclid; extern crate fnv; extern crate num; extern crate rustc_serialize; -extern crate selectors; +#[macro_use(state_pseudo_classes)] extern crate selectors; extern crate serde; extern crate smallvec; extern crate string_cache; @@ -48,6 +48,7 @@ pub mod legacy; pub mod media_queries; pub mod node; pub mod parser; +pub mod restyle_hints; pub mod selector_matching; pub mod stylesheets; #[macro_use] diff --git a/components/style/restyle_hints.rs b/components/style/restyle_hints.rs new file mode 100644 index 00000000000..acb521b0f10 --- /dev/null +++ b/components/style/restyle_hints.rs @@ -0,0 +1,247 @@ +/* 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/. */ + +use selectors::Element; +use selectors::matching::matches_compound_selector; +use selectors::parser::{AttrSelector, Combinator, CompoundSelector, SimpleSelector}; +use selectors::states::*; +use std::clone::Clone; +use std::sync::Arc; +use string_cache::{Atom, Namespace}; + +/// When the ElementState of an element (like IN_HOVER_STATE) changes, certain +/// pseudo-classes (like :hover) may require us to restyle that element, its +/// siblings, and/or its descendants. Doing this conservatively is expensive, +/// and so we RestyleHints to short-circuit work we know is unnecessary. +/// +/// NB: We should extent restyle hints to check for attribute-dependent style +/// in addition to state-dependent style (Gecko does this). + + +bitflags! { + flags RestyleHint: u8 { + #[doc = "Rerun selector matching on the element."] + const RESTYLE_SELF = 0x01, + #[doc = "Rerun selector matching on all of the element's descendants."] + // NB: In Gecko, we have RESTYLE_SUBTREE which is inclusive of self, but heycam isn't aware + // of a good reason for that. + const RESTYLE_DESCENDANTS = 0x02, + #[doc = "Rerun selector matching on all later siblings of the element and all of their descendants."] + const RESTYLE_LATER_SIBLINGS = 0x04, + } +} + +/// In order to compute restyle hints, we perform a selector match against a list of partial +/// selectors whose rightmost simple selector may be sensitive to the thing being changed. We +/// do this matching twice, once for the element as it exists now and once for the element as it +/// existed at the time of the last restyle. If the results of the selector match differ, that means +/// that the given partial selector is sensitive to the change, and we compute a restyle hint +/// based on its combinator. +/// +/// In order to run selector matching against the old element state, we generate a wrapper for +/// the element which claims to have the old state. This is the ElementWrapper logic below. +/// +/// Gecko does this differently for element states, and passes a mask called mStateMask, which +/// indicates the states that need to be ignored during selector matching. This saves an ElementWrapper +/// allocation and an additional selector match call at the expense of additional complexity inside +/// the selector matching logic. This only works for boolean states though, so we still need to +/// take the ElementWrapper approach for attribute-dependent style. So we do it the same both ways for +/// now to reduce complexity, but it's worth measuring the performance impact (if any) of the +/// mStateMask approach. +struct ElementWrapper where E: Element { + element: E, + state_override: ElementState, +} + +impl<'a, E> ElementWrapper where E: Element { + pub fn new(el: E) -> ElementWrapper { + ElementWrapper { element: el, state_override: ElementState::empty() } + } + + pub fn new_with_override(el: E, state: ElementState) -> ElementWrapper { + ElementWrapper { element: el, state_override: state } + } +} + +macro_rules! overridden_state_accessors { + ($( + $(#[$Flag_attr: meta])* + state $css: expr => $variant: ident / $method: ident / + $flag: ident = $value: expr, + )+) => { $( fn $method(&self) -> bool { self.state_override.contains($flag) } )+ + } +} + +impl Element for ElementWrapper where E: Element { + + // Implement the state accessors on Element to use our overridden state. + state_pseudo_classes!(overridden_state_accessors); + + fn parent_element(&self) -> Option { + self.element.parent_element().map(|el| ElementWrapper::new(el)) + } + fn first_child_element(&self) -> Option { + self.element.first_child_element().map(|el| ElementWrapper::new(el)) + } + fn last_child_element(&self) -> Option { + self.element.last_child_element().map(|el| ElementWrapper::new(el)) + } + fn prev_sibling_element(&self) -> Option { + self.element.prev_sibling_element().map(|el| ElementWrapper::new(el)) + } + fn next_sibling_element(&self) -> Option { + self.element.next_sibling_element().map(|el| ElementWrapper::new(el)) + } + fn is_html_element_in_html_document(&self) -> bool { + self.element.is_html_element_in_html_document() + } + fn get_local_name<'b>(&'b self) -> &'b Atom { + self.element.get_local_name() + } + fn get_namespace<'b>(&'b self) -> &'b Namespace { + self.element.get_namespace() + } + fn get_id(&self) -> Option { + self.element.get_id() + } + fn has_class(&self, name: &Atom) -> bool { + self.element.has_class(name) + } + fn match_attr(&self, attr: &AttrSelector, test: F) -> bool + where F: Fn(&str) -> bool { + self.element.match_attr(attr, test) + } + fn is_empty(&self) -> bool { + self.element.is_empty() + } + fn is_root(&self) -> bool { + self.element.is_root() + } + fn is_link(&self) -> bool { + self.element.is_link() + } + fn is_visited_link(&self) -> bool { + self.element.is_visited_link() + } + fn is_unvisited_link(&self) -> bool { + self.element.is_unvisited_link() + } + fn each_class(&self, callback: F) where F: FnMut(&Atom) { + self.element.each_class(callback) + } +} + +macro_rules! gen_selector_to_state { + ($( + $(#[$Flag_attr: meta])* + state $css: expr => $variant: ident / $method: ident / + $flag: ident = $value: expr, + )+) => { + fn selector_to_state(sel: &SimpleSelector) -> ElementState { + match *sel { + $( SimpleSelector::$variant => $flag, )+ + _ => ElementState::empty(), + } + } + } +} + +fn combinator_to_restyle_hint(combinator: Option) -> RestyleHint { + match combinator { + None => RESTYLE_SELF, + Some(c) => match c { + Combinator::Child => RESTYLE_DESCENDANTS, + Combinator::Descendant => RESTYLE_DESCENDANTS, + Combinator::NextSibling => RESTYLE_LATER_SIBLINGS, + Combinator::LaterSibling => RESTYLE_LATER_SIBLINGS, + } + } +} + +state_pseudo_classes!(gen_selector_to_state); + +// Mapping between (partial) CompoundSelectors (and the combinator to their right) +// and the states they depend on. +// +// In general, for all selectors in all applicable stylesheets of the form: +// +// |s _ s:X _ s _ s:Y _ s| +// +// Where: +// * Each |s| is an arbitrary simple selector. +// * Each |s| is an arbitrary combinator (or nothing). +// * X and Y are state-dependent pseudo-classes like :hover. +// +// We generate a StateDependency for both |s _ s:X _| and |s _ s:X _ s _ s:Y _|, even +// though those selectors may not appear on their own in any stylesheet. This allows +// us to quickly scan through the operation points of pseudo-classes and determine the +// maximum effect their associated state changes may have on the style of elements in +// the document. +#[derive(Debug)] +struct StateDependency { + selector: Arc, + combinator: Option, + state: ElementState, +} + +#[derive(Debug)] +pub struct StateDependencySet { + deps: Vec, +} + +impl StateDependencySet { + pub fn new() -> StateDependencySet { + StateDependencySet { deps: Vec::new() } + } + + pub fn compute_hint(&self, el: &E, current_state: ElementState, state_changes: ElementState) + -> RestyleHint where E: Element, E: Clone { + let mut hint = RestyleHint::empty(); + let mut old_state = current_state; + old_state.toggle(state_changes); + for dep in &self.deps { + if state_changes.intersects(dep.state) { + let old_el: ElementWrapper = ElementWrapper::new_with_override(el.clone(), old_state); + let matched_then = matches_compound_selector(&*dep.selector, &old_el, None, &mut false); + let matches_now = matches_compound_selector(&*dep.selector, el, None, &mut false); + if matched_then != matches_now { + hint.insert(combinator_to_restyle_hint(dep.combinator)); + if hint.is_all() { + break + } + } + } + } + hint + } + + pub fn note_selector(&mut self, selector: Arc) { + let mut cur = selector; + let mut combinator: Option = None; + loop { + if let Some(rightmost) = cur.simple_selectors.last() { + let state_dep = selector_to_state(rightmost); + if !state_dep.is_empty() { + self.deps.push(StateDependency { + selector: cur.clone(), + combinator: combinator, + state: state_dep, + }); + } + } + + cur = match cur.next { + Some((ref sel, comb)) => { + combinator = Some(comb); + sel.clone() + } + None => break, + } + } + } + + pub fn clear(&mut self) { + self.deps.clear(); + } +} diff --git a/components/style/selector_matching.rs b/components/style/selector_matching.rs index b758c535da8..53f1a0d3589 100644 --- a/components/style/selector_matching.rs +++ b/components/style/selector_matching.rs @@ -6,11 +6,13 @@ use legacy::PresentationalHintSynthesis; use media_queries::Device; use node::TElementAttributes; use properties::{PropertyDeclaration, PropertyDeclarationBlock}; +use restyle_hints::{RestyleHint, StateDependencySet}; use selectors::Element; use selectors::bloom::BloomFilter; use selectors::matching::DeclarationBlock as GenericDeclarationBlock; use selectors::matching::{Rule, SelectorMap}; use selectors::parser::PseudoElement; +use selectors::states::*; use smallvec::VecLike; use std::process; use style_traits::viewport::ViewportConstraints; @@ -41,6 +43,9 @@ pub struct Stylist { before_map: PerPseudoElementSelectorMap, after_map: PerPseudoElementSelectorMap, rules_source_order: usize, + + // Selector state dependencies used to compute restyle hints. + state_deps: StateDependencySet, } impl Stylist { @@ -55,6 +60,7 @@ impl Stylist { before_map: PerPseudoElementSelectorMap::new(), after_map: PerPseudoElementSelectorMap::new(), rules_source_order: 0, + state_deps: StateDependencySet::new(), }; // FIXME: Add iso-8859-9.css when the document’s encoding is ISO-8859-8. // FIXME: presentational-hints.css should be at author origin with zero specificity. @@ -102,6 +108,7 @@ impl Stylist { self.before_map = PerPseudoElementSelectorMap::new(); self.after_map = PerPseudoElementSelectorMap::new(); self.rules_source_order = 0; + self.state_deps.clear(); for stylesheet in &self.stylesheets { let (mut element_map, mut before_map, mut after_map) = match stylesheet.origin { @@ -151,6 +158,9 @@ impl Stylist { append!(style_rule, normal); append!(style_rule, important); rules_source_order += 1; + for selector in &style_rule.selectors { + self.state_deps.note_selector(selector.compound_selectors.clone()); + } } self.rules_source_order = rules_source_order; } @@ -162,6 +172,14 @@ impl Stylist { false } + pub fn restyle_hint_for_state_change(&self, element: &E, + current_state: ElementState, + state_change: ElementState) + -> RestyleHint + where E: Element + Clone { + self.state_deps.compute_hint(element, current_state, state_change) + } + pub fn set_device(&mut self, device: Device) { let is_dirty = self.is_dirty || self.stylesheets.iter() .flat_map(|stylesheet| stylesheet.rules().media()) From 564170f41bb82c57620541dc7f40bac32d63b4a6 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 29 Oct 2015 17:49:49 -0700 Subject: [PATCH 3/3] State restyle hint tests. This handles the state part of #6942. --- tests/wpt/mozilla/meta/MANIFEST.json | 24 ++++++++++ .../mozilla/tests/css/restyle_hints_state.css | 36 +++++++++++++++ .../tests/css/restyle_hints_state.html | 44 +++++++++++++++++++ .../tests/css/restyle_hints_state_ref.html | 19 ++++++++ 4 files changed, 123 insertions(+) create mode 100644 tests/wpt/mozilla/tests/css/restyle_hints_state.css create mode 100644 tests/wpt/mozilla/tests/css/restyle_hints_state.html create mode 100644 tests/wpt/mozilla/tests/css/restyle_hints_state_ref.html diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index 2c07bd173f4..bc04fb431a2 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -2927,6 +2927,18 @@ "url": "/_mozilla/css/quotes_simple_a.html" } ], + "css/restyle_hints_state.html": [ + { + "path": "css/restyle_hints_state.html", + "references": [ + [ + "/_mozilla/css/restyle_hints_state_ref.html", + "==" + ] + ], + "url": "/_mozilla/css/restyle_hints_state.html" + } + ], "css/root_height_a.html": [ { "path": "css/root_height_a.html", @@ -7560,6 +7572,18 @@ "url": "/_mozilla/css/quotes_simple_a.html" } ], + "css/restyle_hints_state.html": [ + { + "path": "css/restyle_hints_state.html", + "references": [ + [ + "/_mozilla/css/restyle_hints_state_ref.html", + "==" + ] + ], + "url": "/_mozilla/css/restyle_hints_state.html" + } + ], "css/root_height_a.html": [ { "path": "css/root_height_a.html", diff --git a/tests/wpt/mozilla/tests/css/restyle_hints_state.css b/tests/wpt/mozilla/tests/css/restyle_hints_state.css new file mode 100644 index 00000000000..673ac25d572 --- /dev/null +++ b/tests/wpt/mozilla/tests/css/restyle_hints_state.css @@ -0,0 +1,36 @@ +fieldset { + width: 200px; +} + +div { + background-color: black; + padding: 10px; + width: 50px; + height: 50px; +} + +fieldset div { + background-color: red; +} +fieldset:enabled div { + color: aqua; + background-color: green; +} +fieldset:enabled > div { + background-color: yellow; +} +fieldset:enabled ~ div { + color: pink; + background-color: purple; +} +fieldset:enabled + div { + color: brown; + background-color: orange; +} + +input:checked:enabled + span { + color: khaki; +} +input:checked:disabled + span { + color: lawngreen; +} diff --git a/tests/wpt/mozilla/tests/css/restyle_hints_state.html b/tests/wpt/mozilla/tests/css/restyle_hints_state.html new file mode 100644 index 00000000000..52f82bc905d --- /dev/null +++ b/tests/wpt/mozilla/tests/css/restyle_hints_state.html @@ -0,0 +1,44 @@ + + + + + + + +
+
+
sometext
+
+
+
othertext
+
+
+ + I should be lawngreen +
+ + + diff --git a/tests/wpt/mozilla/tests/css/restyle_hints_state_ref.html b/tests/wpt/mozilla/tests/css/restyle_hints_state_ref.html new file mode 100644 index 00000000000..d8d0ad1dd09 --- /dev/null +++ b/tests/wpt/mozilla/tests/css/restyle_hints_state_ref.html @@ -0,0 +1,19 @@ + + + + + + + +
+
+
sometext
+
+
+
othertext
+
+
+ + I should be lawngreen +
+