Command buffer: generate correct GL error for invalid level for compressed texture

Command buffer: generate correct GL error for invalid level for compressed texture

This change can fix some bugs in negativetextureapi.html in WebGL deqp.

BUG=565347
TEST=deqp/functional/gles3/negativetextureapi.html
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel

Committed: https://crrev.com/9ad4a35e2a01d16e8b58a86e417e796ecc81edd1
Cr-Commit-Position: refs/heads/master@{#379697}

[yunchao, 2016-03-02 09:45:48]

Description was changed from

==========
Command buffer: generates correct GL error for invalid level for compressed
texture

BUG=
==========

to

==========
Command buffer: generates correct GL error for invalid level for compressed
texture

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[yunchao, 2016-03-02 09:47:41]

Description was changed from

==========
Command buffer: generates correct GL error for invalid level for compressed
texture

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Command buffer: generates correct GL error for invalid level for compressed
texture

This change can fix some bugs in negativetextureapi.html 
in WebGL deqp.

BUG=565347
TEST=deqp/functional/gles3/negativetextureapi.html
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[yunchao, 2016-03-02 09:47:41]

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

[yunchao, 2016-03-02 09:47:59]

Description was changed from

==========
Command buffer: generates correct GL error for invalid level for compressed
texture

This change can fix some bugs in negativetextureapi.html 
in WebGL deqp.

BUG=565347
TEST=deqp/functional/gles3/negativetextureapi.html
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Command buffer: generates correct GL error for invalid level for compressed
texture

This change can fix some bugs in negativetextureapi.html in WebGL deqp.

BUG=565347
TEST=deqp/functional/gles3/negativetextureapi.html
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[yunchao, 2016-03-02 12:27:04]

It should generate INVALID_VALUE, instead of INVALID_OPERAION. See the
statements for 'level' argument for texSubImage{2D|3D}, compressed texture
related API follow the same criteria:
https://www.khronos.org/registry/gles/specs/3.0/es_spec_3.0.4.pdf#nameddest=s...


PTAL. Thanks a lot!

[yunchao, 2016-03-02 12:27:24]

Description was changed from

==========
Command buffer: generates correct GL error for invalid level for compressed
texture

This change can fix some bugs in negativetextureapi.html in WebGL deqp.

BUG=565347
TEST=deqp/functional/gles3/negativetextureapi.html
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Command buffer: generate correct GL error for invalid level for compressed
texture

This change can fix some bugs in negativetextureapi.html in WebGL deqp.

BUG=565347
TEST=deqp/functional/gles3/negativetextureapi.html
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[Zhenyao Mo, 2016-03-02 17:42:08]

On 2016/03/02 12:27:04, yunchao wrote:
> It should generate INVALID_VALUE, instead of INVALID_OPERAION. See the
> statements for 'level' argument for texSubImage{2D|3D}, compressed texture
> related API follow the same criteria:
>
https://www.khronos.org/registry/gles/specs/3.0/es_spec_3.0.4.pdf#nameddest=s...
> 
> 
> PTAL. Thanks a lot!

Not really.  If level < 0 or level > log2(MAX_SIZE) then INVALID_VALUE, but if 0
<= level <= log2(MAX_SIZE) but not defined by TexImage calls, then it's
INVALID_OPERATION.

Command buffer may not check the INVALID_VALUE cases though.

[yunchao, 2016-03-04 15:22:11]

On 2016/03/02 17:42:08, Zhenyao Mo wrote:
> On 2016/03/02 12:27:04, yunchao wrote:
> > It should generate INVALID_VALUE, instead of INVALID_OPERAION. See the
> > statements for 'level' argument for texSubImage{2D|3D}, compressed texture
> > related API follow the same criteria:
> >
>
https://www.khronos.org/registry/gles/specs/3.0/es_spec_3.0.4.pdf#nameddest=s...
> > 
> > 
> > PTAL. Thanks a lot!
> 
> Not really.  If level < 0 or level > log2(MAX_SIZE) then INVALID_VALUE, but if
0
> <= level <= log2(MAX_SIZE) but not defined by TexImage calls, then it's
> INVALID_OPERATION.
> 
> Command buffer may not check the INVALID_VALUE cases though.

OK, I added a code snippet to check the INVALID_VALUE for
CompressedTexSubImage{2D|3D} in the latest patch set. PTAL.

[Zhenyao Mo, 2016-03-04 17:39:58]

https://codereview.chromium.org/1749183003/diff/20001/gpu/command_buffer/serv...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/1749183003/diff/20001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:11337: if
(!texture_manager()->ValidForTarget(target, level, width, height, 1)) {
This needs to happen after GetTextureInfoForTarget() so we are sure target is
valid.

[yunchao, 2016-03-07 02:58:19]

Thanks for your review, Zhenyao. Code has been updated accordingly. PTAL.

https://codereview.chromium.org/1749183003/diff/20001/gpu/command_buffer/serv...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/1749183003/diff/20001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:11337: if
(!texture_manager()->ValidForTarget(target, level, width, height, 1)) {
On 2016/03/04 17:39:58, Zhenyao Mo wrote:
> This needs to happen after GetTextureInfoForTarget() so we are sure target is
> valid.

Done.

[yunchao, 2016-03-07 03:15:29]

On 2016/03/07 02:58:19, yunchao wrote:
> Thanks for your review, Zhenyao. Code has been updated accordingly. PTAL.
> 
>
https://codereview.chromium.org/1749183003/diff/20001/gpu/command_buffer/serv...
> File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):
> 
>
https://codereview.chromium.org/1749183003/diff/20001/gpu/command_buffer/serv...
> gpu/command_buffer/service/gles2_cmd_decoder.cc:11337: if
> (!texture_manager()->ValidForTarget(target, level, width, height, 1)) {
> On 2016/03/04 17:39:58, Zhenyao Mo wrote:
> > This needs to happen after GetTextureInfoForTarget() so we are sure target
is
> > valid.
> 
> Done.

The caller of DoCompressedTexSubImage2D has checked the "target" argument. So
the patchset 2 is OK. The patchset 3 is OK too. It is not important of the
sequence of these two functions (GetTextureInfoForTarget() and ValidForTarget())
here.

[Zhenyao Mo, 2016-03-07 17:50:18]

You are correct.

lgtm

[yunchao, 2016-03-08 00:08:40]

The CQ bit was checked by yunchao.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-03-08 00:09:05]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1749183003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1749183003/40001

[commit-bot: I haz the power, 2016-03-08 00:55:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-08 00:55:48]

Dry run: This issue passed the CQ dry run.

[yunchao, 2016-03-08 01:35:55]

The CQ bit was checked by yunchao.he@intel.com

[commit-bot: I haz the power, 2016-03-08 01:36:36]

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

[commit-bot: I haz the power, 2016-03-08 01:43:28]

Description was changed from

==========
Command buffer: generate correct GL error for invalid level for compressed
texture

This change can fix some bugs in negativetextureapi.html in WebGL deqp.

BUG=565347
TEST=deqp/functional/gles3/negativetextureapi.html
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Command buffer: generate correct GL error for invalid level for compressed
texture

This change can fix some bugs in negativetextureapi.html in WebGL deqp.

BUG=565347
TEST=deqp/functional/gles3/negativetextureapi.html
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[commit-bot: I haz the power, 2016-03-08 01:43:29]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-03-08 01:44:42]

Description was changed from

==========
Command buffer: generate correct GL error for invalid level for compressed
texture

This change can fix some bugs in negativetextureapi.html in WebGL deqp.

BUG=565347
TEST=deqp/functional/gles3/negativetextureapi.html
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Command buffer: generate correct GL error for invalid level for compressed
texture

This change can fix some bugs in negativetextureapi.html in WebGL deqp.

BUG=565347
TEST=deqp/functional/gles3/negativetextureapi.html
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel

Committed: https://crrev.com/9ad4a35e2a01d16e8b58a86e417e796ecc81edd1
Cr-Commit-Position: refs/heads/master@{#379697}
==========

[commit-bot: I haz the power, 2016-03-08 01:44:43]

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