Mixedrealitytoolkit-unity: Bounding Box Refactor

Created on 17 Jul 2019  路  8Comments  路  Source: microsoft/MixedRealityToolkit-Unity

Describe the bug

BoundingBox is one of the most commonly used components in MRTK but has over time grown into a huge monolithic class that is hard to maintain and understand for developers and users.
BoundingBox refactor should

  • make it's code easier to understand and reuse for MRTK developers
  • make BoundingBox easier to understand and use for consumers of MRTK
  • make it easier to merge common / shared functionality with ManipulationHandler (ObjectManipulator) or other components that have overlapping functionality
  • increase test coverage
  • provide clear documentation on how to set up bounds control and use it

Refactor breakdown:

  • [x] Separate Logic from Visuals in Bounding Box
  • [x] Look into making Bounds Control serialized objects -> scriptable objects
  • [x] Move any transform scale handler logic out of bounding box
  • [x] Add tests for BoundsControl and ProximityEffect
  • [x] (Potentially move ProximityEffect into own component to be reusable for other UX elements)
  • [x] Unify events and constraints / share logic between BoundsControl and ObjectManipulator
  • [x] Make AppBar / Example scripts work with new BoundsControl
  • [x] Improve Inspector for setting up BoundsControl
  • [x] Increase test coverage of BoundsControl #6284
  • [x] Provide migration tool for upgrading from BoundingBox to BoundsControl #6911
  • [x] add move feature https://github.com/microsoft/MixedRealityToolkit-Unity/issues/6927
  • [x] adjust runtime examples #6926

related #6260, #6261

Bug Current Iteration UX Controls UX Controls - Bounds Control

Most helpful comment

Wanted to add a couple of things that I realized while fixing bounds calculation:

  • Almost all changes to Properties recalculate the box. This means you can't update several properties and then recalculate it just once. There should be a Recalculate call you need to call after adjusting the BB settings
  • The BoundingBox is completely destroying all the gizmos and recreating them. That's a bad idea considering garbage collection and instantiate performance
  • SkinnedMeshRenderers are not supported. The current implementation fails in that bounds are calculated in AA world space bounds, while #5111 did not implement them yet
  • The Mixed Reality Portal has better usability with the box. You can grab handles, over-rotate the box or you could even stir it in circles with your motion controllers. This BB only ever deflects to a maximum of orthogonal to your view. A full rotation means re-grabbing handles and continue rotating

All 8 comments

I just want to say something here:

This is just the first time out of 3 times I had to repair the BoundingBox, now actually contributing the change to the source.
The first BoundingBox from the Hololens 2 years ago did not work very well when upside down and needed fixing.
The second Box from MRDL just did not differentiate between Hololens Hand Movement and the MR Motion Controllers, and needed fixing.
It then moved over officially and had those same problems of course and needed fixing.
Now the current implementation is the closest to being fully functional.

So please please please take extra care whoever does this. This box has been broken long enough now and nobody cared.

This certainly ranks at the top of my list of commonly used components that need some love. Looking forward to a refactor!

There are quite a few issues already filed on boundingbox so this is a great idea. #5111 , #5038 , #5006 , #4901 , #4755

Wanted to add a couple of things that I realized while fixing bounds calculation:

  • Almost all changes to Properties recalculate the box. This means you can't update several properties and then recalculate it just once. There should be a Recalculate call you need to call after adjusting the BB settings
  • The BoundingBox is completely destroying all the gizmos and recreating them. That's a bad idea considering garbage collection and instantiate performance
  • SkinnedMeshRenderers are not supported. The current implementation fails in that bounds are calculated in AA world space bounds, while #5111 did not implement them yet
  • The Mixed Reality Portal has better usability with the box. You can grab handles, over-rotate the box or you could even stir it in circles with your motion controllers. This BB only ever deflects to a maximum of orthogonal to your view. A full rotation means re-grabbing handles and continue rotating

I'm working on my fourth point right now and it looks good. I'll report back when I have something to show

https://github.com/HoloSpaces/MixedRealityToolkit-Unity/tree/mrtk_betterBBLogic

Thank you @alexees I agree that all of those bullet points should be addressed at some point. (And, have been pain points for developers.)

Hey you guys,

I spent a couple of days to come up with an improved version of a BoundingBox rotation. Before going to 3D-fiying the logic, I wanted you to have a look, if what you see is what you would expect when using it. Features are:

  • Aiming at the box always perfectly matches the handle with your ray
  • If the cursor is put into the box, you you twirl it about the center
  • if you move the cursor further to the sides, the box will over rotate
  • If you're INSIDE the box, you can just spin around yourself and it will rotate

The white circle represents the circular motion vector of one handle you can grab.
The blue line with blue circle represents a motion controller with its beam and the cursor at the distance by the time of the handle grab.
The green dot is the handle.
The green line appears when the box overextends.

ezgif com-video-to-gif

closing this now - open individual tickets for more work on bounds control - graduation is tracked here https://github.com/microsoft/MixedRealityToolkit-Unity/issues/7358

Was this page helpful?
0 / 5 - 0 ratings