mozilla
/
gecko-dev
зеркало из https://github.com/mozilla/gecko-dev.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

servo: Merge #19935 - style: Cleanup StyleBuilder (from emilio:cleanup-style-builder); r=nox 

style: Cleanup StyleBuilder.

This is in preparation of a cascade optimization for custom properties.

This fixes various fishiness around our StyleBuilder stuff. In particular,
StyleBuilder::for_derived_style (renamed to for_animation) is only used to
compute specified values, and thus doesn't need to know about rules, visited
style, or other things like that.

The flag propagation that was done in StyleAdjuster is now done in StyleBuilder,
since we know beforehand which ones are always inherited, and it simplified the
callers and the StyleAdjuster code. It also fixed some fishiness wrt which flags
were propagated to anon boxes and text.

Source-Repo: https://github.com/servo/servo
Source-Revision: a0d9d3633b99c01868c98d2a5e64bf311f532d58

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : bd28ecd66b845f18aa545761ec4caee972f6fad5
This commit is contained in:
Emilio Cobos Álvarez 2018-02-03 12:24:23 -05:00
Родитель 200a23f376
Коммит ecbccf4248
9 изменённых файлов: 120 добавлений и 155 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

2
servo/components/style/matching.rs
Просмотреть файл

@ -336,7 +336,7 @@ trait PrivateMatchMethods: TElement {
// We need to cascade the children in order to ensure the correct
// propagation of inherited computed value flags.
if old_values.flags.inherited() != new_values.flags.inherited() {
if old_values.flags.maybe_inherited() != new_values.flags.maybe_inherited() {
debug!(" > flags changed: {:?} != {:?}", old_values.flags, new_values.flags);
return ChildCascadeRequirement::MustCascadeChildren;
}

32
servo/components/style/properties/computed_value_flags.rs
Просмотреть файл

@ -71,11 +71,35 @@ bitflags! {
}
impl ComputedValueFlags {
/// Returns the flags that are inherited.
/// Flags that are unconditionally propagated to descendants.
#[inline]
fn inherited_flags() -> Self {
ComputedValueFlags::IS_STYLE_IF_VISITED |
ComputedValueFlags::IS_RELEVANT_LINK_VISITED |
ComputedValueFlags::CAN_BE_FRAGMENTED |
ComputedValueFlags::IS_IN_DISPLAY_NONE_SUBTREE |
ComputedValueFlags::IS_IN_PSEUDO_ELEMENT_SUBTREE |
ComputedValueFlags::HAS_TEXT_DECORATION_LINES
}
/// Flags that may be propagated to descendants.
#[inline]
fn maybe_inherited_flags() -> Self {
Self::inherited_flags() | ComputedValueFlags::SHOULD_SUPPRESS_LINEBREAK
}
/// Returns the flags that are always propagated to descendants.
///
/// See StyleAdjuster::set_bits and StyleBuilder.
#[inline]
pub fn inherited(self) -> Self {
self & !(ComputedValueFlags::INHERITS_DISPLAY |
ComputedValueFlags::INHERITS_CONTENT |
ComputedValueFlags::INHERITS_RESET_STYLE)
self & Self::inherited_flags()
}
/// Flags that are conditionally propagated to descendants, just to handle
/// properly style invalidation.
#[inline]
pub fn maybe_inherited(self) -> Self {
self & Self::maybe_inherited_flags()
}
}

11
servo/components/style/properties/gecko.mako.rs
Просмотреть файл

@ -252,17 +252,6 @@ impl ops::DerefMut for ComputedValues {
}
impl ComputedValuesInner {
/// Clone the visited style. Used for inheriting parent styles in
/// StyleBuilder::for_derived_style.
pub fn clone_visited_style(&self) -> Option<Arc<ComputedValues>> {
self.visited_style.as_ref().map(|x| x.clone_arc())
}
#[inline]
pub fn is_display_contents(&self) -> bool {
self.get_box().clone_display() == longhands::display::computed_value::T::Contents
}
/// Returns true if the value of the `content` property would make a
/// pseudo-element not rendered.
#[inline]

74
servo/components/style/properties/properties.mako.rs
Просмотреть файл

@ -2216,6 +2216,11 @@ pub struct ComputedValues {
}
impl ComputedValues {
/// Returns whether this style's display value is equal to contents.
pub fn is_display_contents(&self) -> bool {
self.get_box().clone_display().is_contents()
}
/// Whether we're a visited style.
pub fn is_style_if_visited(&self) -> bool {
self.flags.contains(ComputedValueFlags::IS_STYLE_IF_VISITED)
@ -2335,17 +2340,6 @@ impl ComputedValuesInner {
/// Servo for obvious reasons.
pub fn has_moz_binding(&self) -> bool { false }
/// Clone the visited style. Used for inheriting parent styles in
/// StyleBuilder::for_derived_style.
pub fn clone_visited_style(&self) -> Option<Arc<ComputedValues>> {
self.visited_style.clone()
}
/// Returns whether this style's display value is equal to contents.
///
/// Since this isn't supported in Servo, this is always false for Servo.
pub fn is_display_contents(&self) -> bool { false }
#[inline]
/// Returns whether the "content" property for the given style is completely
/// ineffective, and would yield an empty `::before` or `::after`
@ -2728,8 +2722,6 @@ impl<'a> StyleBuilder<'a> {
cascade_flags: CascadeFlags,
rules: Option<StrongRuleNode>,
custom_properties: Option<Arc<::custom_properties::CustomPropertiesMap>>,
writing_mode: WritingMode,
mut flags: ComputedValueFlags,
visited_style: Option<Arc<ComputedValues>>,
) -> Self {
debug_assert_eq!(parent_style.is_some(), parent_style_ignoring_first_line.is_some());
@ -2751,6 +2743,7 @@ impl<'a> StyleBuilder<'a> {
reset_style
};
let mut flags = inherited_style.flags.inherited();
if cascade_flags.contains(CascadeFlags::VISITED_DEPENDENT_ONLY) {
flags.insert(ComputedValueFlags::IS_STYLE_IF_VISITED);
}
@ -2765,7 +2758,7 @@ impl<'a> StyleBuilder<'a> {
rules,
modified_reset: false,
custom_properties,
writing_mode,
writing_mode: inherited_style.writing_mode,
flags,
visited_style,
% for style_struct in data.active_style_structs():
@ -2783,13 +2776,16 @@ impl<'a> StyleBuilder<'a> {
self.flags.contains(ComputedValueFlags::IS_STYLE_IF_VISITED)
}
/// Creates a StyleBuilder holding only references to the structs of `s`, in
/// order to create a derived style.
pub fn for_derived_style(
/// NOTE(emilio): This is done so we can compute relative units with respect
/// to the parent style, but all the early properties / writing-mode / etc
/// are already set to the right ones on the kid.
///
/// Do _not_ actually call this to construct a style, this should mostly be
/// used for animations.
pub fn for_animation(
device: &'a Device,
style_to_derive_from: &'a ComputedValues,
parent_style: Option<<&'a ComputedValues>,
pseudo: Option<<&'a PseudoElement>,
) -> Self {
let reset_style = device.default_computed_values();
let inherited_style = parent_style.unwrap_or(reset_style);
@ -2803,13 +2799,13 @@ impl<'a> StyleBuilder<'a> {
// None of our callers pass in ::first-line parent styles.
inherited_style_ignoring_first_line: inherited_style,
reset_style,
pseudo,
pseudo: None,
modified_reset: false,
rules: None, // FIXME(emilio): Dubious...
rules: None,
custom_properties: style_to_derive_from.custom_properties().cloned(),
writing_mode: style_to_derive_from.writing_mode,
flags: style_to_derive_from.flags,
visited_style: style_to_derive_from.clone_visited_style(),
visited_style: None,
% for style_struct in data.active_style_structs():
${style_struct.ident}: StyleStructRef::Borrowed(
style_to_derive_from.${style_struct.name_lower}_arc()
@ -2912,7 +2908,7 @@ impl<'a> StyleBuilder<'a> {
/// computed values that need to be provided as well.
pub fn for_inheritance(
device: &'a Device,
parent: &'a ComputedValues,
parent: Option<<&'a ComputedValues>,
pseudo: Option<<&'a PseudoElement>,
) -> Self {
// Rebuild the visited style from the parent, ensuring that it will also
@ -2920,26 +2916,23 @@ impl<'a> StyleBuilder<'a> {
// produced by this builder. This assumes that the caller doesn't need
// to adjust or process visited style, so we can just build visited
// style here for simplicity.
let visited_style = parent.visited_style().map(|style| {
Self::for_inheritance(
device,
style,
pseudo,
).build()
let visited_style = parent.and_then(|parent| {
parent.visited_style().map(|style| {
Self::for_inheritance(
device,
Some(style),
pseudo,
).build()
})
});
// FIXME(emilio): This Some(parent) here is inconsistent with what we
// usually do if `parent` is the default computed values, but that's
// fine, and we want to eventually get rid of it.
Self::new(
device,
Some(parent),
Some(parent),
parent,
parent,
pseudo,
CascadeFlags::empty(),
/* rules = */ None,
parent.custom_properties().cloned(),
parent.writing_mode,
parent.flags.inherited(),
parent.and_then(|p| p.custom_properties().cloned()),
visited_style,
)
}
@ -3072,9 +3065,10 @@ impl<'a> StyleBuilder<'a> {
&self.inherited_style.writing_mode
}
/// Inherited style flags.
pub fn inherited_flags(&self) -> &ComputedValueFlags {
&self.inherited_style.flags
/// The computed value flags of our parent.
#[inline]
pub fn get_parent_flags(&self) -> ComputedValueFlags {
self.inherited_style.flags
}
/// And access to inherited style structs.
@ -3326,8 +3320,6 @@ where
flags,
Some(rules.clone()),
custom_properties,
WritingMode::empty(),
ComputedValueFlags::empty(),
visited_style,
),
cached_system_font: None,

58
servo/components/style/style_adjuster.rs
Просмотреть файл

@ -8,6 +8,7 @@
use app_units::Au;
use dom::TElement;
use properties::{self, CascadeFlags, ComputedValues, StyleBuilder};
use properties::computed_value_flags::ComputedValueFlags;
use properties::longhands::display::computed_value::T as Display;
use properties::longhands::float::computed_value::T as Float;
use properties::longhands::overflow_x::computed_value::T as Overflow;
@ -111,23 +112,23 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
/// Compute a few common flags for both text and element's style.
pub fn set_bits(&mut self) {
use properties::computed_value_flags::ComputedValueFlags;
let display = self.style.get_box().clone_display();
if self.style.inherited_flags().contains(ComputedValueFlags::IS_IN_DISPLAY_NONE_SUBTREE) ||
self.style.get_box().clone_display() == Display::None {
if !display.is_contents() && !self.style.get_text().clone_text_decoration_line().is_empty() {
self.style.flags.insert(ComputedValueFlags::HAS_TEXT_DECORATION_LINES);
}
if display == Display::None {
self.style.flags.insert(ComputedValueFlags::IS_IN_DISPLAY_NONE_SUBTREE);
}
if self.style.inherited_flags().contains(ComputedValueFlags::IS_IN_PSEUDO_ELEMENT_SUBTREE) ||
self.style.is_pseudo_element() {
if self.style.is_pseudo_element() {
self.style.flags.insert(ComputedValueFlags::IS_IN_PSEUDO_ELEMENT_SUBTREE);
}
#[cfg(feature = "servo")]
{
if self.style.inherited_flags().contains(ComputedValueFlags::CAN_BE_FRAGMENTED) ||
self.style.get_parent_column().is_multicol()
{
if self.style.get_parent_column().is_multicol() {
self.style.flags.insert(ComputedValueFlags::CAN_BE_FRAGMENTED);
}
}
@ -160,7 +161,6 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
fn adjust_for_text_combine_upright(&mut self) {
use computed_values::text_combine_upright::T as TextCombineUpright;
use computed_values::writing_mode::T as WritingMode;
use properties::computed_value_flags::ComputedValueFlags;
let writing_mode =
self.style.get_inheritedbox().clone_writing_mode();
@ -174,15 +174,18 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
}
}
/// Applies the line break suppression flag to text if it is in any ruby
/// box. This is necessary because its parent may not itself have the flag
/// set (e.g. ruby or ruby containers), thus we may not inherit the flag
/// from them.
/// Unconditionally propagates the line break suppression flag to text, and
/// additionally it applies it if it is in any ruby box.
///
/// This is necessary because its parent may not itself have the flag set
/// (e.g. ruby or ruby containers), thus we may not inherit the flag from
/// them.
#[cfg(feature = "gecko")]
fn adjust_for_text_in_ruby(&mut self) {
use properties::computed_value_flags::ComputedValueFlags;
let parent_display = self.style.get_parent_box().clone_display();
if parent_display.is_ruby_type() {
if parent_display.is_ruby_type() ||
self.style.get_parent_flags().contains(ComputedValueFlags::SHOULD_SUPPRESS_LINEBREAK)
{
self.style.flags.insert(ComputedValueFlags::SHOULD_SUPPRESS_LINEBREAK);
}
}
@ -426,18 +429,6 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
self.style.mutate_inheritedtext().set_text_align(TextAlign::Start)
}
/// Set the HAS_TEXT_DECORATION_LINES flag based on parent style.
fn adjust_for_text_decoration_lines(
&mut self,
layout_parent_style: &ComputedValues,
) {
use properties::computed_value_flags::ComputedValueFlags;
if layout_parent_style.flags.contains(ComputedValueFlags::HAS_TEXT_DECORATION_LINES) ||
!self.style.get_text().clone_text_decoration_line().is_empty() {
self.style.flags.insert(ComputedValueFlags::HAS_TEXT_DECORATION_LINES);
}
}
/// Computes the used text decoration for Servo.
///
/// FIXME(emilio): This is a layout tree concept, should move away from
@ -458,7 +449,6 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
&self,
layout_parent_style: &ComputedValues,
) -> bool {
use properties::computed_value_flags::ComputedValueFlags;
// Line break suppression should only be propagated to in-flow children.
if self.style.floated() || self.style.out_of_flow_positioned() {
return false;
@ -503,7 +493,6 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
where
E: TElement,
{
use properties::computed_value_flags::ComputedValueFlags;
use properties::longhands::unicode_bidi::computed_value::T as UnicodeBidi;
let self_display = self.style.get_box().clone_display();
@ -555,8 +544,6 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
where
E: TElement,
{
use properties::computed_value_flags::ComputedValueFlags;
if !self.style.has_visited_style() {
return;
}
@ -565,13 +552,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
self.style.pseudo.is_none() &&
element.map_or(false, |e| e.is_link());
let relevant_link_visited = if is_link_element {
element.unwrap().is_visited_link()
} else {
self.style.inherited_flags().contains(ComputedValueFlags::IS_RELEVANT_LINK_VISITED)
};
if relevant_link_visited {
if is_link_element && element.unwrap().is_visited_link() {
self.style.flags.insert(ComputedValueFlags::IS_RELEVANT_LINK_VISITED);
}
}
@ -674,7 +655,6 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
self.adjust_for_border_width();
self.adjust_for_outline();
self.adjust_for_writing_mode(layout_parent_style);
self.adjust_for_text_decoration_lines(layout_parent_style);
#[cfg(feature = "gecko")]
{
self.adjust_for_ruby(layout_parent_style, element);

9
servo/components/style/stylesheets/viewport_rule.rs
Просмотреть файл

@ -708,19 +708,16 @@ impl MaybeNew for ViewportConstraints {
}
// DEVICE-ADAPT § 6.2.3 Resolve non-auto lengths to pixel lengths
//
// Note: DEVICE-ADAPT § 5. states that relative length values are
// resolved against initial values
let initial_viewport = device.au_viewport_size();
let provider = get_metrics_provider_for_product();
let default_values = device.default_computed_values();
let mut conditions = RuleCacheConditions::default();
let context = Context {
is_root_element: false,
builder: StyleBuilder::for_derived_style(device, default_values, None, None),
// Note: DEVICE-ADAPT § 5. states that relative length values are
// resolved against initial values
builder: StyleBuilder::for_inheritance(device, None, None),
font_metrics_provider: &provider,
cached_system_font: None,
in_media_query: false,

3
servo/components/style/values/computed/mod.rs
Просмотреть файл

@ -174,12 +174,11 @@ impl<'a> Context<'a> {
F: FnOnce(&Context) -> R
{
let mut conditions = RuleCacheConditions::default();
let default_values = device.default_computed_values();
let provider = get_metrics_provider_for_product();
let context = Context {
is_root_element: false,
builder: StyleBuilder::for_derived_style(device, default_values, None, None),
builder: StyleBuilder::for_inheritance(device, None, None),
font_metrics_provider: &provider,
cached_system_font: None,
in_media_query: true,

10
servo/components/style/values/specified/box.rs
Просмотреть файл

@ -217,6 +217,16 @@ impl Display {
}
}
/// Returns true if the value is `Contents`
#[inline]
pub fn is_contents(&self) -> bool {
match *self {
#[cfg(feature = "gecko")]
Display::Contents => true,
_ => false,
}
}
/// Returns true if the value is `None`
#[inline]
pub fn is_none(&self) -> bool {

76
servo/ports/geckolib/glue.rs
Просмотреть файл

@ -1997,10 +1997,11 @@ pub extern "C" fn Servo_FontFeatureValuesRule_GetValueText(rule: RawServoFontFea
}
#[no_mangle]
pub extern "C" fn Servo_ComputedValues_GetForAnonymousBox(parent_style_or_null: ServoStyleContextBorrowedOrNull,
pseudo_tag: *mut nsAtom,
raw_data: RawServoStyleSetBorrowed)
-> ServoStyleContextStrong {
pub extern "C" fn Servo_ComputedValues_GetForAnonymousBox(
parent_style_or_null: ServoStyleContextBorrowedOrNull,
pseudo_tag: *mut nsAtom,
raw_data: RawServoStyleSetBorrowed,
) -> ServoStyleContextStrong {
let global_style_data = &*GLOBAL_STYLE_DATA;
let guard = global_style_data.shared_lock.read();
let guards = StylesheetGuards::same(&guard);
@ -2299,7 +2300,7 @@ fn get_pseudo_style(
Some(style.unwrap_or_else(|| {
StyleBuilder::for_inheritance(
doc_data.stylist.device(),
styles.primary(),
Some(styles.primary()),
Some(pseudo),
).build()
}))
@ -2318,30 +2319,18 @@ pub extern "C" fn Servo_ComputedValues_Inherit(
let atom = Atom::from(pseudo_tag);
let pseudo = PseudoElement::from_anon_box_atom(&atom)
.expect("Not an anon-box? Gah!");
let style = if let Some(reference) = parent_style_context {
let mut style = StyleBuilder::for_inheritance(
data.stylist.device(),
reference,
Some(&pseudo)
);
if for_text {
StyleAdjuster::new(&mut style)
.adjust_for_text();
}
let mut style = StyleBuilder::for_inheritance(
data.stylist.device(),
parent_style_context,
Some(&pseudo)
);
style.build()
} else {
debug_assert!(!for_text);
StyleBuilder::for_derived_style(
data.stylist.device(),
data.default_computed_values(),
/* parent_style = */ None,
Some(&pseudo),
).build()
};
if for_text {
StyleAdjuster::new(&mut style).adjust_for_text();
}
style.into()
style.build().into()
}
#[no_mangle]
@ -3658,22 +3647,20 @@ fn simulate_compute_values_failure(_: &PropertyValuePair) -> bool {
false
}
fn create_context<'a>(
fn create_context_for_animation<'a>(
per_doc_data: &'a PerDocumentStyleDataImpl,
font_metrics_provider: &'a FontMetricsProvider,
style: &'a ComputedValues,
parent_style: Option<&'a ComputedValues>,
pseudo: Option<&'a PseudoElement>,
for_smil_animation: bool,
rule_cache_conditions: &'a mut RuleCacheConditions,
) -> Context<'a> {
Context {
is_root_element: false,
builder: StyleBuilder::for_derived_style(
builder: StyleBuilder::for_animation(
per_doc_data.stylist.device(),
style,
parent_style,
pseudo,
),
font_metrics_provider: font_metrics_provider,
cached_system_font: None,
@ -3751,14 +3738,12 @@ pub extern "C" fn Servo_GetComputedKeyframeValues(
let parent_data = parent_element.as_ref().and_then(|e| e.borrow_data());
let parent_style = parent_data.as_ref().map(|d| d.styles.primary()).map(|x| &**x);
let pseudo = style.pseudo();
let mut conditions = Default::default();
let mut context = create_context(
let mut context = create_context_for_animation(
&data,
&metrics,
&style,
parent_style,
pseudo.as_ref(),
/* for_smil_animation = */ false,
&mut conditions,
);
@ -3860,14 +3845,12 @@ pub extern "C" fn Servo_GetAnimationValues(
let parent_data = parent_element.as_ref().and_then(|e| e.borrow_data());
let parent_style = parent_data.as_ref().map(|d| d.styles.primary()).map(|x| &**x);
let pseudo = style.pseudo();
let mut conditions = Default::default();
let mut context = create_context(
let mut context = create_context_for_animation(
&data,
&metrics,
&style,
parent_style,
pseudo.as_ref(),
/* for_smil_animation = */ true,
&mut conditions,
);
@ -3904,14 +3887,12 @@ pub extern "C" fn Servo_AnimationValue_Compute(
let parent_data = parent_element.as_ref().and_then(|e| e.borrow_data());
let parent_style = parent_data.as_ref().map(|d| d.styles.primary()).map(|x| &**x);
let pseudo = style.pseudo();
let mut conditions = Default::default();
let mut context = create_context(
let mut context = create_context_for_animation(
&data,
&metrics,
style,
parent_style,
pseudo.as_ref(),
/* for_smil_animation = */ false,
&mut conditions,
);
@ -4609,18 +4590,11 @@ pub extern "C" fn Servo_ComputeColor(
let computed_color = match raw_data {
Some(raw_data) => {
let data = PerDocumentStyleData::from_ffi(raw_data).borrow();
let metrics = get_metrics_provider_for_product();
let mut conditions = Default::default();
let context = create_context(
&data,
&metrics,
data.stylist.device().default_computed_values(),
/* parent_style = */ None,
/* pseudo = */ None,
/* for_smil_animation = */ false,
&mut conditions,
);
specified_color.to_computed_color(Some(&context))
let device = data.stylist.device();
let quirks_mode = data.stylist.quirks_mode();
Context::for_media_query_evaluation(device, quirks_mode, |context| {
specified_color.to_computed_color(Some(&context))
})
}
None => {
specified_color.to_computed_color(None)