Initial addition of R8 support

This doesn't/shouldn't do anything different then the current Alpha8 support but does start the process of shifting to R8.

[bsalomon, 2012-04-03 21:51:02]

http://codereview.appspot.com/5967067/diff/1/include/gpu/gl/GrGLDefines.h
File include/gpu/gl/GrGLDefines.h (right):

http://codereview.appspot.com/5967067/diff/1/include/gpu/gl/GrGLDefines.h#new...
include/gpu/gl/GrGLDefines.h:299: // Use GR_GL_R8 only with GL_ARB_texture_rg
Let's either say "or GL version 3.x" or remove the comment. There are many
defines in here that rely on particular versions or extensions.

http://codereview.appspot.com/5967067/diff/1/src/gpu/gl/GrGLCaps.cpp
File src/gpu/gl/GrGLCaps.cpp (right):

http://codereview.appspot.com/5967067/diff/1/src/gpu/gl/GrGLCaps.cpp#newcode139
src/gpu/gl/GrGLCaps.cpp:139: fTextureRGSupport =
ctxInfo.hasExtension("GL_ARB_texture_rg");
Is it a legal ES extension? I guess it doesn't hurt to try.

http://codereview.appspot.com/5967067/diff/1/src/gpu/gl/GrGLCaps.h
File src/gpu/gl/GrGLCaps.h (right):

http://codereview.appspot.com/5967067/diff/1/src/gpu/gl/GrGLCaps.h#newcode161
src/gpu/gl/GrGLCaps.h:161: /// Is there support for GL_RED and GL_R8
Should we call the field/function textureRSupport (or textureRedSupport)? I only
ask because I could imagine an ES extension (possibly for ANGLE) that only added
the single channel formats and not the two channel formats and we only use the
former.

http://codereview.appspot.com/5967067/diff/1/src/gpu/gl/GrGpuGL.cpp
File src/gpu/gl/GrGpuGL.cpp (right):

http://codereview.appspot.com/5967067/diff/1/src/gpu/gl/GrGpuGL.cpp#newcode1935
src/gpu/gl/GrGpuGL.cpp:1935: if (this->glCaps().textureRGSupport()) {
Perhaps we should check the internal format (or assert that it is red if
textureRGSupport()). It'd require that we either pass the GrGLTexture or the
internal format. Our mapping between configs and internal formats will only get
more complicated over time and we should start to be more careful about making
assumptions about configs implying specific internal formats.

Also, rather than make this is a member can we instead pass the caps?  (It
should probably be marked inline as well). I think it is likely the compiler
will inline this guy if we make it easy to do so.

[robertphillips, 2012-04-04 12:51:36]

http://codereview.appspot.com/5967067/diff/1/include/gpu/gl/GrGLDefines.h
File include/gpu/gl/GrGLDefines.h (right):

http://codereview.appspot.com/5967067/diff/1/include/gpu/gl/GrGLDefines.h#new...
include/gpu/gl/GrGLDefines.h:299: // Use GR_GL_R8 only with GL_ARB_texture_rg
On 2012/04/03 21:51:02, bsalomon wrote:
> Let's either say "or GL version 3.x" or remove the comment. There are many
> defines in here that rely on particular versions or extensions.

Done.

http://codereview.appspot.com/5967067/diff/1/src/gpu/gl/GrGLCaps.cpp
File src/gpu/gl/GrGLCaps.cpp (right):

http://codereview.appspot.com/5967067/diff/1/src/gpu/gl/GrGLCaps.cpp#newcode139
src/gpu/gl/GrGLCaps.cpp:139: fTextureRGSupport =
ctxInfo.hasExtension("GL_ARB_texture_rg");
It is not but I didn't think it would hurt to try.

http://codereview.appspot.com/5967067/diff/1/src/gpu/gl/GrGLCaps.h
File src/gpu/gl/GrGLCaps.h (right):

http://codereview.appspot.com/5967067/diff/1/src/gpu/gl/GrGLCaps.h#newcode161
src/gpu/gl/GrGLCaps.h:161: /// Is there support for GL_RED and GL_R8
Renamed to textureRedSupport

http://codereview.appspot.com/5967067/diff/1/src/gpu/gl/GrGpuGL.cpp
File src/gpu/gl/GrGpuGL.cpp (right):

http://codereview.appspot.com/5967067/diff/1/src/gpu/gl/GrGpuGL.cpp#newcode1935
src/gpu/gl/GrGpuGL.cpp:1935: if (this->glCaps().textureRGSupport()) {
The second part (return get_swizzle to being a non-member) is done.

I'm not entirely sure I understand the first bit. It looks like the internal
format is determined from the config in configToGLFormats and then sent off to
OpenGL via TexStorage2D, CompressedTexImage2D or TexImage2D. It does look like
we are caching the internal formats of the stencil buffers but not the regular
textures.

[bsalomon, 2012-04-04 13:45:14]

I thought of another thing after my initial review: What are the implications
for readpixels / writepixels? Will we correctly read/write  the red channel?

I don't know of a code path today that would call read pixels on an
kAlpha_8_GrPixelConfig but it is possible to do so (and both read and write
pixels are public facing functions on the texture object). ES2 is probably not a
concern for this since we don't know of an ES2 implementation that supports R8
and ES2 has very limited glReadPixels support which we already gives us problems
(and needs some love). But we shouldn't regress the desktop.

We have unit tests for read / write pixels that only test the rgba, bgra
configs. Perhaps we should add  test cases for alpha 8.

http://codereview.appspot.com/5967067/diff/1/src/gpu/gl/GrGLCaps.cpp
File src/gpu/gl/GrGLCaps.cpp (right):

http://codereview.appspot.com/5967067/diff/1/src/gpu/gl/GrGLCaps.cpp#newcode139
src/gpu/gl/GrGLCaps.cpp:139: fTextureRGSupport =
ctxInfo.hasExtension("GL_ARB_texture_rg");
On 2012/04/04 12:51:36, robertphillips wrote:
> It is not but I didn't think it would hurt to try.
OK

http://codereview.appspot.com/5967067/diff/1/src/gpu/gl/GrGpuGL.cpp
File src/gpu/gl/GrGpuGL.cpp (right):

http://codereview.appspot.com/5967067/diff/1/src/gpu/gl/GrGpuGL.cpp#newcode1935
src/gpu/gl/GrGpuGL.cpp:1935: if (this->glCaps().textureRGSupport()) {
On 2012/04/04 12:51:36, robertphillips wrote:
> The second part (return get_swizzle to being a non-member) is done.
> 
> I'm not entirely sure I understand the first bit. It looks like the internal
> format is determined from the config in configToGLFormats and then sent off to
> OpenGL via TexStorage2D, CompressedTexImage2D or TexImage2D. It does look like
> we are caching the internal formats of the stencil buffers but not the regular
> textures.


Ok we used to store the internal formats but I removed it (forgot about that).
So I guess we have no way to assert. So let's ignore for now.

http://codereview.appspot.com/5967067/diff/4002/src/gpu/gl/GrGpuGL.cpp
File src/gpu/gl/GrGpuGL.cpp (right):

http://codereview.appspot.com/5967067/diff/4002/src/gpu/gl/GrGpuGL.cpp#newcod...
src/gpu/gl/GrGpuGL.cpp:1951: inline const GrGLenum* get_swizzle(const
GrGLTexture* texture,
If you'd like we can go back to passing the config since its all we're using
from texture.

[robertphillips, 2012-04-04 15:30:40]

It looks like adding testing of reading/writing a single channel texture through
SkCanvas (as used in the existing unit tests) will take a bit of replumbing
(both SkCanvas::writePixels and SkDevice::writePixels take a Config8888). What
do you think about a new unit test just for alpha textures?

[bsalomon, 2012-04-04 15:54:07]

On 2012/04/04 15:30:40, robertphillips wrote:
> It looks like adding testing of reading/writing a single channel texture
through
> SkCanvas (as used in the existing unit tests) will take a bit of replumbing
> (both SkCanvas::writePixels and SkDevice::writePixels take a Config8888). What
> do you think about a new unit test just for alpha textures?

Oh right, that's an annoying wrinkle. Sure, an alpha-texture specific test would
work.

[robertphillips, 2012-04-06 17:41:19]

This version also adds a test case

http://codereview.appspot.com/5967067/diff/4002/src/gpu/gl/GrGpuGL.cpp
File src/gpu/gl/GrGpuGL.cpp (right):

http://codereview.appspot.com/5967067/diff/4002/src/gpu/gl/GrGpuGL.cpp#newcod...
src/gpu/gl/GrGpuGL.cpp:1951: inline const GrGLenum* get_swizzle(const
GrGLTexture* texture,
On 2012/04/04 13:45:14, bsalomon wrote:
> If you'd like we can go back to passing the config since its all we're using
> from texture.

Done.

[bsalomon, 2012-04-06 17:56:26]

LGTM. Nice test. I'm glad that it also exercises backing a device with a kA8
render target.

[robertphillips, 2012-04-06 18:06:59]

committed as r3622