Do not set bounds on SkLiteRecorder

Do not set bounds on SkLiteRecorder

If this can land, we demonstrate that SkLiteRecorder does not need
to know anything about bounds.

This allows us to refactor SkLiteDL to be contained in an Android
drawable (rather than actually being an Android drawable).  Under
this design, we can mutate the bounds of the drawable without
abusing SkLiteDL.

BUG=skia:5780
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2357273003

[msarett, 2016-09-21 17:26:21]

Description was changed from

==========
Do not pass bounds to SkLiteRecorder

If this can land, we demonstrate that SkLiteRecorder does not need
to know anything about bounds.

This allows us to refactor SkLiteDL to contained in an Android
drawable (rather than actually being an Android drawable).  Under
this design, we can mutate the bounds of the drawable without
abusing SkLiteDL.

BUG=skia:
==========

to

==========
Do not pass bounds to SkLiteRecorder

If this can land, we demonstrate that SkLiteRecorder does not need
to know anything about bounds.

This allows us to refactor SkLiteDL to contained in an Android
drawable (rather than actually being an Android drawable).  Under
this design, we can mutate the bounds of the drawable without
abusing SkLiteDL.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2357273003
==========

[msarett, 2016-09-21 17:26:56]

Description was changed from

==========
Do not pass bounds to SkLiteRecorder

If this can land, we demonstrate that SkLiteRecorder does not need
to know anything about bounds.

This allows us to refactor SkLiteDL to contained in an Android
drawable (rather than actually being an Android drawable).  Under
this design, we can mutate the bounds of the drawable without
abusing SkLiteDL.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2357273003
==========

to

==========
Do not pass bounds to SkLiteRecorder

If this can land, we demonstrate that SkLiteRecorder does not need
to know anything about bounds.

This allows us to refactor SkLiteDL to contained in an Android
drawable (rather than actually being an Android drawable).  Under
this design, we can mutate the bounds of the drawable without
abusing SkLiteDL.

BUG=skia:5780
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2357273003
==========

[msarett, 2016-09-21 17:27:10]

Description was changed from

==========
Do not pass bounds to SkLiteRecorder

If this can land, we demonstrate that SkLiteRecorder does not need
to know anything about bounds.

This allows us to refactor SkLiteDL to contained in an Android
drawable (rather than actually being an Android drawable).  Under
this design, we can mutate the bounds of the drawable without
abusing SkLiteDL.

BUG=skia:5780
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2357273003
==========

to

==========
Do not pass bounds to SkLiteRecorder

If this can land, we demonstrate that SkLiteRecorder does not need
to know anything about bounds.

This allows us to refactor SkLiteDL to be contained in an Android
drawable (rather than actually being an Android drawable).  Under
this design, we can mutate the bounds of the drawable without
abusing SkLiteDL.

BUG=skia:5780
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2357273003
==========

[msarett, 2016-09-21 17:27:53]

msarett@google.com changed reviewers:
+ mtklein@google.com, reed@google.com

[msarett, 2016-09-21 17:28:15]

Description was changed from

==========
Do not pass bounds to SkLiteRecorder

If this can land, we demonstrate that SkLiteRecorder does not need
to know anything about bounds.

This allows us to refactor SkLiteDL to be contained in an Android
drawable (rather than actually being an Android drawable).  Under
this design, we can mutate the bounds of the drawable without
abusing SkLiteDL.

BUG=skia:5780
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2357273003
==========

to

==========
Do not set bounds on SkLiteRecorder

If this can land, we demonstrate that SkLiteRecorder does not need
to know anything about bounds.

This allows us to refactor SkLiteDL to be contained in an Android
drawable (rather than actually being an Android drawable).  Under
this design, we can mutate the bounds of the drawable without
abusing SkLiteDL.

BUG=skia:5780
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2357273003
==========

[reed1, 2016-09-21 17:30:30]

canvas lgtm

https://codereview.chromium.org/2357273003/diff/1/src/core/SkCanvas.cpp
File src/core/SkCanvas.cpp (right):

https://codereview.chromium.org/2357273003/diff/1/src/core/SkCanvas.cpp#newco...
src/core/SkCanvas.cpp:2974: this->onDrawPatch(cubics, colors, texCoords, xmode,
paint);
definitely a good change -- we should not quickreject before the virtual

[msarett, 2016-09-21 17:46:09]

On 2016/09/21 17:30:30, reed1 wrote:
> canvas lgtm
> 
> https://codereview.chromium.org/2357273003/diff/1/src/core/SkCanvas.cpp
> File src/core/SkCanvas.cpp (right):
> 
>
https://codereview.chromium.org/2357273003/diff/1/src/core/SkCanvas.cpp#newco...
> src/core/SkCanvas.cpp:2974: this->onDrawPatch(cubics, colors, texCoords,
xmode,
> paint);
> definitely a good change -- we should not quickreject before the virtual

Will split this out and land it...  I think the other stuff is likely not a good
idea.

[mtklein_C, 2016-09-21 17:48:33]

mtklein@chromium.org changed reviewers:
+ mtklein@chromium.org

[mtklein_C, 2016-09-21 17:48:33]

If we want to land this, I'd like to document the list of SkCanvas methods that
will not work in SkLiteRecorder.h, and to override as many of them as possible
with SK_ABORT implementations to assure ourselves they're really not called.

If this is safe to land, I think that should imply it'd also be safe to remove
the SkCanvas::onClipFoo() calls back into SkCanvas in the clips.  We know those
are _not_ safe to remove, so I'm skeptical that this could be safe to land.

[mtklein_C, 2016-09-21 17:49:20]

On 2016/09/21 at 17:46:09, msarett wrote:
> On 2016/09/21 17:30:30, reed1 wrote:
> > canvas lgtm
> > 
> > https://codereview.chromium.org/2357273003/diff/1/src/core/SkCanvas.cpp
> > File src/core/SkCanvas.cpp (right):
> > 
> >
https://codereview.chromium.org/2357273003/diff/1/src/core/SkCanvas.cpp#newco...
> > src/core/SkCanvas.cpp:2974: this->onDrawPatch(cubics, colors, texCoords,
xmode,
> > paint);
> > definitely a good change -- we should not quickreject before the virtual
> 
> Will split this out and land it...  I think the other stuff is likely not a
good idea.

I don't see any major harm in landing the GM changes too, unless you want to use
them as a reminder that there are readback methods on SkCanvas.

[msarett, 2016-09-21 17:52:12]

On 2016/09/21 17:49:20, mtklein_C wrote:
> On 2016/09/21 at 17:46:09, msarett wrote:
> > On 2016/09/21 17:30:30, reed1 wrote:
> > > canvas lgtm
> > > 
> > > https://codereview.chromium.org/2357273003/diff/1/src/core/SkCanvas.cpp
> > > File src/core/SkCanvas.cpp (right):
> > > 
> > >
>
https://codereview.chromium.org/2357273003/diff/1/src/core/SkCanvas.cpp#newco...
> > > src/core/SkCanvas.cpp:2974: this->onDrawPatch(cubics, colors, texCoords,
> xmode,
> > > paint);
> > > definitely a good change -- we should not quickreject before the virtual
> > 
> > Will split this out and land it...  I think the other stuff is likely not a
> good idea.
> 
> I don't see any major harm in landing the GM changes too, unless you want to
use
> them as a reminder that there are readback methods on SkCanvas.

I'll keep them as a reminder...  They can land if we end up circling back to
this strategy.