Unify validation for TexImage{2D|3D}

Unify validation for TexImage{2D|3D}

BUG=295792
TEST=No functional changes, current conformance test covers the test.

Committed: https://crrev.com/3a896bbd9e244c502b04bbd1ec77a1ff34149799
Cr-Commit-Position: refs/heads/master@{#367122}

[qiankun, 2015-12-23 15:47:11]

qiankun.miao@intel.com changed reviewers:
+ bajones@chromium.org, kbr@chromium.org, yunchao.he@intel.com, zmo@chromium.org

[qiankun, 2015-12-23 15:47:12]

PTAL.

[Zhenyao Mo, 2015-12-28 19:30:27]

https://codereview.chromium.org/1543233002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp
(right):

https://codereview.chromium.org/1543233002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp:863:
WebGLTexture* tex = validateTextureBinding("texImage3D", target, true);
This is done in validateTexFunc().

https://codereview.chromium.org/1543233002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp:867: if
(tex->isImmutable()) {
This is also done in validateTexFunc().

https://codereview.chromium.org/1543233002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
(right):

https://codereview.chromium.org/1543233002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp:4209: bool
WebGLRenderingContextBase::validateTexFunc(const char* functionName,
TexImageFunctionType functionType, TexFuncValidationSourceType sourceType,
GLenum target, GLint level, GLenum internalformat, GLsizei width, GLsizei
height, GLsizei depth, GLint border, GLenum format, GLenum type, GLint xoffset,
GLint yoffset)
It is weird that you upgrade to depth but didn't do it for zoffset. Because
either it handles 3D or it does not.  A mixture leads to maintenance cost.

https://codereview.chromium.org/1543233002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp:4231:
synthesizeGLError(GL_INVALID_OPERATION, "texImage2D", "attempted to modify
immutable texture");
use functionName here.

https://codereview.chromium.org/1543233002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp:4235: if
(isNPOTStrict() && level && WebGLTexture::isNPOT(width, height)) {
Worth add a comment that depth is for WebGL 2 only where iSNPOTStrict() is
always false.

[qiankun, 2015-12-29 16:16:58]

Thanks for review. I updated the CL, PTAL.

https://codereview.chromium.org/1543233002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp
(right):

https://codereview.chromium.org/1543233002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp:863:
WebGLTexture* tex = validateTextureBinding("texImage3D", target, true);
On 2015/12/28 19:30:27, Zhenyao Mo wrote:
> This is done in validateTexFunc().

Add an assert(tex). tex is used below.

https://codereview.chromium.org/1543233002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp:867: if
(tex->isImmutable()) {
On 2015/12/28 19:30:27, Zhenyao Mo wrote:
> This is also done in validateTexFunc().

Done.

https://codereview.chromium.org/1543233002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
(right):

https://codereview.chromium.org/1543233002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp:4209: bool
WebGLRenderingContextBase::validateTexFunc(const char* functionName,
TexImageFunctionType functionType, TexFuncValidationSourceType sourceType,
GLenum target, GLint level, GLenum internalformat, GLsizei width, GLsizei
height, GLsizei depth, GLint border, GLenum format, GLenum type, GLint xoffset,
GLint yoffset)
On 2015/12/28 19:30:27, Zhenyao Mo wrote:
> It is weird that you upgrade to depth but didn't do it for zoffset. Because
> either it handles 3D or it does not.  A mixture leads to maintenance cost.

Added depth now. It will be used when validating texSubImage3D. I will do it
next.

https://codereview.chromium.org/1543233002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp:4231:
synthesizeGLError(GL_INVALID_OPERATION, "texImage2D", "attempted to modify
immutable texture");
On 2015/12/28 19:30:27, Zhenyao Mo wrote:
> use functionName here.

Done.

https://codereview.chromium.org/1543233002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp:4235: if
(isNPOTStrict() && level && WebGLTexture::isNPOT(width, height)) {
On 2015/12/28 19:30:27, Zhenyao Mo wrote:
> Worth add a comment that depth is for WebGL 2 only where iSNPOTStrict() is
> always false.

Done.

[Zhenyao Mo, 2015-12-29 22:26:56]

lgtm

[qiankun, 2015-12-30 00:45:37]

The CQ bit was checked by qiankun.miao@intel.com

[commit-bot: I haz the power, 2015-12-30 00:45:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1543233002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1543233002/20001

[commit-bot: I haz the power, 2015-12-30 01:26:21]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-12-30 01:27:09]

Description was changed from

==========
Unify validation for TexImage{2D|3D}

BUG=295792
TEST=No functional changes, current conformance test covers the test.
==========

to

==========
Unify validation for TexImage{2D|3D}

BUG=295792
TEST=No functional changes, current conformance test covers the test.

Committed: https://crrev.com/3a896bbd9e244c502b04bbd1ec77a1ff34149799
Cr-Commit-Position: refs/heads/master@{#367122}
==========

[commit-bot: I haz the power, 2015-12-30 01:27:10]

Patchset 2 (id:??) landed as
https://crrev.com/3a896bbd9e244c502b04bbd1ec77a1ff34149799
Cr-Commit-Position: refs/heads/master@{#367122}