Move WebGL1 attachment image type / attachment point matching check to command buffer.

Move WebGL1 attachment image type / attachment point matching check to command buffer.

Otherwise we will have to validate all texture calls to make sure we
cache the right texture format/type for the check.

BUG=570453
TEST=gpu_unittests,webgl_conformance
R=kbr@chromium.org,bajones@chromium.org,piman@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/08c9d1ef6670f512e6b81a2316973ba7be9a3860
Cr-Commit-Position: refs/heads/master@{#374440}

[Zhenyao Mo, 2016-02-07 22:03:39]

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

[commit-bot: I haz the power, 2016-02-07 22:03:45]

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

[Zhenyao Mo, 2016-02-07 22:03:48]

Patchset #2 (id:20001) has been deleted

[Zhenyao Mo, 2016-02-07 22:03:54]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2016-02-07 23:00:58]

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

[commit-bot: I haz the power, 2016-02-07 23:00:58]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Zhenyao Mo, 2016-02-08 04:56:56]

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

[commit-bot: I haz the power, 2016-02-08 04:57:21]

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

[Zhenyao Mo, 2016-02-08 04:59:06]

kbr,bajones: blink side
piman: command buffer side

PTAL

[commit-bot: I haz the power, 2016-02-08 05:34:00]

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

[commit-bot: I haz the power, 2016-02-08 05:34:01]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[piman, 2016-02-08 17:44:39]

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

https://codereview.chromium.org/1673113002/diff/60001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:10080:
glFramebufferTexture2DEXT(GL_DRAW_FRAMEBUFFER_EXT, GL_STENCIL_ATTACHMENT,
Do we need to split this one too?

[Zhenyao Mo, 2016-02-08 18:52:48]

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

https://codereview.chromium.org/1673113002/diff/60001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:10080:
glFramebufferTexture2DEXT(GL_DRAW_FRAMEBUFFER_EXT, GL_STENCIL_ATTACHMENT,
On 2016/02/08 17:44:39, piman wrote:
> Do we need to split this one too?

What do you mean?  This one is already split into depth and stencil.

[piman, 2016-02-08 20:49:12]

lgtm

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

https://codereview.chromium.org/1673113002/diff/60001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:10080:
glFramebufferTexture2DEXT(GL_DRAW_FRAMEBUFFER_EXT, GL_STENCIL_ATTACHMENT,
On 2016/02/08 18:52:48, Zhenyao Mo wrote:
> On 2016/02/08 17:44:39, piman wrote:
> > Do we need to split this one too?
> 
> What do you mean?  This one is already split into depth and stencil.

Huh, must have misread, apologies.

[Ken Russell (switch to Gerrit), 2016-02-08 21:21:57]

Description was changed from

==========
Move WebGL1 attachment image type / attachment point matching check to command
buffer.

Otherwise we will have to validate all texture alls to make sure we
cache the right texture format/type for the check.

BUG=570453
TEST=gpu_unittests,webgl_conformance
R=kbr@chromium.org,bajones@chromium.org,piman@chromium.org
==========

to

==========
Move WebGL1 attachment image type / attachment point matching check to command
buffer.

Otherwise we will have to validate all texture calls to make sure we
cache the right texture format/type for the check.

BUG=570453
TEST=gpu_unittests,webgl_conformance
R=kbr@chromium.org,bajones@chromium.org,piman@chromium.org
==========

[Ken Russell (switch to Gerrit), 2016-02-08 21:39:46]

Great work and cleanup of the Blink-side code. LGTM

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

https://codereview.chromium.org/1673113002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp:109:
extensionsUtil->ensureExtensionEnabled("GL_OES_packed_depth_stencil");
I wonder whether these checks could be replaced with:
  ASSERT(extensionsUtil->supportsExtension("GL_OES_packed_depth_stencil"));

since, in WebGLRenderingContext::Factory::create, the DrawingBuffer is created
after the WebGraphicsContext3D, and after the new early-out check has been done
for the presence of GL_OES_packed_depth_stencil.

https://codereview.chromium.org/1673113002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTest.cpp
(left):

https://codereview.chromium.org/1673113002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTest.cpp:613:
DepthStencilTestCase(bool requestStencil, bool requestDepth, int
expectedRenderBuffers, bool expectDepthStencil, const char* const testCaseName)
Were these arguments out of order all this time?

[Zhenyao Mo, 2016-02-09 17:20:11]

Description was changed from

==========
Move WebGL1 attachment image type / attachment point matching check to command
buffer.

Otherwise we will have to validate all texture calls to make sure we
cache the right texture format/type for the check.

BUG=570453
TEST=gpu_unittests,webgl_conformance
R=kbr@chromium.org,bajones@chromium.org,piman@chromium.org
==========

to

==========
Move WebGL1 attachment image type / attachment point matching check to command
buffer.

Otherwise we will have to validate all texture calls to make sure we
cache the right texture format/type for the check.

BUG=570453
TEST=gpu_unittests,webgl_conformance
R=kbr@chromium.org,bajones@chromium.org,piman@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Zhenyao Mo, 2016-02-09 17:21:23]

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

https://codereview.chromium.org/1673113002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp:109:
extensionsUtil->ensureExtensionEnabled("GL_OES_packed_depth_stencil");
On 2016/02/08 21:39:45, Ken Russell wrote:
> I wonder whether these checks could be replaced with:
>   ASSERT(extensionsUtil->supportsExtension("GL_OES_packed_depth_stencil"));
> 
> since, in WebGLRenderingContext::Factory::create, the DrawingBuffer is created
> after the WebGraphicsContext3D, and after the new early-out check has been
done
> for the presence of GL_OES_packed_depth_stencil.

Done.

[Zhenyao Mo, 2016-02-09 17:21:27]

The CQ bit was checked by zmo@chromium.org

[Zhenyao Mo, 2016-02-09 17:21:28]

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

[commit-bot: I haz the power, 2016-02-09 17:21:51]

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

[commit-bot: I haz the power, 2016-02-09 20:16:01]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2016-02-09 20:17:13]

Description was changed from

==========
Move WebGL1 attachment image type / attachment point matching check to command
buffer.

Otherwise we will have to validate all texture calls to make sure we
cache the right texture format/type for the check.

BUG=570453
TEST=gpu_unittests,webgl_conformance
R=kbr@chromium.org,bajones@chromium.org,piman@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Move WebGL1 attachment image type / attachment point matching check to command
buffer.

Otherwise we will have to validate all texture calls to make sure we
cache the right texture format/type for the check.

BUG=570453
TEST=gpu_unittests,webgl_conformance
R=kbr@chromium.org,bajones@chromium.org,piman@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/08c9d1ef6670f512e6b81a2316973ba7be9a3860
Cr-Commit-Position: refs/heads/master@{#374440}
==========

[commit-bot: I haz the power, 2016-02-09 20:17:14]

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