microsoft / MixedRealityToolkit-Unity

This repository is for the legacy Mixed Reality Toolkit (MRTK) v2. For the latest version of the MRTK please visit https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity
https://aka.ms/mrtkdocs
MIT License
6.01k stars 2.12k forks source link

When configuring the MRTK the MixedRealityPlayspace is not configured properly #2899

Closed StephenHodgson closed 6 years ago

StephenHodgson commented 6 years ago

Does this affect the legacy HoloToolkit (master) or the Mixed Reality Toolkit (mrtk_release)? Mixed Reality Toolkit

Describe the bug When a developer imports and sets up the MRTK by pressing "configure..." the MixedRealityPlayspace should be what the Main Camera parents itself to. It used to work this way and no longer works as expected. The Main Camera should have a GameObject parent so the Teleport and Input Systems work properly.

Parenting the MixedRealityPlayspace to the Main Camera helps keep startup costs down, and lets developers understand what the scene is doing before entering play mode.

To Reproduce

  1. Open a new project
  2. Import MRTK
  3. Press Configure
  4. See that Main Camera is not parented correctly

Expected behavior The Main Camera should be parented on a new GameObject named MixedRealityPlayspace

Actual behavior Main Camera is not parented correctly

Additional Comments When enabling and disabling either the Input or Teleport Systems they should prepare the scene appropriately, as both rely on the Main Camera to be parented.

When both systems are disabled the Main Camera shouldn't be parented at all (as it's unnecessary, and some platforms may not need it)

keveleigh commented 6 years ago

I don't think "it used to work this way" implies that it's now broken.

Do those systems no longer work properly?

From my investigation, their calls to get the MixedRealityPlayspace reference causes the camera to become parented when needed.

StephenHodgson commented 6 years ago

I don't think "it used to work this way" implies that it's now broken.

I explicitly designed it work work this way, so yes. It is broken.

Do those systems no longer work properly?

They work but the problem is that the workflow isn't the same.

Parenting the MixedRealityPlayspace to the Main Camera helps keep startup costs down, and lets developers understand what the scene is doing before entering play mode.

StephenHodgson commented 6 years ago

I'm also getting this warning in the editor for the Material Gallery:

image

SimonDarksideJ commented 6 years ago

Like the HTK before it. The Camera should have a parent object, to detract it from the scene and have it managed properly, especially when also dealing with the Boundary. It's supposed to have a parent. Originally it was called the Body (and camera was called head), this was then updated to be MRPS and Camera.

All the testing / framework and work (including the transforms for the incoming data) are all centered around the Camera having a parent, so it should definitely have one.

Although yes, we have also added functionality to allow it to work without one, it is not the expected or desired approach for the project.

keveleigh commented 6 years ago

Yes, the only thing that has changed about the Camera being given a parent object is when that happens. The camera still gets a parent object if it doesn't have one when one is asked for.

This issue isn't about allowing the camera to work without a parent, but about updating the time at which the camera is given a parent.

SimonDarksideJ commented 6 years ago

From it's initial inception, this has always been when: a) You select the "MixedRealityManager" configuration profile (it checks for one and then asks to create it) b) You select "MixedRealityToolkit->Configure" in the editor menu.

Basically, if we add the MixedRealityManager to the scene, then the camera should also be setup appropriately.

it should NOT change when the project is run, as it changes the developers expectation on start. So if the developer chooses to parent the camera to something else or remove the Playspace, then we do not prevent that.

david-c-kline commented 6 years ago

it should NOT change when the project is run, as it changes the developers expectation on start. So if the developer chooses to parent the camera to something else or remove the Playspace, then we do not prevent that.

Do we work without a parented camera? I was under the impression that we require parenting (we did in HTK if memory serves). If my impression is correct, then parenting at runtime allows the app to work correctly >even if< the developer unparents the camera.

StephenHodgson commented 6 years ago

True, but we also want to make sure the user understands that, and gets a visual representation before they every run the scene so that they don't accidentally do something that breaks the setup.

StephenHodgson commented 6 years ago

The point is we didn't have to change the way we setup the scene, but we did, and now it doesn't do what we originally intended it to do. That's a problem.