Skip to content

[WebXR] Revert WebXRSession changes from 312788@main#64744

Merged
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
djg:eng/webxr/separate-applyPendingRenderState
May 13, 2026
Merged

[WebXR] Revert WebXRSession changes from 312788@main#64744
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
djg:eng/webxr/separate-applyPendingRenderState

Conversation

@djg
Copy link
Copy Markdown
Contributor

@djg djg commented May 12, 2026

3c73508

[WebXR] Revert WebXRSession changes from 312788@main
https://bugs.webkit.org/show_bug.cgi?id=314623
rdar://176829186

Reviewed by Cameron McCormack.

312788@main moved applyPendingRenderState() from before the rendering block
to after it in WebXRSession::onFrame(), and moved inputSources->update()
to run unconditionally before the render block. These changes were made to
fix an OpenXR-specific crash involving swapchain acquire/release mismatches
when activeLayerHandles changed mid-frame.

The OpenXR author has confirmed these WebXRSession changes are no longer
required for OpenXR -- the OpenXR-side changes in that commit (snapshotting
active layers at beginFrame and filtering at endFrame) are sufficient to
prevent the crash without modifying the shared WebXRSession frame loop.

Reverting restores the original frame loop ordering:
- applyPendingRenderState() before frameShouldBeRendered() (step 7 before 6)
- inputSources->update() at step 6.4 inside the render block

This fixes first-frame rendering on visionOS where the deferred
applyPendingRenderState meant frameShouldBeRendered() would see a null
baseLayer and skip the entire render block.

* Source/WebCore/Modules/webxr/WebXRSession.cpp:
(WebCore::WebXRSession::applyPendingRenderState):
(WebCore::WebXRSession::onFrame):
(WebCore::WebXRSession::updateRequestDataFromActiveRenderState): Deleted.
* Source/WebCore/Modules/webxr/WebXRSession.h:

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

726ac8a

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

@djg djg self-assigned this May 12, 2026
@djg djg added the WebXR For bugs in WebXR label May 12, 2026
@djg djg requested a review from svillar May 12, 2026 08:04
Copy link
Copy Markdown
Contributor

@svillar svillar left a comment

Choose a reason for hiding this comment

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

Are you sure the commit is the one mentioned in the ChangeLog? Shouldn't be b0baaea instead?

// Update m_requestData AFTER submitFrame(). This ensures the
// UIProcess coordinator doesn't see new activeLayerHandles until
// AFTER the current frame's submitFrame() has been processed.
session.updateRequestDataFromActiveRenderState();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually I think we might not even need this. We can keep everything together now because we have applied countermeasures at OpenXR side which will allow us to produce sensible rendering even if the processes disagree about the active layers.

Contrary to what I mentioned in my previous commits applying the render state above would still be conformant to the spec because it happens before calling the frame callbacks. This means that calling updateRenderState() during rAF won't change the current frame because the render state has been applied before.

That's why I think it'd be better to avoid splitting these calls and keep the code together in apply render state as it was before my change. OpenXR should be fine.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So you're fine with me reverting the changes to WebXRSession made in b0baaea?

@djg djg changed the title [WebXR] Split applyPendingRenderState to fix first-frame rendering on visionOS [WebXR] Revert WebXRSession changes from 312788@main May 13, 2026
@djg djg force-pushed the eng/webxr/separate-applyPendingRenderState branch from 6f3921b to 726ac8a Compare May 13, 2026 01:42
@djg djg requested review from mwyrzykowski and svillar May 13, 2026 01:44
@djg djg added the merge-queue Applied to send a pull request to merge-queue label May 13, 2026
https://bugs.webkit.org/show_bug.cgi?id=314623
rdar://176829186

Reviewed by Cameron McCormack.

312788@main moved applyPendingRenderState() from before the rendering block
to after it in WebXRSession::onFrame(), and moved inputSources->update()
to run unconditionally before the render block. These changes were made to
fix an OpenXR-specific crash involving swapchain acquire/release mismatches
when activeLayerHandles changed mid-frame.

The OpenXR author has confirmed these WebXRSession changes are no longer
required for OpenXR -- the OpenXR-side changes in that commit (snapshotting
active layers at beginFrame and filtering at endFrame) are sufficient to
prevent the crash without modifying the shared WebXRSession frame loop.

Reverting restores the original frame loop ordering:
- applyPendingRenderState() before frameShouldBeRendered() (step 7 before 6)
- inputSources->update() at step 6.4 inside the render block

This fixes first-frame rendering on visionOS where the deferred
applyPendingRenderState meant frameShouldBeRendered() would see a null
baseLayer and skip the entire render block.

* Source/WebCore/Modules/webxr/WebXRSession.cpp:
(WebCore::WebXRSession::applyPendingRenderState):
(WebCore::WebXRSession::onFrame):
(WebCore::WebXRSession::updateRequestDataFromActiveRenderState): Deleted.
* Source/WebCore/Modules/webxr/WebXRSession.h:

Canonical link: https://commits.webkit.org/313131@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/webxr/separate-applyPendingRenderState branch from 726ac8a to 3c73508 Compare May 13, 2026 05:18
@webkit-commit-queue
Copy link
Copy Markdown
Collaborator

Committed 313131@main (3c73508): https://commits.webkit.org/313131@main

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

@webkit-commit-queue webkit-commit-queue merged commit 3c73508 into WebKit:main May 13, 2026
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WebXR For bugs in WebXR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants