Factory methods for heap-allocated SkColorFilter objects.

Factory methods for heap-allocated SkColorFilter objects.

This is part of an effort to ensure that all SkPaint effects can only be
allocated on the heap.

This patch makes the constructors of SkColorFilter and its subclasses non-public
and instead provides factory methods for creating these objects on the heap. We
temporarily keep constructor of publicly visible classes public behind a flag.

BUG=skia:2187
Committed: http://code.google.com/p/skia/source/detail?r=13539

[Dominik Grewe, 2014-02-21 16:16:47]

I've looked at SkColorFilter this time. Apart from SkColorMatrixFilter all
subclasses were either not public or their constructors were already
protected/private.

I found one strange case though: SkTable_ColorFilter is a subclass of
SkColorFilter that lives in SkTableColorFilter.cpp. SkTableColorFilter (without
the '_') only contains factory methods to create SkTable_ColorFilter objects.
Why do we have two separate classes? Can't we just move the factory methods into
the subclass and rename it to SkTableColorFilter? Seems a lot cleaner to me.

[scroggo, 2014-02-21 16:25:21]

On 2014/02/21 16:16:47, Dominik Grewe wrote:
> I've looked at SkColorFilter this time. Apart from SkColorMatrixFilter all
> subclasses were either not public or their constructors were already
> protected/private.
> 
> I found one strange case though: SkTable_ColorFilter is a subclass of
> SkColorFilter that lives in SkTableColorFilter.cpp. SkTableColorFilter
(without
> the '_') only contains factory methods to create SkTable_ColorFilter objects.
> Why do we have two separate classes? Can't we just move the factory methods
into
> the subclass and rename it to SkTableColorFilter? Seems a lot cleaner to me.

Generally we do that to hide implementation details. I am not sure why this one
uses that convention whereas others do not.

lgtm

[reed1, 2014-02-21 17:06:23]

lgtm

[Dominik Grewe, 2014-02-21 17:21:49]

The CQ bit was checked by dominikg@chromium.org

[commit-bot: I haz the power, 2014-02-21 17:21:53]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/dominikg@chromium.org/175293002/1

[commit-bot: I haz the power, 2014-02-21 18:23:52]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/dominikg@chromium.org/175293002/1

[commit-bot: I haz the power, 2014-02-21 18:23:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-21 18:23:55]

Failed to apply patch for bench/ColorFilterBench.cpp:
While running git apply --index -p1;
  Created missing directory bench.
  error: bench/ColorFilterBench.cpp: does not exist in index

Patch:       bench/ColorFilterBench.cpp
Index: bench/ColorFilterBench.cpp
diff --git a/bench/ColorFilterBench.cpp b/bench/ColorFilterBench.cpp
index
9fec587b5c0a56bce6677592d97b32e2e7dcdb23..3d9515477c7e52b222db7c2785028d766f35f2ad
100644
--- a/bench/ColorFilterBench.cpp
+++ b/bench/ColorFilterBench.cpp
@@ -33,7 +33,7 @@ protected:
                                 0, 1, 0, 0, amount255,
                                 0, 0, 1, 0, amount255,
                                 0, 0, 0, 1, 0 };
-        SkAutoTUnref<SkColorFilter> filter(new SkColorMatrixFilter(matrix));
+        SkAutoTUnref<SkColorFilter>
filter(SkColorMatrixFilter::Create(matrix));
         return SkColorFilterImageFilter::Create(filter, input);
     }
 
@@ -44,7 +44,7 @@ protected:
         matrix[1] = matrix[6] = matrix[11] = 0.7152f;
         matrix[2] = matrix[7] = matrix[12] = 0.0722f;
         matrix[18] = 1.0f;
-        SkAutoTUnref<SkColorFilter> filter(new SkColorMatrixFilter(matrix));
+        SkAutoTUnref<SkColorFilter>
filter(SkColorMatrixFilter::Create(matrix));
         return SkColorFilterImageFilter::Create(filter, input);
     }

[mtklein, 2014-02-21 18:30:57]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-02-21 18:31:02]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/dominikg@chromium.org/175293002/1

[commit-bot: I haz the power, 2014-02-21 18:46:34]

Change committed as 13539