Extend texture release on DrawState to also handle custom stages

[bsalomon, 2012-06-22 17:54:06]

On 2012/06/22 17:51:46, TomH wrote:

How about disableStages()? It's shorter and still makes sense when textures are
no longer a property of GrDrawState.

[robertphillips, 2012-06-22 17:56:37]

LGTM but wasn't there supposed to be an assert somewhere in your code to make
sure this reset was happening everywhere needed?

[bsalomon, 2012-06-22 18:00:53]

On 2012/06/22 17:56:37, robertphillips wrote:
> LGTM but wasn't there supposed to be an assert somewhere in your code to make
> sure this reset was happening everywhere needed?

We should have an assert at the top of GrContext::SetPaint() that all the stages
are disabled. But that assert is really relevant to Tom's forthcoming change to
what defines an enabled stage.

[TomH, 2012-06-22 18:14:00]

On 2012/06/22 17:54:06, bsalomon wrote:
> How about disableStages()? It's shorter and still makes sense when textures
are
> no longer a property of GrDrawState.
Done.

[bsalomon, 2012-06-22 18:15:53]

http://codereview.appspot.com/6324046/diff/1003/src/gpu/GrDrawState.h
File src/gpu/GrDrawState.h (right):

http://codereview.appspot.com/6324046/diff/1003/src/gpu/GrDrawState.h#newcode229
src/gpu/GrDrawState.h:229: class AutoTextureStageRelease : public
::GrNoncopyable {
I hate to be a pest but can we rename this guy too?

[TomH, 2012-06-22 19:12:56]

So, when I add the asserts (see current patch), they fail during gm.

shadertext, by itself, has a shader already set during text drawing drawing:
  - SkCanvas::drawText() calls SkGpuDevice::drawText(), which sets up a radial
gradient CustomStage
  - GrTextContext::AutoFinish::AutoFinish inherits it, and has a call stack that
looks like:

  GrDefaultTextContext::init
  GrContext::getTextTarget
  GrContext::prepareToDraw (category=kBuffered_DrawCategory)
  GrContext::setPaint

http://codereview.appspot.com/6324046/diff/1003/src/gpu/GrDrawState.h
File src/gpu/GrDrawState.h (right):

http://codereview.appspot.com/6324046/diff/1003/src/gpu/GrDrawState.h#newcode229
src/gpu/GrDrawState.h:229: class AutoTextureStageRelease : public
::GrNoncopyable {
On 2012/06/22 18:15:53, bsalomon wrote:
> I hate to be a pest but can we rename this guy too?

Done.

[TomH, 2012-06-22 19:24:57]

On 2012/06/22 19:12:56, TomH wrote:
> So, when I add the asserts (see current patch), they fail during gm.
> 
> shadertext, by itself, has a shader already set during text drawing drawing:
>   - SkCanvas::drawText() calls SkGpuDevice::drawText(), which sets up a radial
> gradient CustomStage
>   - GrTextContext::AutoFinish::AutoFinish inherits it, and has a call stack
that
> looks like:
> 
>   GrDefaultTextContext::init
>   GrContext::getTextTarget
>   GrContext::prepareToDraw (category=kBuffered_DrawCategory)
>   GrContext::setPaint

Both GrBatchedTextContext::init() and GrDefaultTextContext::init() call
getTextTarget(). GrDefaultTextContext claims that we need to do it a second time
because the matrix has changed?

[bsalomon, 2012-06-22 19:25:06]

Mostly LGTM.

http://codereview.appspot.com/6324046/diff/3003/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

http://codereview.appspot.com/6324046/diff/3003/src/gpu/GrContext.cpp#newcode248
src/gpu/GrContext.cpp:248: SkDebugf("setCustomStage %lu\n", morph.get());
accidentally left in?

http://codereview.appspot.com/6324046/diff/3003/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

http://codereview.appspot.com/6324046/diff/3003/src/gpu/SkGpuDevice.cpp#newco...
src/gpu/SkGpuDevice.cpp:545: SkDebugf("setCustomStage %lu\n",
sampler->getCustomStage());
more debug code?

[TomH, 2012-06-22 19:56:47]

Mostly cleaned up. Mostly PTAL?

[bsalomon, 2012-06-22 20:08:08]

Mostly LGTM

http://codereview.appspot.com/6324046/diff/9002/src/gpu/GrBatchedTextContext.cpp
File src/gpu/GrBatchedTextContext.cpp (right):

http://codereview.appspot.com/6324046/diff/9002/src/gpu/GrBatchedTextContext....
src/gpu/GrBatchedTextContext.cpp:30: //fDrawTarget =
fContext->getTextTarget(fGrPaint);
Should we just axe this line?

http://codereview.appspot.com/6324046/diff/9002/src/gpu/GrDefaultTextContext.cpp
File src/gpu/GrDefaultTextContext.cpp (right):

http://codereview.appspot.com/6324046/diff/9002/src/gpu/GrDefaultTextContext....
src/gpu/GrDefaultTextContext.cpp:134: // this has been already done in the
baseclass, but we need to repeat
           ^^^ not true anymore

[TomH, 2012-06-22 20:11:07]

Committed with r4313.

[TomH, 2012-06-26 19:53:43]

GrContext::setPaint()'s GrAssert(fDrawState->stagesDisabled()) had to be rolled
out in r4332 because it triggers in chrome.

A fairly minimal test is:
 - paint.setShader(SkGradientShader::CreateLinear(...))
 - paint.setTextSize(SkIntToScalar(500))
 - canvas->drawText(..., paint)

If either the shader OR the textSize is omitted, the assert succeeds.