apply xfermode after imagefilter when we create a tmp layer

apply xfermode after imagefilter when we create a tmp layer

BUG=skia:3741
Committed: https://skia.googlesource.com/skia/+/5c476fb2776639bdbf0e974dd38d1c5d4c4ff1aa

[reed1, 2015-04-20 12:34:26]

reed@google.com changed reviewers:
+ robertphillips@google.com, senorblanco@google.com

[reed1, 2015-04-20 12:34:27]

will write some GMs...

[reed1, 2015-04-20 14:02:58]

This adds a GM that shows the problem...
https://codereview.chromium.org/1088773004/#

This CL seems to draw this GM correctly

[Stephen White, 2015-04-20 14:36:46]

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

[Stephen White, 2015-04-20 14:36:46]

LGTM

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

https://codereview.chromium.org/1091173003/diff/1/src/core/SkCanvas.cpp#newco...
src/core/SkCanvas.cpp:322: *  is applied right before the restore, and the
xfermode is applied during the restore.
Nit: this might be slightly misleading: the imagefilter is also applied *during*
restore() (the function), although 
you're correct that it's applied before restore() draws the (filtered) layer to
the canvas.

Maybe something like: "during restore(), the imagefilter is applied, and then
the filtered result is drawn to the main canvas with the xfermode applied".

https://codereview.chromium.org/1091173003/diff/1/src/core/SkCanvas.cpp#newco...
src/core/SkCanvas.cpp:333: fTempLayerForImageFilter = true;
This name is much clearer to me; thanks for that!

[reed1, 2015-04-20 14:49:19]

ptal

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

https://codereview.chromium.org/1091173003/diff/1/src/core/SkCanvas.cpp#newco...
src/core/SkCanvas.cpp:322: *  is applied right before the restore, and the
xfermode is applied during the restore.
On 2015/04/20 14:36:46, Stephen White wrote:
> Nit: this might be slightly misleading: the imagefilter is also applied
*during*
> restore() (the function), although 
> you're correct that it's applied before restore() draws the (filtered) layer
to
> the canvas.
> 
> Maybe something like: "during restore(), the imagefilter is applied, and then
> the filtered result is drawn to the main canvas with the xfermode applied".

Done.

[Stephen White, 2015-04-20 14:55:45]

Still LGTM.

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

https://codereview.chromium.org/1091173003/diff/20001/src/core/SkCanvas.cpp#n...
src/core/SkCanvas.cpp:323: *      return (fPaint). We then draw the primitive
(using srcover) into a cleared
Out of curiosity, would it be equivalent to use Src instead of SrcOver when
drawing into a cleared buffer? From the math:

        kSrcOver_Mode,  //!< [Sa + Da - Sa*Da, Rc = Sc + (1 - Sa)*Dc]
 
if Da is zero, this is [Sa, Sc] which == kSrc_Mode. Might be a small perf win.

[reed1, 2015-04-20 14:57:39]

I should be the same, and it *should* be faster, but for the most part we have
fast-case-blits only for srcover, and often src requires a scanline
function-ptr.

That said, would be fine to try it in a branch and see (1) that it draws the
same, and (2) measure the perf diff.

[reed1, 2015-04-20 14:57:53]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2015-04-20 14:58:11]

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

[commit-bot: I haz the power, 2015-04-20 15:04:26]

Committed patchset #2 (id:20001) as
https://skia.googlesource.com/skia/+/5c476fb2776639bdbf0e974dd38d1c5d4c4ff1aa

[tomhudson, 2015-04-20 15:42:45]

This changed the output of imagefilters_xfermodes GM, and there's now a very
subtle difference between the lower left and lower right images. Mike, can you
check those results? I may have been mis-triaging.

[reed1, 2015-04-20 15:44:33]

Indeed, that was the point of the (earlier) GM, to show a bug and its
(hopefully) fix.

Please don't triage anything yet. I will take care of it after consulting with
the gpu side.