XPFactory lazily initializie in drawstate / GrPaint

XPFactory lazily initializie in drawstate / GrPaint

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/2fdeda04988867625bb9bc2f7f6037406a0a98ec

[joshualitt, 2015-01-21 22:18:42]

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

[joshualitt, 2015-01-21 22:18:53]

joshualitt@google.com changed reviewers:
+ joshualitt@google.com

[joshualitt, 2015-01-21 22:18:53]

ptal

[egdaniel, 2015-01-21 22:31:36]

https://codereview.chromium.org/866573002/diff/20001/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (left):

https://codereview.chromium.org/866573002/diff/20001/src/gpu/GrContext.cpp#ol...
src/gpu/GrContext.cpp:1572: if (paint) {
Do we want to support the case where we have a DrawState but no paint? In
practice I don't think we ever hit this case, but should it be supported?

The confusing thing is the comment for this function says "he paint can be NULL
in which case the draw state is left unmodified". The old code clearly is not
respecting this comment since it resets the draw state if there is no paint.

https://codereview.chromium.org/866573002/diff/20001/src/gpu/GrContext.cpp#ol...
src/gpu/GrContext.cpp:1575: #if GR_DEBUG_PARTIAL_COVERAGE_CHECK
what is this debug check for? is it still useful to sit around in our code?

[bsalomon, 2015-01-21 23:03:06]

https://codereview.chromium.org/866573002/diff/20001/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (left):

https://codereview.chromium.org/866573002/diff/20001/src/gpu/GrContext.cpp#ol...
src/gpu/GrContext.cpp:1572: if (paint) {
On 2015/01/21 22:31:36, egdaniel wrote:
> Do we want to support the case where we have a DrawState but no paint? In
> practice I don't think we ever hit this case, but should it be supported?
> 
> The confusing thing is the comment for this function says "he paint can be
NULL
> in which case the draw state is left unmodified". The old code clearly is not
> respecting this comment since it resets the draw state if there is no paint.

If it's not being used, then let's just update the docs.

https://codereview.chromium.org/866573002/diff/20001/src/gpu/GrContext.cpp#ol...
src/gpu/GrContext.cpp:1575: #if GR_DEBUG_PARTIAL_COVERAGE_CHECK
On 2015/01/21 22:31:36, egdaniel wrote:
> what is this debug check for? is it still useful to sit around in our code?

Since we're on the path to deleting canUFCPP, ok to remove

https://codereview.chromium.org/866573002/diff/20001/src/gpu/GrDrawState.cpp
File src/gpu/GrDrawState.cpp (right):

https://codereview.chromium.org/866573002/diff/20001/src/gpu/GrDrawState.cpp#...
src/gpu/GrDrawState.cpp:17:
//////////////////////////////////////////////////////////////////////////////
remove?

https://codereview.chromium.org/866573002/diff/20001/src/gpu/GrDrawState.cpp#...
src/gpu/GrDrawState.cpp:30: SkASSERT(0 == fBlockEffectRemovalCnt);
no need to assert, we just set it

[joshualitt, 2015-01-21 23:31:14]

On 2015/01/21 23:03:06, bsalomon wrote:
> https://codereview.chromium.org/866573002/diff/20001/src/gpu/GrContext.cpp
> File src/gpu/GrContext.cpp (left):
> 
>
https://codereview.chromium.org/866573002/diff/20001/src/gpu/GrContext.cpp#ol...
> src/gpu/GrContext.cpp:1572: if (paint) {
> On 2015/01/21 22:31:36, egdaniel wrote:
> > Do we want to support the case where we have a DrawState but no paint? In
> > practice I don't think we ever hit this case, but should it be supported?
> > 
> > The confusing thing is the comment for this function says "he paint can be
> NULL
> > in which case the draw state is left unmodified". The old code clearly is
not
> > respecting this comment since it resets the draw state if there is no paint.
> 
> If it's not being used, then let's just update the docs.
> 
>
https://codereview.chromium.org/866573002/diff/20001/src/gpu/GrContext.cpp#ol...
> src/gpu/GrContext.cpp:1575: #if GR_DEBUG_PARTIAL_COVERAGE_CHECK
> On 2015/01/21 22:31:36, egdaniel wrote:
> > what is this debug check for? is it still useful to sit around in our code?
> 
> Since we're on the path to deleting canUFCPP, ok to remove
> 
> https://codereview.chromium.org/866573002/diff/20001/src/gpu/GrDrawState.cpp
> File src/gpu/GrDrawState.cpp (right):
> 
>
https://codereview.chromium.org/866573002/diff/20001/src/gpu/GrDrawState.cpp#...
> src/gpu/GrDrawState.cpp:17:
> //////////////////////////////////////////////////////////////////////////////
> remove?
> 
>
https://codereview.chromium.org/866573002/diff/20001/src/gpu/GrDrawState.cpp#...
> src/gpu/GrDrawState.cpp:30: SkASSERT(0 == fBlockEffectRemovalCnt);
> no need to assert, we just set it

Feedback incorporated

[egdaniel, 2015-01-22 03:12:25]

lgtm

[bsalomon, 2015-01-22 13:43:02]

lgtm

[joshualitt, 2015-01-22 14:17:21]

The CQ bit was checked by joshualitt@google.com

[commit-bot: I haz the power, 2015-01-22 14:18:27]

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

[commit-bot: I haz the power, 2015-01-22 14:20:17]

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

[commit-bot: I haz the power, 2015-01-22 14:20:18]

Try jobs failed on following builders:
  Build-Mac10.8-Clang-x86_64-Release-Trybot on client.skia.compile
(http://build.chromium.org/p/client.skia.compile/builders/Build-Mac10.8-Clang-...)
  Build-Ubuntu13.10-GCC4.8-x86_64-Release-Trybot on client.skia.compile
(http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu13.10-GC...)

[joshualitt, 2015-01-22 15:03:05]

The CQ bit was checked by joshualitt@google.com

[commit-bot: I haz the power, 2015-01-22 15:03:37]

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

[commit-bot: I haz the power, 2015-01-22 15:11:48]

Committed patchset #4 (id:60001) as
https://skia.googlesource.com/skia/+/2fdeda04988867625bb9bc2f7f6037406a0a98ec