Skip to content
This repository has been archived by the owner on Aug 11, 2024. It is now read-only.

Fixed a few things with the new camera rig #405

Merged
merged 16 commits into from
Nov 9, 2019

Conversation

StephenHodgson
Copy link
Contributor

@StephenHodgson StephenHodgson commented Nov 5, 2019

XRTK - Mixed Reality Toolkit Change Request

Overview

Fixed the errors from the camera rig shutting down improperly and syncing the playspace and bodypace transforms correctly.

Mainly we wanna ensure that the body is roughly the same position/rotation of the head (within the adjustment angle threshold)

Target of the change:

Is this enhancement for:

  • Core (core framework, interfaces and definitions)

Changes:

Brief list of the targeted features that are being changed.

  • Fixed When stopping the project in the editor, an error is received about destroying the camera #404 component casting check to ensure we're not trying to destroy components that aren't a IMixedRealityCameraRig type.
  • Added flags to check if app was quitting before setting up objects when we're trying to cleanup.
  • Reverted some changes where we were parenting to the body instead of the playspace.
  • Reverted changes where we were moving the PlaySpace improperly.
  • Added appropriate fallbacks to the cashed camera reference if the camera system isn't enabled

Fixed component casting check to ensure we're not trying to destroy components that aren't a IMixedRealityCameraRig type
@StephenHodgson StephenHodgson added the Ready for review PR finished primary development, open for review label Nov 5, 2019
@StephenHodgson StephenHodgson added In Progress PR currently still being developed and removed Ready for review PR finished primary development, open for review labels Nov 5, 2019
SimonDarksideJ
SimonDarksideJ previously approved these changes Nov 5, 2019
@StephenHodgson StephenHodgson added Ready for review PR finished primary development, open for review and removed In Progress PR currently still being developed labels Nov 6, 2019
SimonDarksideJ
SimonDarksideJ previously approved these changes Nov 6, 2019
@StephenHodgson StephenHodgson added In Progress PR currently still being developed and removed Ready for review PR finished primary development, open for review labels Nov 6, 2019
@StephenHodgson StephenHodgson added Ready for review PR finished primary development, open for review Breaking Change and removed In Progress PR currently still being developed labels Nov 6, 2019
@StephenHodgson StephenHodgson changed the title Fixed new camera rig setup errors when shutting down Fixed a few things with the new camera rig Nov 6, 2019
@StephenHodgson
Copy link
Contributor Author

@SimonDarksideJ tested this on WMR but Open VR and Oculus still need to be tested.

@SimonDarksideJ
Copy link
Contributor

Sadly, utter fail, for all the same reasons we attached everything to the Body. Altering the headpose, does not alter all the things you want to "synchonise" with the head, e.g. Controllers, Teleportation, gaze, etc.
So this update breaks everything that requires the headpose to be updated, hence why it works find on WMR and OpenVR, which automatically translates the pose in the camera.

@StephenHodgson
Copy link
Contributor Author

Gonna close this for now. Yeah we def shouldn't be changing the playspace transform.

We expect that anyone will move that to re-position their players. This would def break in that scenario

@StephenHodgson StephenHodgson added In Progress PR currently still being developed and removed Ready for review PR finished primary development, open for review labels Nov 7, 2019
@StephenHodgson StephenHodgson reopened this Nov 7, 2019
@SimonDarksideJ
Copy link
Contributor

SO before we can merge this, we need to address the height set by platform (or manually) issue. Else we are regressing what is currently in public

@StephenHodgson
Copy link
Contributor Author

Agreed.

@SimonDarksideJ
Copy link
Contributor

Oculus PR unblocked and tested working with this new setup. Good to go.

SimonDarksideJ
SimonDarksideJ previously approved these changes Nov 9, 2019
@StephenHodgson
Copy link
Contributor Author

I'm gonna hold off on merging this until the submodules get their release

@StephenHodgson StephenHodgson added Ready for review PR finished primary development, open for review and removed In Progress PR currently still being developed labels Nov 9, 2019
@StephenHodgson
Copy link
Contributor Author

Should be ready now

@StephenHodgson StephenHodgson merged commit bdc3c6e into development Nov 9, 2019
@StephenHodgson StephenHodgson deleted the fix/camera-rig-errors branch November 9, 2019 17:00
XRTK-Build-Bot pushed a commit that referenced this pull request Nov 9, 2019
* Added flags to check if app was quitting before setting up objects

Fixed component casting check to ensure we're not trying to destroy components that aren't a IMixedRealityCameraRig type

* better camera cleanup

* return a null camera if we're shutting down

* reverted some changes to how we're syncing the body/playspace transforms

* Added an assert for verification

* sync the body's x/z position with the camera

* add comment

* Added back in the body rotation modifications

* updated SDK checkout

* added in inspector options for camera profile and fixed missing serialization attribute

* updated sdk checkout

* reverted a few things and made sure we're not manipulating the playspace

added in more documentation
updated rig transfrom
added in rig transform reset

* Added some null checks for when camera system isn't enabled

* updated oculus checkout

* updated submodules

* updated all the submodules
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Breaking Change Ready for review PR finished primary development, open for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When stopping the project in the editor, an error is received about destroying the camera
2 participants