Skip to content

Commit

Permalink
Use SkColorTable in cc::ColorFilter and make it thread safe
Browse files Browse the repository at this point in the history 
cc::ColorFilter can be used from multiple threads, and the lazy
creation of sk_color_filter_ (which was probably based on a concern
about the cost of TableARGBColorFilter?) was not thread safe.

Now use SkColorTable in TableColorFilter to reduce copy of the color
tables, and always create sk_color_filter_ in the constructors of
ColorFilter subclasses to make them thread safe.

(cherry picked from commit 4034b48)

Bug: 1451102, 1434335
Change-Id: Icca998fb949c5b579ff17b516de17e1a05a0d1f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4654951
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1164409}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4809033
Auto-Submit: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Cr-Commit-Position: refs/branch-heads/5845@{#1596}
Cr-Branched-From: 5a5dff6-refs/heads/main@{#1160321}
  • Loading branch information
@wangxianzhu
wangxianzhu authored and Chromium LUCI CQ committed Aug 24, 2023
1 parent 31af827 commit 5e9baf1
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 84 deletions.
116 changes: 47 additions & 69 deletions cc/paint/color_filter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@
#include "cc/paint/color_filter.h"

#include <algorithm>
#include <utility>

#include "base/check_op.h"
#include "base/memory/values_equivalent.h"
#include "cc/paint/paint_op_reader.h"
#include "cc/paint/paint_op_writer.h"
#include "third_party/skia/include/core/SkColorFilter.h"
#include "third_party/skia/include/core/SkColorTable.h"
#include "third_party/skia/include/effects/SkHighContrastFilter.h"
#include "third_party/skia/include/effects/SkLumaColorFilter.h"

Expand All @@ -21,35 +23,38 @@ namespace {
class MatrixColorFilter final : public ColorFilter {
public:
explicit MatrixColorFilter(const float matrix[20])
: ColorFilter(Type::kMatrix) {
std::copy_n(matrix, 20, matrix_);
}
: ColorFilter(Type::kMatrix, SkColorFilters::Matrix(matrix)) {}

private:
sk_sp<SkColorFilter> CreateSkColorFilter() const override {
return SkColorFilters::Matrix(matrix_);
}
size_t SerializedDataSize() const override {
return PaintOpWriter::SerializedSizeOfElements(matrix_, 20);
float matrix[20];
return PaintOpWriter::SerializedSizeOfElements(matrix, 20);
}
void SerializeData(PaintOpWriter& writer) const override {
for (float f : matrix_) {
// The identity matrix will be used if the constructor failed to create
// sk_color_filter_ due to invalid matrix values.
float matrix[20] = {1, 0, 0, 0, 0, // row 0
0, 1, 0, 0, 0, // row 1
0, 0, 1, 0, 0, // row 2
0, 0, 0, 1, 0}; // row 3
if (sk_color_filter_) {
sk_color_filter_->asAColorMatrix(matrix);
}
for (float f : matrix) {
writer.Write(f);
}
}

float matrix_[20];
};

class BlendColorFilter final : public ColorFilter {
public:
BlendColorFilter(const SkColor4f& color, SkBlendMode blend_mode)
: ColorFilter(Type::kBlend), color_(color), blend_mode_(blend_mode) {}
: ColorFilter(Type::kBlend,
SkColorFilters::Blend(color, nullptr, blend_mode)),
color_(color),
blend_mode_(blend_mode) {}

private:
sk_sp<SkColorFilter> CreateSkColorFilter() const override {
return SkColorFilters::Blend(color_, nullptr, blend_mode_);
}
size_t SerializedDataSize() const override {
return PaintOpWriter::SerializedSize(color_) +
PaintOpWriter::SerializedSize(blend_mode_);
Expand All @@ -65,77 +70,51 @@ class BlendColorFilter final : public ColorFilter {

class SRGBToLinearGammaColorFilter final : public ColorFilter {
public:
SRGBToLinearGammaColorFilter() : ColorFilter(Type::kSRGBToLinearGamma) {}

private:
sk_sp<SkColorFilter> CreateSkColorFilter() const override {
return SkColorFilters::SRGBToLinearGamma();
}
SRGBToLinearGammaColorFilter()
: ColorFilter(Type::kSRGBToLinearGamma,
SkColorFilters::SRGBToLinearGamma()) {}
};

class LinearToSRGBGammaColorFilter final : public ColorFilter {
public:
LinearToSRGBGammaColorFilter() : ColorFilter(Type::kLinearToSRGBGamma) {}

private:
sk_sp<SkColorFilter> CreateSkColorFilter() const override {
return SkColorFilters::LinearToSRGBGamma();
}
LinearToSRGBGammaColorFilter()
: ColorFilter(Type::kLinearToSRGBGamma,
SkColorFilters::LinearToSRGBGamma()) {}
};

class LumaColorFilter final : public ColorFilter {
public:
LumaColorFilter() : ColorFilter(Type::kLuma) {}

private:
sk_sp<SkColorFilter> CreateSkColorFilter() const override {
return SkLumaColorFilter::Make();
}
LumaColorFilter() : ColorFilter(Type::kLuma, SkLumaColorFilter::Make()) {}
};

class TableARGBColorFilter : public ColorFilter {
class TableColorFilter : public ColorFilter {
public:
TableARGBColorFilter(const uint8_t a_table[256],
const uint8_t r_table[256],
const uint8_t g_table[256],
const uint8_t b_table[256])
: ColorFilter(Type::kTableARGB) {
std::copy_n(a_table, 256, a_table_);
std::copy_n(r_table, 256, r_table_);
std::copy_n(g_table, 256, g_table_);
std::copy_n(b_table, 256, b_table_);
}
explicit TableColorFilter(sk_sp<SkColorTable> table)
: ColorFilter(Type::kTableARGB, SkColorFilters::Table(table)),
table_(std::move(table)) {}

private:
sk_sp<SkColorFilter> CreateSkColorFilter() const override {
return SkColorFilters::TableARGB(a_table_, r_table_, g_table_, b_table_);
}
size_t SerializedDataSize() const override {
return PaintOpWriter::SerializedSizeOfBytes(256 * 4);
}
void SerializeData(PaintOpWriter& writer) const override {
writer.WriteData(256, a_table_);
writer.WriteData(256, r_table_);
writer.WriteData(256, g_table_);
writer.WriteData(256, b_table_);
writer.WriteData(256, table_->alphaTable());
writer.WriteData(256, table_->redTable());
writer.WriteData(256, table_->greenTable());
writer.WriteData(256, table_->blueTable());
}

private:
uint8_t a_table_[256];
uint8_t r_table_[256];
uint8_t g_table_[256];
uint8_t b_table_[256];
sk_sp<SkColorTable> table_;
};

class HighContrastColorFilter final : public ColorFilter {
public:
explicit HighContrastColorFilter(const SkHighContrastConfig& config)
: ColorFilter(Type::kHighContrast), config_(config) {}
: ColorFilter(Type::kHighContrast, SkHighContrastFilter::Make(config)),
config_(config) {}

private:
sk_sp<SkColorFilter> CreateSkColorFilter() const override {
return SkHighContrastFilter::Make(config_);
}
size_t SerializedDataSize() const override {
return PaintOpWriter::SerializedSize(config_);
}
Expand All @@ -150,7 +129,8 @@ class HighContrastColorFilter final : public ColorFilter {

ColorFilter::~ColorFilter() = default;

ColorFilter::ColorFilter(Type type) : type_(type) {
ColorFilter::ColorFilter(Type type, sk_sp<SkColorFilter> sk_color_filter)
: type_(type), sk_color_filter_(std::move(sk_color_filter)) {
DCHECK_NE(type, Type::kNull);
}

Expand All @@ -175,7 +155,11 @@ sk_sp<ColorFilter> ColorFilter::MakeTableARGB(const uint8_t a_table[256],
const uint8_t r_table[256],
const uint8_t g_table[256],
const uint8_t b_table[256]) {
return sk_make_sp<TableARGBColorFilter>(a_table, r_table, g_table, b_table);
return MakeTable(SkColorTable::Make(a_table, r_table, g_table, b_table));
}

sk_sp<ColorFilter> ColorFilter::MakeTable(sk_sp<SkColorTable> table) {
return sk_make_sp<TableColorFilter>(std::move(table));
}

sk_sp<ColorFilter> ColorFilter::MakeLuma() {
Expand All @@ -188,20 +172,14 @@ sk_sp<ColorFilter> ColorFilter::MakeHighContrast(
}

SkColor4f ColorFilter::FilterColor(const SkColor4f& color) const {
sk_sp<SkColorFilter> filter = GetSkColorFilter();
return filter ? filter->filterColor4f(color, nullptr, nullptr) : color;
return sk_color_filter_
? sk_color_filter_->filterColor4f(color, nullptr, nullptr)
: color;
}
bool ColorFilter::EqualsForTesting(const ColorFilter& other) const {
return type_ == other.type_;
}

sk_sp<SkColorFilter> ColorFilter::GetSkColorFilter() const {
if (!sk_color_filter_) {
sk_color_filter_ = CreateSkColorFilter();
}
return sk_color_filter_;
}

size_t ColorFilter::SerializedDataSize() const {
return 0u;
}
Expand Down
22 changes: 9 additions & 13 deletions cc/paint/color_filter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "third_party/skia/include/core/SkRefCnt.h"

class SkColorFilter;
class SkColorTable;
struct SkHighContrastConfig;

namespace cc {
Expand All @@ -30,11 +31,11 @@ class CC_PAINT_EXPORT ColorFilter : public SkRefCnt {
static sk_sp<ColorFilter> MakeSRGBToLinearGamma();
static sk_sp<ColorFilter> MakeLinearToSRGBGamma();
static sk_sp<ColorFilter> MakeLuma();
// TODO(wangxianzhu): Use sk_sp<SkColorTable> when it's available.
static sk_sp<ColorFilter> MakeTableARGB(const uint8_t a_table[256],
const uint8_t r_table[256],
const uint8_t g_table[256],
const uint8_t b_table[256]);
static sk_sp<ColorFilter> MakeTable(sk_sp<SkColorTable> table);
static sk_sp<ColorFilter> MakeHighContrast(
const SkHighContrastConfig& config);

Expand All @@ -43,6 +44,11 @@ class CC_PAINT_EXPORT ColorFilter : public SkRefCnt {
bool EqualsForTesting(const ColorFilter& other) const;

protected:
friend class ColorFilterPaintFilter;
friend class PaintFlags;
friend class PaintOpReader;
friend class PaintOpWriter;

enum class Type {
// kNull is for serialization purposes only, to indicate a null color
// filter in a containing object (e.g. PaintFlags).
Expand All @@ -57,24 +63,14 @@ class CC_PAINT_EXPORT ColorFilter : public SkRefCnt {
kMaxValue = kHighContrast,
};

explicit ColorFilter(Type type);

sk_sp<SkColorFilter> GetSkColorFilter() const;

virtual sk_sp<SkColorFilter> CreateSkColorFilter() const = 0;
explicit ColorFilter(Type type, sk_sp<SkColorFilter> sk_color_filter);
// These functions don't handle type_. It's handled in PaintOpWriter/Reader.
virtual size_t SerializedDataSize() const;
virtual void SerializeData(PaintOpWriter& writer) const;
static sk_sp<ColorFilter> Deserialize(PaintOpReader& reader, Type type);

private:
friend class ColorFilterPaintFilter;
friend class PaintFlags;
friend class PaintOpReader;
friend class PaintOpWriter;

Type type_;
mutable sk_sp<SkColorFilter> sk_color_filter_;
sk_sp<SkColorFilter> sk_color_filter_;
};

} // namespace cc
Expand Down
2 changes: 1 addition & 1 deletion cc/paint/paint_filter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ ColorFilterPaintFilter::ColorFilterPaintFilter(sk_sp<ColorFilter> color_filter,
color_filter_(std::move(color_filter)),
input_(std::move(input)) {
cached_sk_filter_ = SkImageFilters::ColorFilter(
color_filter_ ? color_filter_->GetSkColorFilter() : nullptr,
color_filter_ ? color_filter_->sk_color_filter_ : nullptr,
GetSkFilter(input_.get()), crop_rect);
}

Expand Down
2 changes: 1 addition & 1 deletion cc/paint/paint_flags.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ SkPaint PaintFlags::ToSkPaint() const {
paint.setShader(shader_->GetSkShader(getFilterQuality()));
paint.setMaskFilter(mask_filter_);
if (color_filter_) {
paint.setColorFilter(color_filter_->GetSkColorFilter());
paint.setColorFilter(color_filter_->sk_color_filter_);
}
if (image_filter_)
paint.setImageFilter(image_filter_->cached_sk_filter_);
Expand Down

0 comments on commit 5e9baf1

Please sign in to comment.