commit c4fe65e572fea0d79387873075bc93f12d660e69
parent 2a90cd0fc9ad47173b019981ca62dfff459d6d17
Author: Sajid Anwar <sajidanwar94@gmail.com>
Date: Fri, 19 Dec 2025 14:32:25 +0000
Bug 2004948 - Calculate rcap/rch/rex/ric units only after first use in the document. r=emilio,firefox-style-system-reviewers
Differential Revision: https://phabricator.services.mozilla.com/D276143
Diffstat:
4 files changed, 98 insertions(+), 75 deletions(-)
diff --git a/servo/components/style/font_metrics.rs b/servo/components/style/font_metrics.rs
@@ -6,7 +6,7 @@
#![deny(missing_docs)]
-use crate::values::computed::{FontSize, Length};
+use crate::values::computed::Length;
/// Represents the font metrics that style needs from a font to compute the
/// value of certain CSS units like `ex`.
@@ -46,7 +46,7 @@ impl Default for FontMetrics {
impl FontMetrics {
/// Returns the x-height, computing a fallback value if not present
- pub fn x_height_or_default(&self, reference_font_size: &FontSize) -> Length {
+ pub fn x_height_or_default(&self, reference_font_size: Length) -> Length {
// https://drafts.csswg.org/css-values/#ex
//
// In the cases where it is impossible or impractical to
@@ -55,14 +55,13 @@ impl FontMetrics {
//
// (But note we use 0.5em of the used, not computed
// font-size)
- self.x_height
- .unwrap_or_else(|| reference_font_size.used_size() * 0.5)
+ self.x_height.unwrap_or_else(|| reference_font_size * 0.5)
}
/// Returns the zero advance measure, computing a fallback value if not present
pub fn zero_advance_measure_or_default(
&self,
- reference_font_size: &FontSize,
+ reference_font_size: Length,
upright: bool,
) -> Length {
// https://drafts.csswg.org/css-values/#ch
@@ -79,9 +78,9 @@ impl FontMetrics {
// above.
self.zero_advance_measure.unwrap_or_else(|| {
if upright {
- reference_font_size.used_size()
+ reference_font_size
} else {
- reference_font_size.used_size() * 0.5
+ reference_font_size * 0.5
}
})
}
@@ -98,7 +97,7 @@ impl FontMetrics {
}
/// Returns the ideographic advance measure, computing a fallback value if not present
- pub fn ic_width_or_default(&self, reference_font_size: &FontSize) -> Length {
+ pub fn ic_width_or_default(&self, reference_font_size: Length) -> Length {
// https://drafts.csswg.org/css-values/#ic
//
// In the cases where it is impossible or impractical to
@@ -107,8 +106,7 @@ impl FontMetrics {
//
// Same caveat about computed vs. used as for other
// metric-dependent units.
- self.ic_width
- .unwrap_or_else(|| reference_font_size.used_size())
+ self.ic_width.unwrap_or_else(|| reference_font_size)
}
}
diff --git a/servo/components/style/gecko/media_queries.rs b/servo/components/style/gecko/media_queries.rs
@@ -27,27 +27,28 @@ use crate::values::{CustomIdent, KeyframesName};
use app_units::{Au, AU_PER_PX};
use euclid::default::Size2D;
use euclid::{Scale, SideOffsets2D};
+use parking_lot::RwLock;
use servo_arc::Arc;
use std::sync::atomic::{AtomicBool, AtomicU32, AtomicUsize, Ordering};
-use std::{cmp, fmt};
+use std::{cmp, fmt, mem};
use style_traits::{CSSPixel, DevicePixel};
/// The `Device` in Gecko wraps a pres context, has a default values computed,
/// and contains all the viewport rule state.
+///
+/// This structure also contains atomics used for computing root font-relative
+/// units. These atomics use relaxed ordering, since when computing the style
+/// of the root element, there can't be any other style being computed at the
+/// same time (given we need the style of the parent to compute everything else).
pub struct Device {
/// NB: The document owns the styleset, who owns the stylist, and thus the
/// `Device`, so having a raw document pointer here is fine.
document: *const structs::Document,
default_values: Arc<ComputedValues>,
- /// The font size of the root element.
- ///
- /// This is set when computing the style of the root element, and used for
- /// rem units in other elements.
- ///
- /// When computing the style of the root element, there can't be any other
- /// style being computed at the same time, given we need the style of the
- /// parent to compute everything else. So it is correct to just use a
- /// relaxed atomic here.
+ /// Current computed style of the root element, used for calculations of
+ /// root font-relative units.
+ root_style: RwLock<Arc<ComputedValues>>,
+ /// Font size of the root element, used for rem units in other elements.
root_font_size: AtomicU32,
/// Line height of the root element, used for rlh units in other elements.
root_line_height: AtomicU32,
@@ -71,8 +72,9 @@ pub struct Device {
/// by using rlh units.
used_root_line_height: AtomicBool,
/// Whether any styles computed in the document relied on the root font metrics
- /// by using rcap, rch, rex, or ric units.
- used_root_font_metrics: AtomicBool,
+ /// by using rcap, rch, rex, or ric units. This is a lock instead of an atomic
+ /// in order to prevent concurrent writes to the root font metric values.
+ used_root_font_metrics: RwLock<bool>,
/// Whether any styles computed in the document relied on font metrics.
used_font_metrics: AtomicBool,
/// Whether any styles computed in the document relied on the viewport size
@@ -110,9 +112,12 @@ impl Device {
assert!(!document.is_null());
let doc = unsafe { &*document };
let prefs = unsafe { &*bindings::Gecko_GetPrefSheetPrefs(doc) };
+ let default_values = ComputedValues::default_values(doc);
+ let root_style = RwLock::new(Arc::clone(&default_values));
Device {
document,
- default_values: ComputedValues::default_values(doc),
+ default_values: default_values,
+ root_style: root_style,
root_font_size: AtomicU32::new(FONT_MEDIUM_PX.to_bits()),
root_line_height: AtomicU32::new(FONT_MEDIUM_LINE_HEIGHT_PX.to_bits()),
root_font_metrics_ex: AtomicU32::new(FONT_MEDIUM_EX_PX.to_bits()),
@@ -125,7 +130,7 @@ impl Device {
body_text_color: AtomicUsize::new(prefs.mLightColors.mDefault as usize),
used_root_font_size: AtomicBool::new(false),
used_root_line_height: AtomicBool::new(false),
- used_root_font_metrics: AtomicBool::new(false),
+ used_root_font_metrics: RwLock::new(false),
used_font_metrics: AtomicBool::new(false),
used_viewport_size: AtomicBool::new(false),
used_dynamic_viewport_size: AtomicBool::new(false),
@@ -186,6 +191,12 @@ impl Device {
&self.default_values
}
+ /// Store a pointer to the root element's computed style, for use in
+ /// calculation of root font-relative metrics.
+ pub fn set_root_style(&self, style: &Arc<ComputedValues>) {
+ *self.root_style.write() = style.clone();
+ }
+
/// Get the font size of the root element (for rem)
pub fn root_font_size(&self) -> Length {
self.used_root_font_size.store(true, Ordering::Relaxed);
@@ -213,7 +224,7 @@ impl Device {
/// Get the x-height of the root element (for rex)
pub fn root_font_metrics_ex(&self) -> Length {
- self.used_root_font_metrics.store(true, Ordering::Relaxed);
+ self.ensure_root_font_metrics_updated();
Length::new(f32::from_bits(
self.root_font_metrics_ex.load(Ordering::Relaxed),
))
@@ -228,7 +239,7 @@ impl Device {
/// Get the cap-height of the root element (for rcap)
pub fn root_font_metrics_cap(&self) -> Length {
- self.used_root_font_metrics.store(true, Ordering::Relaxed);
+ self.ensure_root_font_metrics_updated();
Length::new(f32::from_bits(
self.root_font_metrics_cap.load(Ordering::Relaxed),
))
@@ -243,7 +254,7 @@ impl Device {
/// Get the advance measure of the root element (for rch)
pub fn root_font_metrics_ch(&self) -> Length {
- self.used_root_font_metrics.store(true, Ordering::Relaxed);
+ self.ensure_root_font_metrics_updated();
Length::new(f32::from_bits(
self.root_font_metrics_ch.load(Ordering::Relaxed),
))
@@ -258,7 +269,7 @@ impl Device {
/// Get the ideographic advance measure of the root element (for ric)
pub fn root_font_metrics_ic(&self) -> Length {
- self.used_root_font_metrics.store(true, Ordering::Relaxed);
+ self.ensure_root_font_metrics_updated();
Length::new(f32::from_bits(
self.root_font_metrics_ic.load(Ordering::Relaxed),
))
@@ -350,6 +361,55 @@ impl Device {
}
}
+ fn ensure_root_font_metrics_updated(&self) {
+ let mut guard = self.used_root_font_metrics.write();
+ let previously_computed = mem::replace(&mut *guard, true);
+ if !previously_computed {
+ self.update_root_font_metrics();
+ }
+ }
+
+ /// Compute the root element's font metrics, and returns a bool indicating whether
+ /// the font metrics have changed since the previous restyle.
+ pub fn update_root_font_metrics(&self) -> bool {
+ let root_style = self.root_style.read();
+ let root_effective_zoom = (*root_style).effective_zoom;
+ let root_font_size = (*root_style).get_font().clone_font_size().computed_size();
+
+ let root_font_metrics = self.query_font_metrics(
+ (*root_style).writing_mode.is_upright(),
+ &(*root_style).get_font(),
+ root_font_size,
+ QueryFontMetricsFlags::USE_USER_FONT_SET
+ | QueryFontMetricsFlags::NEEDS_CH
+ | QueryFontMetricsFlags::NEEDS_IC,
+ /* track_usage = */ false,
+ );
+
+ let mut root_font_metrics_changed = false;
+ root_font_metrics_changed |= self.set_root_font_metrics_ex(
+ root_effective_zoom.unzoom(root_font_metrics.x_height_or_default(root_font_size).px()),
+ );
+ root_font_metrics_changed |= self.set_root_font_metrics_ch(
+ root_effective_zoom.unzoom(
+ root_font_metrics
+ .zero_advance_measure_or_default(
+ root_font_size,
+ (*root_style).writing_mode.is_upright(),
+ )
+ .px(),
+ ),
+ );
+ root_font_metrics_changed |= self.set_root_font_metrics_cap(
+ root_effective_zoom.unzoom(root_font_metrics.cap_height_or_default().px()),
+ );
+ root_font_metrics_changed |= self.set_root_font_metrics_ic(
+ root_effective_zoom.unzoom(root_font_metrics.ic_width_or_default(root_font_size).px()),
+ );
+
+ root_font_metrics_changed
+ }
+
/// Returns the body text color.
pub fn body_text_color(&self) -> AbsoluteColor {
convert_nscolor_to_absolute_color(self.body_text_color.load(Ordering::Relaxed) as u32)
@@ -390,7 +450,7 @@ impl Device {
self.reset_computed_values();
self.used_root_font_size.store(false, Ordering::Relaxed);
self.used_root_line_height.store(false, Ordering::Relaxed);
- self.used_root_font_metrics.store(false, Ordering::Relaxed);
+ self.used_root_font_metrics = RwLock::new(false);
self.used_font_metrics.store(false, Ordering::Relaxed);
self.used_viewport_size.store(false, Ordering::Relaxed);
self.used_dynamic_viewport_size
@@ -409,7 +469,7 @@ impl Device {
/// Returns whether we ever looked up the root font metrics of the device.
pub fn used_root_font_metrics(&self) -> bool {
- self.used_root_font_metrics.load(Ordering::Relaxed)
+ *self.used_root_font_metrics.read()
}
/// Recreates all the temporary state that the `Device` stores.
diff --git a/servo/components/style/matching.rs b/servo/components/style/matching.rs
@@ -28,7 +28,6 @@ use crate::style_resolver::{PseudoElementResolution, ResolvedElementStyles};
use crate::stylesheets::layer_rule::LayerOrder;
use crate::stylist::RuleInclusion;
use crate::traversal_flags::TraversalFlags;
-use crate::values::computed::font::QueryFontMetricsFlags;
use servo_arc::{Arc, ArcBorrow};
/// Represents the result of comparing an element's old and new style.
@@ -984,17 +983,17 @@ pub trait MatchMethods: TElement {
.0
});
+ // Update root font-relative units. If any of these unit values changed
+ // since last time, ensure that we recascade the entire tree.
if is_root {
debug_assert!(self.owner_doc_matches_for_testing(device));
+ device.set_root_style(new_primary_style);
// Update root font size for rem units
if old_font_size != Some(new_font_size) {
let size = new_font_size.computed_size();
device.set_root_font_size(new_primary_style.effective_zoom.unzoom(size.px()));
if device.used_root_font_size() {
- // If the root font-size changed since last time, and something
- // in the document did use rem units, ensure we recascade the
- // entire tree.
restyle_requirement = ChildRestyleRequirement::MustCascadeDescendants;
}
}
@@ -1011,44 +1010,10 @@ pub trait MatchMethods: TElement {
}
}
- // Update root font metrics for rcap, rch, rex, ric units
- let new_font_metrics = device.query_font_metrics(
- new_primary_style.writing_mode.is_upright(),
- &new_primary_style.get_font(),
- new_font_size.computed_size(),
- QueryFontMetricsFlags::USE_USER_FONT_SET
- | QueryFontMetricsFlags::NEEDS_CH
- | QueryFontMetricsFlags::NEEDS_IC,
- /* track_usage = */ false,
- );
- let mut root_font_metrics_changed = false;
- root_font_metrics_changed |= device.set_root_font_metrics_ex(
- new_primary_style
- .effective_zoom
- .unzoom(new_font_metrics.x_height_or_default(&new_font_size).px()),
- );
- root_font_metrics_changed |= device.set_root_font_metrics_ch(
- new_primary_style.effective_zoom.unzoom(
- new_font_metrics
- .zero_advance_measure_or_default(
- &new_font_size,
- new_primary_style.writing_mode.is_upright(),
- )
- .px(),
- ),
- );
- root_font_metrics_changed |= device.set_root_font_metrics_cap(
- new_primary_style
- .effective_zoom
- .unzoom(new_font_metrics.cap_height_or_default().px()),
- );
- root_font_metrics_changed |= device.set_root_font_metrics_ic(
- new_primary_style
- .effective_zoom
- .unzoom(new_font_metrics.ic_width_or_default(&new_font_size).px()),
- );
-
- if device.used_root_font_metrics() && root_font_metrics_changed {
+ // Update root font metrics for rcap, rch, rex, ric units. Since querying
+ // font metrics can be an expensive call, they are only updated if these
+ // units are used in the document.
+ if device.used_root_font_metrics() && device.update_root_font_metrics() {
restyle_requirement = ChildRestyleRequirement::MustCascadeDescendants;
}
}
diff --git a/servo/components/style/values/specified/length.rs b/servo/components/style/values/specified/length.rs
@@ -316,7 +316,7 @@ impl FontRelativeLength {
FontMetricsOrientation::Horizontal,
QueryFontMetricsFlags::empty(),
);
- metrics.x_height_or_default(&reference_font_size)
+ metrics.x_height_or_default(reference_font_size.used_size())
}
fn ch_size(
@@ -338,7 +338,7 @@ impl FontRelativeLength {
QueryFontMetricsFlags::NEEDS_CH,
);
metrics.zero_advance_measure_or_default(
- &reference_font_size,
+ reference_font_size.used_size(),
context.style().writing_mode.is_upright(),
)
}
@@ -364,7 +364,7 @@ impl FontRelativeLength {
FontMetricsOrientation::MatchContextPreferVertical,
QueryFontMetricsFlags::NEEDS_IC,
);
- metrics.ic_width_or_default(&reference_font_size)
+ metrics.ic_width_or_default(reference_font_size.used_size())
}
let reference_font_size = base_size.resolve(context);
