New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 711107 link

Starred by 4 users

Issue metadata

Status: Assigned
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 777640
issue 777748
issue 777750
issue 777756

Blocking:
issue 761424



Sign in to add a comment

WebGL: Respect canvas color space and bit depth

Project Member Reported by ccameron@chromium.org, Apr 13 2017

Issue description

In issue 634542 we are adding support for <canvas> elements that have custom color spaces and have half-float backings.

WebGL DrawingBuffer needs to support this as well.

Of note is that there is no separate bug for HDR support -- that should fall out of this directly.
 
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/914a3662e9947933953bf8e3935797621f48ec93

commit 914a3662e9947933953bf8e3935797621f48ec93
Author: ccameron <ccameron@chromium.org>
Date: Wed Apr 19 07:10:29 2017

Plumb CanvasColorParams to canvas image classes

Send the CanvasColorParams structure instead of the individual
parameters.

Propagate CanvasColorParams to DrawingBuffer, and use the specified
color space instead of defaulting to SRGB.

BUG=711107
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;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/2825183002
Cr-Commit-Position: refs/heads/master@{#465515}

[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/core/html/canvas/CanvasRenderingContext.cpp
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/core/html/canvas/CanvasRenderingContext.h
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/core/paint/HTMLCanvasPainterTest.cpp
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.cpp
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2DTest.cpp
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/platform/graphics/Canvas2DImageBufferSurface.h
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.h
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridgeTest.cpp
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/platform/graphics/CanvasColorParams.cpp
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/platform/graphics/CanvasColorParams.h
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/platform/graphics/ImageBuffer.cpp
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/platform/graphics/ImageBuffer.h
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/platform/graphics/ImageBufferSurface.cpp
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/platform/graphics/ImageBufferSurface.h
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/platform/graphics/RecordingImageBufferSurface.cpp
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/platform/graphics/RecordingImageBufferSurface.h
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/platform/graphics/RecordingImageBufferSurfaceTest.cpp
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/platform/graphics/UnacceleratedImageBufferSurface.cpp
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/platform/graphics/UnacceleratedImageBufferSurface.h
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/platform/graphics/gpu/AcceleratedImageBufferSurface.cpp
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/platform/graphics/gpu/AcceleratedImageBufferSurface.h
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTest.cpp
[modify] https://crrev.com/914a3662e9947933953bf8e3935797621f48ec93/third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTestHelpers.h

Project Member

Comment 3 by bugdroid1@chromium.org, Apr 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2ecea20b01bdccc4f80893e5e14955bad1e9a554

commit 2ecea20b01bdccc4f80893e5e14955bad1e9a554
Author: ccameron <ccameron@chromium.org>
Date: Thu Apr 20 04:25:49 2017

WebGL: Tidy up buffer allocation logic

This patch should not change any of the allocation format logic, it
just makes it more clear what it's doing. Rather than compute the
formats to use, compute the parameter "should we allocate an
alpha channel".

But, there is one bugfix where, if we fail to allocate a GLImage, we
we were not re-computing the color buffer parameters. Now we do.

BUG=711107
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;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/2820373003
Cr-Commit-Position: refs/heads/master@{#465898}

[modify] https://crrev.com/2ecea20b01bdccc4f80893e5e14955bad1e9a554/gpu/command_buffer/common/gpu_memory_buffer_support.cc
[modify] https://crrev.com/2ecea20b01bdccc4f80893e5e14955bad1e9a554/gpu/command_buffer/common/gpu_memory_buffer_support.h
[modify] https://crrev.com/2ecea20b01bdccc4f80893e5e14955bad1e9a554/third_party/WebKit/Source/platform/graphics/DEPS
[modify] https://crrev.com/2ecea20b01bdccc4f80893e5e14955bad1e9a554/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
[modify] https://crrev.com/2ecea20b01bdccc4f80893e5e14955bad1e9a554/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0cf6c5aec94b518ae3d0850466e28bbae0f284b4

commit 0cf6c5aec94b518ae3d0850466e28bbae0f284b4
Author: ccameron <ccameron@chromium.org>
Date: Thu Apr 20 21:07:29 2017

color: Fix custom params IPC

AddBytes and AddData need to be used consistently.

BUG=711107
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2828833002
Cr-Commit-Position: refs/heads/master@{#466130}

[modify] https://crrev.com/0cf6c5aec94b518ae3d0850466e28bbae0f284b4/cc/ipc/struct_traits_unittest.cc
[modify] https://crrev.com/0cf6c5aec94b518ae3d0850466e28bbae0f284b4/ui/gfx/ipc/color/gfx_param_traits.cc

Issue 718404 has been merged into this issue.
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7770a5a2b049424d3fa28d3c574fa49d51a19b33

commit 7770a5a2b049424d3fa28d3c574fa49d51a19b33
Author: Christopher Cameron <ccameron@chromium.org>
Date: Mon Oct 23 20:29:44 2017

Clean up DrawingBuffer RGB emulation logic

Background: We can emulate RGB using RGB based on
  * DISABLE_GL_RGB_FORMAT
    * Tells us not to allocate GL_RGB buffers.
  * DISABLE_WEBGL_RGB_MULTISAMPLING_USAGE
    * Tells us not to allocate GL_RGB multisample buffers.
  * chromium_image_rgb_emulation
    * Tells us that if we allocate a GLImage with internal format
      GL_RGB, it will actually be allocated behind the scenes with an
      alpha channel that we need to be careful not to write to.
We check these variables in a handful of functions.

Break this logic up into 3 separate variables set at initialization:
  * want_alpha_channel_
    * Did the user request an alpha channel?
  * allocate_alpha_channel_
    * Should we use a GL_RGBA versus GL_RGB like internal format when
      allocating our buffer?
    * If want_alpha_channel_ then this is true (of course)
    * True when want_alpha_channel_ is false for workarounds
  * have_alpha_channel_
    * Does our allocation have an alpha channel that we can read from
      and write to?
    * If allocate_alpha_channel_ then this is true (of course)
    * True when allocate_alpha_channel_ for IOSurface reasons

From there, remove all of the places where we check
DISABLE_GL_RGB_FORMAT, DISABLE_WEBGL_RGB_MULTISAMPLING_USAGE, or
chromium_image_rgb_emulation, and instead use one of the above
variables as appropriate.

Because we're adding the allocate_alpha_channel_ variable, we can remove
the ColorBufferParameters::allocate_alpha_channel variable, and replace
the class with just the GL target.

Note that there are bugs when want_alpha_channel_ does not match
allocate_alpha_channel_, as described in crbug.com/776269. When those
are cleaned up, we will be able to merge allocate_alpha_channel_ and
have_alpha_channel_.

Bug: 711107
Change-Id: I12dea9e179336dc2c24c1763e8e9cc7e5403c6b0
Reviewed-on: https://chromium-review.googlesource.com/726955
Commit-Queue: ccameron (ping after 24hr) <ccameron@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510901}
[modify] https://crrev.com/7770a5a2b049424d3fa28d3c574fa49d51a19b33/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
[modify] https://crrev.com/7770a5a2b049424d3fa28d3c574fa49d51a19b33/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h

Blockedon: 777748
Blockedon: 777750
Blockedon: 777756
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/00c0ae3c7fd6af966dd4d476638999da21f20f02

commit 00c0ae3c7fd6af966dd4d476638999da21f20f02
Author: Khushal <khushalsagar@chromium.org>
Date: Tue Oct 24 17:32:03 2017

Revert "Clean up DrawingBuffer RGB emulation logic"

This reverts commit 7770a5a2b049424d3fa28d3c574fa49d51a19b33.

Reason for revert: Suspected culprit for failure on Mac Pro (AMD) bots. See  crbug.com/777640 .

Original change's description:
> Clean up DrawingBuffer RGB emulation logic
> 
> Background: We can emulate RGB using RGB based on
>   * DISABLE_GL_RGB_FORMAT
>     * Tells us not to allocate GL_RGB buffers.
>   * DISABLE_WEBGL_RGB_MULTISAMPLING_USAGE
>     * Tells us not to allocate GL_RGB multisample buffers.
>   * chromium_image_rgb_emulation
>     * Tells us that if we allocate a GLImage with internal format
>       GL_RGB, it will actually be allocated behind the scenes with an
>       alpha channel that we need to be careful not to write to.
> We check these variables in a handful of functions.
> 
> Break this logic up into 3 separate variables set at initialization:
>   * want_alpha_channel_
>     * Did the user request an alpha channel?
>   * allocate_alpha_channel_
>     * Should we use a GL_RGBA versus GL_RGB like internal format when
>       allocating our buffer?
>     * If want_alpha_channel_ then this is true (of course)
>     * True when want_alpha_channel_ is false for workarounds
>   * have_alpha_channel_
>     * Does our allocation have an alpha channel that we can read from
>       and write to?
>     * If allocate_alpha_channel_ then this is true (of course)
>     * True when allocate_alpha_channel_ for IOSurface reasons
> 
> From there, remove all of the places where we check
> DISABLE_GL_RGB_FORMAT, DISABLE_WEBGL_RGB_MULTISAMPLING_USAGE, or
> chromium_image_rgb_emulation, and instead use one of the above
> variables as appropriate.
> 
> Because we're adding the allocate_alpha_channel_ variable, we can remove
> the ColorBufferParameters::allocate_alpha_channel variable, and replace
> the class with just the GL target.
> 
> Note that there are bugs when want_alpha_channel_ does not match
> allocate_alpha_channel_, as described in crbug.com/776269. When those
> are cleaned up, we will be able to merge allocate_alpha_channel_ and
> have_alpha_channel_.
> 
> Bug: 711107
> Change-Id: I12dea9e179336dc2c24c1763e8e9cc7e5403c6b0
> Reviewed-on: https://chromium-review.googlesource.com/726955
> Commit-Queue: ccameron (ping after 24hr) <ccameron@chromium.org>
> Reviewed-by: Kenneth Russell <kbr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#510901}

TBR=ccameron@chromium.org,kbr@chromium.org

Change-Id: I30b1b8ff58c4024e29160cd3a21f2dff7a16eee9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 711107
Reviewed-on: https://chromium-review.googlesource.com/734730
Reviewed-by: Khushal <khushalsagar@chromium.org>
Commit-Queue: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511193}
[modify] https://crrev.com/00c0ae3c7fd6af966dd4d476638999da21f20f02/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
[modify] https://crrev.com/00c0ae3c7fd6af966dd4d476638999da21f20f02/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h

Comment 11 by kbr@chromium.org, Oct 25 2017

Blockedon: 777640
 Issue 751150  has been merged into this issue.
Blocking: 761424
Project Member

Comment 14 by bugdroid1@chromium.org, Nov 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a4305b8005efedf15beb40e3815181388c933296

commit a4305b8005efedf15beb40e3815181388c933296
Author: Christopher Cameron <ccameron@chromium.org>
Date: Mon Nov 20 22:45:42 2017

DrawingBuffer: Add support for HDR WebGL

Add extension checks for half-float support and update GL and GMB
calls to use the appropriate types.

Add several log statements for the various failure mechanisms for
allocation (since they were helpful in debugging).

Bug: 711107
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I22d4f152014042099e4831a9b85931c6fd975798
Reviewed-on: https://chromium-review.googlesource.com/734649
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Justin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517961}
[modify] https://crrev.com/a4305b8005efedf15beb40e3815181388c933296/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
[modify] https://crrev.com/a4305b8005efedf15beb40e3815181388c933296/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h

Cc: fs...@chromium.org
+fserb for <canvas> issues.

WebGL currently does respect requests for float-16 backbuffers.
Cc: -junov@chromium.org
Cc: piman@chromium.org vmi...@chromium.org erikc...@chromium.org
 Issue 663197  has been merged into this issue.

Sign in to add a comment