[June18] App bar positioning bug fix #2042 #2212
Conversation
…egy called TightFollow
…offset along face normal
| /// <summary> | ||
| /// Uses an alternate follow style that works better for very oblong objects | ||
| /// </summary> | ||
| public bool UseTightFollow = false; |
There was a problem hiding this comment.
should probably be a property
| @@ -108,6 +113,8 @@ public BoundingBox BoundingBox | |||
| } | |||
| } | |||
|
|
|||
| public BoundingBoxRig BoundingRig; | |||
cre8ivepark
changed the title
|
changed the two public variables to Properties |
| @@ -150,6 +186,8 @@ public ButtonTemplate[] DefaultButtons | |||
| } | |||
| } | |||
|
|
|||
|
|
|||
|
|
|||
| GameObject clone = GameObject.Instantiate(target); | ||
| clone.transform.localRotation = Quaternion.identity; | ||
| clone.transform.position = Vector3.zero; | ||
| clone.transform.localScale = new Vector3(1,1,1); |
|
2 more changes- extra space and static call |
| for (int i = 0; i < boundsPoints.Count; ++i) | ||
| { | ||
| boundsPoints[i] = target.transform.localToWorldMatrix.MultiplyPoint(boundsPoints[i]); | ||
| //boundsPoints[i] += centroid; |
There was a problem hiding this comment.
will we use this later?
| GameObject clone = GameObject.Instantiate(targetObject); | ||
| clone.transform.localRotation = Quaternion.identity; | ||
| clone.transform.position = Vector3.zero; | ||
| clone.transform.localScale = new Vector3(1, 1, 1); |
| switch (index) | ||
| { | ||
| case 0: | ||
| return new int[] { 0, 1, 3, 2 }; |
There was a problem hiding this comment.
we could make these readonly fields so we don't have to allocate new arrays each time.
| return new int[] { 1, 0, 4, 5}; | ||
| } | ||
|
|
||
| return new int[0]; |
| public Vector3[] GetFaceEdgeMidpoints(int index) | ||
| { | ||
| Vector3[] corners = GetFaceCorners(index); | ||
| Vector3[] midpoints = new Vector3[4]; |
There was a problem hiding this comment.
this could also be a readonly field if we're only ever updating the same 4 array values.
There was a problem hiding this comment.
corners and midpoints will always have unique values.
| return face; | ||
| } | ||
|
|
||
| return new Vector3[0]; |
There was a problem hiding this comment.
could be a cached readonly array
| float hightestDotValue = float.MinValue; | ||
| for (int i = 0; i < 6; ++i) | ||
| { | ||
| Vector3 a = (lookAtPoint - GetFaceCentroid(i) ).normalized; |
There was a problem hiding this comment.
some strange formatting on this line.
| GameObject clone = GameObject.Instantiate(target); | ||
| clone.transform.localRotation = Quaternion.identity; | ||
| clone.transform.position = Vector3.zero; | ||
| clone.transform.localScale = new Vector3(1, 1, 1); |
| /// <param name="target">The gameObject whose bounding box is desired</param> | ||
| /// <param name="boundsPoints">the array of 8 points that will be filled</param> | ||
| /// <param name="ignoreLayers">a LayerMask variable</param> | ||
| public static void GetNonAABoundingBoxCornerPositions(GameObject target, List<Vector3> boundsPoints, LayerMask ignoreLayers) |
There was a problem hiding this comment.
It seems like we have a lot of duplicated code that does almost identical things.
Could consolidate these?
There was a problem hiding this comment.
yes- great idea! both the main functions can share a new static function that does the bulk of the calculations.
|
pushed more changes- including moving duplicated code into static function |
| /// The dynamic functions can be used to obtain boundingcube info on an object's Update loop. Operations | ||
| /// are minimized in the dynamic use scenario. | ||
| /// </summary> | ||
| public class BoundingBoxHelper : MonoBehaviour |
There was a problem hiding this comment.
Does this need to be a MonoBehaviour? Its only use is with the 'new' keyword, which causes a Unity warning.
| @@ -35,6 +35,24 @@ public class AppBar : InteractionReceiver | |||
| /// </summary> | |||
| public float HoverOffsetZ = 0f; | |||
|
|
|||
| private bool useTightFollow = false; | |||
There was a problem hiding this comment.
missing tooltip with summary description.
| /// The dynamic functions can be used to obtain boundingcube info on an object's Update loop. Operations | ||
| /// are minimized in the dynamic use scenario. | ||
| /// </summary> | ||
| public class BoundingBoxHelper : MonoBehaviour |
There was a problem hiding this comment.
Why isn't this class in a namespace?
There was a problem hiding this comment.
It's also in Utilities, while the rest of bounding box is in UX.
There was a problem hiding this comment.
This was merged a month ago.... Please open a new issue so we can track the needed change(s)
Overview
This PR addresses Issue #2042
"Bounding Box and App Bar not working as expected for Rectangular Holograms"
The fix causes the appbar to move to a cleaner location on objects that are very long in one dimension.
Changes
The non-axis-aligned bounding box (NAABB) helper functions are now broken out into a separate file: BoundingBoxHelper.cs.
This script contains a function to update the location of the appbar in its Update() loop:
GetNonAABoundingBoxCornerPositions()
This function finds the face of the non-axis aligned bounding cube of an object that is facing a target point. (this point is the headPosition in the example scene.)
This script also contains a static function which finds the same info if it is only needed once.
Included are additional helper functions for getting the edges of a face of the NAABB, the centroid of a face, the midpoint of the edges, the bottom of a face etc.
the Serialized Field AppBarHoverOffsetZ is now exposed at design time to allow the user to set a distance along the face normal that the appbar sits offset from the actual face.