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
6k stars 2.12k forks source link

BoundsControl rotation issue after Upgrading from BoundingBox #10134

Closed Ahim13 closed 9 months ago

Ahim13 commented 3 years ago

Hi We upgraded from the BoundingBox to the BoundsControl to get the fix for the flickering issue, but it brought up a new issue

Describe the bug

BoundsControl does not rotate around the object's center but the object's pivot. Most of our assets and Gameobjects have their pivot elsewhere because the medical devices are using a different coordinate system. So they are rotating around an invisible point when the user grabs one of the Rotation handles.

As a reference here is the change in the code: BoundingBox.cs

Target.transform.RotateAround(rigRoot.transform.position, axis, angle);

BoundsControl.cs

Target.transform.rotation = smoothingActive ?
                            Smoothing.SmoothTo(Target.transform.rotation, constraintRotation.Rotation, rotateLerpTime, Time.deltaTime) :
                            constraintRotation.Rotation;

A clear and concise description of what the bug is.

BoundsControl does not rotate around rigroot anymore which results in an unexpected rotation when trying to rotate with one of the rotationhandles.

To reproduce

Have a gamobject that has it's pivot point outside of the object. Then apply Boundscontrol on it. When grabbing the Rotation handle it will rotate around an invisible point, and not the center of the object.

Your setup

Target platform

Additional context

Also tried creating our own BoundsControl, by copying the original code and change the rotation only, but the IBoundsTargetProvider is internal, hence we can not use it outside.

Zee2 commented 3 years ago

Thanks; we can take a look into this.

tommensink commented 3 years ago

This is a very disturbing bug.

m-the-hoff commented 3 years ago

I recommend we maintain backwards compatibility by adding a choice between Rotate around center of bounds / rotate around pivot option where the default is the prior behavior around pivot. Open to not adding an option if we feel confident no one is depending on the prior pivot behavior.

tommensink commented 3 years ago

Agreed, and I cannot think myself about a reason to rotate around the pivot. The pivot is most often an art designer's choice. But of course I do not know all the use cases. At least it is then as it was, which was the most natural way for a human.

tommensink commented 2 years ago

Is this already being looked at @m-the-hoff ? It is a pretty bad bug. If you want I can fix it, have not done PRs yet for MRTK, but maybe nice to start with that and contribute?

Zee2 commented 2 years ago

Hi @tommensink , this is fixed in our internal MRTK v3 build, and will be backported as soon as we can.

IssueSyncBot commented 9 months ago

We appreciate your feedback and thank you for reporting this issue.

Microsoft Mixed Reality Toolkit version 2 (MRTK2) is currently in limited support. This means that Microsoft is only fixing high priority security issues. Unfortunately, this issue does not meet the necessary priority and will be closed. If you strongly feel that this issue deserves more attention, please open a new issue and explain why it is important.

Microsoft recommends that all new HoloLens 2 Unity applications use MRTK3 instead of MRTK2.

Please note that MRTK3 was released in August 2023. It features an all new architecture for developing rich mixed reality experiences and has a minimum requirement of Unity 2021.3 LTS. For more information about MRTK3, please visithttps://www.mixedrealitytoolkit.org.

Thank you for your continued support of the Mixed Reality Toolkit!