Skip to content

Iframes with HDR images don't always become composited to show the HDR #47340

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.

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

Conversation

smfr
Copy link
Contributor

@smfr smfr commented Jun 28, 2025

6f89b28

Iframes with HDR images don't always become composited to show the HDR
https://bugs.webkit.org/show_bug.cgi?id=294873
rdar://154137509

Reviewed by Matt Woodrow.

296315@main wasn't quite complete; there's a race condition, revealed by `iframe-with-hdr-image.html`,
where the iframe may not actually do a compositing update after we detect the HDR image. The new
test, `iframe-gains-hdr-image.html`, targets this case.

When `RenderElement::imageContentChanged()` is called, we may not have any compositing layers in
the iframe yet, so we'd never hit `layer->contentChanged()`. We also need to explicitly trigger
a compositing update for this image, via `invalidateStyleAndLayerComposition()`.

There's no need to find the `enclosingCompositingLayer()` to call `contentChanged()` on; we can just
call it on the enclosing RenderLayer.

`ContentChangeType::HDRImage` is added since HDR images are specifically a compositing trigger
which prevents us from needlessly triggering compositing updates for SDR images.

* LayoutTests/compositing/hdr/iframe-gains-hdr-image-expected.txt: Added.
* LayoutTests/compositing/hdr/iframe-gains-hdr-image.html: Added.
* LayoutTests/compositing/hdr/iframe-with-hdr-image.html:
* LayoutTests/compositing/hdr/layer-gains-hdr-image-expected.txt: Added.
* LayoutTests/compositing/hdr/layer-gains-hdr-image.html: Copied from LayoutTests/compositing/hdr/iframe-with-hdr-image.html.
* LayoutTests/platform/ios/TestExpectations:
* LayoutTests/platform/mac-wk2/TestExpectations:
* Source/WebCore/rendering/RenderBoxModelObject.h:
* Source/WebCore/rendering/RenderElement.cpp:
(WebCore::RenderElement::imageContentChanged):
* Source/WebCore/rendering/RenderLayer.cpp:
(WebCore::RenderLayer::contentChanged):

Canonical link: https://commits.webkit.org/296835@main

929de7c

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
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@smfr smfr self-assigned this Jun 28, 2025
@smfr smfr added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label Jun 28, 2025
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 29, 2025
@smfr smfr removed the merging-blocked Applied to prevent a change from being merged label Jun 29, 2025
@smfr smfr force-pushed the eng/NEW-TEST-296315-main-macOS-iOS-compositing-hdr-iframe-with-hdr-image-html-is-flaky-failure branch from 74d6f42 to 96e855d Compare June 29, 2025 17:30
@smfr smfr force-pushed the eng/NEW-TEST-296315-main-macOS-iOS-compositing-hdr-iframe-with-hdr-image-html-is-flaky-failure branch from 96e855d to 1e2e806 Compare June 30, 2025 15:14
@smfr smfr force-pushed the eng/NEW-TEST-296315-main-macOS-iOS-compositing-hdr-iframe-with-hdr-image-html-is-flaky-failure branch from 1e2e806 to 929de7c Compare June 30, 2025 16:48
@smfr smfr added the merge-queue Applied to send a pull request to merge-queue label Jun 30, 2025
https://bugs.webkit.org/show_bug.cgi?id=294873
rdar://154137509

Reviewed by Matt Woodrow.

296315@main wasn't quite complete; there's a race condition, revealed by `iframe-with-hdr-image.html`,
where the iframe may not actually do a compositing update after we detect the HDR image. The new
test, `iframe-gains-hdr-image.html`, targets this case.

When `RenderElement::imageContentChanged()` is called, we may not have any compositing layers in
the iframe yet, so we'd never hit `layer->contentChanged()`. We also need to explicitly trigger
a compositing update for this image, via `invalidateStyleAndLayerComposition()`.

There's no need to find the `enclosingCompositingLayer()` to call `contentChanged()` on; we can just
call it on the enclosing RenderLayer.

`ContentChangeType::HDRImage` is added since HDR images are specifically a compositing trigger
which prevents us from needlessly triggering compositing updates for SDR images.

* LayoutTests/compositing/hdr/iframe-gains-hdr-image-expected.txt: Added.
* LayoutTests/compositing/hdr/iframe-gains-hdr-image.html: Added.
* LayoutTests/compositing/hdr/iframe-with-hdr-image.html:
* LayoutTests/compositing/hdr/layer-gains-hdr-image-expected.txt: Added.
* LayoutTests/compositing/hdr/layer-gains-hdr-image.html: Copied from LayoutTests/compositing/hdr/iframe-with-hdr-image.html.
* LayoutTests/platform/ios/TestExpectations:
* LayoutTests/platform/mac-wk2/TestExpectations:
* Source/WebCore/rendering/RenderBoxModelObject.h:
* Source/WebCore/rendering/RenderElement.cpp:
(WebCore::RenderElement::imageContentChanged):
* Source/WebCore/rendering/RenderLayer.cpp:
(WebCore::RenderLayer::contentChanged):

Canonical link: https://commits.webkit.org/296835@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/NEW-TEST-296315-main-macOS-iOS-compositing-hdr-iframe-with-hdr-image-html-is-flaky-failure branch from 929de7c to 6f89b28 Compare June 30, 2025 22:40
@webkit-commit-queue
Copy link
Collaborator

Committed 296835@main (6f89b28): https://commits.webkit.org/296835@main

Reviewed commits have been landed. Closing PR #47340 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 6f89b28 into WebKit:main Jun 30, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jun 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Bugs Unclassified bugs are placed in this component until the correct component can be determined.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants