Move filter/wrap out of GrSamplerState into GrTextureParams

[bsalomon, 2012-07-25 14:40:10]

GrSamplerState is being eviscerated as the stuff it controlled is migrating into
specialized custom stages.

The idea of this CL is to pull out the parts of SamplerState that will be
preserved. The custom stages will have to communicate the wrap and filter info
the the code that binds/configures textures. Also, these fields are needed by
the texture cache on ES2 devices with limited NPOT support.

The new class, GrTextureParams, is a member of GrSamplerState. However, the next
step will be to modify GrCustomStage to provide a GrTextureParams for each of
its GrTextures. Once all texture accesses happen within custom effects then the
GrTextureParams memeber can be removed from GrSamplerState.

GrSingleTextureEffect will take a GrTextureParams in its constructor. Other
stages may be able to have a single static GrTextureParams because they always
use the same params. Gradient effects will construct a GrTextureParams based on
the generating SkGradientShader's tile mode.

Jeff, I suspect after this may play into or interact with the texture access /
GrGLShaderBuilder stuff you're looking at.

Tom, I put you on the CC since you're WK gardener. This CL is right up your
alley so if you are able to review that'd be great but if not that's OK.

[bsalomon, 2012-07-25 14:41:23]

Updated review line. I meant to include Riley, not Tom's @chromium.

[TomH, 2012-07-25 15:35:10]

LGTM.

If we expected to keep GrSamplerState around I'd probably have a few more nits,
but this will do just fine.

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

http://codereview.appspot.com/6440046/diff/1/include/gpu/GrContext.h#newcode122
include/gpu/GrContext.h:122: *  @param params   The params used to draw a
texture may be used to
Nit: "used" twice in the same sentence when it's a really vague verb to start
with. Repeated in the next function's comment.

http://codereview.appspot.com/6440046/diff/1/src/effects/SkGradientShader.cpp
File src/effects/SkGradientShader.cpp (right):

http://codereview.appspot.com/6440046/diff/1/src/effects/SkGradientShader.cpp...
src/effects/SkGradientShader.cpp:1058:
sampler->textureParams()->setFilter(true);
Nit: is true/false really more informative than the enums? Or are we just
deciding that we only ever need two filter modes? If so, should it be
setBilinearFilter()?

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

http://codereview.appspot.com/6440046/diff/1/src/gpu/GrDrawTarget.cpp#newcode767
src/gpu/GrDrawTarget.cpp:767: // WRT to filtering. We'd have to filter each
texel before filtering. Until Skia itself
Nit: you lost the word "distributive" here, which loses the sense of the
sentence. Also consider getting rid of abbreviations for the sake of
intelligibility.

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

http://codereview.appspot.com/6440046/diff/1/src/gpu/GrTexture.cpp#newcode166
src/gpu/GrTexture.cpp:166: bool tiled = NULL != params && params->isTiled();
New convenience functions are very nice through here.

[bsalomon, 2012-07-25 15:52:27]

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

http://codereview.appspot.com/6440046/diff/1/include/gpu/GrContext.h#newcode122
include/gpu/GrContext.h:122: *  @param params   The params used to draw a
texture may be used to
On 2012/07/25 15:35:10, TomH wrote:
> Nit: "used" twice in the same sentence when it's a really vague verb to start
> with. Repeated in the next function's comment.

Done.

http://codereview.appspot.com/6440046/diff/1/src/effects/SkGradientShader.cpp
File src/effects/SkGradientShader.cpp (right):

http://codereview.appspot.com/6440046/diff/1/src/effects/SkGradientShader.cpp...
src/effects/SkGradientShader.cpp:1058:
sampler->textureParams()->setFilter(true);
On 2012/07/25 15:35:10, TomH wrote:
> Nit: is true/false really more informative than the enums? Or are we just
> deciding that we only ever need two filter modes? If so, should it be
> setBilinearFilter()?

Prolly only bilerp or not for the foreseeable future. If we add fancier filters
they'd be written as custom effects. I'll change the name.

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

http://codereview.appspot.com/6440046/diff/1/src/gpu/GrDrawTarget.cpp#newcode767
src/gpu/GrDrawTarget.cpp:767: // WRT to filtering. We'd have to filter each
texel before filtering. Until Skia itself
On 2012/07/25 15:35:10, TomH wrote:
> Nit: you lost the word "distributive" here, which loses the sense of the
> sentence. Also consider getting rid of abbreviations for the sake of
> intelligibility.

Done.

[bsalomon, 2012-07-25 20:50:02]

If no one objects I'll commit this soon on Tom's review.

[bsalomon, 2012-07-26 15:02:52]

Closed with r4773