Clean up DrawingBuffer.

Clean up DrawingBuffer.

This CL is a refactor and should have no functional effect.

Changes in this CL:
1. The constructor of DrawingBuffer now takes wantDepth and wantStencil
arguments. These parameters, which never change, no longer need to be passed to
Initialize() or reset().
2. Renamed requiresRGBEmulation() -> requiresAlphaMask. Renamed
attachColorBufferToCurrentFBO() -> attachColorBufferToReadFramebuffer().
3. Added documentation.
4. Moved member initialization into the header file.
5. There used to be multiple methods named resize*, some of which were
incorrectly named, and some of which had odd dependencies on each other. The
methods resizeMultisampleFramebuffer() and resizeDepthStencil() now have no
external dependencies. There is a new method resizeDefaultFramebuffer() which
resizes all buffers attached to the default framebuffer, and all associated
framebuffers.

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

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

[erikchen, 2016-05-04 20:25:09]

Description was changed from

==========
Clean up DrawingBuffer.

This is in preparation for adding some workarounds. The goal is to prevent the
complexity of this class from exploding.

This CL is a refactor and should have no functional effect.

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

to

==========
Clean up DrawingBuffer.

This is in preparation for adding some workarounds. The goal is to prevent the
complexity of this class from exploding.

This CL is a refactor and should have no functional effect.

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

[erikchen, 2016-05-04 20:25:10]

Description was changed from

==========
Clean up DrawingBuffer.

This is in preparation for adding some workarounds. The goal is to prevent the
complexity of this class from exploding.

This CL is a refactor and should have no functional effect.

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

to

==========
Clean up DrawingBuffer.

This is in preparation for adding some workarounds. The goal is to prevent the
complexity of this class from exploding.

This CL is a refactor and should have no functional effect.

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

[erikchen, 2016-05-04 20:29:03]

Description was changed from

==========
Clean up DrawingBuffer.

This is in preparation for adding some workarounds. The goal is to prevent the
complexity of this class from exploding.

This CL is a refactor and should have no functional effect.

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

to

==========
Clean up DrawingBuffer.

This CL is a refactor and should have no functional effect.

Changes in this CL include, but are not limited to:
1.

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

[erikchen, 2016-05-04 20:37:06]

Description was changed from

==========
Clean up DrawingBuffer.

This CL is a refactor and should have no functional effect.

Changes in this CL include, but are not limited to:
1.

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

to

==========
Clean up DrawingBuffer.

This CL is a refactor and should have no functional effect.

Changes in this CL:
1. The constructor of DrawingBuffer now takes wantDepth and wantStencil
arguments. These parameters, which enver change, no longer need to be passed to
Initialize() or reset().
2. Renamed requiresRGBEmulation() -> requiresAlphaMask. Renamed
attachColorBufferToCurrentFBO() -> attachColorBufferToReadFramebuffer().
3. Added documentation.
4. Moved member initialization into the header file.
5. There used to be multiple methods named resize*, some of which were
incorrectly named, and some of which had odd dependencies on each other. The
methods resizeMultisampleFramebuffer() and resizeDepthStencil() now have no
external dependencies. There is a new method resizeDefaultFramebuffer() which
resizes all buffers attached to the default framebuffer, and all associated
framebuffers.

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

[erikchen, 2016-05-04 20:37:17]

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

[commit-bot: I haz the power, 2016-05-04 20:37:34]

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

[erikchen, 2016-05-04 20:40:48]

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

[erikchen, 2016-05-04 20:40:49]

kbr: Please review.

This cleanup is in preparation for the upcoming workarounds and making sure that
the complexity doesn't explode.

[erikchen, 2016-05-04 20:41:04]

Description was changed from

==========
Clean up DrawingBuffer.

This CL is a refactor and should have no functional effect.

Changes in this CL:
1. The constructor of DrawingBuffer now takes wantDepth and wantStencil
arguments. These parameters, which enver change, no longer need to be passed to
Initialize() or reset().
2. Renamed requiresRGBEmulation() -> requiresAlphaMask. Renamed
attachColorBufferToCurrentFBO() -> attachColorBufferToReadFramebuffer().
3. Added documentation.
4. Moved member initialization into the header file.
5. There used to be multiple methods named resize*, some of which were
incorrectly named, and some of which had odd dependencies on each other. The
methods resizeMultisampleFramebuffer() and resizeDepthStencil() now have no
external dependencies. There is a new method resizeDefaultFramebuffer() which
resizes all buffers attached to the default framebuffer, and all associated
framebuffers.

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

to

==========
Clean up DrawingBuffer.

This CL is a refactor and should have no functional effect.

Changes in this CL:
1. The constructor of DrawingBuffer now takes wantDepth and wantStencil
arguments. These parameters, which never change, no longer need to be passed to
Initialize() or reset().
2. Renamed requiresRGBEmulation() -> requiresAlphaMask. Renamed
attachColorBufferToCurrentFBO() -> attachColorBufferToReadFramebuffer().
3. Added documentation.
4. Moved member initialization into the header file.
5. There used to be multiple methods named resize*, some of which were
incorrectly named, and some of which had odd dependencies on each other. The
methods resizeMultisampleFramebuffer() and resizeDepthStencil() now have no
external dependencies. There is a new method resizeDefaultFramebuffer() which
resizes all buffers attached to the default framebuffer, and all associated
framebuffers.

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

[Ken Russell (switch to Gerrit), 2016-05-04 21:13:12]

Thanks for doing this reorganization. LGTM. Couple of minor style comments.

https://codereview.chromium.org/1950053003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp (right):

https://codereview.chromium.org/1950053003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp:242: //
Resolve the multisampled buffer into m_colorBuffertexture.
Space was lost in comment.

https://codereview.chromium.org/1950053003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h (right):

https://codereview.chromium.org/1950053003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h:190: bool
requiresAlphaMask();
It's confusing to understand whether alpha is masked "on" so that alpha writes
are allowed or "off" so that the alpha channel is preserved. Maybe
"requiresAlphaChannelToBePreserved"?

https://codereview.chromium.org/1950053003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h:352: bool
m_scissorEnabled = false;
I don't remember whether these inline default values are compatible with the
style guide. If they are, this is nice.

[danakj, 2016-05-04 21:13:59]

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

[danakj, 2016-05-04 21:14:00]

https://codereview.chromium.org/1950053003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h (right):

https://codereview.chromium.org/1950053003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h:352: bool
m_scissorEnabled = false;
On 2016/05/04 21:13:11, Ken Russell wrote:
> I don't remember whether these inline default values are compatible with the
> style guide. If they are, this is nice.

They are allowed

[erikchen, 2016-05-04 21:17:53]

https://codereview.chromium.org/1950053003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp (right):

https://codereview.chromium.org/1950053003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp:242: //
Resolve the multisampled buffer into m_colorBuffertexture.
On 2016/05/04 21:13:11, Ken Russell wrote:
> Space was lost in comment.

Done.

https://codereview.chromium.org/1950053003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h (right):

https://codereview.chromium.org/1950053003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h:190: bool
requiresAlphaMask();
On 2016/05/04 21:13:11, Ken Russell wrote:
> It's confusing to understand whether alpha is masked "on" so that alpha writes
> are allowed or "off" so that the alpha channel is preserved. Maybe
> "requiresAlphaChannelToBePreserved"?

Done.

[erikchen, 2016-05-04 21:18:00]

The CQ bit was checked by erikchen@chromium.org

[erikchen, 2016-05-04 21:18:01]

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

[commit-bot: I haz the power, 2016-05-04 21:18:21]

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

[commit-bot: I haz the power, 2016-05-04 22:14:08]

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

[commit-bot: I haz the power, 2016-05-04 22:14:10]

Try jobs failed on following builders:
  linux_optional_gpu_tests_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional_...)

[erikchen, 2016-05-05 00:03:57]

The CQ bit was checked by erikchen@chromium.org

[erikchen, 2016-05-05 00:03:57]

The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org
Link to the patchset: https://codereview.chromium.org/1950053003/#ps80001
(title: "Fix completeness checks.")

[commit-bot: I haz the power, 2016-05-05 00:04:31]

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

[commit-bot: I haz the power, 2016-05-05 02:21:45]

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

[commit-bot: I haz the power, 2016-05-05 02:21:47]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[erikchen, 2016-05-05 08:27:13]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2016-05-05 08:27:23]

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

[commit-bot: I haz the power, 2016-05-05 09:10:10]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-05-05 09:11:12]

Description was changed from

==========
Clean up DrawingBuffer.

This CL is a refactor and should have no functional effect.

Changes in this CL:
1. The constructor of DrawingBuffer now takes wantDepth and wantStencil
arguments. These parameters, which never change, no longer need to be passed to
Initialize() or reset().
2. Renamed requiresRGBEmulation() -> requiresAlphaMask. Renamed
attachColorBufferToCurrentFBO() -> attachColorBufferToReadFramebuffer().
3. Added documentation.
4. Moved member initialization into the header file.
5. There used to be multiple methods named resize*, some of which were
incorrectly named, and some of which had odd dependencies on each other. The
methods resizeMultisampleFramebuffer() and resizeDepthStencil() now have no
external dependencies. There is a new method resizeDefaultFramebuffer() which
resizes all buffers attached to the default framebuffer, and all associated
framebuffers.

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

to

==========
Clean up DrawingBuffer.

This CL is a refactor and should have no functional effect.

Changes in this CL:
1. The constructor of DrawingBuffer now takes wantDepth and wantStencil
arguments. These parameters, which never change, no longer need to be passed to
Initialize() or reset().
2. Renamed requiresRGBEmulation() -> requiresAlphaMask. Renamed
attachColorBufferToCurrentFBO() -> attachColorBufferToReadFramebuffer().
3. Added documentation.
4. Moved member initialization into the header file.
5. There used to be multiple methods named resize*, some of which were
incorrectly named, and some of which had odd dependencies on each other. The
methods resizeMultisampleFramebuffer() and resizeDepthStencil() now have no
external dependencies. There is a new method resizeDefaultFramebuffer() which
resizes all buffers attached to the default framebuffer, and all associated
framebuffers.

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

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

[commit-bot: I haz the power, 2016-05-05 09:11:13]

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