LibWeb: Compare font keys by reference
Some checks are pending
CI / Lagom (false, FUZZ, ubuntu-24.04, Linux, Clang) (push) Waiting to run
CI / Lagom (false, NO_FUZZ, macos-15, macOS, Clang) (push) Waiting to run
CI / Lagom (false, NO_FUZZ, ubuntu-24.04, Linux, GNU) (push) Waiting to run
CI / Lagom (true, NO_FUZZ, ubuntu-24.04, Linux, Clang) (push) Waiting to run
Package the js repl as a binary artifact / build-and-package (macos-14, macOS, macOS-universal2) (push) Waiting to run
Package the js repl as a binary artifact / build-and-package (ubuntu-24.04, Linux, Linux-x86_64) (push) Waiting to run
Run test262 and test-wasm / run_and_update_results (push) Waiting to run
Lint Code / lint (push) Waiting to run
Push notes / build (push) Waiting to run

`StyleComputer::font_matching_algorithm` was creating a copy of a
`FlyString` every time a `MatchingFontCandidate` was constructed or
copied, causing millions of unnecessairy reference updates when a
lot of fonts are loaded.

While a more permanent solution would be to not load so many unused
fonts, let's do the right thing and remove the unnecessairy copies of
`FlyString`.
This commit is contained in:
Jonne Ransijn 2024-10-26 23:35:58 +02:00 committed by Andreas Kling
parent ec5ea0d686
commit 356507284e
Notes: github-actions[bot] 2024-11-20 14:39:06 +00:00
2 changed files with 76 additions and 25 deletions

View file

@ -14,6 +14,7 @@
#include <AK/Function.h> #include <AK/Function.h>
#include <AK/HashMap.h> #include <AK/HashMap.h>
#include <AK/Math.h> #include <AK/Math.h>
#include <AK/NonnullRawPtr.h>
#include <AK/QuickSort.h> #include <AK/QuickSort.h>
#include <AK/TemporaryChange.h> #include <AK/TemporaryChange.h>
#include <LibGfx/Font/Font.h> #include <LibGfx/Font/Font.h>
@ -85,12 +86,33 @@
#include <math.h> #include <math.h>
#include <stdio.h> #include <stdio.h>
namespace Web::CSS {
struct FontFaceKey {
NonnullRawPtr<FlyString const> family_name;
int weight { 0 };
int slope { 0 };
};
}
namespace AK { namespace AK {
// traits for FontFaceKey namespace Detail {
template<>
inline constexpr bool IsHashCompatible<Web::CSS::FontFaceKey, Web::CSS::OwnFontFaceKey> = true;
template<>
inline constexpr bool IsHashCompatible<Web::CSS::OwnFontFaceKey, Web::CSS::FontFaceKey> = true;
}
template<> template<>
struct Traits<Web::CSS::FontFaceKey> : public DefaultTraits<Web::CSS::FontFaceKey> { struct Traits<Web::CSS::FontFaceKey> : public DefaultTraits<Web::CSS::FontFaceKey> {
static unsigned hash(Web::CSS::FontFaceKey const& key) { return pair_int_hash(key.family_name.hash(), pair_int_hash(key.weight, key.slope)); } static unsigned hash(Web::CSS::FontFaceKey const& key) { return pair_int_hash(key.family_name->hash(), pair_int_hash(key.weight, key.slope)); }
};
template<>
struct Traits<Web::CSS::OwnFontFaceKey> : public DefaultTraits<Web::CSS::OwnFontFaceKey> {
static unsigned hash(Web::CSS::OwnFontFaceKey const& key) { return pair_int_hash(key.family_name.hash(), pair_int_hash(key.weight, key.slope)); }
}; };
} }
@ -124,6 +146,29 @@ FlyString const& MatchingRule::qualified_layer_name() const
VERIFY_NOT_REACHED(); VERIFY_NOT_REACHED();
} }
OwnFontFaceKey::OwnFontFaceKey(FontFaceKey const& other)
: family_name(other.family_name)
, weight(other.weight)
, slope(other.slope)
{
}
OwnFontFaceKey::operator FontFaceKey() const
{
return FontFaceKey {
family_name,
weight,
slope
};
}
[[nodiscard]] bool OwnFontFaceKey::operator==(FontFaceKey const& other) const
{
return family_name == other.family_name
&& weight == other.weight
&& slope == other.slope;
}
static DOM::Element const* element_to_inherit_style_from(DOM::Element const*, Optional<CSS::Selector::PseudoElement::Type>); static DOM::Element const* element_to_inherit_style_from(DOM::Element const*, Optional<CSS::Selector::PseudoElement::Type>);
StyleComputer::StyleComputer(DOM::Document& document) StyleComputer::StyleComputer(DOM::Document& document)
@ -1738,16 +1783,16 @@ RefPtr<Gfx::FontCascadeList const> StyleComputer::find_matching_font_weight_desc
// Partial implementation of the font-matching algorithm: https://www.w3.org/TR/css-fonts-4/#font-matching-algorithm // Partial implementation of the font-matching algorithm: https://www.w3.org/TR/css-fonts-4/#font-matching-algorithm
// FIXME: This should be replaced by the full CSS font selection algorithm. // FIXME: This should be replaced by the full CSS font selection algorithm.
RefPtr<Gfx::FontCascadeList const> StyleComputer::font_matching_algorithm(FontFaceKey const& key, float font_size_in_pt) const RefPtr<Gfx::FontCascadeList const> StyleComputer::font_matching_algorithm(FlyString const& family_name, int weight, int slope, float font_size_in_pt) const
{ {
// If a font family match occurs, the user agent assembles the set of font faces in that family and then // If a font family match occurs, the user agent assembles the set of font faces in that family and then
// narrows the set to a single face using other font properties in the order given below. // narrows the set to a single face using other font properties in the order given below.
Vector<MatchingFontCandidate> matching_family_fonts; Vector<MatchingFontCandidate> matching_family_fonts;
for (auto const& font_key_and_loader : m_loaded_fonts) { for (auto const& font_key_and_loader : m_loaded_fonts) {
if (font_key_and_loader.key.family_name.equals_ignoring_ascii_case(key.family_name)) if (font_key_and_loader.key.family_name.equals_ignoring_ascii_case(family_name))
matching_family_fonts.empend(font_key_and_loader.key, const_cast<FontLoaderList*>(&font_key_and_loader.value)); matching_family_fonts.empend(font_key_and_loader.key, const_cast<FontLoaderList*>(&font_key_and_loader.value));
} }
Gfx::FontDatabase::the().for_each_typeface_with_family_name(key.family_name, [&](Gfx::Typeface const& typeface) { Gfx::FontDatabase::the().for_each_typeface_with_family_name(family_name, [&](Gfx::Typeface const& typeface) {
matching_family_fonts.empend( matching_family_fonts.empend(
FontFaceKey { FontFaceKey {
.family_name = typeface.family(), .family_name = typeface.family(),
@ -1764,24 +1809,24 @@ RefPtr<Gfx::FontCascadeList const> StyleComputer::font_matching_algorithm(FontFa
// We don't have complete support of italic and oblique fonts, so matching on font-style can be simplified to: // We don't have complete support of italic and oblique fonts, so matching on font-style can be simplified to:
// If a matching slope is found, all faces which don't have that matching slope are excluded from the matching set. // If a matching slope is found, all faces which don't have that matching slope are excluded from the matching set.
auto style_it = find_if(matching_family_fonts.begin(), matching_family_fonts.end(), auto style_it = find_if(matching_family_fonts.begin(), matching_family_fonts.end(),
[&](auto const& matching_font_candidate) { return matching_font_candidate.key.slope == key.slope; }); [&](auto const& matching_font_candidate) { return matching_font_candidate.key.slope == slope; });
if (style_it != matching_family_fonts.end()) { if (style_it != matching_family_fonts.end()) {
matching_family_fonts.remove_all_matching([&](auto const& matching_font_candidate) { matching_family_fonts.remove_all_matching([&](auto const& matching_font_candidate) {
return matching_font_candidate.key.slope != key.slope; return matching_font_candidate.key.slope != slope;
}); });
} }
// 3. font-weight is matched next. // 3. font-weight is matched next.
// If the desired weight is inclusively between 400 and 500, weights greater than or equal to the target weight // If the desired weight is inclusively between 400 and 500, weights greater than or equal to the target weight
// are checked in ascending order until 500 is hit and checked, followed by weights less than the target weight // are checked in ascending order until 500 is hit and checked, followed by weights less than the target weight
// in descending order, followed by weights greater than 500, until a match is found. // in descending order, followed by weights greater than 500, until a match is found.
if (key.weight >= 400 && key.weight <= 500) { if (weight >= 400 && weight <= 500) {
auto it = find_if(matching_family_fonts.begin(), matching_family_fonts.end(), auto it = find_if(matching_family_fonts.begin(), matching_family_fonts.end(),
[&](auto const& matching_font_candidate) { return matching_font_candidate.key.weight >= key.weight; }); [&](auto const& matching_font_candidate) { return matching_font_candidate.key.weight >= weight; });
for (; it != matching_family_fonts.end() && it->key.weight <= 500; ++it) { for (; it != matching_family_fonts.end() && it->key.weight <= 500; ++it) {
if (auto found_font = it->font_with_point_size(font_size_in_pt)) if (auto found_font = it->font_with_point_size(font_size_in_pt))
return found_font; return found_font;
} }
if (auto found_font = find_matching_font_weight_descending(matching_family_fonts, key.weight, font_size_in_pt, false)) if (auto found_font = find_matching_font_weight_descending(matching_family_fonts, weight, font_size_in_pt, false))
return found_font; return found_font;
for (; it != matching_family_fonts.end(); ++it) { for (; it != matching_family_fonts.end(); ++it) {
if (auto found_font = it->font_with_point_size(font_size_in_pt)) if (auto found_font = it->font_with_point_size(font_size_in_pt))
@ -1790,18 +1835,18 @@ RefPtr<Gfx::FontCascadeList const> StyleComputer::font_matching_algorithm(FontFa
} }
// If the desired weight is less than 400, weights less than or equal to the desired weight are checked in descending order // If the desired weight is less than 400, weights less than or equal to the desired weight are checked in descending order
// followed by weights above the desired weight in ascending order until a match is found. // followed by weights above the desired weight in ascending order until a match is found.
if (key.weight < 400) { if (weight < 400) {
if (auto found_font = find_matching_font_weight_descending(matching_family_fonts, key.weight, font_size_in_pt, true)) if (auto found_font = find_matching_font_weight_descending(matching_family_fonts, weight, font_size_in_pt, true))
return found_font; return found_font;
if (auto found_font = find_matching_font_weight_ascending(matching_family_fonts, key.weight, font_size_in_pt, false)) if (auto found_font = find_matching_font_weight_ascending(matching_family_fonts, weight, font_size_in_pt, false))
return found_font; return found_font;
} }
// If the desired weight is greater than 500, weights greater than or equal to the desired weight are checked in ascending order // If the desired weight is greater than 500, weights greater than or equal to the desired weight are checked in ascending order
// followed by weights below the desired weight in descending order until a match is found. // followed by weights below the desired weight in descending order until a match is found.
if (key.weight > 500) { if (weight > 500) {
if (auto found_font = find_matching_font_weight_ascending(matching_family_fonts, key.weight, font_size_in_pt, true)) if (auto found_font = find_matching_font_weight_ascending(matching_family_fonts, weight, font_size_in_pt, true))
return found_font; return found_font;
if (auto found_font = find_matching_font_weight_descending(matching_family_fonts, key.weight, font_size_in_pt, false)) if (auto found_font = find_matching_font_weight_descending(matching_family_fonts, weight, font_size_in_pt, false))
return found_font; return found_font;
} }
return {}; return {};
@ -1963,7 +2008,6 @@ RefPtr<Gfx::FontCascadeList const> StyleComputer::compute_font_for_style_values(
.weight = weight, .weight = weight,
.slope = slope, .slope = slope,
}; };
auto result = Gfx::FontCascadeList::create(); auto result = Gfx::FontCascadeList::create();
if (auto it = m_loaded_fonts.find(key); it != m_loaded_fonts.end()) { if (auto it = m_loaded_fonts.find(key); it != m_loaded_fonts.end()) {
auto const& loaders = it->value; auto const& loaders = it->value;
@ -1974,7 +2018,7 @@ RefPtr<Gfx::FontCascadeList const> StyleComputer::compute_font_for_style_values(
return result; return result;
} }
if (auto found_font = font_matching_algorithm(key, font_size_in_pt); found_font && !found_font->is_empty()) { if (auto found_font = font_matching_algorithm(family, weight, slope, font_size_in_pt); found_font && !found_font->is_empty()) {
return found_font; return found_font;
} }
@ -2800,7 +2844,7 @@ Optional<FontLoader&> StyleComputer::load_font_face(ParsedFontFace const& font_f
} else { } else {
FontLoaderList loaders; FontLoaderList loaders;
loaders.append(move(loader)); loaders.append(move(loader));
const_cast<StyleComputer&>(*this).m_loaded_fonts.set(key, move(loaders)); const_cast<StyleComputer&>(*this).m_loaded_fonts.set(OwnFontFaceKey(key), move(loaders));
} }
// Actual object owned by font loader list inside m_loaded_fonts, this isn't use-after-move/free // Actual object owned by font loader list inside m_loaded_fonts, this isn't use-after-move/free
return loader_ref; return loader_ref;

View file

@ -101,13 +101,20 @@ struct MatchingRule {
FlyString const& qualified_layer_name() const; FlyString const& qualified_layer_name() const;
}; };
struct FontFaceKey { struct FontFaceKey;
struct OwnFontFaceKey {
explicit OwnFontFaceKey(FontFaceKey const& other);
operator FontFaceKey() const;
[[nodiscard]] u32 hash() const { return pair_int_hash(family_name.hash(), pair_int_hash(weight, slope)); }
[[nodiscard]] bool operator==(OwnFontFaceKey const& other) const = default;
[[nodiscard]] bool operator==(FontFaceKey const& other) const;
FlyString family_name; FlyString family_name;
int weight { 0 }; int weight { 0 };
int slope { 0 }; int slope { 0 };
[[nodiscard]] u32 hash() const { return pair_int_hash(family_name.hash(), pair_int_hash(weight, slope)); }
[[nodiscard]] bool operator==(FontFaceKey const&) const = default;
}; };
class FontLoader; class FontLoader;
@ -180,7 +187,7 @@ private:
void compute_cascaded_values(StyleProperties&, DOM::Element&, Optional<CSS::Selector::PseudoElement::Type>, bool& did_match_any_pseudo_element_rules, ComputeStyleMode) const; void compute_cascaded_values(StyleProperties&, DOM::Element&, Optional<CSS::Selector::PseudoElement::Type>, bool& did_match_any_pseudo_element_rules, ComputeStyleMode) const;
static RefPtr<Gfx::FontCascadeList const> find_matching_font_weight_ascending(Vector<MatchingFontCandidate> const& candidates, int target_weight, float font_size_in_pt, bool inclusive); static RefPtr<Gfx::FontCascadeList const> find_matching_font_weight_ascending(Vector<MatchingFontCandidate> const& candidates, int target_weight, float font_size_in_pt, bool inclusive);
static RefPtr<Gfx::FontCascadeList const> find_matching_font_weight_descending(Vector<MatchingFontCandidate> const& candidates, int target_weight, float font_size_in_pt, bool inclusive); static RefPtr<Gfx::FontCascadeList const> find_matching_font_weight_descending(Vector<MatchingFontCandidate> const& candidates, int target_weight, float font_size_in_pt, bool inclusive);
RefPtr<Gfx::FontCascadeList const> font_matching_algorithm(FontFaceKey const& key, float font_size_in_pt) const; RefPtr<Gfx::FontCascadeList const> font_matching_algorithm(FlyString const& family_name, int weight, int slope, float font_size_in_pt) const;
void compute_font(StyleProperties&, DOM::Element const*, Optional<CSS::Selector::PseudoElement::Type>) const; void compute_font(StyleProperties&, DOM::Element const*, Optional<CSS::Selector::PseudoElement::Type>) const;
void compute_math_depth(StyleProperties&, DOM::Element const*, Optional<CSS::Selector::PseudoElement::Type>) const; void compute_math_depth(StyleProperties&, DOM::Element const*, Optional<CSS::Selector::PseudoElement::Type>) const;
void compute_defaulted_values(StyleProperties&, DOM::Element const*, Optional<CSS::Selector::PseudoElement::Type>) const; void compute_defaulted_values(StyleProperties&, DOM::Element const*, Optional<CSS::Selector::PseudoElement::Type>) const;
@ -246,7 +253,7 @@ private:
GC::Root<CSSStyleSheet> m_user_style_sheet; GC::Root<CSSStyleSheet> m_user_style_sheet;
using FontLoaderList = Vector<NonnullOwnPtr<FontLoader>>; using FontLoaderList = Vector<NonnullOwnPtr<FontLoader>>;
HashMap<FontFaceKey, FontLoaderList> m_loaded_fonts; HashMap<OwnFontFaceKey, FontLoaderList> m_loaded_fonts;
Length::FontMetrics m_default_font_metrics; Length::FontMetrics m_default_font_metrics;
Length::FontMetrics m_root_element_font_metrics; Length::FontMetrics m_root_element_font_metrics;