Added function CreateFrom2Procs() in new namespace GrXfermodeFragmentProcessor

Brian requested a new namespace for factories that create fragment processors that combine 2 things with a xfermode.  This way, we can change the implementation of these factories in the future to not use GrComposeEffect if desired.

BUG=skia:4182
Committed: https://skia.googlesource.com/skia/+/809e5afdd95962465a51e3dbab707abf9d594fab

[wangyix, 2015-09-02 15:42:50]

Patchset #2 (id:20001) has been deleted

[wangyix, 2015-09-02 15:43:14]

Patchset #1 (id:1) has been deleted

[wangyix, 2015-09-02 15:48:50]

wangyix@google.com changed reviewers:
+ bsalomon@google.com, egdaniel@google.com, joshualitt@google.com,
tomhudson@google.com

[joshualitt, 2015-09-02 17:36:09]

On 2015/09/02 15:48:50, wangyix wrote:

lgtm

[tomhudson, 2015-09-02 20:02:36]

https://codereview.chromium.org/1306163005/diff/60001/include/core/SkComposeS...
File include/core/SkComposeShader.h (right):

https://codereview.chromium.org/1306163005/diff/60001/include/core/SkComposeS...
include/core/SkComposeShader.h:93: #include "SkString.h"
Move the #includes to the top, please.

[egdaniel, 2015-09-02 20:12:44]

https://codereview.chromium.org/1306163005/diff/60001/include/core/SkComposeS...
File include/core/SkComposeShader.h (right):

https://codereview.chromium.org/1306163005/diff/60001/include/core/SkComposeS...
include/core/SkComposeShader.h:93: #include "SkString.h"
On 2015/09/02 20:02:36, tomhudson wrote:
> Move the #includes to the top, please.

So I'm not sure what our official style is, but it very common in our code for
these "SkEffect" classes which have GPU implementations in them surrounded by
#if SK_SUPPORT_GPU to have the includes needed only by the gpu down with the
code.

But I'm just saying what is done in practice, not necessarily what our style is.

[wangyix, 2015-09-02 20:13:21]

Patchset #3 (id:80001) has been deleted

[tomhudson, 2015-09-02 20:36:18]

On 2015/09/02 20:12:44, egdaniel wrote:
>
https://codereview.chromium.org/1306163005/diff/60001/include/core/SkComposeS...
> File include/core/SkComposeShader.h (right):
> 
>
https://codereview.chromium.org/1306163005/diff/60001/include/core/SkComposeS...
> include/core/SkComposeShader.h:93: #include "SkString.h"
> On 2015/09/02 20:02:36, tomhudson wrote:
> > Move the #includes to the top, please.
> 
> So I'm not sure what our official style is, but it very common in our code for
> these "SkEffect" classes which have GPU implementations in them surrounded by
> #if SK_SUPPORT_GPU to have the includes needed only by the gpu down with the
> code.
> 
> But I'm just saying what is done in practice, not necessarily what our style
is.

Looks like https://skia.org/dev/contrib/style is silent and I'm just being
ornery. Objection withdrawn, your honor.

[bsalomon, 2015-09-08 15:10:57]

https://codereview.chromium.org/1306163005/diff/60001/include/core/SkComposeS...
File include/core/SkComposeShader.h (right):

https://codereview.chromium.org/1306163005/diff/60001/include/core/SkComposeS...
include/core/SkComposeShader.h:91: #if SK_SUPPORT_GPU
I don't think we should be exposing this object via this header file (or really
any header file).

Can you GrComposeEffect to GrXfermodeFragmentProcessor.cpp? Also, can you remove
the word "effect". It's use in older processors is legacy we want to drop. Maybe
GrComposeTwoFragmentProcessor or something like that.

https://codereview.chromium.org/1306163005/diff/60001/include/gpu/effects/GrX...
File include/gpu/effects/GrXfermodeFragmentProcessor.h (right):

https://codereview.chromium.org/1306163005/diff/60001/include/gpu/effects/GrX...
include/gpu/effects/GrXfermodeFragmentProcessor.h:16: const GrFragmentProcessor*
CreateFrom2Procs(const GrFragmentProcessor* src,
Not crazy about "Procs". AFAIK it isn't a standard term we use in other code
that deals with fragment processors. I'd like to restrict adding new terminology
to cases that aren't duplicated with other terms and add value.

Ok with avoiding the full "FragmentProcessors" to keep the name shorter.
CreateBlendBetweenTwo? CreateComposeTwo?

[wangyix, 2015-09-09 17:32:35]

https://codereview.chromium.org/1306163005/diff/60001/include/core/SkComposeS...
File include/core/SkComposeShader.h (right):

https://codereview.chromium.org/1306163005/diff/60001/include/core/SkComposeS...
include/core/SkComposeShader.h:91: #if SK_SUPPORT_GPU
On 2015/09/08 15:10:57, bsalomon wrote:
> I don't think we should be exposing this object via this header file (or
really
> any header file).
> 
> Can you GrComposeEffect to GrXfermodeFragmentProcessor.cpp? Also, can you
remove
> the word "effect". It's use in older processors is legacy we want to drop.
Maybe
> GrComposeTwoFragmentProcessor or something like that.
> 

GrComposeEffect::onCreateGLInstance() needs GrGLComposeEffect's class
definition. How should I expose GrGLComposeEffect to it can be used in
GrXfermodeFragmentProcessor.cpp?

[bsalomon, 2015-09-09 17:34:56]

https://codereview.chromium.org/1306163005/diff/60001/include/core/SkComposeS...
File include/core/SkComposeShader.h (right):

https://codereview.chromium.org/1306163005/diff/60001/include/core/SkComposeS...
include/core/SkComposeShader.h:91: #if SK_SUPPORT_GPU
On 2015/09/09 17:32:35, wangyix wrote:
> On 2015/09/08 15:10:57, bsalomon wrote:
> > I don't think we should be exposing this object via this header file (or
> really
> > any header file).
> > 
> > Can you GrComposeEffect to GrXfermodeFragmentProcessor.cpp? Also, can you
> remove
> > the word "effect". It's use in older processors is legacy we want to drop.
> Maybe
> > GrComposeTwoFragmentProcessor or something like that.
> > 
> 
> GrComposeEffect::onCreateGLInstance() needs GrGLComposeEffect's class
> definition. How should I expose GrGLComposeEffect to it can be used in
> GrXfermodeFragmentProcessor.cpp?    

move it there too?

[wangyix, 2015-09-09 17:41:08]

https://codereview.chromium.org/1306163005/diff/60001/include/core/SkComposeS...
File include/core/SkComposeShader.h (right):

https://codereview.chromium.org/1306163005/diff/60001/include/core/SkComposeS...
include/core/SkComposeShader.h:91: #if SK_SUPPORT_GPU
On 2015/09/09 17:34:56, bsalomon wrote:
> On 2015/09/09 17:32:35, wangyix wrote:
> > On 2015/09/08 15:10:57, bsalomon wrote:
> > > I don't think we should be exposing this object via this header file (or
> > really
> > > any header file).
> > > 
> > > Can you GrComposeEffect to GrXfermodeFragmentProcessor.cpp? Also, can you
> > remove
> > > the word "effect". It's use in older processors is legacy we want to drop.
> > Maybe
> > > GrComposeTwoFragmentProcessor or something like that.
> > > 
> > 
> > GrComposeEffect::onCreateGLInstance() needs GrGLComposeEffect's class
> > definition. How should I expose GrGLComposeEffect to it can be used in
> > GrXfermodeFragmentProcessor.cpp?    
> 
> move it there too?

Is it weird to have GL-specific stuff in GrXfermodeFragmentProcessor.cpp?

[bsalomon, 2015-09-09 17:57:05]

On 2015/09/09 17:41:08, wangyix wrote:
>
https://codereview.chromium.org/1306163005/diff/60001/include/core/SkComposeS...
> File include/core/SkComposeShader.h (right):
> 
>
https://codereview.chromium.org/1306163005/diff/60001/include/core/SkComposeS...
> include/core/SkComposeShader.h:91: #if SK_SUPPORT_GPU
> On 2015/09/09 17:34:56, bsalomon wrote:
> > On 2015/09/09 17:32:35, wangyix wrote:
> > > On 2015/09/08 15:10:57, bsalomon wrote:
> > > > I don't think we should be exposing this object via this header file (or
> > > really
> > > > any header file).
> > > > 
> > > > Can you GrComposeEffect to GrXfermodeFragmentProcessor.cpp? Also, can
you
> > > remove
> > > > the word "effect". It's use in older processors is legacy we want to
drop.
> > > Maybe
> > > > GrComposeTwoFragmentProcessor or something like that.
> > > > 
> > > 
> > > GrComposeEffect::onCreateGLInstance() needs GrGLComposeEffect's class
> > > definition. How should I expose GrGLComposeEffect to it can be used in
> > > GrXfermodeFragmentProcessor.cpp?    
> > 
> > move it there too?
> 
> Is it weird to have GL-specific stuff in GrXfermodeFragmentProcessor.cpp?

Most processors have their GL implementation in the same cpp.

[wangyix, 2015-09-09 19:10:21]

I did Brian's changes; the only weird thing is that
SkComposeShader::asFragmentProcessor() has mode optimizations, and at the end it
calls CreateFromTwoProcessors() if the mode is nontrivial.  However,
CreateFromTwoProcessors() internally does those same mode optimizations.  I
decided to leave the mode optimizations in
SkComposeShader::asFragmentProcessor() since they can prevent unnecessary
fragment processor instances from being allocated.

[bsalomon, 2015-09-09 19:22:23]

On 2015/09/09 19:10:21, wangyix wrote:
> I did Brian's changes; the only weird thing is that
> SkComposeShader::asFragmentProcessor() has mode optimizations, and at the end
it
> calls CreateFromTwoProcessors() if the mode is nontrivial.  However,
> CreateFromTwoProcessors() internally does those same mode optimizations.  I
> decided to leave the mode optimizations in
> SkComposeShader::asFragmentProcessor() since they can prevent unnecessary
> fragment processor instances from being allocated.

I think that's ok. Maybe we could print a message in CFTP() to alert devs to
check in the calling code. Anyway, lgtm.

[wangyix, 2015-09-09 19:51:55]

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

[commit-bot: I haz the power, 2015-09-09 19:52:08]

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

[commit-bot: I haz the power, 2015-09-09 19:57:07]

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

[commit-bot: I haz the power, 2015-09-09 19:57:08]

Dry run: This issue passed the CQ dry run.

[wangyix, 2015-09-09 19:57:34]

The CQ bit was checked by wangyix@google.com

[wangyix, 2015-09-09 19:57:34]

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

[commit-bot: I haz the power, 2015-09-09 19:57:46]

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

[commit-bot: I haz the power, 2015-09-09 19:58:35]

Committed patchset #6 (id:160001) as
https://skia.googlesource.com/skia/+/809e5afdd95962465a51e3dbab707abf9d594fab