Remove arbitrary limit on number of composed color filters

This effectively reverts https://codereview.chromium.org/972153010/

Note that we have no such hard limit on SkShaders, and it's never
been a problem. Complex SkPaint objects can generate too many FPs
for us to handle, but those are rare, and better managed by just
reporting the shader compile error (which we do).

Change-Id: Iee5dc3d65ec130f2ce0a29e55fbe3c25b00dc828
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/279836
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This commit is contained in:
Brian Osman 2020-03-27 10:06:54 -04:00 committed by Skia Commit-Bot
parent e1bbfab135
commit 9a3ee1913a
3 changed files with 3 additions and 65 deletions

View File

@ -89,9 +89,6 @@ public:
* this filter, applied to the output of the inner filter.
*
* result = this(inner(...))
*
* Due to internal limits, it is possible that this will return NULL, so the caller must
* always check.
*/
sk_sp<SkColorFilter> makeComposed(sk_sp<SkColorFilter> inner) const;
@ -137,22 +134,11 @@ protected:
virtual bool onAsAColorMode(SkColor* color, SkBlendMode* bmode) const;
private:
/*
* Returns 1 if this is a single filter (not a composition of other filters), otherwise it
* reutrns the number of leaf-node filters in a composition. This should be the same value
* as the number of GrFragmentProcessors returned by asFragmentProcessors's array parameter.
*
* e.g. compose(filter, compose(compose(filter, filter), filter)) --> 4
*/
virtual int privateComposedFilterCount() const { return 1; }
virtual bool onAppendStages(const SkStageRec& rec, bool shaderIsOpaque) const = 0;
virtual skvm::Color onProgram(skvm::Builder*, skvm::Color,
SkColorSpace* dstCS, skvm::Uniforms*, SkArenaAlloc*) const;
friend class SkComposeColorFilter;
typedef SkFlattenable INHERITED;
};

View File

@ -99,16 +99,6 @@ SkColor4f SkColorFilter::filterColor4f(const SkColor4f& origSrcColor, SkColorSpa
///////////////////////////////////////////////////////////////////////////////////////////////////
/*
* Since colorfilters may be used on the GPU backend, and in that case we may string together
* many GrFragmentProcessors, we might exceed some internal instruction/resource limit.
*
* Since we don't yet know *what* those limits might be when we construct the final shader,
* we just set an arbitrary limit during construction. If later we find smarter ways to know what
* the limnits are, we can change this constant (or remove it).
*/
#define SK_MAX_COMPOSE_COLORFILTER_COUNT 4
class SkComposeColorFilter : public SkColorFilter {
public:
uint32_t getFlags() const override {
@ -154,23 +144,12 @@ protected:
private:
SK_FLATTENABLE_HOOKS(SkComposeColorFilter)
SkComposeColorFilter(sk_sp<SkColorFilter> outer, sk_sp<SkColorFilter> inner,
int composedFilterCount)
SkComposeColorFilter(sk_sp<SkColorFilter> outer, sk_sp<SkColorFilter> inner)
: fOuter(std::move(outer))
, fInner(std::move(inner))
, fComposedFilterCount(composedFilterCount)
{
SkASSERT(composedFilterCount >= 2);
SkASSERT(composedFilterCount <= SK_MAX_COMPOSE_COLORFILTER_COUNT);
}
int privateComposedFilterCount() const override {
return fComposedFilterCount;
}
, fInner(std::move(inner)) {}
sk_sp<SkColorFilter> fOuter;
sk_sp<SkColorFilter> fInner;
const int fComposedFilterCount;
friend class SkColorFilter;
@ -183,17 +162,12 @@ sk_sp<SkFlattenable> SkComposeColorFilter::CreateProc(SkReadBuffer& buffer) {
return outer ? outer->makeComposed(std::move(inner)) : inner;
}
sk_sp<SkColorFilter> SkColorFilter::makeComposed(sk_sp<SkColorFilter> inner) const {
if (!inner) {
return sk_ref_sp(this);
}
int count = inner->privateComposedFilterCount() + this->privateComposedFilterCount();
if (count > SK_MAX_COMPOSE_COLORFILTER_COUNT) {
return nullptr;
}
return sk_sp<SkColorFilter>(new SkComposeColorFilter(sk_ref_sp(this), std::move(inner), count));
return sk_sp<SkColorFilter>(new SkComposeColorFilter(sk_ref_sp(this), std::move(inner)));
}
///////////////////////////////////////////////////////////////////////////////////////////////////

View File

@ -33,26 +33,6 @@ static sk_sp<SkColorFilter> reincarnate_colorfilter(SkFlattenable* obj) {
///////////////////////////////////////////////////////////////////////////////
static sk_sp<SkColorFilter> make_filter() {
// pick a filter that cannot compose with itself via newComposed()
return SkColorFilters::Blend(SK_ColorRED, SkBlendMode::kColorBurn);
}
static void test_composecolorfilter_limit(skiatest::Reporter* reporter) {
// Test that CreateComposeFilter() has some finite limit (i.e. that the factory can return null)
const int way_too_many = 100;
auto parent(make_filter());
for (int i = 2; i < way_too_many; ++i) {
auto filter(make_filter());
parent = parent->makeComposed(filter);
if (nullptr == parent) {
REPORTER_ASSERT(reporter, i > 2); // we need to have succeeded at least once!
return;
}
}
REPORTER_ASSERT(reporter, false); // we never saw a nullptr :(
}
#define ILLEGAL_MODE ((SkBlendMode)-1)
DEF_TEST(ColorFilter, reporter) {
@ -109,6 +89,4 @@ DEF_TEST(ColorFilter, reporter) {
REPORTER_ASSERT(reporter, m2 == expectedMode);
}
}
test_composecolorfilter_limit(reporter);
}