Skip to content

Commit

Permalink
Bug 1708240 - Implement the from-font keyword for CSS font-size-adjus…
Browse files Browse the repository at this point in the history
…t. r=emilio,devtools-reviewers

Depends on D182450

Differential Revision: https://phabricator.services.mozilla.com/D182451
  • Loading branch information
jfkthame committed Aug 1, 2023
1 parent 3b3eafa commit 8b2747c
Show file tree
Hide file tree
Showing 16 changed files with 207 additions and 116 deletions.
1 change: 1 addition & 0 deletions devtools/shared/css/generated/properties-db.js
Original file line number Diff line number Diff line change
Expand Up @@ -7136,6 +7136,7 @@ exports.CSS_PROPERTIES = {
"values": [
"cap-height",
"ch-width",
"from-font",
"ic-height",
"ic-width",
"inherit",
Expand Down
49 changes: 28 additions & 21 deletions gfx/thebes/gfxFont.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4617,33 +4617,40 @@ gfxFontStyle::gfxFontStyle(FontSlantStyle aStyle, FontWeight aWeight,
noFallbackVariantFeatures(true) {
MOZ_ASSERT(!std::isnan(size));

sizeAdjustBasis = uint8_t(aSizeAdjust.tag);
// sizeAdjustBasis is currently a small bitfield, so let's assert that the
// tag value was not truncated.
MOZ_ASSERT(FontSizeAdjust::Tag(sizeAdjustBasis) == aSizeAdjust.tag,
"gfxFontStyle.sizeAdjustBasis too small?");

// If we're created with aSizeAdjust holding a FromFont value, we ignore it
// here; this is the style system retrieving font metrics in order to resolve
// FromFont to an actual ratio, which it can do using the unmodified metrics.

#define HANDLE_TAG(TAG) \
case FontSizeAdjust::Tag::TAG: \
if (aSizeAdjust.As##TAG().IsFromFont()) { \
sizeAdjustBasis = uint8_t(FontSizeAdjust::Tag::None); \
sizeAdjust = 0.0f; \
break; \
} \
sizeAdjust = aSizeAdjust.As##TAG().AsNumber(); \
break;

switch (aSizeAdjust.tag) {
case FontSizeAdjust::Tag::None:
sizeAdjust = 0.0f;
break;
case FontSizeAdjust::Tag::ExHeight:
sizeAdjust = aSizeAdjust.AsExHeight();
break;
case FontSizeAdjust::Tag::CapHeight:
sizeAdjust = aSizeAdjust.AsCapHeight();
break;
case FontSizeAdjust::Tag::ChWidth:
sizeAdjust = aSizeAdjust.AsChWidth();
break;
case FontSizeAdjust::Tag::IcWidth:
sizeAdjust = aSizeAdjust.AsIcWidth();
break;
case FontSizeAdjust::Tag::IcHeight:
sizeAdjust = aSizeAdjust.AsIcHeight();
break;
HANDLE_TAG(ExHeight)
HANDLE_TAG(CapHeight)
HANDLE_TAG(ChWidth)
HANDLE_TAG(IcWidth)
HANDLE_TAG(IcHeight)
}
MOZ_ASSERT(!std::isnan(sizeAdjust));

sizeAdjustBasis = uint8_t(aSizeAdjust.tag);
// sizeAdjustBasis is currently a small bitfield, so let's assert that the
// tag value was not truncated.
MOZ_ASSERT(FontSizeAdjust::Tag(sizeAdjustBasis) == aSizeAdjust.tag,
"gfxFontStyle.sizeAdjustBasis too small?");
#undef HANDLE_TAG

MOZ_ASSERT(!std::isnan(sizeAdjust));

if (weight > FontWeight::FromInt(1000)) {
weight = FontWeight::FromInt(1000);
Expand Down
4 changes: 2 additions & 2 deletions layout/base/StaticPresData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,8 @@ void LangGroupFontPrefs::Initialize(nsStaticAtom* aLangGroupAtom) {
nsAutoCString cvalue;
Preferences::GetCString(pref.get(), cvalue);
if (!cvalue.IsEmpty()) {
font->sizeAdjust =
StyleFontSizeAdjust::ExHeight((float)atof(cvalue.get()));
font->sizeAdjust = StyleFontSizeAdjust::ExHeight(
StyleFontSizeAdjustFactor::Number((float)atof(cvalue.get())));
}

#ifdef DEBUG_rbs
Expand Down
21 changes: 11 additions & 10 deletions layout/base/nsLayoutUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9792,29 +9792,30 @@ void nsLayoutUtils::ComputeSystemFont(nsFont* aSystemFont,
aSystemFont->size = Length::FromPixels(fontStyle.size);

// aSystemFont->langGroup = fontStyle.langGroup;

switch (StyleFontSizeAdjust::Tag(fontStyle.sizeAdjustBasis)) {
case StyleFontSizeAdjust::Tag::None:
aSystemFont->sizeAdjust = StyleFontSizeAdjust::None();
break;
case StyleFontSizeAdjust::Tag::ExHeight:
aSystemFont->sizeAdjust =
StyleFontSizeAdjust::ExHeight(fontStyle.sizeAdjust);
aSystemFont->sizeAdjust = StyleFontSizeAdjust::ExHeight(
StyleFontSizeAdjustFactor::Number(fontStyle.sizeAdjust));
break;
case StyleFontSizeAdjust::Tag::CapHeight:
aSystemFont->sizeAdjust =
StyleFontSizeAdjust::CapHeight(fontStyle.sizeAdjust);
aSystemFont->sizeAdjust = StyleFontSizeAdjust::CapHeight(
StyleFontSizeAdjustFactor::Number(fontStyle.sizeAdjust));
break;
case StyleFontSizeAdjust::Tag::ChWidth:
aSystemFont->sizeAdjust =
StyleFontSizeAdjust::ChWidth(fontStyle.sizeAdjust);
aSystemFont->sizeAdjust = StyleFontSizeAdjust::ChWidth(
StyleFontSizeAdjustFactor::Number(fontStyle.sizeAdjust));
break;
case StyleFontSizeAdjust::Tag::IcWidth:
aSystemFont->sizeAdjust =
StyleFontSizeAdjust::IcWidth(fontStyle.sizeAdjust);
aSystemFont->sizeAdjust = StyleFontSizeAdjust::IcWidth(
StyleFontSizeAdjustFactor::Number(fontStyle.sizeAdjust));
break;
case StyleFontSizeAdjust::Tag::IcHeight:
aSystemFont->sizeAdjust =
StyleFontSizeAdjust::IcHeight(fontStyle.sizeAdjust);
aSystemFont->sizeAdjust = StyleFontSizeAdjust::IcHeight(
StyleFontSizeAdjustFactor::Number(fontStyle.sizeAdjust));
break;
}

Expand Down
3 changes: 2 additions & 1 deletion layout/style/ServoBindings.toml
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,8 @@ cbindgen-types = [
{ gecko = "StyleGenericClipRect", servo = "crate::values::generics::GenericClipRect" },
{ gecko = "StyleGenericCursorImage", servo = "crate::values::generics::ui::GenericCursorImage" },
{ gecko = "StyleFontFamily", servo = "crate::values::computed::font::FontFamily" },
{ gecko = "StyleGenericFontSizeAdjust", servo = "crate::values::generics::font::GenericFontSizeAdjust" },
{ gecko = "StyleFontSizeAdjust", servo = "crate::values::computed::font::FontSizeAdjust" },
{ gecko = "StyleFontSizeAdjustFactor", servo = "crate::values::computed::font::FontSizeAdjustFactor" },
{ gecko = "StyleFontFamilyNameSyntax", servo = "crate::values::computed::font::FontFamilyNameSyntax" },
{ gecko = "StyleGenericColor", servo = "crate::values::generics::color::Color" },
{ gecko = "StyleSystemColor", servo = "crate::values::specified::color::SystemColor" },
Expand Down
26 changes: 24 additions & 2 deletions layout/style/test/property_database.js
Original file line number Diff line number Diff line change
Expand Up @@ -5783,8 +5783,30 @@ var gCSSProperties = {
applies_to_placeholder: true,
applies_to_cue: true,
initial_values: ["none"],
other_values: ["0.3", "0.5", "0.7", "0.0", "0", "3"],
invalid_values: ["-0.3", "-1"],
other_values: [
"0.7",
"0.0",
"0",
"3",
"from-font",
"cap-height 0.8",
"ch-width 0.4",
"ic-width 0.4",
"ic-height 0.9",
"ch-width from-font",
],
invalid_values: [
"-0.3",
"-1",
"normal",
"none none",
"cap-height none",
"none from-font",
"from-font none",
"0.5 from-font",
"0.5 cap-height",
"cap-height, 0.8",
],
},
"font-stretch": {
domProp: "fontStretch",
Expand Down
58 changes: 57 additions & 1 deletion servo/components/style/properties/cascade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1221,6 +1221,61 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
font.mScriptUnconstrainedSize = NonNegative(new_unconstrained_size);
}

/// If font-size-adjust used the from-font value, we need to resolve it to an actual number
/// using metrics from the font.
fn resolve_font_size_adjust_from_font_if_needed(&mut self) {
use crate::values::computed::font::{FontSizeAdjust, FontSizeAdjustFactor as Factor};

if !self.seen.contains(LonghandId::FontSizeAdjust) {
return;
}

let font_metrics = |vertical| {
let orient = if vertical {
FontMetricsOrientation::MatchContextPreferVertical
} else {
FontMetricsOrientation::Horizontal
};
let metrics = self.context.query_font_metrics(FontBaseSize::CurrentStyle, orient, false);
let font_size = self.context.style().get_font().clone_font_size().used_size.0;
(metrics, font_size)
};

// Macro to resolve a from-font value using the given metric field. If not present,
// returns the fallback value, or if that is negative, resolves using ascent instead
// of the missing field (this is the fallback for cap-height).
macro_rules! resolve {
($basis:ident, $value:expr, $vertical:expr, $field:ident, $fallback:expr) => {
{
if $value != Factor::FromFont {
return;
}
let (metrics, font_size) = font_metrics($vertical);
let ratio = if let Some(metric) = metrics.$field {
metric / font_size
} else if $fallback >= 0.0 {
$fallback
} else {
metrics.ascent / font_size
};
FontSizeAdjust::$basis(Factor::new(ratio))
}
};
}

// If sizeAdjust is currently FromFont, we need to resolve it to a number.
let resolved = match self.context.builder.get_font().mFont.sizeAdjust {
FontSizeAdjust::None => return,
FontSizeAdjust::ExHeight(val) => resolve!(ExHeight, val, false, x_height, 0.5),
FontSizeAdjust::CapHeight(val) => resolve!(CapHeight, val, false, cap_height, -1.0 /* fall back to ascent */),
FontSizeAdjust::ChWidth(val) => resolve!(ChWidth, val, false, zero_advance_measure, 0.5),
FontSizeAdjust::IcWidth(val) => resolve!(IcWidth, val, false, ic_width, 1.0),
FontSizeAdjust::IcHeight(val) => resolve!(IcHeight, val, true, ic_width, 1.0),
};

self.context.builder.mutate_font().mFont.sizeAdjust = resolved
}

/// Various properties affect how font-size and font-family are computed.
///
/// These need to be handled here, since relative lengths and ex / ch units
Expand All @@ -1233,7 +1288,8 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
self.prioritize_user_fonts_if_needed();
self.recompute_keyword_font_size_if_needed();
self.recompute_math_font_size_if_needed();
self.constrain_font_size_if_needed()
self.constrain_font_size_if_needed();
self.resolve_font_size_adjust_from_font_if_needed()
}
}
}
16 changes: 14 additions & 2 deletions servo/components/style/values/computed/font.rs
Original file line number Diff line number Diff line change
Expand Up @@ -746,8 +746,20 @@ impl FontFamilyList {
}
}

/// Preserve the readability of text when font fallback occurs
pub type FontSizeAdjust = generics::GenericFontSizeAdjust<NonNegativeNumber>;
/// A factor for one of the font-size-adjust metrics, which may be either a number
/// or the `from-font` keyword.
pub type FontSizeAdjustFactor = generics::GenericNumberOrFromFont<NonNegativeNumber>;

impl FontSizeAdjustFactor {
#[inline]
/// Create a ratio from a raw number
pub fn new(val: f32) -> Self {
FontSizeAdjustFactor::Number(NonNegative(val))
}
}

/// Preserve the readability of text when font fallback occurs.
pub type FontSizeAdjust = generics::GenericFontSizeAdjust<FontSizeAdjustFactor>;

impl FontSizeAdjust {
#[inline]
Expand Down
2 changes: 1 addition & 1 deletion servo/components/style/values/computed/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ pub use self::effects::{BoxShadow, Filter, SimpleShadow};
pub use self::flex::FlexBasis;
pub use self::font::{FontFamily, FontLanguageOverride, FontPalette, FontStyle};
pub use self::font::{FontFeatureSettings, FontVariantLigatures, FontVariantNumeric};
pub use self::font::{FontSize, FontSizeAdjust, FontStretch, FontSynthesis};
pub use self::font::{FontSize, FontSizeAdjust, FontSizeAdjustFactor, FontStretch, FontSynthesis};
pub use self::font::{FontVariantAlternates, FontWeight};
pub use self::font::{FontVariantEastAsian, FontVariationSettings};
pub use self::font::{MathDepth, MozScriptMinSize, MozScriptSizeMultiplier, XLang, XTextScale};
Expand Down
45 changes: 35 additions & 10 deletions servo/components/style/values/generics/font.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,11 +213,36 @@ pub enum FontStyle<Angle> {
Oblique(Angle),
}

/// A generic value that holds either a generic Number or the keyword
/// `from-font`; used for values of font-size-adjust.
#[repr(u8)]
#[derive(
Animate,
Clone,
ComputeSquaredDistance,
Copy,
Debug,
MallocSizeOf,
Parse,
PartialEq,
SpecifiedValueInfo,
ToAnimatedValue,
ToAnimatedZero,
ToComputedValue,
ToResolvedValue,
ToCss,
ToShmem,
)]
pub enum GenericNumberOrFromFont<N> {
/// An explicitly-specified number.
Number(N),
/// The from-font keyword: resolve the number from font metrics.
FromFont,
}

/// A generic value for the `font-size-adjust` property.
///
/// https://www.w3.org/TR/css-fonts-4/#font-size-adjust-prop
/// https://github.com/w3c/csswg-drafts/issues/6160
/// https://github.com/w3c/csswg-drafts/issues/6288
/// https://drafts.csswg.org/css-fonts-5/#font-size-adjust-prop
#[allow(missing_docs)]
#[repr(u8)]
#[derive(
Expand All @@ -236,22 +261,22 @@ pub enum FontStyle<Angle> {
ToResolvedValue,
ToShmem,
)]
pub enum GenericFontSizeAdjust<Number> {
pub enum GenericFontSizeAdjust<Factor> {
#[animation(error)]
None,
// 'ex-height' is the implied basis, so the keyword can be omitted
ExHeight(Number),
ExHeight(Factor),
#[value_info(starts_with_keyword)]
CapHeight(Number),
CapHeight(Factor),
#[value_info(starts_with_keyword)]
ChWidth(Number),
ChWidth(Factor),
#[value_info(starts_with_keyword)]
IcWidth(Number),
IcWidth(Factor),
#[value_info(starts_with_keyword)]
IcHeight(Number),
IcHeight(Factor),
}

impl<Number: ToCss> ToCss for GenericFontSizeAdjust<Number> {
impl<Factor: ToCss> ToCss for GenericFontSizeAdjust<Factor> {
fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
where
W: Write,
Expand Down
Loading

0 comments on commit 8b2747c

Please sign in to comment.