Do writepixels alpha-premul using gpu

[reed1, 2011-11-11 16:43:41]

lgtm w/ suggestion for a comment

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

http://codereview.appspot.com/5373064/diff/1/src/gpu/GrDrawTarget.cpp#oldcode814
src/gpu/GrDrawTarget.cpp:814:
GrPixelConfigIsUnpremultiplied(fCurrDrawState.fTextures[s]->config())) {
// If the texture is unpremulitplied, we can't use the built-in bilerp, as the
operation of applying-alpha and bilerp are not symmetric (associative?), hence
we only support these with nearest_filter.
// To add support, we'd need to perform the filter (bilper) ourselves in the
shader...

[TomH, 2011-11-11 19:02:16]

Think about kPremul_InConfigFlag. If there isn't something better that jumps out
at you, LGTM - as long as I read the header first and learn your naming
convention, it makes sense in situ.

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

http://codereview.appspot.com/5373064/diff/1/include/gpu/GrTypes.h#newcode276
include/gpu/GrTypes.h:276: * has an unpremultiplied config only draws that use
blend coeffs coeffs 1,0
This word appears twice twice.

http://codereview.appspot.com/5373064/diff/1/src/gpu/GrGLProgram.h
File src/gpu/GrGLProgram.h (right):

http://codereview.appspot.com/5373064/diff/1/src/gpu/GrGLProgram.h#newcode153
src/gpu/GrGLProgram.h:153: kPremul_InConfigFlag        = 0x4,
Although it makes sense here, seen in use without reading the header comments
first Premul (or Alphamul) is very misleading. It's consistent with your naming
pattern, though, so may not need a redo.

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

http://codereview.appspot.com/5373064/diff/1/src/gpu/GrGpuGLShaders.cpp#newco...
src/gpu/GrGpuGLShaders.cpp:188: bool GrGpuGLShaders::programUnitTest() {
Tangent: is this getting large enough it'd be worth moving to a separate file,
even though it needs to be compiled in with the class?

[bsalomon, 2011-11-11 19:34:55]

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

http://codereview.appspot.com/5373064/diff/1/include/gpu/GrTypes.h#newcode276
include/gpu/GrTypes.h:276: * has an unpremultiplied config only draws that use
blend coeffs coeffs 1,0
On 2011/11/11 19:02:16, TomH wrote:
> This word appears twice twice.

I reworked the comment a bit.

http://codereview.appspot.com/5373064/diff/1/src/gpu/GrGLProgram.h
File src/gpu/GrGLProgram.h (right):

http://codereview.appspot.com/5373064/diff/1/src/gpu/GrGLProgram.h#newcode153
src/gpu/GrGLProgram.h:153: kPremul_InConfigFlag        = 0x4,
On 2011/11/11 19:02:16, TomH wrote:
> Although it makes sense here, seen in use without reading the header comments
> first Premul (or Alphamul) is very misleading. It's consistent with your
naming
> pattern, though, so may not need a redo.

I verbized it as kMulRGBByAlpha. Better/worse?

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

http://codereview.appspot.com/5373064/diff/1/src/gpu/GrGpuGLShaders.cpp#newco...
src/gpu/GrGpuGLShaders.cpp:188: bool GrGpuGLShaders::programUnitTest() {
On 2011/11/11 19:02:16, TomH wrote:
> Tangent: is this getting large enough it'd be worth moving to a separate file,
> even though it needs to be compiled in with the class?

Good idea, will do in a follow up change.

[TomH, 2011-11-11 19:40:40]

On 2011/11/11 19:34:55, bsalomon wrote:
> On 2011/11/11 19:02:16, TomH wrote:
> > Although it makes sense here, seen in use without reading the header
comments
> > first Premul (or Alphamul) is very misleading. It's consistent with your
> naming
> > pattern, though, so may not need a redo.
> 
> I verbized it as kMulRGBByAlpha. Better/worse?

It's growing on me. Ship it.

Tom