Initialize depth texture using tex{Sub}Image2D except for ANGLE ES2

Initialize depth texture using tex{Sub}Image2D except for ANGLE ES2

ANGLE_depth_texture does not allow uploading depth texture through tex{Sub}Image2D.

BUG=666384
TEST=Linux Intel bot on GPU FYI
R=jmadill@chromium.org
NOTRY=true
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/79d05bf972b24b752029c2d9c727bfe975f2f208
Cr-Commit-Position: refs/heads/master@{#432970}

[Zhenyao Mo, 2016-11-17 19:17:14]

Description was changed from

==========
Initialize depth texture using tex{Sub}Image2D except for ANGLE ES2

ANGLE_depth_texture does not allow uploading depth texture through
tex{Sub}Image2D.

BUG=666384
TEST=Linux Intel bot on GPU FYI
R=jmadill@chromium.org
==========

to

==========
Initialize depth texture using tex{Sub}Image2D except for ANGLE ES2

ANGLE_depth_texture does not allow uploading depth texture through
tex{Sub}Image2D.

BUG=666384
TEST=Linux Intel bot on GPU FYI
R=jmadill@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Zhenyao Mo, 2016-11-17 19:17:38]

Jamie: PTAL

[Zhenyao Mo, 2016-11-17 19:17:43]

The CQ bit was checked by zmo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-17 19:18:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ken Russell (switch to Gerrit), 2016-11-17 19:23:47]

kbr@chromium.org changed reviewers:
+ kbr@chromium.org

[Ken Russell (switch to Gerrit), 2016-11-17 19:23:48]

I see. Yes, I can imagine that that driver might report an incomplete
framebuffer with only a depth attachment attached.

LGTM but let's let either Jamie or Kai look at this change too.

[Jamie Madill, 2016-11-17 19:24:41]

jmadill@chromium.org changed reviewers:
- kbr@chromium.org

[Jamie Madill, 2016-11-17 19:24:41]

lgtm, but not an owner. the command buffer validation code will catch invalid
cases presumably, like es2+no extension+trying to subimage a depth texture.

[Kai Ninomiya, 2016-11-17 20:41:22]

kainino@chromium.org changed reviewers:
+ kainino@chromium.org

[Kai Ninomiya, 2016-11-17 20:41:22]

https://codereview.chromium.org/2510163003/diff/1/gpu/command_buffer/service/...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/2510163003/diff/1/gpu/command_buffer/service/...
gpu/command_buffer/service/gles2_cmd_decoder.cc:12204:
feature_info_->gl_version_info().is_es2) {
I think the logic here is valid but doesn't quite make sense. If
angle_depth_texture is false, I think neither path will work. Perhaps it should
throw an error in that case:

if ((channels & GLES2Util::kDepth) != 0 &&
    !feature_info_->feature_flags().angle_depth_texture) {
  // this is an error
}

if ((channels & GLES2Util::kDepth) != 0 &&
    feature_info_->gl_version_info().is_es2) {

[Zhenyao Mo, 2016-11-17 20:46:58]

https://codereview.chromium.org/2510163003/diff/1/gpu/command_buffer/service/...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/2510163003/diff/1/gpu/command_buffer/service/...
gpu/command_buffer/service/gles2_cmd_decoder.cc:12204:
feature_info_->gl_version_info().is_es2) {
On 2016/11/17 20:41:22, Kai Ninomiya wrote:
> I think the logic here is valid but doesn't quite make sense. If
> angle_depth_texture is false, I think neither path will work. Perhaps it
should
> throw an error in that case:
> 
> if ((channels & GLES2Util::kDepth) != 0 &&
>     !feature_info_->feature_flags().angle_depth_texture) {
>   // this is an error
> }
> 
> if ((channels & GLES2Util::kDepth) != 0 &&
>     feature_info_->gl_version_info().is_es2) {

You are right.  I can move feature_info_->feature_flags().angle_depth_texture
into the if body as a DCHECK

[Zhenyao Mo, 2016-11-17 20:51:45]

Description was changed from

==========
Initialize depth texture using tex{Sub}Image2D except for ANGLE ES2

ANGLE_depth_texture does not allow uploading depth texture through
tex{Sub}Image2D.

BUG=666384
TEST=Linux Intel bot on GPU FYI
R=jmadill@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Initialize depth texture using tex{Sub}Image2D except for ANGLE ES2

ANGLE_depth_texture does not allow uploading depth texture through
tex{Sub}Image2D.

BUG=666384
TEST=Linux Intel bot on GPU FYI
R=jmadill@chromium.org
NOTRY=true
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Zhenyao Mo, 2016-11-17 20:51:50]

The CQ bit was unchecked by zmo@chromium.org

[Zhenyao Mo, 2016-11-17 20:51:57]

The CQ bit was checked by zmo@chromium.org

[commit-bot: I haz the power, 2016-11-17 20:52:27]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-17 21:41:57]

Description was changed from

==========
Initialize depth texture using tex{Sub}Image2D except for ANGLE ES2

ANGLE_depth_texture does not allow uploading depth texture through
tex{Sub}Image2D.

BUG=666384
TEST=Linux Intel bot on GPU FYI
R=jmadill@chromium.org
NOTRY=true
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Initialize depth texture using tex{Sub}Image2D except for ANGLE ES2

ANGLE_depth_texture does not allow uploading depth texture through
tex{Sub}Image2D.

BUG=666384
TEST=Linux Intel bot on GPU FYI
R=jmadill@chromium.org
NOTRY=true
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[commit-bot: I haz the power, 2016-11-17 21:41:58]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-11-17 21:45:11]

Description was changed from

==========
Initialize depth texture using tex{Sub}Image2D except for ANGLE ES2

ANGLE_depth_texture does not allow uploading depth texture through
tex{Sub}Image2D.

BUG=666384
TEST=Linux Intel bot on GPU FYI
R=jmadill@chromium.org
NOTRY=true
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Initialize depth texture using tex{Sub}Image2D except for ANGLE ES2

ANGLE_depth_texture does not allow uploading depth texture through
tex{Sub}Image2D.

BUG=666384
TEST=Linux Intel bot on GPU FYI
R=jmadill@chromium.org
NOTRY=true
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/79d05bf972b24b752029c2d9c727bfe975f2f208
Cr-Commit-Position: refs/heads/master@{#432970}
==========

[commit-bot: I haz the power, 2016-11-17 21:45:13]

Patchset 1 (id:??) landed as
https://crrev.com/79d05bf972b24b752029c2d9c727bfe975f2f208
Cr-Commit-Position: refs/heads/master@{#432970}