Commit 7ca178b8 authored by Dominik Röttsches's avatar Dominik Röttsches Committed by Commit Bot

Fix FontDescription cache keys with identical FontVariationSettings

Two identical FontDescriptions with identical font-variation-settings
axis + value pairs should produce the same FontCacheKey hash. Fix an issue
in FontCacheKey computation that only performs pointer comparisons on
the FontVariationSettings.

This fixes a smaller percentage of FontCache misses during variable font
animations.

Bug: 1046257
Change-Id: I05ae3ac64b289dc7e1c41b03ab15d7e651195d9b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2023650Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736280}
parent ceeabef2
...@@ -93,13 +93,19 @@ struct FontCacheKey { ...@@ -93,13 +93,19 @@ struct FontCacheKey {
} }
bool operator==(const FontCacheKey& other) const { bool operator==(const FontCacheKey& other) const {
bool variation_settings_equal =
(!variation_settings_ && !other.variation_settings_) ||
(variation_settings_ && other.variation_settings_ &&
*variation_settings_ == *other.variation_settings_);
return creation_params_ == other.creation_params_ && return creation_params_ == other.creation_params_ &&
font_size_ == other.font_size_ && options_ == other.options_ && font_size_ == other.font_size_ && options_ == other.options_ &&
device_scale_factor_ == other.device_scale_factor_ && device_scale_factor_ == other.device_scale_factor_ &&
variation_settings_ == other.variation_settings_ && variation_settings_equal &&
is_unique_match_ == other.is_unique_match_; is_unique_match_ == other.is_unique_match_;
} }
bool operator!=(const FontCacheKey& other) const { return !(*this == other); }
static constexpr unsigned PrecisionMultiplier() { static constexpr unsigned PrecisionMultiplier() {
return kFontSizePrecisionMultiplier; return kFontSizePrecisionMultiplier;
} }
......
...@@ -63,6 +63,78 @@ TEST(FontDescriptionTest, TestHashCollision) { ...@@ -63,6 +63,78 @@ TEST(FontDescriptionTest, TestHashCollision) {
} }
} }
TEST(FontDescriptionTest, VariationSettingsIdentical) {
FontDescription a;
FontDescription b(a);
scoped_refptr<FontVariationSettings> settings_a =
FontVariationSettings::Create();
settings_a->Append(FontVariationAxis("test", 1));
scoped_refptr<FontVariationSettings> settings_b =
FontVariationSettings::Create();
settings_b->Append(FontVariationAxis("test", 1));
ASSERT_EQ(*settings_a, *settings_b);
a.SetVariationSettings(settings_a);
b.SetVariationSettings(settings_b);
ASSERT_EQ(a, b);
FontFaceCreationParams test_creation_params;
FontCacheKey cache_key_a = a.CacheKey(test_creation_params, false);
FontCacheKey cache_key_b = b.CacheKey(test_creation_params, false);
ASSERT_EQ(cache_key_a, cache_key_b);
}
TEST(FontDescriptionTest, VariationSettingsDifferent) {
FontDescription a;
FontDescription b(a);
scoped_refptr<FontVariationSettings> settings_a =
FontVariationSettings::Create();
settings_a->Append(FontVariationAxis("test", 1));
scoped_refptr<FontVariationSettings> settings_b =
FontVariationSettings::Create();
settings_b->Append(FontVariationAxis("0000", 1));
ASSERT_NE(*settings_a, *settings_b);
a.SetVariationSettings(settings_a);
b.SetVariationSettings(settings_b);
ASSERT_NE(a, b);
FontFaceCreationParams test_creation_params;
FontCacheKey cache_key_a = a.CacheKey(test_creation_params, false);
FontCacheKey cache_key_b = b.CacheKey(test_creation_params, false);
ASSERT_NE(cache_key_a, cache_key_b);
scoped_refptr<FontVariationSettings> second_settings_a =
FontVariationSettings::Create();
second_settings_a->Append(FontVariationAxis("test", 1));
scoped_refptr<FontVariationSettings> second_settings_b =
FontVariationSettings::Create();
ASSERT_NE(*second_settings_a, *second_settings_b);
a.SetVariationSettings(second_settings_a);
b.SetVariationSettings(second_settings_b);
ASSERT_NE(a, b);
FontCacheKey second_cache_key_a = a.CacheKey(test_creation_params, false);
FontCacheKey second_cache_key_b = b.CacheKey(test_creation_params, false);
ASSERT_NE(second_cache_key_a, second_cache_key_b);
}
TEST(FontDescriptionTest, ToString) { TEST(FontDescriptionTest, ToString) {
FontDescription description; FontDescription description;
......
...@@ -48,6 +48,7 @@ class FontSettings { ...@@ -48,6 +48,7 @@ class FontSettings {
bool operator==(const FontSettings& other) const { bool operator==(const FontSettings& other) const {
return list_ == other.list_; return list_ == other.list_;
} }
bool operator!=(const FontSettings& other) const { return !(*this == other); }
String ToString() const { String ToString() const {
StringBuilder builder; StringBuilder builder;
wtf_size_t num_features = size(); wtf_size_t num_features = size();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment