Make sure SkDrawLooper objects can only be allocated on the heap.

Make sure SkDrawLooper objects can only be allocated on the heap.

Make constructors of SkLayerDrawLooper and SkBlurDrawLooper non-public.
Remove addLayer* methods from SkLayerDrawLooper. SkLayerDrawLooper::Builder is
used to create new objects.
Provide factory method for creating SkBlurDrawLooper.

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

[Dominik Grewe, 2014-04-11 09:38:26]

I didn't use guards for the addLayer* methods last time, so I missed a few uses
in Skia. They've now all been removed.

PTAL, thanks.

[reed1, 2014-04-11 12:57:10]

Derek: What is the status of removing blurdrawlooper from android entirely, and
just using layerdrawlooper instead?

https://codereview.chromium.org/232913003/diff/20001/include/effects/SkBlurDr...
File include/effects/SkBlurDrawLooper.h (right):

https://codereview.chromium.org/232913003/diff/20001/include/effects/SkBlurDr...
include/effects/SkBlurDrawLooper.h:44: SkBlurDrawLooper(SkScalar radius,
SkScalar dx, SkScalar dy, SkColor color,
How will this compile when the flag is set, if we have the same constructor in
protected?

[Dominik Grewe, 2014-04-11 12:59:21]

https://codereview.chromium.org/232913003/diff/20001/include/effects/SkBlurDr...
File include/effects/SkBlurDrawLooper.h (right):

https://codereview.chromium.org/232913003/diff/20001/include/effects/SkBlurDr...
include/effects/SkBlurDrawLooper.h:44: SkBlurDrawLooper(SkScalar radius,
SkScalar dx, SkScalar dy, SkColor color,
On 2014/04/11 12:57:10, reed1 wrote:
> How will this compile when the flag is set, if we have the same constructor in
> protected?

Without the flag we don't have that constructor any more (the protected one
takes different params). Looked like it was deprecated and wasn't used any more.
If that's not the case, I'll change it back.

[djsollen, 2014-04-11 13:44:43]

I had no plans to remove BlurDrawLooper in Android?  Are you planning on
removing the ::Create function for that class as well?

[Dominik Grewe, 2014-04-11 13:50:46]

On 2014/04/11 13:44:43, djsollen wrote:
> I had no plans to remove BlurDrawLooper in Android?  Are you planning on
> removing the ::Create function for that class as well?

The Create function is going to stay. It's only the constructor taking the
radius that I wanted to remove, because it seems to be deprecated (although this
was commented out so I'm not 100% sure).

Could you (or someone else) change the Android code to use the Create(color,
sigma, ...) method instead? You can use SkBlurMask::ConvertRadiusToSigma().

[djsollen, 2014-04-11 13:57:17]

On 2014/04/11 13:50:46, Dominik Grewe wrote:
> On 2014/04/11 13:44:43, djsollen wrote:
> > I had no plans to remove BlurDrawLooper in Android?  Are you planning on
> > removing the ::Create function for that class as well?
> 
> The Create function is going to stay. It's only the constructor taking the
> radius that I wanted to remove, because it seems to be deprecated (although
this
> was commented out so I'm not 100% sure).
> 
> Could you (or someone else) change the Android code to use the Create(color,
> sigma, ...) method instead? You can use SkBlurMask::ConvertRadiusToSigma().

Yes, I can and that is in fact one of the changes I'm trying to get in Android
now.  Can we leave in the radius constructor (even behind a flag) until I can
get Android switched to using sigma?

[Dominik Grewe, 2014-04-11 14:03:01]

On 2014/04/11 13:57:17, djsollen wrote:
> On 2014/04/11 13:50:46, Dominik Grewe wrote:
> > On 2014/04/11 13:44:43, djsollen wrote:
> > > I had no plans to remove BlurDrawLooper in Android?  Are you planning on
> > > removing the ::Create function for that class as well?
> > 
> > The Create function is going to stay. It's only the constructor taking the
> > radius that I wanted to remove, because it seems to be deprecated (although
> this
> > was commented out so I'm not 100% sure).
> > 
> > Could you (or someone else) change the Android code to use the Create(color,
> > sigma, ...) method instead? You can use SkBlurMask::ConvertRadiusToSigma().
> 
> Yes, I can and that is in fact one of the changes I'm trying to get in Android
> now.  Can we leave in the radius constructor (even behind a flag) until I can
> get Android switched to using sigma?

Sure. I was going to leave the constructor behind the flag for now (as it is in
the CL) and then remove it later when all uses have been removed.

[scroggo, 2014-04-11 14:31:03]

On 2014/04/11 12:57:10, reed1 wrote:
> Derek: What is the status of removing blurdrawlooper from android entirely,
and
> just using layerdrawlooper instead?

What is the motivation for removing blurdrawlooper? It seems like a client can
accomplish the same thing with a layerdrawlooper, but it takes a bit more work.

https://codereview.chromium.org/232913003/diff/20001/include/effects/SkLayerD...
File include/effects/SkLayerDrawLooper.h (left):

https://codereview.chromium.org/232913003/diff/20001/include/effects/SkLayerD...
include/effects/SkLayerDrawLooper.h:82: SkPaint* addLayer(const LayerInfo&);
Should these go behind a flag until we update Android as well?

https://codereview.chromium.org/232913003/diff/20001/src/effects/SkLayerDrawL...
File src/effects/SkLayerDrawLooper.cpp (left):

https://codereview.chromium.org/232913003/diff/20001/src/effects/SkLayerDrawL...
src/effects/SkLayerDrawLooper.cpp:39: SkPaint* SkLayerDrawLooper::addLayer(const
LayerInfo& info) {
Same with the definitions/

[Dominik Grewe, 2014-04-11 17:48:04]

https://codereview.chromium.org/232913003/diff/20001/include/effects/SkLayerD...
File include/effects/SkLayerDrawLooper.h (left):

https://codereview.chromium.org/232913003/diff/20001/include/effects/SkLayerD...
include/effects/SkLayerDrawLooper.h:82: SkPaint* addLayer(const LayerInfo&);
On 2014/04/11 14:31:03, scroggo wrote:
> Should these go behind a flag until we update Android as well?

I don't see any calls to these functions in Android, but maybe I'm not looking
in all the right places.

[scroggo, 2014-04-11 17:56:07]

https://codereview.chromium.org/232913003/diff/20001/include/effects/SkLayerD...
File include/effects/SkLayerDrawLooper.h (left):

https://codereview.chromium.org/232913003/diff/20001/include/effects/SkLayerD...
include/effects/SkLayerDrawLooper.h:82: SkPaint* addLayer(const LayerInfo&);
On 2014/04/11 17:48:05, Dominik Grewe wrote:
> On 2014/04/11 14:31:03, scroggo wrote:
> > Should these go behind a flag until we update Android as well?
> 
> I don't see any calls to these functions in Android, but maybe I'm not looking
> in all the right places.

Oh no, I think you're correct. Nvm

[Dominik Grewe, 2014-04-15 12:29:17]

Any more comments?

[scroggo, 2014-04-15 13:10:53]

On 2014/04/15 12:29:17, Dominik Grewe wrote:
> Any more comments?

lgtm

[reed1, 2014-04-15 13:47:52]

lgtm

[Dominik Grewe, 2014-04-15 15:20:49]

The CQ bit was checked by dominikg@chromium.org

[commit-bot: I haz the power, 2014-04-15 15:21:08]

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

[commit-bot: I haz the power, 2014-04-15 15:48:43]

Change committed as 14200