Fix blits from multisampled renderbuffers to alpha:false WebGL back buffer.

Fix blits from multisampled renderbuffers to alpha:false WebGL back buffer.

Blitting from a multisampled RGB8 renderbuffer to an alpha:false WebGL
back buffer is supposed to work, and from an RGBA8 renderbuffer is
supposed to not work. The emulation of IOSurface-backed RGB textures
using RGBA textures didn't handle this case.

Add BindTexImage2DWithInternalformatCHROMIUM to the command buffer to
allow internal format to be overridden; currently this is only
supported on macOS. Use this to bind a second texture to the IOSurface
and use it only for BlitFramebuffer calls from WebGL, fixing up the
alpha channel afterward.

Covered by new conformance test in
https://github.com/KhronosGroup/WebGL/pull/2377 .

BUG=699566
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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/2831733003
Cr-Commit-Position: refs/heads/master@{#466523}
Committed: https://chromium.googlesource.com/chromium/src/+/3d4d5d7a9198bea6f0018a36fc6825637f9a12c4

[Ken Russell (switch to Gerrit), 2017-04-19 22:38:42]

Description was changed from

==========
Fix blits from multisampled renderbuffers to alpha:false WebGL back buffer.

Blitting from a multisampled RGB8 renderbuffer to an alpha:false WebGL
back buffer is supposed to work, and from an RGBA8 renderbuffer is
supposed to not work. The emulation of IOSurface-backed RGB textures
using RGBA textures didn't handle this case.

Add BindTexImage2DWithInternalformatCHROMIUM to the command buffer to
allow internal format to be overridden; currently this is only
supported on macOS. Use this to bind a second texture to the IOSurface
and use it only for BlitFramebuffer calls from WebGL, fixing up the
alpha channel afterward.

Covered by new conformance test in
https://github.com/KhronosGroup/WebGL/pull/2377 .

BUG=699566
==========

to

==========
Fix blits from multisampled renderbuffers to alpha:false WebGL back buffer.

Blitting from a multisampled RGB8 renderbuffer to an alpha:false WebGL
back buffer is supposed to work, and from an RGBA8 renderbuffer is
supposed to not work. The emulation of IOSurface-backed RGB textures
using RGBA textures didn't handle this case.

Add BindTexImage2DWithInternalformatCHROMIUM to the command buffer to
allow internal format to be overridden; currently this is only
supported on macOS. Use this to bind a second texture to the IOSurface
and use it only for BlitFramebuffer calls from WebGL, fixing up the
alpha channel afterward.

Covered by new conformance test in
https://github.com/KhronosGroup/WebGL/pull/2377 .

BUG=699566
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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
==========

[Ken Russell (switch to Gerrit), 2017-04-19 22:41:33]

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

[Ken Russell (switch to Gerrit), 2017-04-19 22:41:34]

zmo: please review.

ccameron, erikchen: FYI only, but any comments welcome.

While this CL looks large, it's only due to the number of files touched. The
actual code changes are very small, and are all in support of the small changes
to DrawingBuffer.

While I was touching this code, I expanded the CHROMIUM_image extension
documentation and put a couple of the entry points in the correct place in
gl2extchromium.h.

I'll aim to roll in the new conformance test before this lands, with a failure
suppression, and remove the failure suppression in this CL.

[Zhenyao Mo, 2017-04-19 23:07:34]

LGTM

https://codereview.chromium.org/2831733003/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp (right):

https://codereview.chromium.org/2831733003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp:1353: target,
back_color_buffer_->texture_id, 0);
Please make sure this was bound before SetupRGBEmulationForBlitFramebuffer

[Ken Russell (switch to Gerrit), 2017-04-20 01:49:18]

Thanks for your review.

https://codereview.chromium.org/2831733003/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp (right):

https://codereview.chromium.org/2831733003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp:1353: target,
back_color_buffer_->texture_id, 0);
On 2017/04/19 23:07:34, Zhenyao Mo wrote:
> Please make sure this was bound before SetupRGBEmulationForBlitFramebuffer

Confirmed that it is, in AttachColorBufferToReadFramebuffer, called by
ResizeDefaultFramebuffer and FinishPrepareTextureMailboxGpu.

[ccameron, 2017-04-20 03:38:18]

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

[ccameron, 2017-04-20 03:38:19]

lgtm ... I'd like to know the comparative power implications, but,
realistically, let's fix the bug!

https://codereview.chromium.org/2831733003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp
(right):

https://codereview.chromium.org/2831733003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp:430:
GetDrawingBuffer());
This incurs a call to CGLTexImageIOSurface2D at every blitFramebuffer call,
which may be more expensive than expected (esp, in comparison WRT using an RGB
texture as the backbuffer and doing an explicit copy at swap).

[Ken Russell (switch to Gerrit), 2017-04-20 17:12:33]

https://codereview.chromium.org/2831733003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp
(right):

https://codereview.chromium.org/2831733003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp:430:
GetDrawingBuffer());
On 2017/04/20 03:38:19, ccameron wrote:
> This incurs a call to CGLTexImageIOSurface2D at every blitFramebuffer call,
> which may be more expensive than expected (esp, in comparison WRT using an RGB
> texture as the backbuffer and doing an explicit copy at swap).

No, the RGB texture is cached inside the DrawingBuffer's ColorBuffer instances
as rgb_workaround_texture_id. It definitely isn't supposed to call
CGLTexImageIOSurface2D every time.

[ccameron, 2017-04-20 17:14:39]

On 2017/04/20 17:12:33, Ken Russell wrote:
>
https://codereview.chromium.org/2831733003/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp
> (right):
> 
>
https://codereview.chromium.org/2831733003/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp:430:
> GetDrawingBuffer());
> On 2017/04/20 03:38:19, ccameron wrote:
> > This incurs a call to CGLTexImageIOSurface2D at every blitFramebuffer call,
> > which may be more expensive than expected (esp, in comparison WRT using an
RGB
> > texture as the backbuffer and doing an explicit copy at swap).
> 
> No, the RGB texture is cached inside the DrawingBuffer's ColorBuffer instances
> as rgb_workaround_texture_id. It definitely isn't supposed to call
> CGLTexImageIOSurface2D every time.

Oops, yes, you're right.

[Ken Russell (switch to Gerrit), 2017-04-21 21:32:43]

The CQ bit was checked by kbr@chromium.org

[Ken Russell (switch to Gerrit), 2017-04-21 21:32:43]

The patchset sent to the CQ was uploaded after l-g-t-m from zmo@chromium.org,
ccameron@chromium.org
Link to the patchset: https://codereview.chromium.org/2831733003/#ps40001
(title: "Removed failure expectation for associated test.")

[commit-bot: I haz the power, 2017-04-21 21:33:06]

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-04-21 22:54:56]

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

[commit-bot: I haz the power, 2017-04-21 22:54:58]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Ken Russell (switch to Gerrit), 2017-04-21 23:42:29]

The CQ bit was checked by kbr@chromium.org

[Ken Russell (switch to Gerrit), 2017-04-21 23:42:30]

The patchset sent to the CQ was uploaded after l-g-t-m from zmo@chromium.org,
ccameron@chromium.org
Link to the patchset: https://codereview.chromium.org/2831733003/#ps60001
(title: "Add PLATFORM_EXPORT to fix link failure on Windows.")

[commit-bot: I haz the power, 2017-04-21 23:43:09]

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-04-22 02:09:07]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1492818149750880,
"parent_rev": "9a47d86f8e4000d54119e7f860b99684bf72b5ca", "commit_rev":
"3d4d5d7a9198bea6f0018a36fc6825637f9a12c4"}

[commit-bot: I haz the power, 2017-04-22 02:09:18]

Description was changed from

==========
Fix blits from multisampled renderbuffers to alpha:false WebGL back buffer.

Blitting from a multisampled RGB8 renderbuffer to an alpha:false WebGL
back buffer is supposed to work, and from an RGBA8 renderbuffer is
supposed to not work. The emulation of IOSurface-backed RGB textures
using RGBA textures didn't handle this case.

Add BindTexImage2DWithInternalformatCHROMIUM to the command buffer to
allow internal format to be overridden; currently this is only
supported on macOS. Use this to bind a second texture to the IOSurface
and use it only for BlitFramebuffer calls from WebGL, fixing up the
alpha channel afterward.

Covered by new conformance test in
https://github.com/KhronosGroup/WebGL/pull/2377 .

BUG=699566
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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 blits from multisampled renderbuffers to alpha:false WebGL back buffer.

Blitting from a multisampled RGB8 renderbuffer to an alpha:false WebGL
back buffer is supposed to work, and from an RGBA8 renderbuffer is
supposed to not work. The emulation of IOSurface-backed RGB textures
using RGBA textures didn't handle this case.

Add BindTexImage2DWithInternalformatCHROMIUM to the command buffer to
allow internal format to be overridden; currently this is only
supported on macOS. Use this to bind a second texture to the IOSurface
and use it only for BlitFramebuffer calls from WebGL, fixing up the
alpha channel afterward.

Covered by new conformance test in
https://github.com/KhronosGroup/WebGL/pull/2377 .

BUG=699566
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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/2831733003
Cr-Commit-Position: refs/heads/master@{#466523}
Committed:
https://chromium.googlesource.com/chromium/src/+/3d4d5d7a9198bea6f0018a36fc68...
==========

[commit-bot: I haz the power, 2017-04-22 02:09:19]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/3d4d5d7a9198bea6f0018a36fc68...

[pkalinnikov, 2017-04-24 12:00:53]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/2837833002/ by pkalinnikov@chromium.org.

The reason for reverting is: Reverting because 4
gpu_tests.context_lost_integration_test.ContextLostIntegrationTest.ContextLost_WebGLContextLost*
tests and GpuCrash_GPUProcessCrashesExactlyOncePerVisitToAboutGpuCrash crash on
"Mac Retina Debug (AMD)" bot..