Fix a bug in drawBuffers optimization.

Fix a bug in drawBuffers optimization.

We filter out draw buffers that are not written to and are not NONE, but
currently we only do that on Draw calls, not Clear.  This CL fixes this.

BUG=678153
TEST=webgl_conformance, test case from the bug
R=kbr@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

Review-Url: https://codereview.chromium.org/2611403002
Cr-Commit-Position: refs/heads/master@{#442131}
Committed: https://chromium.googlesource.com/chromium/src/+/3e2c0df8485250d7bd94ab90fc6a260bf89cef71

[Zhenyao Mo, 2017-01-06 22:30:16]

Description was changed from

==========
Fix a bug in drawBuffers optimization.

We filter out draw buffers that are not written to and are not NONE, but
currently we only do that on Draw calls, not Clear.  This CL fixes this.

BUG=678153
TEST=webgl_conformance, test case from the bug
R=kbr@chromium.org
==========

to

==========
Fix a bug in drawBuffers optimization.

We filter out draw buffers that are not written to and are not NONE, but
currently we only do that on Draw calls, not Clear.  This CL fixes this.

BUG=678153
TEST=webgl_conformance, test case from the bug
R=kbr@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, 2017-01-06 22:30:37]

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

[commit-bot: I haz the power, 2017-01-06 22:31:14]

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

[Zhenyao Mo, 2017-01-06 22:32:03]

WebGL conformance test to catch this bug will be uploaded shortly after.

kbr: Please review.
Others: FYI (feel free to review).

[Ken Russell (switch to Gerrit), 2017-01-06 22:43:07]

Basically looks good.

Any chance of a unit test for this change?

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

https://codereview.chromium.org/2611403002/diff/1/gpu/command_buffer/service/...
gpu/command_buffer/service/framebuffer_manager.cc:558: }
Per offline discussion: this is the same block of code as in
ValidateAndAdjustDrawBuffers. Could you at least refactor it into some sort of
AdjustDrawBuffersImpl and pass in draw_buffer_bound_mask_ as "mask"?

[commit-bot: I haz the power, 2017-01-06 23:38:31]

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

[commit-bot: I haz the power, 2017-01-06 23:38:32]

Dry run: This issue passed the CQ dry run.

[Zhenyao Mo, 2017-01-07 00:22:24]

Conformance test is uploaded https://github.com/KhronosGroup/WebGL/pull/2246

As you can see, it take a quite some steps to create the state where this bug
manifest, so it seems to me a unit test isn't the best fit whereas a WebGL
conformance test will prevent from regression.

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

https://codereview.chromium.org/2611403002/diff/1/gpu/command_buffer/service/...
gpu/command_buffer/service/framebuffer_manager.cc:558: }
On 2017/01/06 22:43:07, Ken Russell wrote:
> Per offline discussion: this is the same block of code as in
> ValidateAndAdjustDrawBuffers. Could you at least refactor it into some sort of
> AdjustDrawBuffersImpl and pass in draw_buffer_bound_mask_ as "mask"?

Done.

[Zhenyao Mo, 2017-01-07 00:22:29]

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

[commit-bot: I haz the power, 2017-01-07 00:22:52]

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), 2017-01-07 00:24:35]

Sounds good. LGTM

[Zhenyao Mo, 2017-01-07 00:40:19]

The CQ bit was unchecked by zmo@chromium.org

[Zhenyao Mo, 2017-01-07 00:40:45]

The CQ bit was checked by zmo@chromium.org

[commit-bot: I haz the power, 2017-01-07 00:41:15]

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, 2017-01-07 01:38:39]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1483749645821450,
"parent_rev": "355fef64c80c70bf0787534f029ee84c7e58d171", "commit_rev":
"3e2c0df8485250d7bd94ab90fc6a260bf89cef71"}

[commit-bot: I haz the power, 2017-01-07 01:39:10]

Description was changed from

==========
Fix a bug in drawBuffers optimization.

We filter out draw buffers that are not written to and are not NONE, but
currently we only do that on Draw calls, not Clear.  This CL fixes this.

BUG=678153
TEST=webgl_conformance, test case from the bug
R=kbr@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

==========
Fix a bug in drawBuffers optimization.

We filter out draw buffers that are not written to and are not NONE, but
currently we only do that on Draw calls, not Clear.  This CL fixes this.

BUG=678153
TEST=webgl_conformance, test case from the bug
R=kbr@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

Review-Url: https://codereview.chromium.org/2611403002
Cr-Commit-Position: refs/heads/master@{#442131}
Committed:
https://chromium.googlesource.com/chromium/src/+/3e2c0df8485250d7bd94ab90fc6a...
==========

[commit-bot: I haz the power, 2017-01-07 01:39:11]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/3e2c0df8485250d7bd94ab90fc6a...