Remove duplicate validateTexFuncLevel

Remove duplicate validateTexFuncLevel

validateTexFuncLevel is called twice from validateTexFunc and
validateTexFuncParameters. Move validateTexFuncLevel out of
validateTexFuncParameters.

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

[qiankun, 2015-09-01 14:15:02]

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

[qiankun, 2015-09-01 14:15:03]

PTAL.

[Zhenyao Mo, 2015-09-01 21:26:43]

On 2015/09/01 14:15:03, qiankun wrote:
> PTAL.

LGTM

[Ken Russell (switch to Gerrit), 2015-09-01 22:52:49]

LGTM too

Please double-check the conformance tests and make sure there are tests
verifying texSubImage2D with out-of-range values for the texture's level (less
than 0, and greater than the maximum possible level for both 2D and cube map
textures -- see the computation of WebGLRenderingContextBase::m_maxTextureLevel
and m_maxCubeMapTextureLevel). Thanks.

https://codereview.chromium.org/1326653002/diff/1/Source/modules/webgl/WebGLR...
File Source/modules/webgl/WebGLRenderingContextBase.cpp (left):

https://codereview.chromium.org/1326653002/diff/1/Source/modules/webgl/WebGLR...
Source/modules/webgl/WebGLRenderingContextBase.cpp:5517: bool
WebGLRenderingContextBase::validateTexFuncData(const char* functionName, GLint
level, GLsizei width, GLsizei height, GLenum internalformat, GLenum format,
GLenum type, DOMArrayBufferView* pixels, NullDisposition disposition)
It's surprising that this argument was unused. I wonder whether the validation
was changed recently and some check was removed. The command buffer code has
back-to-back checks involving the internal_format in
TextureManager::ValidateTextureParameters. Not sure what the second one checks
that the first one doesn't.

[qiankun, 2015-09-02 09:49:07]

Thanks for review. I updated conformance test to check texture level in
texSubImage2D is in valid range:
https://github.com/KhronosGroup/WebGL/pull/1173.

https://codereview.chromium.org/1326653002/diff/1/Source/modules/webgl/WebGLR...
File Source/modules/webgl/WebGLRenderingContextBase.cpp (left):

https://codereview.chromium.org/1326653002/diff/1/Source/modules/webgl/WebGLR...
Source/modules/webgl/WebGLRenderingContextBase.cpp:5517: bool
WebGLRenderingContextBase::validateTexFuncData(const char* functionName, GLint
level, GLsizei width, GLsizei height, GLenum internalformat, GLenum format,
GLenum type, DOMArrayBufferView* pixels, NullDisposition disposition)
On 2015/09/01 22:52:49, Ken Russell wrote:
> It's surprising that this argument was unused. I wonder whether the validation
> was changed recently and some check was removed. The command buffer code has
> back-to-back checks involving the internal_format in
> TextureManager::ValidateTextureParameters. Not sure what the second one checks
> that the first one doesn't.

I move formats and types validation out of validateTexFuncData in
https://codereview.chromium.org/1311413003/. So internalformat isn't used any
more. internal_format/format/type is checked in
WebGLRenderingContextBase::validateTexFuncParameters now.

[Ken Russell (switch to Gerrit), 2015-09-02 20:58:18]

The CQ bit was checked by kbr@chromium.org

[commit-bot: I haz the power, 2015-09-02 20:58:34]

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

[commit-bot: I haz the power, 2015-09-02 23:07:55]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=201679