Added some missing bits of support for PVRTC, ATC, and ETC1 textures

Added some missing bits of support for PVRTC, ATC, and ETC1 textures

BUG=364564
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=171927

[bajones, 2014-04-17 22:23:50]

PTAL. Has a corresponding command buffer change incoming.

[Zhenyao Mo, 2014-04-17 22:40:27]

https://codereview.chromium.org/241163003/diff/20001/Source/core/html/canvas/...
File Source/core/html/canvas/WebGLRenderingContextBase.cpp (right):

https://codereview.chromium.org/241163003/diff/20001/Source/core/html/canvas/...
Source/core/html/canvas/WebGLRenderingContextBase.cpp:4942:
synthesizeGLError(GL_INVALID_OPERATION, functionName, "not valid for this
format");
I think if these formats are not supported, they should be caught while checking
the format (in this case, whether it's matching the internal format) and an
error should be generated there.  Here probably you can just ASSERT(FALSE),
assuming format checking is before this.

Also, while reading this, I realized validateCompressedTexFormat() doesn't
generate an error message, which is a bug.

[Zhenyao Mo, 2014-04-17 22:40:49]

On 2014/04/17 22:40:27, Zhenyao Mo wrote:
>
https://codereview.chromium.org/241163003/diff/20001/Source/core/html/canvas/...
> File Source/core/html/canvas/WebGLRenderingContextBase.cpp (right):
> 
>
https://codereview.chromium.org/241163003/diff/20001/Source/core/html/canvas/...
> Source/core/html/canvas/WebGLRenderingContextBase.cpp:4942:
> synthesizeGLError(GL_INVALID_OPERATION, functionName, "not valid for this
> format");
> I think if these formats are not supported, they should be caught while
checking
> the format (in this case, whether it's matching the internal format) and an
> error should be generated there.  Here probably you can just ASSERT(FALSE),
> assuming format checking is before this.
> 
> Also, while reading this, I realized validateCompressedTexFormat() doesn't
> generate an error message, which is a bug.

I mean an GL ERROR, not an error message.

[Ken Russell (switch to Gerrit), 2014-04-18 01:56:34]

In order to unblock, LGTM with a couple of minor issues.

https://codereview.chromium.org/241163003/diff/20001/Source/core/html/canvas/...
File Source/core/html/canvas/WebGLRenderingContextBase.cpp (right):

https://codereview.chromium.org/241163003/diff/20001/Source/core/html/canvas/...
Source/core/html/canvas/WebGLRenderingContextBase.cpp:4828: bytesRequired =
(max(width, 8) * max(height, 8) * 4 + 7) / 8;
Please submit a pull request updating the extension registry.

https://codereview.chromium.org/241163003/diff/20001/Source/core/html/canvas/...
Source/core/html/canvas/WebGLRenderingContextBase.cpp:4834: bytesRequired =
(max(width, 16) * max(height, 8) * 2 + 7) / 8;
Same for this change.

https://codereview.chromium.org/241163003/diff/20001/Source/core/html/canvas/...
Source/core/html/canvas/WebGLRenderingContextBase.cpp:4928:
synthesizeGLError(GL_INVALID_OPERATION, functionName, "xoffset or yoffset must
be zero");
"xoffset and yoffset" would be clearer.

https://codereview.chromium.org/241163003/diff/20001/Source/core/html/canvas/...
Source/core/html/canvas/WebGLRenderingContextBase.cpp:4942:
synthesizeGLError(GL_INVALID_OPERATION, functionName, "not valid for this
format");
On 2014/04/17 22:40:27, Zhenyao Mo wrote:
> I think if these formats are not supported, they should be caught while
checking
> the format (in this case, whether it's matching the internal format) and an
> error should be generated there.  Here probably you can just ASSERT(FALSE),
> assuming format checking is before this.

I think this is here because texSubImage2D isn't supported for these compressed
texture formats.

[bajones, 2014-04-18 05:45:44]

On 2014/04/18 01:56:34, Ken Russell wrote:
>
https://codereview.chromium.org/241163003/diff/20001/Source/core/html/canvas/...
> Source/core/html/canvas/WebGLRenderingContextBase.cpp:4828: bytesRequired =
> (max(width, 8) * max(height, 8) * 4 + 7) / 8;
> Please submit a pull request updating the extension registry.

Was already planning on it. Done.

>
https://codereview.chromium.org/241163003/diff/20001/Source/core/html/canvas/...
> Source/core/html/canvas/WebGLRenderingContextBase.cpp:4834: bytesRequired =
> (max(width, 16) * max(height, 8) * 2 + 7) / 8;
> Same for this change.

Done.

>
https://codereview.chromium.org/241163003/diff/20001/Source/core/html/canvas/...
> Source/core/html/canvas/WebGLRenderingContextBase.cpp:4928:
> synthesizeGLError(GL_INVALID_OPERATION, functionName, "xoffset or yoffset must
> be zero");
> "xoffset and yoffset" would be clearer.

Good call. Done.
 
>
https://codereview.chromium.org/241163003/diff/20001/Source/core/html/canvas/...
> Source/core/html/canvas/WebGLRenderingContextBase.cpp:4942:
> synthesizeGLError(GL_INVALID_OPERATION, functionName, "not valid for this
> format");
> On 2014/04/17 22:40:27, Zhenyao Mo wrote:
> > I think if these formats are not supported, they should be caught while
> checking
> > the format (in this case, whether it's matching the internal format) and an
> > error should be generated there.  Here probably you can just ASSERT(FALSE),
> > assuming format checking is before this.
> 
> I think this is here because texSubImage2D isn't supported for these
compressed
> texture formats.

That is correct. I've updated the error message to make that clearer.

> Also, while reading this, I realized validateCompressedTexFormat() doesn't
> generate an error message, which is a bug.

Looking at the code it actually generates an error in both locations that call
the function, so I don't think there's actually a bug here. You could argue that
it could be consolidated into that function, but that's probably material for a
different patch.

[bajones, 2014-04-18 05:46:00]

The CQ bit was checked by bajones@chromium.org

[commit-bot: I haz the power, 2014-04-18 05:46:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bajones@chromium.org/241163003/40001

[commit-bot: I haz the power, 2014-04-18 06:51:59]

Change committed as 171927