feat: ✨ Implement co-location using markers #3115

zmerp · 2025-11-23T10:47:01Z

Tested on Quest 3 + Windows. It should work on the Samsung Galaxy XR also.

The-personified-devil

Preliminary review, didn't do the openxr extension impl yet

alvr/session/src/settings.rs

alvr/common/src/average.rs

alvr/client_openxr/src/stream.rs

alvr/server_core/src/tracking/mod.rs

alvr/common/src/average.rs

The-personified-devil · 2025-11-30T17:59:23Z

Also is perpetual recentering really the best idea? Afaik headsets are actually pretty good at keeping position so wouldn't it make more sense to always collect the data but only recenter on a recenter request? Or at least that should also be a feature since it shouldn't be too hard to implement. Especially because if you stare away from the qr code for a while and only catch glimpses of it every now and then this could cause bad location prediction and then the recenter gets messed up. Whilst if you only do it at fixed intervalls you can just stare at the qr code for a second and then do the recenter.

zmerp · 2025-12-01T17:14:25Z

The appeal of this feature is not needing to recenter at all. And about your concern of worsening the pose prediction if the QR code is far, you are right and I can use the relative position from the headset as heuristic and weight for the angle and position estimation. This improvement was planned, sort of (depending on the resulting tracking quality)

The-personified-devil

There's some specific stuff I want changed, but I think at the heart of it we're utterly misusing the spatial entity api. It's very much intended to be partially static and only be refreshed if something actually changes, and we're totally ignoring that. (Tho I'm not 100% sure if the runtime is effectively allowed to "rename" an already discovered marker during an update cycle or if we can actually count on that being fixed. (renaming makes zero sense, idk what would be going on there))

alvr/session/src/settings.rs

The-personified-devil · 2025-12-03T18:01:52Z

alvr/server_core/src/tracking/mod.rs

    }

+    pub fn recenter_from_marker(&mut self, config: &MarkerColocationConfig) {
+        if let Some(marker_pose) = self.markers.get(&config.qr_code_string) {


Why not just do this:

// In case the marker isn't found, don't recenter to keep the last `inverse_recentering_origin` let Some(marker_pose) = self.markers.get(&config.qr_code_string);

This makes it even more obvious what happens in case we don't find the marker instead of having to scroll all the way down.

The-personified-devil · 2025-12-03T18:03:34Z

alvr/server_core/src/tracking/mod.rs

+
+            let recentering_origin = Pose {
+                position,
+                orientation: Quat::from_rotation_y(recentering_marker.average_angle.get_average()),


You already have average_angle as a variable

alvr/server_core/src/tracking/mod.rs

alvr/client_openxr/src/extra_extensions/spatial_marker_tracking.rs

The-personified-devil · 2025-12-03T23:31:38Z

alvr/client_openxr/src/extra_extensions/spatial_marker_tracking.rs

+            ))?;
+        }
+
+        let mut out_markers = vec![];


Might be worth initializing with marker_count capacity

Since we're filtering now this should either be with the codes to track size or just not done

The-personified-devil · 2025-12-03T23:32:05Z

alvr/client_openxr/src/extra_extensions/spatial_marker_tracking.rs

+        }
+
+        let mut out_markers = vec![];
+        for idx in 0..query_result.entity_id_count_output as usize {


We already have marker_count, use that.

alvr/client_openxr/src/extra_extensions/spatial_marker_tracking.rs

The-personified-devil · 2025-12-04T00:03:12Z

alvr/client_openxr/src/extra_extensions/spatial_marker_tracking.rs

+            out_markers.push((string, pose));
+        }
+
+        Ok(Some(out_markers))


Since the data doesn't shift if you actually only call the update function, it might make a lot of sense to send over what markers we want from the server and optionally send over an association from string to id on initial discovery within the struct and then only send over the ids with the data.

I know this adds some complexity, but imo it could save quit a bit of unnecessary bandwidth if you start getting into multi marker territory (this might happen in the future when we wanna extend this to use them for tracking moving objects for whatever reason) to only send over the association between id and string when that changes just alongside the current poses for the ids and then only send over id + pose live. Since this is entirely one way it should be pretty easy to do, however this is essentially it's own thing so totally fine if u don't wanna do it.

zmerp · 2025-12-08T11:21:00Z

Thanks for the tips @The-personified-devil, this is working 200% better now. Since now I discover the marker evey single tracking frame, the anchoring converges basically instantaneously when discovering the marker for the first time, and re-converges in about 1 second when the marker is moved.

I made the marker colocation setting disjointed from the recentering settings. Since the marker colocation acts like recentering, it will continuously override the manual recentering.

The-personified-devil

This is starting to look pretty good now, however it seems that you either forgot to push a commit or you forgot about a lot of the previous comments

The-personified-devil · 2025-12-08T16:00:35Z

alvr/client_openxr/src/extra_extensions/spatial_marker_tracking.rs

+const MAX_MARKERS_COUNT: usize = 32;
+const DISCOVERY_TIMEOUT: Duration = Duration::from_secs(1);
+
+struct SpatialContextReadyData {


Why are all of these types named like ur trying to do scoping in C?

The-personified-devil · 2025-12-08T16:12:44Z

alvr/client_openxr/src/extra_extensions/spatial_marker_tracking.rs

+    time::{Duration, Instant},
+};
+
+const CAPABILITY: sys::SpatialCapabilityEXT = sys::SpatialCapabilityEXT::MARKER_TRACKING_QR_CODE;


I was thinking more like QR_CODE_CAP or QR_CAPABILITY, since that's what it is

The-personified-devil · 2025-12-08T16:15:10Z

alvr/client_openxr/src/interaction.rs

+        if self.platform.is_quest() {
+            #[cfg(target_os = "android")]
+            {
+                alvr_system_info::try_get_permission("com.oculus.permission.USE_ANCHOR_API");
+                alvr_system_info::try_get_permission("com.oculus.permission.USE_SCENE")
+            }


Since we now only create the spatial context when the setting is actually enabled we should also only conditionally request the permission

The-personified-devil · 2025-12-08T16:17:48Z

alvr/client_openxr/src/lib.rs

-                    xr::Event::Unknown => {
-                        // use event_storage.as_raw(), reinterpret as sys::BaseInStructure, get type
-                        // and then reinterpret as the event struct
-                    }


You mentioned you got 1s timing on having new markers appear in frame or something, did you check if the discover events actually work now and if they could improve timing? (And if so reduce the default rechecking interval)

Oh that was about moving the thing, well I guess the movement won't happen in reality for trackers used to do recentering and the 1s is an entirely recentering averaging side thing, but this still holds for actually discovering new trackers...

The-personified-devil · 2025-12-08T16:20:18Z

alvr/client_openxr/src/extra_extensions/spatial_marker_tracking.rs

+// Note: The Meta implementation is currently bugged and the buffer capacity cannot be changed once
+// the fist call of query_spatial_component_data is made.
+const MAX_MARKERS_COUNT: usize = 32;


I'm curious if you rechecked if the runtime is bugged with all the code improvements, I'm ok with this ig, but it'd be nice if we could avoid it. (And if you can please file a bug, we can literally just link this repo as a reproduction source.)

The-personified-devil · 2025-12-08T16:23:12Z

alvr/client_openxr/src/extra_extensions/spatial_marker_tracking.rs

+    entity_ids: [sys::SpatialEntityIdEXT; MAX_MARKERS_COUNT],
+    entity_states: [sys::SpatialEntityTrackingStateEXT; MAX_MARKERS_COUNT],
+    bounded_2d_arr: [sys::SpatialBounded2DDataEXT; MAX_MARKERS_COUNT],
+    marker_arr: [sys::SpatialMarkerDataEXT; MAX_MARKERS_COUNT],
+    string_buffer: [c_char; 256],


Rust blows up the stack when creating big structs (mainly in debug mode, it creates them on the stack and then moves them to memory) and I think the entire interaction context ends up a a humongous stack, but realistically this shouldn't actually be an issue because stack size limits are huge nowadays. But yea scoping was the main advantage I had in mind.

The-personified-devil · 2025-12-08T16:26:50Z

alvr/client_openxr/src/extra_extensions/spatial_marker_tracking.rs

+                // Custom enums don't have the method insert(), so we cannot get back a mutable
+                // reference directly.
+                return Ok(None);


Why not if let Some State::Creating(future) = ... {// do creation} and follow that up with let Some State::Ready(data) = ... else {return Ok(None)}?

The-personified-devil · 2025-12-08T16:30:44Z

alvr/client_openxr/src/extra_extensions/spatial_marker_tracking.rs

+        // Get snapshot for already discovered entities if no discovery happened
+        // Note: We are not generating a snapshot for the already discovered entities if we got a
+        // discovery snapshot. This is because the


Where did half the comment go?

The-personified-devil · 2025-12-08T16:38:08Z

alvr/client_openxr/src/extra_extensions/spatial_marker_tracking.rs

+            ))?;
+        }
+
+        let mut out_markers = vec![];


Since we're filtering now this should either be with the codes to track size or just not done

The-personified-devil · 2025-12-08T16:39:14Z

alvr/client_openxr/src/extra_extensions/spatial_marker_tracking.rs

+                    .map_err(|_| sys::Result::ERROR_SPATIAL_BUFFER_ID_INVALID_EXT)?
+                    .to_owned();
+
+                if self.codes_to_track.is_empty() || self.codes_to_track.contains(&string) {


Why all if the codes_to_track are empty? In the server the data isn't processed in that case anyway...

The-personified-devil · 2025-12-08T16:46:40Z

alvr/client_openxr/src/extra_extensions/spatial_marker_tracking.rs

+        unsafe {
+            super::xr_res((self.spatial_entity_fns.destroy_spatial_snapshot)(
+                snapshot_handle,
+            ))?;
+        }


Oh yea, forgot to say, I'd highly prefer the snapshot being wrapped in some type with a destructor because now it won't be ran when we encounter an error after the snapshot has been created.

The-personified-devil · 2025-12-08T17:11:04Z

alvr/client_openxr/src/extra_extensions/spatial_marker_tracking.rs

+                || spatial_context_data.marker_arr[idx].data.buffer_id
+                    == sys::SpatialBufferIdEXT::NULL


So the state buffer id being NULL corresponds to the runtime not yet having decoded the qr code. So it'd probably be a good idea to throw those objects into a separate list of spatial entities that's appended to the other list so we can still get them if the runtime eventually-resolves them. Same goes for entities that are not currently tracking (I wonder if keeping the discovery snapshot longer actually results in better behavior with re-picking up lost qr codes?), we wanna pick them up during the discovery snapshot and only ignore them during update snapshots (or rather when parsing for the data vs checking which to place into our spatial entities queue). We should probably do some additional handling for discovery snapshots. (And we should be removing entities with tracking state stopped from our updates because they'll never come back alive (double check the spec on that tho pls)).

Maybe something similar to how the cts code does things, i.e.:
Extract the fetching and live data processing and then when doing a discovery snapshot do:
fetching + discovery processing (i.e. picking entities we're interested) + live processing
and then on an update snapshot only do:
fetching + live processing

Alternatively we could rely on only getting the strings on the discovery snapshot (which we're effectively doing currently), this is not something guaranteed by the runtime tho, and then on future updates just not fetch the additional marker data and only the bounds.

If we don't do that we're already fetching the buffer_ids anyway and so I think we should be mapping the buffer_id rather than the entitiy_id to the string because whilst it'd make enormous sense for the buffer, i.e. the string, to not be allowed to change after the initial decoding has happened (the fucking qr code gonna rename itself???), the spec doesn't directly guarantee this and only guarantees that the same buffer ids must carry the same data (i.e. the string). This would be more correct and add zero overhead to the existing method. (I'm also not sure, you'd have to consult the spec again but iirc buffer ids can also stay the same across discoveries).

The-personified-devil · 2025-12-08T17:21:20Z

alvr/client_openxr/src/interaction.rs

        }

        self.body_source = None;
+        self.marker_spatial_context = None;


I'm aware that we do the clean slate wiping to reduce friction with colliding exensions and I think that's great design, but maybe violating that and trying to keep the context if the feature stays enabled would be worthwhile since multi-second (background) startup times every time the stream reconnects doesn't seem optimal, your call tho (this is something that should happen in a separate pr tho).

zmerp · 2025-12-08T17:40:07Z

I did have a moment where I git reset (not hard) by mistake and had to re-pull and fix the merging issues. I might have accidentally deleted some of the edits because I remember having done them already. In any case I will work on all the remaining issues tomorrow.

zmerp marked this pull request as draft November 23, 2025 10:47

zmerp force-pushed the marker-colocation branch 2 times, most recently from 9418db0 to cb44823 Compare November 25, 2025 09:29

zmerp requested a review from The-personified-devil November 28, 2025 04:00

zmerp marked this pull request as ready for review November 28, 2025 04:00

The-personified-devil requested changes Nov 30, 2025

View reviewed changes

feat: ✨ Implement co-location using markers

5ddbb99

zmerp force-pushed the marker-colocation branch from 7fd2cb2 to 5ddbb99 Compare December 3, 2025 15:41

The-personified-devil requested changes Dec 4, 2025

View reviewed changes

Fix impl + update every frame

bf0e688

zmerp force-pushed the marker-colocation branch from 74b9dac to bf0e688 Compare December 8, 2025 09:56

The-personified-devil requested changes Dec 8, 2025

View reviewed changes

The-personified-devil reviewed Dec 8, 2025

View reviewed changes

		\|\| spatial_context_data.marker_arr[idx].data.buffer_id
		== sys::SpatialBufferIdEXT::NULL

Uh oh!

feat: ✨ Implement co-location using markers #3115

Are you sure you want to change the base?

feat: ✨ Implement co-location using markers #3115

Uh oh!

Conversation

zmerp commented Nov 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

The-personified-devil left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

The-personified-devil commented Nov 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zmerp commented Dec 1, 2025

Uh oh!

The-personified-devil left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zmerp commented Dec 8, 2025

Uh oh!

The-personified-devil left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zmerp commented Nov 23, 2025 •

edited

Loading

The-personified-devil commented Nov 30, 2025 •

edited

Loading