Skip to content

Commit

Permalink
Filter visited cascade to only visited dependent properties
Browse files Browse the repository at this point in the history
Speed up the visited cascade by only running it for the properties that are
actually visited dependent.  (These are only the properties where the separate
set of visited styles is even read at all, so running the rest is wasted work.)

MozReview-Commit-ID: 5B7wYtuH974
  • Loading branch information
jryans committed May 24, 2017
1 parent 47c8574 commit f12af6c
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 18 deletions.
14 changes: 12 additions & 2 deletions components/style/matching.rs
Expand Up @@ -13,7 +13,8 @@ use data::{ComputedStyle, ElementData, RestyleData};
use dom::{TElement, TNode};
use font_metrics::FontMetricsProvider;
use log::LogLevel::Trace;
use properties::{AnimationRules, CascadeFlags, ComputedValues, SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP, cascade};
use properties::{AnimationRules, CascadeFlags, ComputedValues};
use properties::{SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP, VISITED_DEPENDENT_ONLY, cascade};
use properties::longhands::display::computed_value as display;
use restyle_hints::{RESTYLE_CSS_ANIMATIONS, RESTYLE_CSS_TRANSITIONS, RestyleReplacements};
use restyle_hints::{RESTYLE_STYLE_ATTRIBUTE, RESTYLE_SMIL};
Expand Down Expand Up @@ -188,7 +189,7 @@ impl CascadeVisitedMode {
*self == CascadeVisitedMode::Unvisited
}

/// Returns whether animations should be processed based on the cascade
/// Returns whether animations should be processed based on the cascade
/// mode. At the moment, it appears we don't need to support animating
/// visited styles.
fn should_process_animations(&self) -> bool {
Expand All @@ -202,6 +203,12 @@ impl CascadeVisitedMode {
fn should_accumulate_damage(&self) -> bool {
*self == CascadeVisitedMode::Unvisited
}

/// Returns whether the cascade should filter to only visited dependent
/// properties based on the cascade mode.
fn visited_dependent_only(&self) -> bool {
*self == CascadeVisitedMode::Visited
}
}

trait PrivateMatchMethods: TElement {
Expand Down Expand Up @@ -246,6 +253,9 @@ trait PrivateMatchMethods: TElement {
if self.skip_root_and_item_based_display_fixup() {
cascade_flags.insert(SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP)
}
if cascade_visited.visited_dependent_only() {
cascade_flags.insert(VISITED_DEPENDENT_ONLY);
}

// Grab the inherited values.
let parent_el;
Expand Down
78 changes: 62 additions & 16 deletions components/style/properties/properties.mako.rs
Expand Up @@ -576,6 +576,57 @@ impl LonghandId {
% endfor
}
}

/// Only a few properties are allowed to depend on the visited state of
/// links. When cascading visited styles, we can save time by only
/// processing these properties.
fn is_visited_dependent(&self) -> bool {
matches!(*self,
% if product == "gecko":
LonghandId::ColumnRuleColor |
LonghandId::TextEmphasisColor |
LonghandId::WebkitTextFillColor |
LonghandId::WebkitTextStrokeColor |
LonghandId::TextDecorationColor |
LonghandId::Fill |
LonghandId::Stroke |
LonghandId::CaretColor |
% endif
LonghandId::Color |
LonghandId::BackgroundColor |
LonghandId::BorderTopColor |
LonghandId::BorderRightColor |
LonghandId::BorderBottomColor |
LonghandId::BorderLeftColor |
LonghandId::OutlineColor
)
}

/// The computed value of some properties depends on the (sometimes
/// computed) value of *other* properties.
///
/// So we classify properties into "early" and "other", such that the only
/// dependencies can be from "other" to "early".
///
/// Unfortunately, it’s not easy to check that this classification is
/// correct.
fn is_early_property(&self) -> bool {
matches!(*self,
% if product == 'gecko':
LonghandId::TextOrientation |
LonghandId::AnimationName |
LonghandId::TransitionProperty |
LonghandId::XLang |
LonghandId::MozScriptLevel |
% endif
LonghandId::FontSize |
LonghandId::FontFamily |
LonghandId::Color |
LonghandId::TextDecorationLine |
LonghandId::WritingMode |
LonghandId::Direction
)
}
}

/// An identifier for a given shorthand property.
Expand Down Expand Up @@ -2403,6 +2454,8 @@ bitflags! {
/// Whether to skip any root element and flex/grid item display style
/// fixup.
const SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP = 0x02,
/// Whether to only cascade properties that are visited dependent.
const VISITED_DEPENDENT_ONLY = 0x04,
}
}

Expand Down Expand Up @@ -2582,6 +2635,14 @@ pub fn apply_declarations<'a, F, I>(device: &Device,
PropertyDeclarationId::Custom(..) => continue,
};

// Only a few properties are allowed to depend on the visited state
// of links. When cascading visited styles, we can save time by
// only processing these properties.
if flags.contains(VISITED_DEPENDENT_ONLY) &&
!longhand_id.is_visited_dependent() {
continue
}

// The computed value of some properties depends on the
// (sometimes computed) value of *other* properties.
//
Expand All @@ -2593,26 +2654,11 @@ pub fn apply_declarations<'a, F, I>(device: &Device,
//
// Unfortunately, it’s not easy to check that this
// classification is correct.
let is_early_property = matches!(longhand_id,
LonghandId::FontSize |
LonghandId::FontFamily |
LonghandId::Color |
LonghandId::TextDecorationLine |
LonghandId::WritingMode |
LonghandId::Direction
% if product == 'gecko':
| LonghandId::TextOrientation
| LonghandId::AnimationName
| LonghandId::TransitionProperty
| LonghandId::XLang
| LonghandId::MozScriptLevel
% endif
);
if
% if category_to_cascade_now == "early":
!
% endif
is_early_property
longhand_id.is_early_property()
{
continue
}
Expand Down

0 comments on commit f12af6c

Please sign in to comment.