Part 2 of separate coverage / color

[bsalomon, 2011-05-17 14:05:16]

This actually uses the separate coverage and color functionality previously
added. Glyph masks, super-sample masks, and mask filters are now part of the
"coverage" phase of the frag shader.

[reed1, 2011-05-17 15:11:01]

LGTM + nits

http://codereview.appspot.com/4548048/diff/1/gpu/include/GrContext.h
File gpu/include/GrContext.h (right):

http://codereview.appspot.com/4548048/diff/1/gpu/include/GrContext.h#newcode606
gpu/include/GrContext.h:606: const GrPaint& paint,
const array

http://codereview.appspot.com/4548048/diff/1/gpu/include/GrContext_impl.h
File gpu/include/GrContext_impl.h (right):

http://codereview.appspot.com/4548048/diff/1/gpu/include/GrContext_impl.h#new...
gpu/include/GrContext_impl.h:47: bool hasTexCoords[GrPaint::kTotalStages] = {
const bool ...

http://codereview.appspot.com/4548048/diff/1/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (left):

http://codereview.appspot.com/4548048/diff/1/src/gpu/SkGpuDevice.cpp#oldcode319
src/gpu/SkGpuDevice.cpp:319: paint->setTexture(fTexture);
enum for the 0 ?

[junov, 2011-05-17 15:39:22]

http://codereview.appspot.com/4548048/diff/1/gpu/include/GrPaint.h
File gpu/include/GrPaint.h (right):

http://codereview.appspot.com/4548048/diff/1/gpu/include/GrPaint.h#newcode37
gpu/include/GrPaint.h:37: kNumMaskStages    = 1,
How about having kFirstTextureStage = 0 and kFirstMaskStage = kNumTextureStages?

http://codereview.appspot.com/4548048/diff/1/gpu/include/GrPaint.h#newcode209
gpu/include/GrPaint.h:209: kMaskStageShift = kNumTextureStages
This enum would no longer be needed (see comment at line 37)

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

http://codereview.appspot.com/4548048/diff/1/gpu/src/GrContext.cpp#newcode1036
gpu/src/GrContext.cpp:1036: if (NULL == paint.getTexture(0)) {
0 => kFirstTextureStage

http://codereview.appspot.com/4548048/diff/1/gpu/src/GrContext.cpp#newcode1338
gpu/src/GrContext.cpp:1338: int s = GrPaint::kNumTextureStages + i;
kNumTextureStages => kFirstMaskStage

[bsalomon, 2011-05-17 17:44:03]

The latest patch adds kFirstTextureState and kFirstMaskStage. Also renamed the
kNum...Stages values.

[junov, 2011-05-17 18:21:14]

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

http://codereview.appspot.com/4548048/diff/9002/src/gpu/SkGpuDevice.cpp#newco...
src/gpu/SkGpuDevice.cpp:906: GrSamplerState* sampler =
grPaint.getTextureSampler(0);
0 => kFirstTextureStage?
5x in this file

[bsalomon, 2011-05-17 18:56:12]

On 2011/05/17 18:21:14, junov wrote:
http://codereview.appspot.com/4548048/diff/9002/src/gpu/SkGpuDevice.cpp#newco...
> src/gpu/SkGpuDevice.cpp:906: GrSamplerState* sampler =
> grPaint.getTextureSampler(0);
> 0 => kFirstTextureStage?
> 5x in this file

The stages are really "internal" to GrContext and below (GrDrawTarget, etc).
From the SkGpuDevice pt of view the grpaint just has an array of textures and
array of masks. They should use the named constant, though (kBitmapTextureIndex/
kShaderTextureIdx). Fixed in latest patch.

[junov, 2011-05-17 20:13:24]

LGTM
On 2011/05/17 18:56:12, bsalomon wrote:
> On 2011/05/17 18:21:14, junov wrote:
>
http://codereview.appspot.com/4548048/diff/9002/src/gpu/SkGpuDevice.cpp#newco...
> > src/gpu/SkGpuDevice.cpp:906: GrSamplerState* sampler =
> > grPaint.getTextureSampler(0);
> > 0 => kFirstTextureStage?
> > 5x in this file
> 
> The stages are really "internal" to GrContext and below (GrDrawTarget, etc).
> From the SkGpuDevice pt of view the grpaint just has an array of textures and
> array of masks. They should use the named constant, though
(kBitmapTextureIndex/
> kShaderTextureIdx). Fixed in latest patch.

[bsalomon, 2011-05-17 20:16:36]

Closed with r1356