Spatial Observer Volume ignores MRTK Playspace transform

[fast-slow-still]

Describe the bug

This one is my bad, I missed it when fixing up other SpatialMesh issues with Playspace transform.

When the WindowsMixedRealitySpatialMeshObserver sets the ObserverVolume, it incorrectly accounts for the MRTK Playspace transform (and any other transform on the camera hierarchy).

The tangible outcome is that when there is a non-identity transform on the Playspace, the Spatial Meshes will no longer be centered around the camera

To reproduce

Steps to reproduce the behavior:

1. Set a large translation on the playspace (e.g. teleport a few meters).
2. Notice that the spatial meshes are centered around where you were, rather than where you are.

Expected behavior

The spatial meshes to recenter around the viewer as the viewer teleports around.

Additional context

There are two conflicting approaches to this problem. It is important to note that, whatever else, the volume passed into observer.SetVolumeXXX(volume) is expected to be in the same space as the head pose. That is, in the space of the camera's parent.

Approach 1) Treat WindowsMixedRealitySpatialMeshObserver.ObserverOrigin as being in the same space as the head pose (i.e. already in the camera's parent's space). In that case, the error is in UpdateObserver(), where ObserverOrigin should be CameraCache.Main.transform.localPosition, rather than position.

Approach 2) Treat ObserverOrigin as being in world space (Unity's root coordinate system). In that case, UpdateObserver() is correct, but ConfigureObserverVolume() needs to transform the world space ObserverOrigin into the camera's parent's space before passing it to Observer.SetVolumeXXXX(). This would presumably use CameraCache.Main.transform.parent.InverseTransformPoint(ObserverOrigin), with appropriate null check(s).

Since ObserverOrigin is publicly settable, it is probably expected that it is in world space, but the expected space isn't actually explicitly stated, so I might be wrong there. But if it is generally in world space, then Approach 2) would be preferred. Otherwise, Approach 1) is a simpler fix.

Note that everything here also applies to ObserverRotation.

[wiwei]

I'm guessing this is referring to:

https://github.com/microsoft/MixedRealityToolkit-Unity/pull/4474/files

[fast-slow-still]

Yes, I fixed the mesh placement there, but spaced and forgot to correct the observer volume.

[david-c-kline]

Yes, I fixed the mesh placement there, but spaced and forgot to correct the observer volume.

Got any cycles to write up a fix?

[david-c-kline]

This feels like the correct option

Approach 2) Treat ObserverOrigin as being in world space (Unity's root coordinate system). In that case, UpdateObserver() is correct, but ConfigureObserverVolume() needs to transform the world space ObserverOrigin into the camera's parent's space before passing it to Observer.SetVolumeXXXX(). This would presumably use CameraCache.Main.transform.parent.InverseTransformPoint(ObserverOrigin), with appropriate null check(s).

[fast-slow-still]

Sorry, @davidkline-ms , I didn't see your post before I left last night. The joys of time-zone differences.

Your fix looks good, I'll see if I can get access to the CI fail to see what happened there. My ungrounded suspicion is that in some of the tests there might not be a camera setup, in which case camera parent space should be treated as world space, rather than skipping entirely. But I'll need to find the CI logs to verify that.