Skip to content

Simply restyle damage macros (and convert them to be plain functions) #222

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 7, 2025
Merged

Simply restyle damage macros (and convert them to be plain functions) #222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5079b1d
Simplify restyle damage macros
nicoburns Aug 4, 2025
a58aed7
Don't diff custom properties if we've already set damage anyway
nicoburns Aug 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 10 additions & 23 deletions style/properties/properties.mako.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2964,28 +2964,15 @@ const_assert!(std::mem::size_of::<longhands::${longhand.ident}::SpecifiedValue>(

% if engine == "servo":
% for effect_name in ["repaint", "recalculate_overflow", "rebuild_stacking_context", "rebuild_box"]:
macro_rules! restyle_damage_${effect_name} {
($old: ident, $new: ident, $damage: ident, [ $($effect:expr),* ]) => ({
restyle_damage_${effect_name}!($old, $new, $damage, [$($effect),*], false)
});
($old: ident, $new: ident, $damage: ident, [ $($effect:expr),* ], $extra:expr) => ({
if
% for style_struct in data.active_style_structs():
% for longhand in style_struct.longhands:
% if effect_name in longhand.servo_restyle_damage.split() and not longhand.logical:
$old.get_${style_struct.name_lower}().${longhand.ident} !=
$new.get_${style_struct.name_lower}().${longhand.ident} ||
% endif
% endfor
% endfor

$extra || false {
$damage.insert($($effect)|*);
true
} else {
false
}
});
}
pub(crate) fn restyle_damage_${effect_name} (old: &ComputedValues, new: &ComputedValues) -> bool {
% for style_struct in data.active_style_structs():
Copy link
Member

@emilio emilio Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW you really want to check arc::ptr_eq(old.get_${style_struct}(), new${style_struct}()) or so to avoid most of these comparisons. That's equivalent to what gecko does in CalcStyleDifference: https://searchfox.org/mozilla-central/rev/f6a806c38c459e0e0d797d264ca0e8ad46005105/layout/style/ComputedStyle.cpp#133

% for longhand in style_struct.longhands:
% if effect_name in longhand.servo_restyle_damage.split() and not longhand.logical:
old.get_${style_struct.name_lower}().${longhand.ident} != new.get_${style_struct.name_lower}().${longhand.ident} ||
% endif
% endfor
% endfor
false
}
% endfor
% endif
116 changes: 61 additions & 55 deletions style/servo/restyle_damage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ use crate::computed_values::mix_blend_mode::T as MixBlendMode;
use crate::computed_values::transform_style::T as TransformStyle;
use crate::dom::TElement;
use crate::matching::{StyleChange, StyleDifference};
use crate::properties::{style_structs, ComputedValues};
use crate::properties::{
restyle_damage_rebuild_box, restyle_damage_rebuild_stacking_context,
restyle_damage_recalculate_overflow, restyle_damage_repaint, style_structs, ComputedValues,
};
use crate::values::computed::basic_shape::ClipPath;
use crate::values::computed::Perspective;
use crate::values::generics::transform::{GenericRotate, GenericScale, GenericTranslate};
Expand Down Expand Up @@ -125,64 +128,67 @@ impl fmt::Display for ServoRestyleDamage {
}
}

fn augmented_restyle_damage_rebuild_box(old: &ComputedValues, new: &ComputedValues) -> bool {
let old_box = old.get_box();
let new_box = new.get_box();
restyle_damage_rebuild_box(old, new)
|| old_box.original_display != new_box.original_display
|| old_box.has_transform_or_perspective() != new_box.has_transform_or_perspective()
|| old.get_effects().filter.0.is_empty() != new.get_effects().filter.0.is_empty()
}

fn augmented_restyle_damage_rebuild_stacking_context(
old: &ComputedValues,
new: &ComputedValues,
) -> bool {
restyle_damage_rebuild_stacking_context(old, new)
|| old.guarantees_stacking_context() != new.guarantees_stacking_context()
}
fn compute_damage(old: &ComputedValues, new: &ComputedValues) -> ServoRestyleDamage {
let mut damage = ServoRestyleDamage::empty();

let has_transform_or_perspective_style = |style_box: &style_structs::Box| {
!style_box.transform.0.is_empty() ||
style_box.scale != GenericScale::None ||
style_box.rotate != GenericRotate::None ||
style_box.translate != GenericTranslate::None ||
style_box.perspective != Perspective::None ||
style_box.transform_style == TransformStyle::Preserve3d
};

let rebuild_box_extra = || {
let old_box = old.get_box();
let new_box = new.get_box();
old_box.original_display != new_box.original_display ||
has_transform_or_perspective_style(old_box) !=
has_transform_or_perspective_style(new_box) ||
old.get_effects().filter.0.is_empty() != new.get_effects().filter.0.is_empty()
};

// Some properties establish a stacking context when they are set to a non-initial value.
// In that case, the damage is only set to `ServoRestyleDamage::REPAINT` because we don't
// need to rebuild stacking contexts when the style changes between different non-initial
// values. This function checks whether any of these properties is set to a value that
// guarantees a stacking context, so that we only do the work when this changes.
// Note that it's still possible to establish a stacking context when this returns false.
let guarantees_stacking_context = |style: &ComputedValues| {
style.get_effects().opacity != 1.0 ||
old.get_effects().mix_blend_mode != MixBlendMode::Normal ||
old.get_svg().clip_path != ClipPath::None ||
style.get_box().isolation == Isolation::Isolate
};

// This uses short-circuiting boolean OR for its side effects and ignores the result.
let _ = restyle_damage_rebuild_box!(
old,
new,
damage,
[ServoRestyleDamage::RELAYOUT],
rebuild_box_extra()
) || restyle_damage_recalculate_overflow!(
old,
new,
damage,
[ServoRestyleDamage::RECALCULATE_OVERFLOW]
) || restyle_damage_rebuild_stacking_context!(
old,
new,
damage,
[ServoRestyleDamage::REBUILD_STACKING_CONTEXT],
guarantees_stacking_context(old) != guarantees_stacking_context(new)
) || restyle_damage_repaint!(old, new, damage, [ServoRestyleDamage::REPAINT]);

// Paint worklets may depend on custom properties,
// so if they have changed we should repaint.
if !old.custom_properties_equal(new) {
// Damage flags higher up the if-else chain imply damage flags lower down the if-else chain,
// so we can skip the diffing process for later flags if an earlier flag is true
if augmented_restyle_damage_rebuild_box(old, new) {
damage.insert(ServoRestyleDamage::RELAYOUT)
} else if restyle_damage_recalculate_overflow(old, new) {
damage.insert(ServoRestyleDamage::RECALCULATE_OVERFLOW)
} else if augmented_restyle_damage_rebuild_stacking_context(old, new) {
damage.insert(ServoRestyleDamage::REBUILD_STACKING_CONTEXT);
} else if restyle_damage_repaint(old, new) {
damage.insert(ServoRestyleDamage::REPAINT);
}
// Paint worklets may depend on custom properties, so if they have changed we should repaint.
else if !old.custom_properties_equal(new) {
damage.insert(ServoRestyleDamage::REPAINT);
}

damage
}

impl ComputedValues {
/// Some properties establish a stacking context when they are set to a non-initial value.
/// In that case, the damage is only set to `ServoRestyleDamage::REPAINT` because we don't
/// need to rebuild stacking contexts when the style changes between different non-initial
/// values. This function checks whether any of these properties is set to a value that
/// guarantees a stacking context, so that we only do the work when this changes.
/// Note that it's still possible to establish a stacking context when this returns false.
pub fn guarantees_stacking_context(&self) -> bool {
self.get_effects().opacity != 1.0
|| self.get_effects().mix_blend_mode != MixBlendMode::Normal
|| self.get_svg().clip_path != ClipPath::None
|| self.get_box().isolation == Isolation::Isolate
}
}

impl style_structs::Box {
/// Whether there is a non-default transform or perspective style set
pub fn has_transform_or_perspective(&self) -> bool {
!self.transform.0.is_empty()
|| self.scale != GenericScale::None
|| self.rotate != GenericRotate::None
|| self.translate != GenericTranslate::None
|| self.perspective != Perspective::None
|| self.transform_style == TransformStyle::Preserve3d
}
}
Loading