Add compressed texture capabilities for GPU devices

Add compressed texture capabilities for GPU devices

BUG=skia:

Committed: http://code.google.com/p/skia/source/detail?r=14880

Committed: http://code.google.com/p/skia/source/detail?r=14901

[robertphillips, 2014-05-22 22:00:26]

https://codereview.chromium.org/292323003/diff/20001/src/gpu/GrDrawTarget.cpp
File src/gpu/GrDrawTarget.cpp (right):

https://codereview.chromium.org/292323003/diff/20001/src/gpu/GrDrawTarget.cpp...
src/gpu/GrDrawTarget.cpp:1141: 
1?

https://codereview.chromium.org/292323003/diff/40001/include/gpu/GrTypes.h
File include/gpu/GrTypes.h (right):

https://codereview.chromium.org/292323003/diff/40001/include/gpu/GrTypes.h#ne...
include/gpu/GrTypes.h:622: /**
that _can be_ ?
Do we really need/use the "Uncompressed" value?

https://codereview.chromium.org/292323003/diff/40001/src/gpu/gl/GrGLCaps.cpp
File src/gpu/gl/GrGLCaps.cpp (right):

https://codereview.chromium.org/292323003/diff/40001/src/gpu/gl/GrGLCaps.cpp#...
src/gpu/gl/GrGLCaps.cpp:485: // ETC2 is a superset of ETC1, so we can just check
for that, too.
Is this an && or an || ?

[krajcevski, 2014-05-22 22:17:11]

https://codereview.chromium.org/292323003/diff/20001/src/gpu/GrDrawTarget.cpp
File src/gpu/GrDrawTarget.cpp (right):

https://codereview.chromium.org/292323003/diff/20001/src/gpu/GrDrawTarget.cpp...
src/gpu/GrDrawTarget.cpp:1141: 
On 2014/05/22 22:00:26, robertphillips wrote:
> 1?

Done.

https://codereview.chromium.org/292323003/diff/40001/include/gpu/GrTypes.h
File include/gpu/GrTypes.h (right):

https://codereview.chromium.org/292323003/diff/40001/include/gpu/GrTypes.h#ne...
include/gpu/GrTypes.h:622: /**
On 2014/05/22 22:00:26, robertphillips wrote:
> that _can be_ ?

Fixed the phrasing.

> Do we really need/use the "Uncompressed" value?

Here there needs to be a way to distinguish compressed textures from
uncompressed textures. We could make a separate GrCompressedTextureDesc that
gets passed around all of the calls, but I think that would be more trouble than
it's worth. In my opinion it'd be better to just have a field in the existing
GrTextureDesc that contains the compressed format, or uncompressed if it's
uncompressed (the default).

https://codereview.chromium.org/292323003/diff/40001/src/gpu/gl/GrGLCaps.cpp
File src/gpu/gl/GrGLCaps.cpp (right):

https://codereview.chromium.org/292323003/diff/40001/src/gpu/gl/GrGLCaps.cpp#...
src/gpu/gl/GrGLCaps.cpp:485: // ETC2 is a superset of ETC1, so we can just check
for that, too.
On 2014/05/22 22:00:26, robertphillips wrote:
> Is this an && or an || ?

We have ETC1 if any of the following are true:
- OpenGL ES version is 3.0 or greater
- We have the ETC1 extension
- We have the ETC2 extension

The last one has many extension strings, and I didn't know how to handle all of
them, so I just checked that we had both of the main format:

OES_compressed_ETC2_RGB8_texture
OES_compressed_ETC2_sRGB8_texture
OES_compressed_ETC2_punchthroughA_RGBA8_texture
OES_compressed_ETC2_punchthroughA_sRGB8_alpha_texture
OES_compressed_ETC2_RGBA8_texture
OES_compressed_ETC2_sRGB8_alpha8_texture

[krajcevski, 2014-05-22 22:20:09]

Forgot to actually change the comments in GrTypes.h

[bsalomon, 2014-05-23 13:40:04]

I have a couple minor comments, but this lgtm.

https://codereview.chromium.org/292323003/diff/80001/include/gpu/GrTypes.h
File include/gpu/GrTypes.h (right):

https://codereview.chromium.org/292323003/diff/80001/include/gpu/GrTypes.h#ne...
include/gpu/GrTypes.h:625: * GrDrawTargetCaps::compressedTextureSupport
Maybe we should throw the 8bit index format in here.

https://codereview.chromium.org/292323003/diff/80001/include/gpu/GrTypes.h#ne...
include/gpu/GrTypes.h:628: kUncompressed_GrCompressedFormat = -1,
Is this used anywhere? Or is it related to a coming CL?

https://codereview.chromium.org/292323003/diff/80001/src/gpu/gl/GrGLCaps.cpp
File src/gpu/gl/GrGLCaps.cpp (right):

https://codereview.chromium.org/292323003/diff/80001/src/gpu/gl/GrGLCaps.cpp#...
src/gpu/gl/GrGLCaps.cpp:468: // however, it is only available on the core
profile after version 1.3
I think "core profile" is slightly confusing since OpenGL has a notion of a core
vs compatibility profile begining in version 3 where the core profile deprecates
a lot of functionality.  Maybe just "OpenGL" or "non-ES OpenGL". We and others
also sometimes refer to regular OpenGL as "desktop GL", though that can be
confusing as well.

[krajcevski, 2014-05-23 15:11:46]

https://codereview.chromium.org/292323003/diff/80001/include/gpu/GrTypes.h
File include/gpu/GrTypes.h (right):

https://codereview.chromium.org/292323003/diff/80001/include/gpu/GrTypes.h#ne...
include/gpu/GrTypes.h:625: * GrDrawTargetCaps::compressedTextureSupport
On 2014/05/23 13:40:04, bsalomon wrote:
> Maybe we should throw the 8bit index format in here.

For paletted textures, OpenGL ES uses glCompressedTexImage2D but OpenGL uses
glTexImage2D. It seems like it would be better to keep them separate, i.e.
GrCompressedFormat should be the ETC1/2, DXT1, etc.

Conversely, we could add the compressed formats to GrPixelConfig

https://codereview.chromium.org/292323003/diff/80001/include/gpu/GrTypes.h#ne...
include/gpu/GrTypes.h:628: kUncompressed_GrCompressedFormat = -1,
On 2014/05/23 13:40:04, bsalomon wrote:
> Is this used anywhere? Or is it related to a coming CL?

It will be used to denote uncompressed textures in GrTextureDesc in an upcoming
CL.

https://codereview.chromium.org/292323003/diff/80001/src/gpu/gl/GrGLCaps.cpp
File src/gpu/gl/GrGLCaps.cpp (right):

https://codereview.chromium.org/292323003/diff/80001/src/gpu/gl/GrGLCaps.cpp#...
src/gpu/gl/GrGLCaps.cpp:468: // however, it is only available on the core
profile after version 1.3
On 2014/05/23 13:40:04, bsalomon wrote:
> I think "core profile" is slightly confusing since OpenGL has a notion of a
core
> vs compatibility profile begining in version 3 where the core profile
deprecates
> a lot of functionality.  Maybe just "OpenGL" or "non-ES OpenGL". We and others
> also sometimes refer to regular OpenGL as "desktop GL", though that can be
> confusing as well.

Done.

[bsalomon, 2014-05-23 15:14:27]

lgtm

[krajcevski, 2014-05-23 17:40:23]

The CQ bit was checked by krajcevski@google.com

[commit-bot: I haz the power, 2014-05-23 17:40:47]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/krajcevski@google.com/292323003/100001

[commit-bot: I haz the power, 2014-05-23 20:00:56]

Change committed as 14880

[krajcevski, 2014-05-23 20:17:03]

A revert of this CL has been created in
https://codereview.chromium.org/296313003/ by krajcevski@google.com.

The reason for reverting is: Signeness comparison breaks build. After working
some more, I think the API design will be cleaner without the Uncompressed enum,
so I'm removing it and retesting everything..

[krajcevski, 2014-05-23 22:45:11]

The CQ bit was checked by krajcevski@google.com

[krajcevski, 2014-05-23 22:45:11]

The CQ bit was unchecked by krajcevski@google.com

[krajcevski, 2014-05-23 22:56:32]

So, there was a little SNAFU when trying to commit this:

Apparently Clang does some funky optimizations when you have an enum with only a
single field. If you have a function call that takes such an enum as a type it
can only ever be a single value, so you know as a compiler what that value will
be. In making "forward-looking" enum declarations and asserts that guard against
malformed input, clang spits out a warning that the assert condition will always
be true.

There are two ways to fix this and I'm not sure which one is better:

1. Remove the assert
2. Add more enum values

I've added a few more compressed formats to the enum that we may want to support
in the future. However, since they're not supported now (or plan on being
supported) I didn't check in my change before I got feedback...

[robertphillips, 2014-05-27 12:20:35]

It think it is okay to build in space for ETC2 and DXT1. I think you need to
update this CL to correctly prepare for them though.

https://codereview.chromium.org/292323003/diff/160001/src/gpu/GrDrawTarget.cpp
File src/gpu/GrDrawTarget.cpp (right):

https://codereview.chromium.org/292323003/diff/160001/src/gpu/GrDrawTarget.cp...
src/gpu/GrDrawTarget.cpp:1135: 
add new enums here !?

https://codereview.chromium.org/292323003/diff/160001/src/gpu/gl/GrGLCaps.cpp
File src/gpu/gl/GrGLCaps.cpp (right):

https://codereview.chromium.org/292323003/diff/160001/src/gpu/gl/GrGLCaps.cpp...
src/gpu/gl/GrGLCaps.cpp:489: fCompressedFormatSupport[kETC1_GrCompressedFormat]
= hasETC1;
set other enums to false here !?

[krajcevski, 2014-05-27 14:31:19]

https://codereview.chromium.org/292323003/diff/160001/src/gpu/gl/GrGLCaps.cpp
File src/gpu/gl/GrGLCaps.cpp (right):

https://codereview.chromium.org/292323003/diff/160001/src/gpu/gl/GrGLCaps.cpp...
src/gpu/gl/GrGLCaps.cpp:489: fCompressedFormatSupport[kETC1_GrCompressedFormat]
= hasETC1;
On 2014/05/27 12:20:35, robertphillips wrote:
> set other enums to false here !?

Done.

[krajcevski, 2014-05-27 14:32:01]

https://codereview.chromium.org/292323003/diff/160001/src/gpu/GrDrawTarget.cpp
File src/gpu/GrDrawTarget.cpp (right):

https://codereview.chromium.org/292323003/diff/160001/src/gpu/GrDrawTarget.cp...
src/gpu/GrDrawTarget.cpp:1135: 
On 2014/05/27 12:20:35, robertphillips wrote:
> add new enums here !?

Done.

[robertphillips, 2014-05-27 17:01:41]

lgtm

[krajcevski, 2014-05-27 17:23:06]

The CQ bit was checked by krajcevski@google.com

[krajcevski, 2014-05-27 17:23:07]

The CQ bit was unchecked by krajcevski@google.com

[krajcevski, 2014-05-27 18:46:36]

The CQ bit was checked by krajcevski@google.com

[commit-bot: I haz the power, 2014-05-27 18:47:08]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/krajcevski@google.com/292323003/170001

[commit-bot: I haz the power, 2014-05-27 18:52:28]

Change committed as 14901

[epoger, 2014-05-27 19:06:25]

A revert of this CL has been created in
https://codereview.chromium.org/307543002/ by epoger@google.com.

The reason for reverting is: RunDecodingTests failing on multiple platforms. 
Please use trybots to validate and re-land..