Added minimal support to command buffer for GL_ARB_texture_rectangle

Added minimal support to command buffer for GL_ARB_texture_rectangle
textures, and added Mac OS X-specific extension for binding IOSurfaces
to textures. This is infrastructure work for rendering Core Animation
plugins via Chrome's compositor.

Refactored the texture initialization in the command buffer and
associated unit tests. Added unit tests for rectangular textures.

These changes will be hooked up in a subsequent CL. They were split
off for easier review.

There are ongoing discussions about unifying the various mechanisms
for sharing textures across processes, but it's been agreed that those
should not block this work.

BUG=38967
TEST=GPU unit tests; manual testing with forthcoming changes
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=111426

[Ken Russell (switch to Gerrit), 2011-11-23 06:19:39]

Please review. Thanks.

If it seems undesirable to expose this legacy texture format (I personally think
it is), please keep in mind that on Mac OS X, IOSurfaces are only compatible
with GL_TEXTURE_RECTANGLE_ARB textures. It might be possible to lie to the
application level and treat these as GL_TEXTURE_EXTERNAL_OES textures, assuming
that that form of texture will never be supported natively on Mac OS; but I
think it's probably best to expose reality. Note also that the associated WebKit
and ANGLE changes assuming the use of rectangular textures have already landed.

[apatrick_chromium, 2011-11-23 20:12:41]

lgtm

[Ken Russell (switch to Gerrit), 2011-11-23 20:15:03]

Thanks.

Gregg, sorry for committing this before you got a chance to look at it, but I
will gladly make any changes you deem necessary. Right now I am rushing to get
the dependent CL on this one out for review for M17.

[greggman, 2011-11-28 22:58:11]

http://codereview.chromium.org/8680002/diff/1/gpu/command_buffer/service/gles...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

http://codereview.chromium.org/8680002/diff/1/gpu/command_buffer/service/gles...
gpu/command_buffer/service/gles2_cmd_decoder.cc:7576: if (it !=
texture_to_io_surface_map_.end()) {
I don't know the use case but is there any issue with texture_to_io_surface_map
being in GLES2DecoderImpl instead of ContextGroup or TextureManager or
IOSurfaceManager? If you have a shared resource then you can delete the texture
in another context and this map will be invalid.

I know that you probably aren't using in in that way but it seems wrong to make
the decoder brittle and at the mercy of the clients not using it correctly.

http://codereview.chromium.org/8680002/diff/1/gpu/command_buffer/service/gles...
gpu/command_buffer/service/gles2_cmd_decoder.cc:7619: if (info ==
texture_manager()->GetDefaultTextureInfo(target)) {
This check is not needed. GetTextureInfoForTarget will never return an info for
the default texture.

http://codereview.chromium.org/8680002/diff/1/gpu/command_buffer/service/gles...
gpu/command_buffer/service/gles2_cmd_decoder.cc:7669: GL_BGRA,
GL_UNSIGNED_INT_8_8_8_8_REV, true);
Do you need to check in feature_info that GL_BGRA support exists before turning
on this feature? Otherwise, passing in GL_BGRA here will mean you can get enums
from glGetIntergerv that won't validate later

Same with GL_UNSIGNED_INT_8_8_8_8_REV, it's not been added to the validators.

[Ken Russell (switch to Gerrit), 2011-11-29 01:34:49]

On 2011/11/28 22:58:11, greggman wrote:
>
http://codereview.chromium.org/8680002/diff/1/gpu/command_buffer/service/gles...
> File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):
> 
>
http://codereview.chromium.org/8680002/diff/1/gpu/command_buffer/service/gles...
> gpu/command_buffer/service/gles2_cmd_decoder.cc:7576: if (it !=
> texture_to_io_surface_map_.end()) {
> I don't know the use case but is there any issue with
texture_to_io_surface_map
> being in GLES2DecoderImpl instead of ContextGroup or TextureManager or
> IOSurfaceManager? If you have a shared resource then you can delete the
texture
> in another context and this map will be invalid.
> 
> I know that you probably aren't using in in that way but it seems wrong to
make
> the decoder brittle and at the mercy of the clients not using it correctly.

Thanks, good point; I've filed http://crbug.com/105662 to clean this up.


> 
>
http://codereview.chromium.org/8680002/diff/1/gpu/command_buffer/service/gles...
> gpu/command_buffer/service/gles2_cmd_decoder.cc:7619: if (info ==
> texture_manager()->GetDefaultTextureInfo(target)) {
> This check is not needed. GetTextureInfoForTarget will never return an info
for
> the default texture.

Will fix.

>
http://codereview.chromium.org/8680002/diff/1/gpu/command_buffer/service/gles...
> gpu/command_buffer/service/gles2_cmd_decoder.cc:7669: GL_BGRA,
> GL_UNSIGNED_INT_8_8_8_8_REV, true);
> Do you need to check in feature_info that GL_BGRA support exists before
turning
> on this feature? Otherwise, passing in GL_BGRA here will mean you can get
enums
> from glGetIntergerv that won't validate later
> 
> Same with GL_UNSIGNED_INT_8_8_8_8_REV, it's not been added to the validators.

Argh, thanks, good point. Can we discuss this one offline? I really wanted to
avoid adding these enums to the public API. Maybe we can lie about the format
and internal format of the texture, since its contents can intentionally only be
set up with this entry point and not with TexImage2D / TexSubImage2D.