Fix merge crop rect computation.

Fix SkMergeImageFilter crop rect computation.

The crop rect should be applied to the union of the input bounds, not
to the src input's bounds. These are often the same, since the saveLayer
offscreen size is computed as the union of all the required bounds, but
is not correct if the input primitive is not used (e.g., if all inputs
are connected to SkImageSources). But this will change as we
move to more accurate intermediate bounds computations (getting rid of the join() hacks as described in skbug.com/3194).

Since we can't know this without actually processing the inputs,
split SkMergeImageFilter processing into:

- filter all inputs
- applyCropRect to the union'ed bounds
- allocate the destination
- do the merge

BUG=3194
Committed: https://skia.googlesource.com/skia/+/4a24398391536f7b176b44a8c45dc288655b4adb

[Stephen White, 2015-11-24 19:47:45]

Description was changed from

==========
Fix merge crop rect computation.

The crop rect should be applied to the union of the input bounds, not
to the src input's bounds. These are often the same, since the saveLayer
offscreen size is computed as the union of all the required bounds, but
is not correct if the input primitive is not used (e.g., if all inputs
are connected to SkImageSources).

BUG=3194
==========

to

==========
Fix merge crop rect computation.

The crop rect should be applied to the union of the input bounds, not
to the src input's bounds. These are often the same, since the saveLayer
offscreen size is computed as the union of all the required bounds, but
is not correct if the input primitive is not used (e.g., if all inputs
are connected to SkImageSources). But this will change as we
move to more accurate intermediate bounds computations (getting rid of the
join() hacks as described in skbug.com/3194).

Since we can't know this without actually processing the inputs,
split SkMergeImageFilter processing into:

- filter all inputs
- applyCropRect to the union'ed bounds
- allocate the destination
- do the merge

BUG=3194
==========

[Stephen White, 2015-11-24 19:47:45]

senorblanco@chromium.org changed reviewers:
+ robertphillips@google.com

[Stephen White, 2015-11-24 19:48:06]

Description was changed from

==========
Fix merge crop rect computation.

The crop rect should be applied to the union of the input bounds, not
to the src input's bounds. These are often the same, since the saveLayer
offscreen size is computed as the union of all the required bounds, but
is not correct if the input primitive is not used (e.g., if all inputs
are connected to SkImageSources). But this will change as we
move to more accurate intermediate bounds computations (getting rid of the
join() hacks as described in skbug.com/3194).

Since we can't know this without actually processing the inputs,
split SkMergeImageFilter processing into:

- filter all inputs
- applyCropRect to the union'ed bounds
- allocate the destination
- do the merge

BUG=3194
==========

to

==========
Fix SkMergeImageFilter crop rect computation.

The crop rect should be applied to the union of the input bounds, not
to the src input's bounds. These are often the same, since the saveLayer
offscreen size is computed as the union of all the required bounds, but
is not correct if the input primitive is not used (e.g., if all inputs
are connected to SkImageSources). But this will change as we
move to more accurate intermediate bounds computations (getting rid of the
join() hacks as described in skbug.com/3194).

Since we can't know this without actually processing the inputs,
split SkMergeImageFilter processing into:

- filter all inputs
- applyCropRect to the union'ed bounds
- allocate the destination
- do the merge

BUG=3194
==========

[Stephen White, 2015-11-24 19:53:23]

Rob: PTAL. Thanks!

[robertphillips, 2015-11-24 20:58:16]

https://codereview.chromium.org/1475793002/diff/40001/src/effects/SkMergeImag...
File src/effects/SkMergeImageFilter.cpp (right):

https://codereview.chromium.org/1475793002/diff/40001/src/effects/SkMergeImag...
src/effects/SkMergeImageFilter.cpp:66: 
this->countInputs() ?

// Basically move the CL's description in here ?

https://codereview.chromium.org/1475793002/diff/40001/src/effects/SkMergeImag...
src/effects/SkMergeImageFilter.cpp:105: for (int i = 0; i < inputCount; ++i) {
if (!inputs[i]) {
   continue;
}

?

https://codereview.chromium.org/1475793002/diff/40001/src/effects/SkMergeImag...
src/effects/SkMergeImageFilter.cpp:108: } else {
Do we actually need this else clause ?

https://codereview.chromium.org/1475793002/diff/40001/src/effects/SkMergeImag...
src/effects/SkMergeImageFilter.cpp:110: }
\n somewhere ?

[Stephen White, 2015-11-24 21:15:53]

Thanks for your review; new patch up.

https://codereview.chromium.org/1475793002/diff/40001/src/effects/SkMergeImag...
File src/effects/SkMergeImageFilter.cpp (right):

https://codereview.chromium.org/1475793002/diff/40001/src/effects/SkMergeImag...
src/effects/SkMergeImageFilter.cpp:66: 
On 2015/11/24 20:58:16, robertphillips wrote:
> this->countInputs() ?

Done (here and elsewhere).

> // Basically move the CL's description in here ?

Done.

https://codereview.chromium.org/1475793002/diff/40001/src/effects/SkMergeImag...
src/effects/SkMergeImageFilter.cpp:105: for (int i = 0; i < inputCount; ++i) {
On 2015/11/24 20:58:16, robertphillips wrote:
> if (!inputs[i]) {
>    continue;
> }
> 
> ?

canvas.drawBitmap() does this as its first line; I think it's redundant here.

https://codereview.chromium.org/1475793002/diff/40001/src/effects/SkMergeImag...
src/effects/SkMergeImageFilter.cpp:108: } else {
On 2015/11/24 20:58:16, robertphillips wrote:
> Do we actually need this else clause ?

Good point. Fixed.

https://codereview.chromium.org/1475793002/diff/40001/src/effects/SkMergeImag...
src/effects/SkMergeImageFilter.cpp:110: }
On 2015/11/24 20:58:16, robertphillips wrote:
> \n somewhere ?

Done.

[Stephen White, 2015-11-24 21:22:41]

New patch up for realz this time.

[Stephen White, 2015-11-24 21:25:37]

The CQ bit was checked by senorblanco@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-24 21:25:48]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1475793002/70001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1475793002/70001

[commit-bot: I haz the power, 2015-11-24 21:38:18]

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

[commit-bot: I haz the power, 2015-11-24 21:38:19]

Dry run: This issue passed the CQ dry run.

[robertphillips, 2015-11-25 14:39:35]

lgtm

[Stephen White, 2015-11-25 15:05:58]

The CQ bit was checked by senorblanco@chromium.org

[commit-bot: I haz the power, 2015-11-25 15:06:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1475793002/70001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1475793002/70001

[commit-bot: I haz the power, 2015-11-25 15:06:58]

Committed patchset #5 (id:70001) as
https://skia.googlesource.com/skia/+/4a24398391536f7b176b44a8c45dc288655b4adb