diff options
-rw-r--r-- | include/core/SkColorFilter.h | 13 | ||||
-rw-r--r-- | src/core/SkColorFilter.cpp | 40 | ||||
-rw-r--r-- | tests/ColorFilterTest.cpp | 22 |
3 files changed, 67 insertions, 8 deletions
diff --git a/include/core/SkColorFilter.h b/include/core/SkColorFilter.h index 5a25814c6b..fe151a2533 100644 --- a/include/core/SkColorFilter.h +++ b/include/core/SkColorFilter.h @@ -134,6 +134,9 @@ public: /** Construct a colorfilter whose effect is to first apply the inner filter and then apply * the outer filter to the result of the inner's. * The reference counts for outer and inner are incremented. + * + * Due to internal limits, it is possible that this will return NULL, so the caller must + * always check. */ static SkColorFilter* CreateComposeFilter(SkColorFilter* outer, SkColorFilter* inner); @@ -162,6 +165,16 @@ protected: SkColorFilter() {} 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; } + friend class SkComposeColorFilter; + typedef SkFlattenable INHERITED; }; diff --git a/src/core/SkColorFilter.cpp b/src/core/SkColorFilter.cpp index fd76dd1c9b..0a9cd93e34 100644 --- a/src/core/SkColorFilter.cpp +++ b/src/core/SkColorFilter.cpp @@ -39,13 +39,18 @@ SkColor SkColorFilter::filterColor(SkColor c) const { /////////////////////////////////////////////////////////////////////////////////////////////////// +/* + * 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: - SkComposeColorFilter(SkColorFilter* outer, SkColorFilter* inner) - : fOuter(SkRef(outer)) - , fInner(SkRef(inner)) - {} - uint32_t getFlags() const SK_OVERRIDE { // Can only claim alphaunchanged and 16bit support if both our proxys do. return fOuter->getFlags() & fInner->getFlags(); @@ -89,8 +94,22 @@ protected: } private: + SkComposeColorFilter(SkColorFilter* outer, SkColorFilter* inner, int composedFilterCount) + : fOuter(SkRef(outer)) + , fInner(SkRef(inner)) + , fComposedFilterCount(composedFilterCount) + { + SkASSERT(composedFilterCount >= 2); + SkASSERT(composedFilterCount <= SK_MAX_COMPOSE_COLORFILTER_COUNT); + } + + int privateComposedFilterCount() const SK_OVERRIDE { + return fComposedFilterCount; + } + SkAutoTUnref<SkColorFilter> fOuter; SkAutoTUnref<SkColorFilter> fInner; + const int fComposedFilterCount; friend class SkColorFilter; @@ -115,10 +134,15 @@ SkColorFilter* SkColorFilter::CreateComposeFilter(SkColorFilter* outer, SkColorF // Give the subclass a shot at a more optimal composition... SkColorFilter* composition = outer->newComposed(inner); - if (NULL == composition) { - composition = SkNEW_ARGS(SkComposeColorFilter, (outer, inner)); + if (composition) { + return composition; + } + + int count = inner->privateComposedFilterCount() + outer->privateComposedFilterCount(); + if (count > SK_MAX_COMPOSE_COLORFILTER_COUNT) { + return NULL; } - return composition; + return SkNEW_ARGS(SkComposeColorFilter, (outer, inner, count)); } SK_DEFINE_FLATTENABLE_REGISTRAR_GROUP_START(SkColorFilter) diff --git a/tests/ColorFilterTest.cpp b/tests/ColorFilterTest.cpp index f3f6a0a8e8..b2e37183b5 100644 --- a/tests/ColorFilterTest.cpp +++ b/tests/ColorFilterTest.cpp @@ -30,6 +30,26 @@ static SkColorFilter* reincarnate_colorfilter(SkFlattenable* obj) { /////////////////////////////////////////////////////////////////////////////// +static SkColorFilter* make_filter() { + // pick a filter that cannot compose with itself via newComposed() + return SkColorFilter::CreateModeFilter(SK_ColorRED, SkXfermode::kColorBurn_Mode); +} + +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; + SkAutoTUnref<SkColorFilter> parent(make_filter()); + for (int i = 2; i < way_too_many; ++i) { + SkAutoTUnref<SkColorFilter> filter(make_filter()); + parent.reset(SkColorFilter::CreateComposeFilter(parent, filter)); + if (NULL == parent) { + REPORTER_ASSERT(reporter, i > 2); // we need to have succeeded at least once! + return; + } + } + REPORTER_ASSERT(reporter, false); // we never saw a NULL :( +} + #define ILLEGAL_MODE ((SkXfermode::Mode)-1) DEF_TEST(ColorFilter, reporter) { @@ -89,6 +109,8 @@ DEF_TEST(ColorFilter, reporter) { REPORTER_ASSERT(reporter, m2 == expectedMode); } } + + test_composecolorfilter_limit(reporter); } /////////////////////////////////////////////////////////////////////////////// |