add src-rect-constraint to drawImageRect

add src-rect-constraint to drawImageRect

Follow-on work
- unify around SrcRectConstraint (i.e. drawBitmapRect)
- remove silly drawBitmapRectToRect alias
- clean-up (possibly remove) alias problems around drawBitmapRect + IRect parameter

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/a5517e2b190a8083b38964972b031c13e99f1012

[reed1, 2015-07-09 20:54:33]

WIP -- I had having to stage changes to canvas virtuals

[reed1, 2015-07-09 20:57:47]

reed@google.com changed reviewers:
+ fmalita@chromium.org, robertphillips@google.com

[reed1, 2015-07-09 20:57:48]

still need to do more picture-serialize and piperead work

[f(malita), 2015-07-09 21:28:36]

https://codereview.chromium.org/1228083004/diff/20001/include/core/SkCanvas.h
File include/core/SkCanvas.h (right):

https://codereview.chromium.org/1228083004/diff/20001/include/core/SkCanvas.h...
include/core/SkCanvas.h:46: #define SRC_RECT_CONSTRAINT_ARG(arg)        , arg
For some reason I recall trying this same approach a while back (saveFlags?),
and finding that commas are not valid define substitutions.  Does this work on
all platforms?  I guess we'll find out soon enough :)

https://codereview.chromium.org/1228083004/diff/20001/include/core/SkCanvas.h...
include/core/SkCanvas.h:819: };
Awesome documentation!

https://codereview.chromium.org/1228083004/diff/20001/include/core/SkCanvas.h...
include/core/SkCanvas.h:834: SrcRectConstraint constraint =
kStrict_SrcRectConstraint);
I don't feel strongly, but aren't multiple default param values discouraged?

https://codereview.chromium.org/1228083004/diff/20001/include/core/SkCanvas.h...
include/core/SkCanvas.h:906: DrawBitmapRectFlags flags =
kNone_DrawBitmapRectFlag);
Are we going to switch this to the new enum at some point?  Or just deprecate
this variant?

[reed1, 2015-07-10 15:07:50]

https://codereview.chromium.org/1228083004/diff/20001/include/core/SkCanvas.h
File include/core/SkCanvas.h (right):

https://codereview.chromium.org/1228083004/diff/20001/include/core/SkCanvas.h...
include/core/SkCanvas.h:834: SrcRectConstraint constraint =
kStrict_SrcRectConstraint);
On 2015/07/09 21:28:36, f(malita) wrote:
> I don't feel strongly, but aren't multiple default param values discouraged?

Good point. Fixed.

https://codereview.chromium.org/1228083004/diff/20001/include/core/SkCanvas.h...
include/core/SkCanvas.h:906: DrawBitmapRectFlags flags =
kNone_DrawBitmapRectFlag);
On 2015/07/09 21:28:36, f(malita) wrote:
> Are we going to switch this to the new enum at some point?  Or just deprecate
> this variant?

Plan to switch, as I now expose a version of drawBitmapRect that takes the
constraint. However, removing flags is complicated due to blink/chrome, so I
will do that in a follow-on CL.

Note: SkDevice has already been converted over to SrcRectConstraint

[reed1, 2015-07-10 15:09:01]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-07-10 15:09:10]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1228083004/40001

[reed1, 2015-07-10 15:13:11]

The CQ bit was unchecked by reed@google.com

[reed1, 2015-07-10 15:43:35]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-07-10 15:43:46]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1228083004/60001

[robertphillips, 2015-07-10 15:44:34]

lgtm

https://codereview.chromium.org/1228083004/diff/40001/include/core/SkBitmapDe...
File include/core/SkBitmapDevice.h (right):

https://codereview.chromium.org/1228083004/diff/40001/include/core/SkBitmapDe...
include/core/SkBitmapDevice.h:83: */
Does this need any staging for Chrome ?

[reed1, 2015-07-10 15:47:24]

On 2015/07/10 15:44:34, robertphillips wrote:
> lgtm
> 
>
https://codereview.chromium.org/1228083004/diff/40001/include/core/SkBitmapDe...
> File include/core/SkBitmapDevice.h (right):
> 
>
https://codereview.chromium.org/1228083004/diff/40001/include/core/SkBitmapDe...
> include/core/SkBitmapDevice.h:83: */
> Does this need any staging for Chrome ?

I *think* chrome no longer subclasses device (or at least I didn't see an
override on drawBitmapRect). I am building locally to confirm.

[reed1, 2015-07-10 15:47:30]

The CQ bit was unchecked by reed@google.com

[f(malita), 2015-07-10 16:46:32]

On 2015/07/10 15:47:24, reed1 wrote:
> On 2015/07/10 15:44:34, robertphillips wrote:
> > lgtm
> > 
> >
>
https://codereview.chromium.org/1228083004/diff/40001/include/core/SkBitmapDe...
> > File include/core/SkBitmapDevice.h (right):
> > 
> >
>
https://codereview.chromium.org/1228083004/diff/40001/include/core/SkBitmapDe...
> > include/core/SkBitmapDevice.h:83: */
> > Does this need any staging for Chrome ?
> 
> I *think* chrome no longer subclasses device (or at least I didn't see an
> override on drawBitmapRect). I am building locally to confirm.

GatherPixelRefDevice still does, but looks like you've addressed that in your
last patch.

LGTM % whatever-is-triggering-SkPictureRecord-asserts
(http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[reed1, 2015-07-10 18:02:51]

The CQ bit was checked by reed@google.com to run a CQ dry run

[reed1, 2015-07-10 18:02:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertphillips@google.com, fmalita@chromium.org
Link to the patchset: https://codereview.chromium.org/1228083004/#ps100001
(title: "update canvas virtual as well")

[commit-bot: I haz the power, 2015-07-10 18:03:07]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1228083004/100001

[reed1, 2015-07-10 18:05:30]

The CQ bit was unchecked by reed@google.com

[reed1, 2015-07-10 18:28:31]

The CQ bit was checked by reed@google.com to run a CQ dry run

[reed1, 2015-07-10 18:28:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
fmalita@chromium.org, robertphillips@google.com
Link to the patchset: https://codereview.chromium.org/1228083004/#ps110001
(title: " ")

[commit-bot: I haz the power, 2015-07-10 18:28:43]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1228083004/110001

[reed1, 2015-07-10 18:29:21]

The CQ bit was unchecked by reed@google.com

[reed1, 2015-07-10 20:27:34]

Will wait until Monday/Tuesday (after the Chrome branch) to try to land this.

[reed1, 2015-07-14 14:17:59]

The CQ bit was checked by reed@google.com

[reed1, 2015-07-14 14:18:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
fmalita@chromium.org, robertphillips@google.com
Link to the patchset: https://codereview.chromium.org/1228083004/#ps130001
(title: "rebase")

[commit-bot: I haz the power, 2015-07-14 14:18:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1228083004/130001

[commit-bot: I haz the power, 2015-07-14 17:54:17]

Committed patchset #8 (id:130001) as
https://skia.googlesource.com/skia/+/a5517e2b190a8083b38964972b031c13e99f1012

[scroggo, 2015-07-14 20:59:42]

On 2015/07/14 17:54:17, commit-bot: I haz the power wrote:
> Committed patchset #8 (id:130001) as
> https://skia.googlesource.com/skia/+/a5517e2b190a8083b38964972b031c13e99f1012

FYI: This broke the Android build. You forgot to add the define when building
for Android.

I have added it here: https://codereview.chromium.org/1237903002/