Skip to content

Conversation

@m4gr3d
Copy link
Contributor

@m4gr3d m4gr3d commented Dec 24, 2025

No description provided.

@m4gr3d m4gr3d added this to the 4.7 milestone Dec 24, 2025
@m4gr3d m4gr3d force-pushed the support_composition_layers_anywhere_in_tree branch from 87fc625 to 6627795 Compare December 24, 2025 22:50

void OpenXRCompositionLayer::_remove_fallback_node() {
ERR_FAIL_COND(fallback != nullptr);
ERR_FAIL_COND(fallback == nullptr);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dsnopek Looks like the cond check was inverted, or am I misreading this logic?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, at first glance, I think you're right! I wonder why we haven't had issues with this in the past, though

if (Object::cast_to<XROrigin3D>(get_parent()) != nullptr) {
xf = get_transform();
} else {
xf = get_global_transform();
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this still work when the XROrigin3D is moved away from (0, 0, 0) and/or rotated?

I would have expected this to need something like (untested):

Suggested change
xf = get_global_transform();
xf = XRServer::get_singleton()->get_world_origin().affine_inverse() * get_global_transform();

Comment on lines -687 to +694
case NOTIFICATION_LOCAL_TRANSFORM_CHANGED: {
case NOTIFICATION_TRANSFORM_CHANGED: {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we still use NOTIFICATION_LOCAL_TRANSFORM_CHANGED when XROrigin3D is the parent, and only use NOTIFICATION_TRANSFORM_CHANGED when putting it elsewhere? NOTIFICATION_TRANSFORM_CHANGED can happen way more frequently than NOTIFICATION_LOCAL_TRANSFORM_CHANGED

Comment on lines -765 to +786
warnings.push_back(RTR("OpenXR composition layers must have an XROrigin3D node as their parent."));
warnings.push_back(RTR("OpenXR composition layers must have an XROrigin3D node as their ancestor."));
Copy link
Contributor

Choose a reason for hiding this comment

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

Wait, I thought the idea was that you could put it anywhere? Why does it still need to have XROrigin3D as an ancestor?

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.

2 participants