corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1963212 - Reuse FontFaceImpl more aggressively. r=jfkthame

Browse Source 
When deep-cloning a stylesheet (like we do for devtools), we end up with
new (identical) LockedFontFaceRule object, but we'd fail to re-use it,
triggering new font loads. Since we don't cache failed loads, this
caused flashing and a temporary state where the used fonts were not what
this DevTools test expects.

Rejigger the code a little bit to push down the decision of whether to
reuse the FontFaceImpl object further down. That allows us to reuse the
FontFace object for the stylesheet caching / deep cloning use-case.

Differential Revision: https://phabricator.services.mozilla.com/D247704
This commit is contained in:
Emilio Cobos Álvarez
2025-05-07 15:36:09 +00:00
committed by ealvarez@mozilla.com
parent fd3e0c3fcd
commit a0a14bcfb9
5 changed files with 147 additions and 125 deletions
Download Patch File Download Diff File Expand all files Collapse all files

92
layout/style/FontFaceImpl.cpp
View File

@@ -570,8 +570,35 @@ bool FontFaceImpl::GetAttributes(gfxUserFontAttributes& aAttr) {
if (!data) {
return false;
}
return GetAttributesFromRule(data, aAttr,
Some(GetUnicodeRangeAsCharacterMap()));
}
nsAtom* fontFamily = Servo_FontFaceRule_GetFamilyName(data);
static already_AddRefed<gfxCharacterMap> ComputeCharacterMap(StyleLockedFontFaceRule* aData) {
size_t len;
const StyleUnicodeRange* rangesPtr =
Servo_FontFaceRule_GetUnicodeRanges(aData, &len);
Span<const StyleUnicodeRange> ranges(rangesPtr, len);
if (ranges.IsEmpty()) {
return nullptr;
}
auto charMap = MakeRefPtr<gfxCharacterMap>();
for (auto& range : ranges) {
charMap->SetRange(range.start, range.end);
}
charMap->Compact();
// As it's common for multiple font resources to have the same
// unicode-range list, look for an existing copy of this map to share,
// or add this one to the sharing cache if not already present.
return gfxPlatformFontList::PlatformFontList()->FindCharMap(charMap);
}
bool FontFaceImpl::GetAttributesFromRule(
StyleLockedFontFaceRule* aData, gfxUserFontAttributes& aAttr,
const Maybe<gfxCharacterMap*>& aKnownCharMap) {
nsAtom* fontFamily = Servo_FontFaceRule_GetFamilyName(aData);
if (!fontFamily) {
return false;
}
@@ -579,20 +606,20 @@ bool FontFaceImpl::GetAttributes(gfxUserFontAttributes& aAttr) {
aAttr.mFamilyName = nsAtomCString(fontFamily);
StyleComputedFontWeightRange weightRange;
if (Servo_FontFaceRule_GetFontWeight(data, &weightRange)) {
if (Servo_FontFaceRule_GetFontWeight(aData, &weightRange)) {
aAttr.mRangeFlags &= ~gfxFontEntry::RangeFlags::eAutoWeight;
aAttr.mWeight = WeightRange(FontWeight::FromFloat(weightRange._0),
FontWeight::FromFloat(weightRange._1));
}
StyleComputedFontStretchRange stretchRange;
if (Servo_FontFaceRule_GetFontStretch(data, &stretchRange)) {
if (Servo_FontFaceRule_GetFontStretch(aData, &stretchRange)) {
aAttr.mRangeFlags &= ~gfxFontEntry::RangeFlags::eAutoStretch;
aAttr.mStretch = StretchRange(stretchRange._0, stretchRange._1);
}
auto styleDesc = StyleComputedFontStyleDescriptor::Normal();
if (Servo_FontFaceRule_GetFontStyle(data, &styleDesc)) {
if (Servo_FontFaceRule_GetFontStyle(aData, &styleDesc)) {
aAttr.mRangeFlags &= ~gfxFontEntry::RangeFlags::eAutoSlantStyle;
switch (styleDesc.tag) {
case StyleComputedFontStyleDescriptor::Tag::Italic:
@@ -609,47 +636,40 @@ bool FontFaceImpl::GetAttributes(gfxUserFontAttributes& aAttr) {
}
StylePercentage ascent{0};
if (Servo_FontFaceRule_GetAscentOverride(data, &ascent)) {
if (Servo_FontFaceRule_GetAscentOverride(aData, &ascent)) {
aAttr.mAscentOverride = ascent._0;
}
StylePercentage descent{0};
if (Servo_FontFaceRule_GetDescentOverride(data, &descent)) {
if (Servo_FontFaceRule_GetDescentOverride(aData, &descent)) {
aAttr.mDescentOverride = descent._0;
}
StylePercentage lineGap{0};
if (Servo_FontFaceRule_GetLineGapOverride(data, &lineGap)) {
if (Servo_FontFaceRule_GetLineGapOverride(aData, &lineGap)) {
aAttr.mLineGapOverride = lineGap._0;
}
StylePercentage sizeAdjust;
if (Servo_FontFaceRule_GetSizeAdjust(data, &sizeAdjust)) {
if (Servo_FontFaceRule_GetSizeAdjust(aData, &sizeAdjust)) {
aAttr.mSizeAdjust = sizeAdjust._0;
}
StyleFontLanguageOverride langOverride;
if (Servo_FontFaceRule_GetFontLanguageOverride(data, &langOverride)) {
if (Servo_FontFaceRule_GetFontLanguageOverride(aData, &langOverride)) {
aAttr.mLanguageOverride = langOverride._0;
}
Servo_FontFaceRule_GetFontDisplay(data, &aAttr.mFontDisplay);
Servo_FontFaceRule_GetFeatureSettings(data, &aAttr.mFeatureSettings);
Servo_FontFaceRule_GetVariationSettings(data, &aAttr.mVariationSettings);
Servo_FontFaceRule_GetSources(data, &aAttr.mSources);
aAttr.mUnicodeRanges = GetUnicodeRangeAsCharacterMap();
return true;
}
bool FontFaceImpl::HasLocalSrc() const {
AutoTArray<StyleFontFaceSourceListComponent, 8> components;
Servo_FontFaceRule_GetSources(GetData(), &components);
for (auto& component : components) {
if (component.tag == StyleFontFaceSourceListComponent::Tag::Local) {
return true;
Servo_FontFaceRule_GetFontDisplay(aData, &aAttr.mFontDisplay);
Servo_FontFaceRule_GetFeatureSettings(aData, &aAttr.mFeatureSettings);
Servo_FontFaceRule_GetVariationSettings(aData, &aAttr.mVariationSettings);
Servo_FontFaceRule_GetSources(aData, &aAttr.mSources);
if (aKnownCharMap) {
aAttr.mUnicodeRanges = aKnownCharMap.value();
} else {
aAttr.mUnicodeRanges = ComputeCharacterMap(aData);
}
}
return false;
return true;
}
nsAtom* FontFaceImpl::GetFamilyName() const {
@@ -725,27 +745,7 @@ gfxCharacterMap* FontFaceImpl::GetUnicodeRangeAsCharacterMap() {
if (!mUnicodeRangeDirty) {
return mUnicodeRange;
}
size_t len;
const StyleUnicodeRange* rangesPtr =
Servo_FontFaceRule_GetUnicodeRanges(GetData(), &len);
Span<const StyleUnicodeRange> ranges(rangesPtr, len);
if (!ranges.IsEmpty()) {
auto charMap = MakeRefPtr<gfxCharacterMap>();
for (auto& range : ranges) {
charMap->SetRange(range.start, range.end);
}
charMap->Compact();
// As it's common for multiple font resources to have the same
// unicode-range list, look for an existing copy of this map to share,
// or add this one to the sharing cache if not already present.
mUnicodeRange =
gfxPlatformFontList::PlatformFontList()->FindCharMap(charMap);
} else {
mUnicodeRange = nullptr;
}
mUnicodeRange = ComputeCharacterMap(GetData());
mUnicodeRangeDirty = false;
return mUnicodeRange;
}

22
layout/style/FontFaceImpl.h
View File

@@ -98,8 +98,9 @@ class FontFaceImpl final {
StyleLockedFontFaceRule* GetRule() { return mRule; }
bool HasLocalSrc() const;
static bool GetAttributesFromRule(
StyleLockedFontFaceRule*, gfxUserFontAttributes& aAttr,
const Maybe<gfxCharacterMap*>& aKnownCharMap = Nothing());
bool GetAttributes(gfxUserFontAttributes& aAttr);
gfxUserFontEntry* CreateUserFontEntry();
gfxUserFontEntry* GetUserFontEntry() const { return mUserFontEntry; }
@@ -128,6 +129,13 @@ class FontFaceImpl final {
*/
bool HasRule() const { return mRule; }
/** Set the font-face block we're reflecting when reusing a FontFace object */
void SetRule(StyleLockedFontFaceRule* aData) {
MOZ_ASSERT(HasRule());
AssertIsOnOwningThread();
mRule = aData;
}
/**
* Breaks the connection between this FontFace and its @font-face rule.
*/
@@ -202,6 +210,11 @@ class FontFaceImpl final {
bool SetDescriptors(const nsACString& aFamily,
const FontFaceDescriptors& aDescriptors);
StyleLockedFontFaceRule* GetData() const {
AssertIsOnOwningThread();
return HasRule() ? mRule : mDescriptors;
}
private:
~FontFaceImpl();
@@ -228,11 +241,6 @@ class FontFaceImpl final {
void GetDesc(nsCSSFontDesc aDescID, nsACString& aResult) const;
StyleLockedFontFaceRule* GetData() const {
AssertIsOnOwningThread();
return HasRule() ? mRule : mDescriptors;
}
/**
* Returns and takes ownership of the buffer storing the font data.
*/

86
layout/style/FontFaceSetDocumentImpl.cpp
View File

@@ -406,16 +406,6 @@ bool FontFaceSetDocumentImpl::UpdateRules(
// same rules are still present.
nsTArray<FontFaceRecord> oldRecords = std::move(mRuleFaces);
// reuse existing FontFace objects mapped to particular rules already
nsTHashMap<nsPtrHashKey<StyleLockedFontFaceRule>, FontFaceImpl*> ruleFaceMap;
for (const FontFaceRecord& record : oldRecords) {
FontFaceImpl* f = record.mFontFace;
if (!f || !f->GetOwner()) {
continue;
}
ruleFaceMap.InsertOrUpdate(f->GetRule(), f);
}
// Remove faces from the font family records; we need to re-insert them
// because we might end up with faces in a different order even if they're
// the same font entries as before. (The order can affect font selection
@@ -441,13 +431,7 @@ bool FontFaceSetDocumentImpl::UpdateRules(
// rule was already present in the hashtable
continue;
}
RefPtr<FontFaceImpl> faceImpl = ruleFaceMap.Get(rule);
RefPtr<FontFace> face = faceImpl ? faceImpl->GetOwner() : nullptr;
if (mOwner && (!faceImpl || !face)) {
face = FontFace::CreateForRule(mOwner->GetParentObject(), mOwner, rule);
faceImpl = face->GetImpl();
}
InsertRuleFontFace(faceImpl, face, container.mOrigin, oldRecords, modified);
modified |= InsertRuleFontFace(rule, container.mOrigin, oldRecords);
}
for (const FontFaceRecord& record : mNonRuleFaces) {
@@ -512,16 +496,20 @@ bool FontFaceSetDocumentImpl::UpdateRules(
return modified;
}
void FontFaceSetDocumentImpl::InsertRuleFontFace(
FontFaceImpl* aFontFace, FontFace* aFontFaceOwner, StyleOrigin aSheetType,
nsTArray<FontFaceRecord>& aOldRecords, bool& aFontSetModified) {
bool FontFaceSetDocumentImpl::InsertRuleFontFace(
StyleLockedFontFaceRule* aRule, StyleOrigin aSheetType,
nsTArray<FontFaceRecord>& aOldRecords) {
RecursiveMutexAutoLock lock(mMutex);
if (MOZ_UNLIKELY(!mOwner)) {
return false;
}
gfxUserFontAttributes attr;
if (!aFontFace->GetAttributes(attr)) {
if (!FontFaceImpl::GetAttributesFromRule(aRule, attr)) {
// If there is no family name, this rule cannot contribute a
// usable font, so there is no point in processing it further.
return;
return false;
}
bool remove = false;
@@ -533,11 +521,27 @@ void FontFaceSetDocumentImpl::InsertRuleFontFace(
for (size_t i = 0; i < aOldRecords.Length(); ++i) {
FontFaceRecord& rec = aOldRecords[i];
if (rec.mFontFace == aFontFace && rec.mOrigin == Some(aSheetType)) {
const bool matches =
rec.mOrigin == Some(aSheetType) &&
Servo_FontFaceRule_Equals(rec.mFontFace->GetData(), aRule);
if (!matches) {
continue;
}
FontFace* owner = rec.mFontFace->GetOwner();
// if local rules were used, don't use the old font entry
// for rules containing src local usage
if (mLocalRulesUsed && mRebuildLocalRules) {
if (aFontFace->HasLocalSrc()) {
const bool hasLocalSource = [&] {
for (auto& source : attr.mSources) {
if (source.IsLocal()) {
return true;
}
}
return false;
}();
if (hasLocalSource) {
// Remove the old record, but wait to see if we successfully create a
// new user font entry below.
remove = true;
@@ -546,6 +550,7 @@ void FontFaceSetDocumentImpl::InsertRuleFontFace(
}
}
rec.mFontFace->SetRule(aRule);
gfxUserFontEntry* entry = rec.mFontFace->GetUserFontEntry();
MOZ_ASSERT(entry, "FontFace should have a gfxUserFontEntry by now");
@@ -557,26 +562,25 @@ void FontFaceSetDocumentImpl::InsertRuleFontFace(
mRuleFaces.AppendElement(rec);
aOldRecords.RemoveElementAt(i);
if (mOwner && aFontFaceOwner) {
mOwner->InsertRuleFontFace(aFontFaceOwner, aSheetType);
if (owner) {
mOwner->InsertRuleFontFace(owner, aSheetType);
}
// note the set has been modified if an old rule was skipped to find
// this one - something has been dropped, or ordering changed
if (i > 0) {
aFontSetModified = true;
}
return;
}
return i > 0;
}
RefPtr<FontFace> fontFace =
FontFace::CreateForRule(mOwner->GetParentObject(), mOwner, aRule);
RefPtr<FontFaceImpl> impl = fontFace->GetImpl();
// this is a new rule:
nsAutoCString family(attr.mFamilyName);
RefPtr<gfxUserFontEntry> entry = FindOrCreateUserFontEntryFromFontFace(
aFontFace, std::move(attr), aSheetType);
RefPtr<gfxUserFontEntry> entry =
FindOrCreateUserFontEntryFromFontFace(impl, std::move(attr), aSheetType);
if (!entry) {
return;
return false;
}
if (remove) {
@@ -590,22 +594,17 @@ void FontFaceSetDocumentImpl::InsertRuleFontFace(
}
FontFaceRecord rec;
rec.mFontFace = aFontFace;
rec.mFontFace = impl;
rec.mOrigin = Some(aSheetType);
aFontFace->SetUserFontEntry(entry);
impl->SetUserFontEntry(entry);
MOZ_ASSERT(!HasRuleFontFace(aFontFace),
MOZ_ASSERT(!HasRuleFontFace(impl),
"FontFace should not occur in mRuleFaces twice");
mRuleFaces.AppendElement(rec);
if (mOwner && aFontFaceOwner) {
mOwner->InsertRuleFontFace(aFontFaceOwner, aSheetType);
}
// this was a new rule and font entry, so note that the set was modified
aFontSetModified = true;
mOwner->InsertRuleFontFace(fontFace, aSheetType);
// Add the entry to the end of the list. If an existing userfont entry was
// returned by FindOrCreateUserFontEntryFromFontFace that was already stored
@@ -613,6 +612,7 @@ void FontFaceSetDocumentImpl::InsertRuleFontFace(
// calls, will automatically remove the earlier occurrence of the same
// userfont entry.
AddUserFontEntry(family, entry);
return true;
}
StyleLockedFontFaceRule* FontFaceSetDocumentImpl::FindRuleForEntry(

7
layout/style/FontFaceSetDocumentImpl.h
View File

@@ -91,10 +91,9 @@ class FontFaceSetDocumentImpl final : public FontFaceSetImpl,
void FindMatchingFontFaces(const nsTHashSet<FontFace*>& aMatchingFaces,
nsTArray<FontFace*>& aFontFaces) override;
void InsertRuleFontFace(FontFaceImpl* aFontFace, FontFace* aFontFaceOwner,
StyleOrigin aOrigin,
nsTArray<FontFaceRecord>& aOldRecords,
bool& aFontSetModified);
/* Returns whether the font set was modified. */
bool InsertRuleFontFace(StyleLockedFontFaceRule*, StyleOrigin,
nsTArray<FontFaceRecord>& aOldRecords);
// Helper function for HasLoadingFontFaces.
void UpdateHasLoadingFontFaces() override;

15
servo/ports/geckolib/glue.rs
View File

@@ -3251,6 +3251,21 @@ pub unsafe extern "C" fn Servo_FontFaceRule_Clone(
with_maybe_worker_shared_lock(|lock| Arc::new(lock.wrap(clone)).into())
}
#[no_mangle]
pub unsafe extern "C" fn Servo_FontFaceRule_Equals(
a: &LockedFontFaceRule,
b: &LockedFontFaceRule,
) -> bool {
if a as *const _ == b as *const _ {
return true;
}
read_locked_arc_worker(a, |a: &FontFaceRule| {
read_locked_arc_worker(b, |b: &FontFaceRule| {
a == b
})
})
}
#[no_mangle]
pub unsafe extern "C" fn Servo_FontFaceRule_GetSourceLocation(
rule: &LockedFontFaceRule,