Enforce no uncleared OES external textures. Clear in AVDA, not GVD.

Enforce no uncleared OES external textures. Clear in AVDA, not GVD.

A few of us on the team have been bitten by the fact that GVD is
the one clearing textures instead of AVDA when it comes to OES
external textures. Lets save a few hours of debugging for someone
in the future.

OES external textures are now always pre-cleared by AVDA (the only
VDA using these) and checked by GpuVideoDecoder. Instead of an opaque
GL_INVALID_ENUM error in GLES2DecoderImpl::PrepareTexturesForRender()
a DCHECK will now fire in ClearLevel() indicating the exact problem.

BUG=662251
TEST=implement fancy new feature, notice there are no holes in foot.
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/d3704fcabb140cd633890479194658742863c881
Cr-Commit-Position: refs/heads/master@{#431064}

[DaleCurtis, 2016-11-08 00:58:48]

Description was changed from

==========
Enforce no uncleared OES external textures. Clear in AVDA, not GVD.

A few of us on the team have been bitten by the fact that GVD is
the one clearing textures instead of AVDA when it comes to OES
external textures. Lets save a few hours of debugging for someone
in the future.

OES external textures are now always pre-cleared by AVDA (the only
VDA using these) and checked by GpuVideoDecoder. Instead of an opaque
GL_INVALID_ENUM error in GLES2DecoderImpl::PrepareTexturesForRender()
a DCHECK will now fire in ClearLevel() indicating the exact problem.

BUG=662251
TEST=implement fancy new feature, notice there are no holes in foot.
==========

to

==========
Enforce no uncleared OES external textures. Clear in AVDA, not GVD.

A few of us on the team have been bitten by the fact that GVD is
the one clearing textures instead of AVDA when it comes to OES
external textures. Lets save a few hours of debugging for someone
in the future.

OES external textures are now always pre-cleared by AVDA (the only
VDA using these) and checked by GpuVideoDecoder. Instead of an opaque
GL_INVALID_ENUM error in GLES2DecoderImpl::PrepareTexturesForRender()
a DCHECK will now fire in ClearLevel() indicating the exact problem.

BUG=662251
TEST=implement fancy new feature, notice there are no holes in foot.
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
==========

[DaleCurtis, 2016-11-08 00:59:41]

dalecurtis@chromium.org changed reviewers:
+ piman@chromium.org, sandersd@chromium.org

[DaleCurtis, 2016-11-08 00:59:42]

Found while implementing surface changes for Android MediaCodec:
https://codereview.chromium.org/2461073002

Thanks watk@ for the solution.

[sandersd (OOO until July 31), 2016-11-08 01:05:44]

lgtm.

I would like someone to double-check whether the concept of clearing makes sense
when combined with level images, though. It's possible that what we actually
want to check is (cleared || has_image).

https://codereview.chromium.org/2481943003/diff/1/media/gpu/ipc/service/gpu_v...
File media/gpu/ipc/service/gpu_video_decode_accelerator.cc (right):

https://codereview.chromium.org/2481943003/diff/1/media/gpu/ipc/service/gpu_v...
media/gpu/ipc/service/gpu_video_decode_accelerator.cc:523: // OES textures are a
special case and expected to already be cleared.
Should say 'External textures are'.

[DaleCurtis, 2016-11-08 01:05:54]

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

[DaleCurtis, 2016-11-08 01:05:59]

The CQ bit was unchecked by dalecurtis@chromium.org

[DaleCurtis, 2016-11-08 01:08:08]

https://codereview.chromium.org/2481943003/diff/1/media/gpu/ipc/service/gpu_v...
File media/gpu/ipc/service/gpu_video_decode_accelerator.cc (right):

https://codereview.chromium.org/2481943003/diff/1/media/gpu/ipc/service/gpu_v...
media/gpu/ipc/service/gpu_video_decode_accelerator.cc:523: // OES textures are a
special case and expected to already be cleared.
On 2016/11/08 at 01:05:44, sandersd wrote:
> Should say 'External textures are'.

Done.

[DaleCurtis, 2016-11-08 01:08:21]

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

[commit-bot: I haz the power, 2016-11-08 01:09:07]

Dry run: 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-08 01:33:14]

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

[commit-bot: I haz the power, 2016-11-08 01:33:15]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[DaleCurtis, 2016-11-08 19:03:11]

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

[commit-bot: I haz the power, 2016-11-08 19:03:51]

Dry run: 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-08 20:51:57]

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

[commit-bot: I haz the power, 2016-11-08 20:51:59]

Dry run: This issue passed the CQ dry run.

[DaleCurtis, 2016-11-09 00:28:06]

piman: ping for gpu/ changes.

[piman, 2016-11-09 16:27:44]

lgtm

[DaleCurtis, 2016-11-09 20:04:18]

The CQ bit was checked by dalecurtis@chromium.org

[DaleCurtis, 2016-11-09 20:04:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
sandersd@chromium.org
Link to the patchset: https://codereview.chromium.org/2481943003/#ps40001
(title: "Fix mismerge.")

[commit-bot: I haz the power, 2016-11-09 20:04:50]

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-09 22:44:45]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-09 22:58:27]

Description was changed from

==========
Enforce no uncleared OES external textures. Clear in AVDA, not GVD.

A few of us on the team have been bitten by the fact that GVD is
the one clearing textures instead of AVDA when it comes to OES
external textures. Lets save a few hours of debugging for someone
in the future.

OES external textures are now always pre-cleared by AVDA (the only
VDA using these) and checked by GpuVideoDecoder. Instead of an opaque
GL_INVALID_ENUM error in GLES2DecoderImpl::PrepareTexturesForRender()
a DCHECK will now fire in ClearLevel() indicating the exact problem.

BUG=662251
TEST=implement fancy new feature, notice there are no holes in foot.
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

==========
Enforce no uncleared OES external textures. Clear in AVDA, not GVD.

A few of us on the team have been bitten by the fact that GVD is
the one clearing textures instead of AVDA when it comes to OES
external textures. Lets save a few hours of debugging for someone
in the future.

OES external textures are now always pre-cleared by AVDA (the only
VDA using these) and checked by GpuVideoDecoder. Instead of an opaque
GL_INVALID_ENUM error in GLES2DecoderImpl::PrepareTexturesForRender()
a DCHECK will now fire in ClearLevel() indicating the exact problem.

BUG=662251
TEST=implement fancy new feature, notice there are no holes in foot.
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/d3704fcabb140cd633890479194658742863c881
Cr-Commit-Position: refs/heads/master@{#431064}
==========

[commit-bot: I haz the power, 2016-11-09 22:58:28]

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