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.

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

Add support for XR_META_spatial_entity_mesh to get global scene mesh #122

Merged
merged 1 commit into from
Apr 25, 2024

Conversation

dsnopek
Copy link
Collaborator

@dsnopek dsnopek commented Apr 16, 2024

Here's a screen shot showing a wire frame (the light white lines) of the global scene mesh, which is what's added to the demo:

org godotengine openxr vendors demo-20240416-175351

This PR also fixes a bug that I noticed in OpenXRFbSceneManager, with storing the scenes based on semantic label (it was inconsistently converting upper to lower case, causing the properties to not really get set).

@dsnopek dsnopek added the enhancement New feature or request label Apr 16, 2024
@dsnopek dsnopek requested review from m4gr3d and BastiaanOlij April 16, 2024 22:58
@dsnopek dsnopek added this to the 3.0.0 milestone Apr 16, 2024
@dsnopek dsnopek force-pushed the meta-spatial-entity-mesh branch from 2796620 to 863216b Compare April 16, 2024 22:59
@dsnopek dsnopek force-pushed the meta-spatial-entity-mesh branch 2 times, most recently from 12adf7b to 3e9e485 Compare April 22, 2024 15:05
@dsnopek dsnopek requested a review from m4gr3d April 22, 2024 15:08
Copy link
Collaborator

@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!

@@ -98,7 +98,7 @@ void OpenXRFbSceneExtensionWrapper::_on_instance_destroyed() {
}

const PackedStringArray &OpenXRFbSceneExtensionWrapper::get_supported_semantic_labels() {
static PackedStringArray semantic_labels = String(SUPPORTED_SEMANTIC_LABELS).split(",");
static PackedStringArray semantic_labels = String(SUPPORTED_SEMANTIC_LABELS).to_lower().split(",");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a comment describing why we're converting to lower case (e.g: to match Godot's automatically converting them to lower-case when saving properties).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, I added a comment at both the places where we convert the semantic labels to lower-case

@dsnopek dsnopek force-pushed the meta-spatial-entity-mesh branch from 3e9e485 to 91ed23f Compare April 22, 2024 15:41
@m4gr3d
Copy link
Collaborator

m4gr3d commented Apr 25, 2024

@dsnopek Can you rebase the PR?

@dsnopek dsnopek force-pushed the meta-spatial-entity-mesh branch from 91ed23f to a882cb0 Compare April 25, 2024 18:04
@dsnopek
Copy link
Collaborator Author

dsnopek commented Apr 25, 2024

@m4gr3d Rebased!

@m4gr3d m4gr3d merged commit 5be9896 into GodotVR:master Apr 25, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants