Don't create a GXPFactory when blend is SrcOver

Don't create a GXPFactory when blend is SrcOver

BUG=skia:

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

[egdaniel, 2015-11-23 20:10:15]

egdaniel@google.com changed reviewers:
+ bsalomon@google.com, robertphillips@google.com

[egdaniel, 2015-11-23 20:10:15]

This is an attempt to remove the cost of creating SrcOver XPFactories that we
have seen on profiling. We just keep the XPFactory as a nullptr when we want
SrcOver.

https://codereview.chromium.org/1471053002/diff/40001/include/gpu/effects/GrP...
File include/gpu/effects/GrPorterDuffXferProcessor.h (right):

https://codereview.chromium.org/1471053002/diff/40001/include/gpu/effects/GrP...
include/gpu/effects/GrPorterDuffXferProcessor.h:29: if (SkXfermode::kSrc_Mode ==
mode) {
Since Src is our second most common blend mode, and we explicitly creating a
SrcMode XP in multiple spots throughout the code, I figured we could do a little
to try and speed it up.

This is totally optional and happy to remove it if we don't want it.

[bsalomon, 2015-11-23 20:19:52]

https://codereview.chromium.org/1471053002/diff/40001/include/gpu/effects/GrP...
File include/gpu/effects/GrPorterDuffXferProcessor.h (right):

https://codereview.chromium.org/1471053002/diff/40001/include/gpu/effects/GrP...
include/gpu/effects/GrPorterDuffXferProcessor.h:29: if (SkXfermode::kSrc_Mode ==
mode) {
On 2015/11/23 20:10:15, egdaniel wrote:
> Since Src is our second most common blend mode, and we explicitly creating a
> SrcMode XP in multiple spots throughout the code, I figured we could do a
little
> to try and speed it up.
> 
> This is totally optional and happy to remove it if we don't want it.

second most common in real content? My instinct is to remove it unless src is
really > ~10% of draws since this whole thing a very micro optimization.

https://codereview.chromium.org/1471053002/diff/40001/include/gpu/effects/GrP...
include/gpu/effects/GrPorterDuffXferProcessor.h:62: GrXferProcessor*
GrCreateSrcOverXferProcessor(const GrCaps& caps,
Should this be GrPorterDuffXPFactory::CreateSrcOverXferProcessor?

https://codereview.chromium.org/1471053002/diff/40001/src/core/SkXfermode.cpp
File src/core/SkXfermode.cpp (right):

https://codereview.chromium.org/1471053002/diff/40001/src/core/SkXfermode.cpp...
src/core/SkXfermode.cpp:667: bool SkXfermode::AsXPFactory(SkXfermode* xfermode,
GrXPFactory** xpf) {
should we inline the top part of this function in SkXfermode.h?

https://codereview.chromium.org/1471053002/diff/40001/src/gpu/GrPipeline.cpp
File src/gpu/GrPipeline.cpp (right):

https://codereview.chromium.org/1471053002/diff/40001/src/gpu/GrPipeline.cpp#...
src/gpu/GrPipeline.cpp:25: GrXferProcessor* xferProcessor;
Why not

SkAutoTUnref xferProcessor;
if (xpFactory) {
  xferProcessor.reset(...);
} else {
  xferProcessor.reset(...);
}

?

Not a big deal, just curious.

[egdaniel, 2015-11-23 20:49:04]

https://codereview.chromium.org/1471053002/diff/40001/include/gpu/effects/GrP...
File include/gpu/effects/GrPorterDuffXferProcessor.h (right):

https://codereview.chromium.org/1471053002/diff/40001/include/gpu/effects/GrP...
include/gpu/effects/GrPorterDuffXferProcessor.h:29: if (SkXfermode::kSrc_Mode ==
mode) {
On 2015/11/23 20:19:51, bsalomon wrote:
> On 2015/11/23 20:10:15, egdaniel wrote:
> > Since Src is our second most common blend mode, and we explicitly creating a
> > SrcMode XP in multiple spots throughout the code, I figured we could do a
> little
> > to try and speed it up.
> > 
> > This is totally optional and happy to remove it if we don't want it.
> 
> second most common in real content? My instinct is to remove it unless src is
> really > ~10% of draws since this whole thing a very micro optimization.

removed

https://codereview.chromium.org/1471053002/diff/40001/include/gpu/effects/GrP...
include/gpu/effects/GrPorterDuffXferProcessor.h:62: GrXferProcessor*
GrCreateSrcOverXferProcessor(const GrCaps& caps,
On 2015/11/23 20:19:51, bsalomon wrote:
> Should this be GrPorterDuffXPFactory::CreateSrcOverXferProcessor?

Done.

https://codereview.chromium.org/1471053002/diff/40001/src/core/SkXfermode.cpp
File src/core/SkXfermode.cpp (right):

https://codereview.chromium.org/1471053002/diff/40001/src/core/SkXfermode.cpp...
src/core/SkXfermode.cpp:667: bool SkXfermode::AsXPFactory(SkXfermode* xfermode,
GrXPFactory** xpf) {
On 2015/11/23 20:19:52, bsalomon wrote:
> should we inline the top part of this function in SkXfermode.h?

Done.

https://codereview.chromium.org/1471053002/diff/40001/src/gpu/GrPipeline.cpp
File src/gpu/GrPipeline.cpp (right):

https://codereview.chromium.org/1471053002/diff/40001/src/gpu/GrPipeline.cpp#...
src/gpu/GrPipeline.cpp:25: GrXferProcessor* xferProcessor;
On 2015/11/23 20:19:52, bsalomon wrote:
> Why not
> 
> SkAutoTUnref xferProcessor;
> if (xpFactory) {
>   xferProcessor.reset(...);
> } else {
>   xferProcessor.reset(...);
> }
> 
> ?
> 
> Not a big deal, just curious.

Done.

[bsalomon, 2015-11-23 20:58:38]

lgtm

[egdaniel, 2015-11-23 20:59:45]

The CQ bit was checked by egdaniel@google.com

[commit-bot: I haz the power, 2015-11-23 20:59:53]

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

[commit-bot: I haz the power, 2015-11-23 21:00:29]

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

[commit-bot: I haz the power, 2015-11-23 21:00:30]

Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[egdaniel, 2015-11-23 21:07:14]

The CQ bit was checked by egdaniel@google.com

[egdaniel, 2015-11-23 21:07:15]

The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/1471053002/#ps100001
(title: "fix compile")

[commit-bot: I haz the power, 2015-11-23 21:07:24]

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

[commit-bot: I haz the power, 2015-11-23 21:20:44]

Committed patchset #6 (id:100001) as
https://skia.googlesource.com/skia/+/c4b72720e75313079212e69e46a5ef7c474b2305