Skip to content

Apply dynamic-range-limit to HDR WebGPU canvas #44915

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Apply dynamic-range-limit to HDR WebGPU canvas #44915

wants to merge 1 commit into from

Conversation

squelart
Copy link
Contributor

@squelart squelart commented May 4, 2025 •
edited by webkit-early-warning-system
Loading

b0b01fe

Apply dynamic-range-limit to HDR WebGPU canvas
https://bugs.webkit.org/show_bug.cgi?id=294831
rdar://148558614

Reviewed by NOBODY (OOPS!).

*** TEMPORARY EXTRA LOGGING, DO NOT MERGE ***

* Source/WebCore/Modules/WebGPU/Implementation/WebGPUCompositorIntegrationImpl.cpp:
(WebCore::WebGPU::CompositorIntegrationImpl::updateContentsHeadroom):
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::shouldSuppressHDRDidChange):
* Source/WebCore/html/HTMLCanvasElement.cpp:
(WebCore::HTMLCanvasElement::computeContextDynamicRangeLimit const):
(WebCore::HTMLCanvasElement::dynamicRangeLimitDidChange):
Store the current dynamic-range-limit, and set the effective limit
(which accounts for the suppress-HDR state) in the rendering context.

* Source/WebCore/html/HTMLCanvasElement.h:
* Source/WebCore/html/canvas/CanvasRenderingContext.h:
(WebCore::CanvasRenderingContext::setDynamicRangeLimit):
Default dynamic-range-limit handler, does nothing.

* Source/WebCore/html/canvas/GPUCanvasContextCocoa.h:
* Source/WebCore/html/canvas/GPUCanvasContextCocoa.mm:
(WebCore::GPUCanvasContextCocoa::updateContentsHeadroom):
(WebCore::GPUCanvasContextCocoa::setDynamicRangeLimit):
WebGPU dynamic-range-limit handler, stores the limit in the display
delegate, which sets the corresponding property on the actual display
PlatformCALayer (remote or not).

* Source/WebCore/page/Page.cpp:
(WebCore::Page::setShouldSuppressHDR):
* Source/WebCore/platform/graphics/ca/PlatformCALayer.h:
dynamic-range-limit property interface.

* Source/WebCore/platform/graphics/ca/cocoa/PlatformCALayerCocoa.h:
* Source/WebCore/platform/graphics/ca/cocoa/PlatformCALayerCocoa.mm:
(WebCore::PlatformCALayerCocoa::dynamicRangeLimit const):
(WebCore::PlatformCALayerCocoa::setDynamicRangeLimit):
Handle setting the dynamic-range-limit on the actual CALayer.
* Source/WebCore/rendering/RenderHTMLCanvas.cpp:
(WebCore::RenderHTMLCanvas::styleDidChange):
Catch dynamic-range-limit changes, and notify the canvas.

* Source/WebCore/rendering/RenderHTMLCanvas.h:
* Source/WebKit/Shared/RemoteLayerTree/LayerProperties.h:
* Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTree.serialization.in:
* Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTreePropertyApplier.mm:
(WebKit::RemoteLayerTreePropertyApplier::applyPropertiesToLayer):
* Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:
(WebKit::WebProcessPool::screenPropertiesChanged):
* Source/WebKit/UIProcess/mac/WebViewImpl.mm:
(WebKit::WebViewImpl::applicationShouldSuppressHDR):
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.h:
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.mm:
(WebKit::PlatformCALayerRemote::dynamicRangeLimit const):
(WebKit::PlatformCALayerRemote::setDynamicRangeLimit):
Handle setting the dynamic-range-limit, and forward it as a layer
property to the counterpart layer in the appropriate process.

b0b01fe

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
  🧪 style   🛠 ios   🛠 mac   🛠 wpe   🛠 win
  🧪 bindings   🛠 ios-sim   🛠 mac-AS-debug   🧪 wpe-wk2   🧪 win-tests
  🧪 webkitperl   🧪 ios-wk2   🧪 api-mac   🧪 api-wpe
  🧪 ios-wk2-wpt   🧪 mac-wk1   🛠 wpe-cairo
  🧪 api-ios   🧪 mac-wk2   🛠 gtk
  🛠 vision   🧪 mac-AS-debug-wk2   🧪 gtk-wk2
  🛠 vision-sim   🧪 mac-wk2-stress   🧪 api-gtk
⏳ 🧪 vision-wk2   🧪 mac-intel-wk2   🛠 playstation
  🛠 tv   🛠 mac-safer-cpp
  🛠 tv-sim
  🛠 watch
  🛠 watch-sim

@squelart squelart self-assigned this May 4, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented May 4, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 0fa9248)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ❌ 🛠 mac ❌ 🛠 wpe ❌ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ❌ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ❌ 🧪 api-mac ❌ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ❌ 🛠 wpe-cairo
✅ 🧪 api-ios ❌ 🧪 mac-wk2 ❌ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ❌ 🧪 gtk-wk2
✅ 🛠 vision-sim ❌ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
✅ 🧪 vision-wk2 ❌ 🧪 mac-intel-wk2 ❌ 🛠 playstation
❌ 🛠 tv ✅ 🛠 mac-safer-cpp
❌ 🛠 tv-sim
❌ 🛠 watch
❌ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 5, 2025
@squelart squelart removed the merging-blocked Applied to prevent a change from being merged label May 5, 2025
@squelart squelart force-pushed the eng/webgpu-dynamic-range-limit branch from 0fa9248 to 216fdd0 Compare May 5, 2025 01:02
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented May 5, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 216fdd0)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ❌ 🛠 mac ❌ 🛠 wpe ❌ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ❌ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ❌ 🧪 api-mac ❌ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ❌ 🛠 wpe-cairo
✅ 🧪 api-ios ❌ 🧪 mac-wk2 ❌ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ❌ 🧪 gtk-wk2
✅ 🛠 vision-sim ❌ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
✅ 🧪 vision-wk2 ❌ 🧪 mac-intel-wk2 ❌ 🛠 playstation
❌ 🛠 tv ✅ 🛠 mac-safer-cpp
❌ 🛠 tv-sim
❌ 🛠 watch
❌ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 5, 2025
@squelart squelart removed the merging-blocked Applied to prevent a change from being merged label May 5, 2025
@squelart squelart force-pushed the eng/webgpu-dynamic-range-limit branch from 216fdd0 to 95fd691 Compare May 5, 2025 05:12
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented May 5, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 95fd691)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@squelart squelart changed the title WebGPU dynamic-range-limit Apply dynamic-range-limit to HDR WebGPU canvas Jun 23, 2025
@squelart squelart force-pushed the eng/webgpu-dynamic-range-limit branch from 95fd691 to 1d52d53 Compare June 23, 2025 10:09
@squelart squelart added the WebGPU For bugs in WebGPU label Jun 23, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 23, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 1d52d53)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ❌ 🛠 mac ❌ 🛠 wpe ❌ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ❌ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ❌ 🧪 api-mac ❌ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ❌ 🛠 wpe-cairo
✅ 🧪 api-ios ❌ 🧪 mac-wk2 ❌ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ❌ 🧪 gtk-wk2
✅ 🛠 vision-sim ❌ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
loading 🧪 vision-wk2 ❌ 🧪 mac-intel-wk2 ❌ 🛠 playstation
❌ 🛠 tv ✅ 🛠 mac-safer-cpp
❌ 🛠 tv-sim
❌ 🛠 watch
❌ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 23, 2025
@squelart squelart removed the merging-blocked Applied to prevent a change from being merged label Jun 23, 2025
@squelart squelart force-pushed the eng/webgpu-dynamic-range-limit branch from 1d52d53 to 5710014 Compare June 23, 2025 14:53
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 23, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 5710014)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@squelart
Apply dynamic-range-limit to HDR WebGPU canvas
b0b01fe 
https://bugs.webkit.org/show_bug.cgi?id=294831
rdar://148558614

Reviewed by NOBODY (OOPS!).

*** TEMPORARY EXTRA LOGGING, DO NOT MERGE ***

* Source/WebCore/Modules/WebGPU/Implementation/WebGPUCompositorIntegrationImpl.cpp:
(WebCore::WebGPU::CompositorIntegrationImpl::updateContentsHeadroom):
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::shouldSuppressHDRDidChange):
* Source/WebCore/html/HTMLCanvasElement.cpp:
(WebCore::HTMLCanvasElement::computeContextDynamicRangeLimit const):
(WebCore::HTMLCanvasElement::dynamicRangeLimitDidChange):
Store the current dynamic-range-limit, and set the effective limit
(which accounts for the suppress-HDR state) in the rendering context.

* Source/WebCore/html/HTMLCanvasElement.h:
* Source/WebCore/html/canvas/CanvasRenderingContext.h:
(WebCore::CanvasRenderingContext::setDynamicRangeLimit):
Default dynamic-range-limit handler, does nothing.

* Source/WebCore/html/canvas/GPUCanvasContextCocoa.h:
* Source/WebCore/html/canvas/GPUCanvasContextCocoa.mm:
(WebCore::GPUCanvasContextCocoa::updateContentsHeadroom):
(WebCore::GPUCanvasContextCocoa::setDynamicRangeLimit):
WebGPU dynamic-range-limit handler, stores the limit in the display
delegate, which sets the corresponding property on the actual display
PlatformCALayer (remote or not).

* Source/WebCore/page/Page.cpp:
(WebCore::Page::setShouldSuppressHDR):
* Source/WebCore/platform/graphics/ca/PlatformCALayer.h:
dynamic-range-limit property interface.

* Source/WebCore/platform/graphics/ca/cocoa/PlatformCALayerCocoa.h:
* Source/WebCore/platform/graphics/ca/cocoa/PlatformCALayerCocoa.mm:
(WebCore::PlatformCALayerCocoa::dynamicRangeLimit const):
(WebCore::PlatformCALayerCocoa::setDynamicRangeLimit):
Handle setting the dynamic-range-limit on the actual CALayer.
* Source/WebCore/rendering/RenderHTMLCanvas.cpp:
(WebCore::RenderHTMLCanvas::styleDidChange):
Catch dynamic-range-limit changes, and notify the canvas.

* Source/WebCore/rendering/RenderHTMLCanvas.h:
* Source/WebKit/Shared/RemoteLayerTree/LayerProperties.h:
* Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTree.serialization.in:
* Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTreePropertyApplier.mm:
(WebKit::RemoteLayerTreePropertyApplier::applyPropertiesToLayer):
* Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:
(WebKit::WebProcessPool::screenPropertiesChanged):
* Source/WebKit/UIProcess/mac/WebViewImpl.mm:
(WebKit::WebViewImpl::applicationShouldSuppressHDR):
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.h:
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.mm:
(WebKit::PlatformCALayerRemote::dynamicRangeLimit const):
(WebKit::PlatformCALayerRemote::setDynamicRangeLimit):
Handle setting the dynamic-range-limit, and forward it as a layer
property to the counterpart layer in the appropriate process.
@squelart squelart force-pushed the eng/webgpu-dynamic-range-limit branch from 5710014 to b0b01fe Compare June 24, 2025 09:45
@squelart squelart added the skip-ews Applied to prevent a change from being run on EWS label Jun 24, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 24, 2025 •
edited
Loading

EWS run on current version of this PR (hash b0b01fe)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
  🧪 style   🛠 ios   🛠 mac   🛠 wpe   🛠 win
  🧪 bindings   🛠 ios-sim   🛠 mac-AS-debug   🧪 wpe-wk2   🧪 win-tests
  🧪 webkitperl   🧪 ios-wk2   🧪 api-mac   🧪 api-wpe
  🧪 ios-wk2-wpt   🧪 mac-wk1   🛠 wpe-cairo
  🧪 api-ios   🧪 mac-wk2   🛠 gtk
  🛠 vision   🧪 mac-AS-debug-wk2   🧪 gtk-wk2
  🛠 vision-sim   🧪 mac-wk2-stress   🧪 api-gtk
⏳ 🧪 vision-wk2   🧪 mac-intel-wk2   🛠 playstation
  🛠 tv   🛠 mac-safer-cpp
  🛠 tv-sim
  🛠 watch
  🛠 watch-sim

@squelart squelart added the merging-blocked Applied to prevent a change from being merged label Jun 24, 2025
mwyrzykowski
mwyrzykowski reviewed Jun 24, 2025
View reviewed changes
Source/WebCore/platform/graphics/ca/cocoa/PlatformCALayerCocoa.mm

m_dynamicRangeLimit = dynamicRangeLimit;

setLayerDynamicRangeLimit(m_layer.get(), dynamicRangeLimit);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think using setLayerDynamicRangeLimit results in the intended behavior and it also does not allow us to correctly implement https://www.w3.org/TR/css-color-hdr-1/#dynamic-range-limit-mix

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the 2nd point, I think this is moot: dynamic-range-limit-mix() is currently disabled anyway, because [CALayer setPreferredDynamicRange:] (called by setLayerDynamicRangeLimit) doesn't support arbitrary values. Once CALayer supports these, mix can be implemented.

I'm not sure why you think setLayerDynamicRangeLimit and therefore setPreferredDynamicRange doesn't have the intended behavior? Maybe we should discuss more... (Later this week, I won't be available much until then.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless we can resolve the issue where SDR white becomes gray, I don't think we can use it. It is very noticeable in content and does not look correct

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But please remember that dynamic-range-limit is an author's choice (unlike OS-imposed dimming/suppressing), so the same author that chooses to use standard should be able to see the effects and deal with them, for example by having their WebGPU code generate pixel values that are appropriate to the SDR case.

mattwoodrow
mattwoodrow reviewed Jun 24, 2025
View reviewed changes
Source/WebCore/Modules/WebGPU/Implementation/WebGPUCompositorIntegrationImpl.cpp
@@ -62,8 +62,10 @@ void CompositorIntegrationImpl::prepareForDisplay(uint32_t frameIndex, Completio
void CompositorIntegrationImpl::updateContentsHeadroom(float headroom)
{
#if HAVE(SUPPORT_HDR_DISPLAY)
for (auto& ioSurface : m_renderBuffers)
for (auto& ioSurface : m_renderBuffers) {
ALWAYS_LOG_WITH_STREAM(stream << "__GS__ CompositorIntegrationImpl[" << this << "]::updateContentsHeadroom(" << headroom << ") -> ioSurface[" << &ioSurface << "]->setContentEDRHeadroom(" << headroom << ")");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an 'HDR' log channel in WebCore now, if you want to make some of this logging more-permanent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for letting me know. But this is just temporary logging while we're debugging; I'll remove them all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattwoodrow mattwoodrow mattwoodrow left review comments

@mwyrzykowski mwyrzykowski mwyrzykowski left review comments

@cdumez cdumez Awaiting requested review from cdumez cdumez will be requested when the pull request is marked ready for review cdumez is a code owner

@rniwa rniwa Awaiting requested review from rniwa rniwa will be requested when the pull request is marked ready for review rniwa is a code owner

Assignees

@squelart squelart

Labels
merging-blocked Applied to prevent a change from being merged skip-ews Applied to prevent a change from being run on EWS WebGPU For bugs in WebGPU
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@squelart @webkit-early-warning-system @mattwoodrow @mwyrzykowski @webkit-ews-buildbot