Skip to content

Add support for running hybrid apps from the XR editor #103972

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

Merged
merged 1 commit into from
Jun 20, 2025

Conversation

m4gr3d
Copy link
Contributor

@m4gr3d m4gr3d commented Mar 11, 2025

As the title mentions, this PR adds support for developing and debugging hybrid apps directly from the XR editor.

For context, Hybrid Apps are applications capable of swapping between an immersive 3D mode and a 2D panel mode at runtime.
The Godot XR editor itself is an hybrid app, and with this update, it gains and provides the ability to developers to create hybrid apps directly from a standalone XR device.

Recording using the Hybrid App sample project from a Quest 3 device:

com.oculus.metacam-20250610-214441-0.mp4

Note: Hybrid apps require v4 of the Godot OpenXR Vendor plugin

@m4gr3d m4gr3d added this to the 4.5 milestone Mar 11, 2025
@m4gr3d m4gr3d force-pushed the xr_editor_hybrid_support branch 2 times, most recently from 62d2e1b to 538830b Compare June 6, 2025 18:30
@m4gr3d m4gr3d force-pushed the xr_editor_hybrid_support branch 3 times, most recently from 6171a82 to a3a7f8c Compare June 11, 2025 03:57
@m4gr3d m4gr3d marked this pull request as ready for review June 11, 2025 05:11
@m4gr3d m4gr3d requested review from a team as code owners June 11, 2025 05:11
Copy link
Contributor

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

I just tested on Quest 3, and this is pretty impressive! When switching back to panel app, it opens the app embedded and the debugger is connected.

Skimming the code, it all looks good! I only had a question about a change outside of the Android platform

if (dbg->is_session_active()) {
dbg->_stop_and_notify();
}
dbg->stop();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this change necessary? It would seem to affect all platforms

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a bug I found while testing, which I think affect all platforms.

When EditorDebuggerNode::stop(...) is invoked, if the debugging session doesn't have an active session, it won't be stopped alongside EditorDebuggerNode, so it'll continue processing because set_process(false) is only invoked in ScriptEditorDebugger::stop().

This'll cause:

  1. _stop_and_notify() in to be invoked
  2. which will fire a stopped signal
  3. which will cause EditorDebuggerNode::_debugger_stopped to be invoked because of
    node->connect("stopped", callable_mp(this, &EditorDebuggerNode::_debugger_stopped).bind(id));
  4. which will trigger notify_all_debug_sessions_exited
  5. which invokes project_run_bar->stop_playing()
  6. which in turn invokes EditorDebuggerNode::get_singleton()->stop() again

In most cases, it's not an issue because EditorRunBar::stop_playing() abort early when nothing is playing, but it becomes an issue when a project is playing after the first call to EditorDebuggerNode::stop(), as the call in (5) will cause the running project to be stopped as a consequence of the described flow.

So the fix is to always invoke ScriptEditorDebugger::stop() when EditorDebuggerNode::stop() is invoked in order to also stop processing for ScriptEditorDebugger. _stop_and_notify() is not used to avoid invoking EditorDebuggerNode::stop() again when the stopped signal is fired.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, ok. I walked through the code, and the chain of events you describe makes sense. However, are you sure there aren't any cases where something is expecting EditorDebuggerNode::stop() to lead to ScriptEditorDebugger's "stopped" signal being emitted, and now it won't be?

This change should probably be reviewed by someone who understands the debugger really well. Perhaps @Faless? I don't know who else knows this code? cc @akien-mga

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested switching to _stop_and_notify() and it seems to work as expected, so I've restored that call.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, that seems like a less potentially impactful change to me

@m4gr3d m4gr3d force-pushed the xr_editor_hybrid_support branch 2 times, most recently from e4cb5e6 to 1b1905f Compare June 13, 2025 00:09
@m4gr3d m4gr3d force-pushed the xr_editor_hybrid_support branch from 1b1905f to 09f5be7 Compare June 15, 2025 14:05
@Repiteo Repiteo merged commit f7c6f00 into godotengine:master Jun 20, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Jun 20, 2025

Thanks!

@m4gr3d m4gr3d deleted the xr_editor_hybrid_support branch June 20, 2025 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants