Skip to content
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

Apply reference frame transform to OpenXRCompositionLayer nodes #92339

Merged
merged 1 commit into from
May 29, 2024
Merged

Apply reference frame transform to OpenXRCompositionLayer nodes #92339

merged 1 commit into from
May 29, 2024

Conversation

devloglogan
Copy link
Contributor

@devloglogan devloglogan commented May 24, 2024 •
edited
Loading

Currently when utilizing any OpenXRCompositionLayer nodes, if a user tries to recenter their environment using XRServer.center_on_hmd(), the composition layers will not be relocated at all. With this change, they should now move as expected.

@devloglogan devloglogan requested a review from a team May 24, 2024 21:27
@Calinou Calinou added this to the 4.3 milestone May 24, 2024
@devloglogan devloglogan mentioned this pull request May 25, 2024
Merged
m4gr3d
m4gr3d reviewed May 25, 2024
View reviewed changes
Copy link
Contributor

@m4gr3d m4gr3d left a comment

Choose a reason for hiding this comment

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

Do we have any other events we can listen to detect when the reference frame has been updated instead of computing the transform every frame?

@dsnopek
Copy link
Contributor

dsnopek commented May 28, 2024

Do we have any other events we can listen to detect when the reference frame has been updated instead of computing the transform every frame?

It would be kind of nice to have a signal on XRServer for when the reference frame is updated, given that it isn't something that happens very often, but it affects lots and lots of things.

@devloglogan
Copy link
Contributor Author

@m4gr3d @dsnopek I've updated this PR to instead add a reference_frame_changed signal to XRServer, and the composition layer nodes now use that, rather than applying their transform every frame. I find this preferable!

@devloglogan devloglogan requested a review from a team as a code owner May 28, 2024 23:08
m4gr3d
m4gr3d approved these changes May 28, 2024
View reviewed changes
Copy link
Contributor

@m4gr3d m4gr3d left a comment

Choose a reason for hiding this comment

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

Looks good!

AThousandShips
AThousandShips approved these changes May 29, 2024
View reviewed changes
doc/classes/XRServer.xml Outdated Show resolved Hide resolved
modules/openxr/scene/openxr_composition_layer_cylinder.cpp Outdated Show resolved Hide resolved
modules/openxr/scene/openxr_composition_layer_equirect.cpp Outdated Show resolved Hide resolved
modules/openxr/scene/openxr_composition_layer_quad.cpp Outdated Show resolved Hide resolved
dsnopek
dsnopek approved these changes May 29, 2024
View reviewed changes
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.

Thanks! I haven't tested, but the code looks good to me :-)

@akien-mga akien-mga merged commit d1f6d4c into godotengine:master May 29, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

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

@dsnopek dsnopek dsnopek approved these changes

@m4gr3d m4gr3d m4gr3d approved these changes

@AThousandShips AThousandShips AThousandShips approved these changes

@BastiaanOlij BastiaanOlij Awaiting requested review from BastiaanOlij

Assignees
No one assigned
Labels
bug topic:xr
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.
6 participants
@devloglogan @dsnopek @akien-mga @m4gr3d @AThousandShips @Calinou