Commit cbaf8ca8 authored by Prashant Nevase's avatar Prashant Nevase Committed by Commit Bot

Implement inverted color cache in DarkModeFilter.

Implements cache for inverted colors using WTF::LruCache so that
paint ops can use the cached values over calculating heavy invert
color operations using color filters.

With cache, the filter operations are improved by 15x to 20x if all
colors are stored in cache and 20% to 30% if there are more colors
than cache size. This patch improves the dark mode paint ops by 2%.

Bug: 1060899
Change-Id: Ia9d1fe0912d50c72835f597a731be137169a6819
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2102388Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Commit-Queue: Prashant Nevase <prashant.n@samsung.com>
Cr-Commit-Position: refs/heads/master@{#774792}
parent ac42fb1e
...@@ -16,6 +16,8 @@ ...@@ -16,6 +16,8 @@
#include "third_party/blink/renderer/platform/graphics/dark_mode_image_classifier.h" #include "third_party/blink/renderer/platform/graphics/dark_mode_image_classifier.h"
#include "third_party/blink/renderer/platform/graphics/graphics_context.h" #include "third_party/blink/renderer/platform/graphics/graphics_context.h"
#include "third_party/blink/renderer/platform/graphics/graphics_types.h" #include "third_party/blink/renderer/platform/graphics/graphics_types.h"
#include "third_party/blink/renderer/platform/wtf/hash_functions.h"
#include "third_party/blink/renderer/platform/wtf/lru_cache.h"
#include "third_party/skia/include/core/SkColorFilter.h" #include "third_party/skia/include/core/SkColorFilter.h"
#include "third_party/skia/include/effects/SkColorMatrix.h" #include "third_party/skia/include/effects/SkColorMatrix.h"
...@@ -45,6 +47,8 @@ void VerifySettingsAreUnchanged(const DarkModeSettings& a, ...@@ -45,6 +47,8 @@ void VerifySettingsAreUnchanged(const DarkModeSettings& a,
#endif // DCHECK_IS_ON() #endif // DCHECK_IS_ON()
const size_t kMaxCacheSize = 1024u;
bool ShouldApplyToImage(const DarkModeSettings& settings, bool ShouldApplyToImage(const DarkModeSettings& settings,
const FloatRect& src_rect, const FloatRect& src_rect,
const FloatRect& dest_rect, const FloatRect& dest_rect,
...@@ -88,10 +92,36 @@ sk_sp<SkColorFilter> MakeGrayscaleFilter(float grayscale_percent) { ...@@ -88,10 +92,36 @@ sk_sp<SkColorFilter> MakeGrayscaleFilter(float grayscale_percent) {
} // namespace } // namespace
// DarkModeInvertedColorCache - Implements cache for inverted colors.
class DarkModeInvertedColorCache {
public:
DarkModeInvertedColorCache() : cache_(kMaxCacheSize) {}
~DarkModeInvertedColorCache() = default;
SkColor GetInvertedColor(DarkModeColorFilter* filter, SkColor color) {
WTF::IntegralWithAllKeys<SkColor> key(color);
SkColor* cached_value = cache_.Get(key);
if (cached_value)
return *cached_value;
SkColor inverted_color = filter->InvertColor(Color(color)).Rgb();
cache_.Put(key, std::move(inverted_color));
return inverted_color;
}
void Clear() { cache_.Clear(); }
size_t size() { return cache_.size(); }
private:
WTF::LruCache<WTF::IntegralWithAllKeys<SkColor>, SkColor> cache_;
};
DarkModeFilter::DarkModeFilter() DarkModeFilter::DarkModeFilter()
: text_classifier_(nullptr), : text_classifier_(nullptr),
color_filter_(nullptr), color_filter_(nullptr),
image_filter_(nullptr) { image_filter_(nullptr),
inverted_color_cache_(new DarkModeInvertedColorCache()) {
DarkModeSettings default_settings; DarkModeSettings default_settings;
default_settings.mode = DarkModeInversionAlgorithm::kOff; default_settings.mode = DarkModeInversionAlgorithm::kOff;
UpdateSettings(default_settings); UpdateSettings(default_settings);
...@@ -111,6 +141,8 @@ void DarkModeFilter::UpdateSettings(const DarkModeSettings& new_settings) { ...@@ -111,6 +141,8 @@ void DarkModeFilter::UpdateSettings(const DarkModeSettings& new_settings) {
return; return;
} }
inverted_color_cache_->Clear();
settings_ = new_settings; settings_ = new_settings;
color_filter_ = DarkModeColorFilter::FromSettings(settings_); color_filter_ = DarkModeColorFilter::FromSettings(settings_);
if (!color_filter_) { if (!color_filter_) {
...@@ -137,8 +169,11 @@ Color DarkModeFilter::InvertColorIfNeeded(const Color& color, ...@@ -137,8 +169,11 @@ Color DarkModeFilter::InvertColorIfNeeded(const Color& color,
if (role_override_.has_value()) if (role_override_.has_value())
role = role_override_.value(); role = role_override_.value();
if (ShouldApplyToColor(color, role)) if (ShouldApplyToColor(color, role)) {
return color_filter_->InvertColor(color); return Color(inverted_color_cache_->GetInvertedColor(color_filter_.get(),
color.Rgb()));
}
return color; return color;
} }
...@@ -165,10 +200,8 @@ base::Optional<cc::PaintFlags> DarkModeFilter::ApplyToFlagsIfNeeded( ...@@ -165,10 +200,8 @@ base::Optional<cc::PaintFlags> DarkModeFilter::ApplyToFlagsIfNeeded(
if (flags.HasShader()) { if (flags.HasShader()) {
dark_mode_flags.setColorFilter(color_filter_->ToSkColorFilter()); dark_mode_flags.setColorFilter(color_filter_->ToSkColorFilter());
} else if (ShouldApplyToColor(flags.getColor(), role)) { } else if (ShouldApplyToColor(flags.getColor(), role)) {
Color inverted_color = color_filter_->InvertColor(flags.getColor()); dark_mode_flags.setColor(inverted_color_cache_->GetInvertedColor(
dark_mode_flags.setColor( color_filter_.get(), flags.getColor()));
SkColorSetARGB(inverted_color.Alpha(), inverted_color.Red(),
inverted_color.Green(), inverted_color.Blue()));
} }
return base::make_optional<cc::PaintFlags>(std::move(dark_mode_flags)); return base::make_optional<cc::PaintFlags>(std::move(dark_mode_flags));
...@@ -213,6 +246,10 @@ bool DarkModeFilter::ShouldApplyToColor(const Color& color, ElementRole role) { ...@@ -213,6 +246,10 @@ bool DarkModeFilter::ShouldApplyToColor(const Color& color, ElementRole role) {
NOTREACHED(); NOTREACHED();
} }
size_t DarkModeFilter::GetInvertedColorCacheSizeForTesting() {
return inverted_color_cache_->size();
}
ScopedDarkModeElementRoleOverride::ScopedDarkModeElementRoleOverride( ScopedDarkModeElementRoleOverride::ScopedDarkModeElementRoleOverride(
GraphicsContext* graphics_context, GraphicsContext* graphics_context,
DarkModeFilter::ElementRole role) DarkModeFilter::ElementRole role)
......
...@@ -22,6 +22,7 @@ namespace blink { ...@@ -22,6 +22,7 @@ namespace blink {
class DarkModeColorClassifier; class DarkModeColorClassifier;
class DarkModeColorFilter; class DarkModeColorFilter;
class ScopedDarkModeElementRoleOverride; class ScopedDarkModeElementRoleOverride;
class DarkModeInvertedColorCache;
class PLATFORM_EXPORT DarkModeFilter { class PLATFORM_EXPORT DarkModeFilter {
public: public:
...@@ -51,6 +52,7 @@ class PLATFORM_EXPORT DarkModeFilter { ...@@ -51,6 +52,7 @@ class PLATFORM_EXPORT DarkModeFilter {
cc::PaintFlags* flags); cc::PaintFlags* flags);
SkColorFilter* GetImageFilterForTesting() { return image_filter_.get(); } SkColorFilter* GetImageFilterForTesting() { return image_filter_.get(); }
size_t GetInvertedColorCacheSizeForTesting();
private: private:
friend class ScopedDarkModeElementRoleOverride; friend class ScopedDarkModeElementRoleOverride;
...@@ -64,6 +66,7 @@ class PLATFORM_EXPORT DarkModeFilter { ...@@ -64,6 +66,7 @@ class PLATFORM_EXPORT DarkModeFilter {
std::unique_ptr<DarkModeColorFilter> color_filter_; std::unique_ptr<DarkModeColorFilter> color_filter_;
sk_sp<SkColorFilter> image_filter_; sk_sp<SkColorFilter> image_filter_;
base::Optional<ElementRole> role_override_; base::Optional<ElementRole> role_override_;
std::unique_ptr<DarkModeInvertedColorCache> inverted_color_cache_;
}; };
// Temporarily override the element role for the scope of this object's // Temporarily override the element role for the scope of this object's
......
...@@ -66,5 +66,56 @@ TEST(DarkModeFilterTest, ApplyDarkModeToColorsAndFlags) { ...@@ -66,5 +66,56 @@ TEST(DarkModeFilterTest, ApplyDarkModeToColorsAndFlags) {
EXPECT_NE(nullptr, filter.GetImageFilterForTesting()); EXPECT_NE(nullptr, filter.GetImageFilterForTesting());
} }
TEST(DarkModeFilterTest, InvertedColorCacheSize) {
DarkModeFilter filter;
DarkModeSettings settings;
settings.mode = DarkModeInversionAlgorithm::kSimpleInvertForTesting;
filter.UpdateSettings(settings);
EXPECT_EQ(0u, filter.GetInvertedColorCacheSizeForTesting());
EXPECT_EQ(Color::kBlack,
filter.InvertColorIfNeeded(
Color::kWhite, DarkModeFilter::ElementRole::kBackground));
EXPECT_EQ(1u, filter.GetInvertedColorCacheSizeForTesting());
// Should get cached value.
EXPECT_EQ(Color::kBlack,
filter.InvertColorIfNeeded(
Color::kWhite, DarkModeFilter::ElementRole::kBackground));
EXPECT_EQ(1u, filter.GetInvertedColorCacheSizeForTesting());
// On changing DarkModeSettings, cache should be reset.
settings.mode = DarkModeInversionAlgorithm::kInvertLightness;
filter.UpdateSettings(settings);
EXPECT_EQ(0u, filter.GetInvertedColorCacheSizeForTesting());
}
TEST(DarkModeFilterTest, InvertedColorCacheZeroMaxKeys) {
DarkModeFilter filter;
DarkModeSettings settings;
settings.mode = DarkModeInversionAlgorithm::kSimpleInvertForTesting;
filter.UpdateSettings(settings);
EXPECT_EQ(0u, filter.GetInvertedColorCacheSizeForTesting());
EXPECT_EQ(Color::kBlack, filter.InvertColorIfNeeded(
Color(SK_ColorWHITE),
DarkModeFilter::ElementRole::kBackground));
EXPECT_EQ(1u, filter.GetInvertedColorCacheSizeForTesting());
EXPECT_EQ(
Color(SK_ColorTRANSPARENT),
filter.InvertColorIfNeeded(Color(SK_ColorTRANSPARENT),
DarkModeFilter::ElementRole::kBackground));
EXPECT_EQ(2u, filter.GetInvertedColorCacheSizeForTesting());
// Results returned from cache.
EXPECT_EQ(Color::kBlack, filter.InvertColorIfNeeded(
Color(SK_ColorWHITE),
DarkModeFilter::ElementRole::kBackground));
EXPECT_EQ(
Color(SK_ColorTRANSPARENT),
filter.InvertColorIfNeeded(Color(SK_ColorTRANSPARENT),
DarkModeFilter::ElementRole::kBackground));
EXPECT_EQ(2u, filter.GetInvertedColorCacheSizeForTesting());
}
} // namespace } // namespace
} // namespace blink } // namespace blink
...@@ -27,6 +27,7 @@ ...@@ -27,6 +27,7 @@
#include "base/bit_cast.h" #include "base/bit_cast.h"
#include "base/memory/scoped_refptr.h" #include "base/memory/scoped_refptr.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "third_party/blink/renderer/platform/wtf/hash_table_deleted_value_type.h"
#include "third_party/blink/renderer/platform/wtf/std_lib_extras.h" #include "third_party/blink/renderer/platform/wtf/std_lib_extras.h"
namespace WTF { namespace WTF {
...@@ -277,6 +278,59 @@ struct DefaultHash<std::pair<T, U>> { ...@@ -277,6 +278,59 @@ struct DefaultHash<std::pair<T, U>> {
using Hash = PairHash<T, U>; using Hash = PairHash<T, U>;
}; };
// Wrapper for integral type to extend to have 0 and max keys.
template <typename T>
struct IntegralWithAllKeys {
IntegralWithAllKeys() : IntegralWithAllKeys(0, ValueType::kEmpty) {}
explicit IntegralWithAllKeys(T value)
: IntegralWithAllKeys(value, ValueType::kValid) {}
explicit IntegralWithAllKeys(HashTableDeletedValueType)
: IntegralWithAllKeys(0, ValueType::kDeleted) {}
bool IsHashTableDeletedValue() const {
return value_type_ == ValueType::kDeleted;
}
unsigned Hash() const {
return HashInts(value_, static_cast<unsigned>(value_type_));
}
bool operator==(const IntegralWithAllKeys& b) const {
return value_ == b.value_ && value_type_ == b.value_type_;
}
private:
enum class ValueType : uint8_t { kEmpty, kValid, kDeleted };
IntegralWithAllKeys(T value, ValueType value_type)
: value_(value), value_type_(value_type) {
static_assert(std::is_integral<T>::value,
"Only integral types are supported.");
}
T value_;
ValueType value_type_;
};
// Specialization for integral type to have all possible values for key
// including 0 and max.
template <typename T>
struct IntegralWithAllKeysHash {
static unsigned GetHash(const IntegralWithAllKeys<T>& key) {
return key.Hash();
}
static bool Equal(const IntegralWithAllKeys<T>& a,
const IntegralWithAllKeys<T>& b) {
return a == b;
}
static const bool safe_to_compare_to_empty_or_deleted = true;
};
template <typename T>
struct DefaultHash<IntegralWithAllKeys<T>> {
using Hash = IntegralWithAllKeysHash<T>;
};
} // namespace WTF } // namespace WTF
using WTF::DefaultHash; using WTF::DefaultHash;
......
...@@ -231,6 +231,12 @@ struct SimpleClassHashTraits : GenericHashTraits<T> { ...@@ -231,6 +231,12 @@ struct SimpleClassHashTraits : GenericHashTraits<T> {
} }
}; };
// Default traits disallow both 0 and max as keys -- use these traits to allow
// all values as keys.
template <typename T>
struct HashTraits<IntegralWithAllKeys<T>>
: SimpleClassHashTraits<IntegralWithAllKeys<T>> {};
template <typename P> template <typename P>
struct HashTraits<scoped_refptr<P>> : SimpleClassHashTraits<scoped_refptr<P>> { struct HashTraits<scoped_refptr<P>> : SimpleClassHashTraits<scoped_refptr<P>> {
static_assert(sizeof(void*) == sizeof(scoped_refptr<P>), static_assert(sizeof(void*) == sizeof(scoped_refptr<P>),
......
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