Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OnManipulationEnded doesn't fire for rotation or scale #5213

Closed
fast-slow-still opened this issue Jul 5, 2019 · 9 comments
Closed

OnManipulationEnded doesn't fire for rotation or scale #5213

fast-slow-still opened this issue Jul 5, 2019 · 9 comments

Comments

@fast-slow-still
Copy link
Contributor

@fast-slow-still fast-slow-still commented Jul 5, 2019

Description

The ManipulationHandler event OnManipulationEnded doesn't seem to fire at the end of a rotation or scale, only at the end of an object translation.

To reproduce

Create a component which, on Start, adds a BoundingBox and ManipulationHandler components. For example:

        private void Start()
        {
            boundingBox = gameObject.AddComponent<BoundingBox>();

            boundingBox.BoundingBoxActivation = BoundingBox.BoundingBoxActivationType.ActivateByProximityAndPointer;

            manipulationHandler = gameObject.AddComponent<ManipulationHandler>();
            manipulationHandler.OneHandRotationModeFar = ManipulationHandler.RotateInOneHandType.MaintainOriginalRotation;
            manipulationHandler.OneHandRotationModeNear = ManipulationHandler.RotateInOneHandType.MaintainOriginalRotation;
            manipulationHandler.ConstraintOnRotation = Toolkit.Utilities.RotationConstraintType.YAxisOnly;
            manipulationHandler.OnManipulationEnded.AddListener(FinishManipulation);

            nearGrabbable = gameObject.AddComponent<NearInteractionGrabbable>();

            boundingBox.Active = false;
        }

Implement FinishManipulation(ManipulationEventData) to do something clearly visible (e.g. return the object to its starting point).

Attach the component to an object (e.g. cube) in the scene and run the scene.

If you move the object, its FinishManipulation gets called. If you rotate or scale the object using the boundingbox handles, the object will be properly rotated or scaled, but there will be no callback at the end.

Expected behavior

ManipulationEnded event should fire for any type of manipulation.

Your Setup (please complete the following information)

  • Unity Version [2018.4.1f1]
  • MRTK Version [e.g. mrtk_development, 2019-07-05, rc2.1]

Target Platform (please complete the following information)

  • HoloLens
@lukastoenneMS
Copy link
Contributor

@lukastoenneMS lukastoenneMS commented Jul 16, 2019

I think the reason for this is

  1. When you translate the object by clicking on a face it does actually use the ManipulationHandler. Note how BoundingBox.HandleType does not include Translate.
  2. When you rotate or scale the object using the BoundingBox it does not use the ManipulationHandler at all, so no manipulation events can be expected.

I'm not 100% certain about this because the BoundingBox class is huge ... we should really separate the graphics elements from the transform logic there.

Loading

@julenka
Copy link
Contributor

@julenka julenka commented Jul 17, 2019

That's true lukas, and 100% agree bounding box is massive....would love to have one of us dedicate some time to refactoring bounding box after GA and other bugs are fixed.

Loading

@julenka
Copy link
Contributor

@julenka julenka commented Jul 17, 2019

I think this is technically by design, since "manipulation ended" event would need to be raised by the bounding box.

Loading

@julenka
Copy link
Contributor

@julenka julenka commented Jul 17, 2019

Note that you could rotate or scale using two hand manip, and then manipulationended would get called.

Loading

@thalbern
Copy link
Contributor

@thalbern thalbern commented Jul 17, 2019

there's also rotate and scale events on the bounding box you could subscribe to.

Loading

@thalbern thalbern mentioned this issue Jul 17, 2019
12 tasks
@thalbern
Copy link
Contributor

@thalbern thalbern commented Jul 17, 2019

I'll close this now as per design - we can think about having better events in bounding box when we do the big refactor post GA. Created a follow up issue: #5340

Loading

@thalbern thalbern closed this Jul 17, 2019
@fast-slow-still
Copy link
Contributor Author

@fast-slow-still fast-slow-still commented Jul 18, 2019

Recapping here:

  1. ManiuplationEnded may or may not get called depending on whether you are using one or two handed.
  2. Suggested workaround is to handle manipulationHandler.ManipulationEnded for translate and two handed rotate/scale, but then boundingbox.RotateStopped/ScaleStopped for rotation and scale manipulations?
  3. I hate to ask, but will RotateStopped/ScaleStopped get called redundantly with ManipulationEnded for two handed rotate/scale, or does two handed rotation not trigger boundingBox.RotateStopped?

Loading

@julenka
Copy link
Contributor

@julenka julenka commented Jul 22, 2019

I think a way that will help to think about this is that Manipulation* events come from the manipulation handler, not bounding box. When you rotate / scale an object using one of the bounding box handles, you are using the bounding box component. When you move / scale / rotate by interacting with the object's collidable, you are using manipulation handler.

ManiuplationEnded may or may not get called depending on whether you are using one or two handed.

Not entirely. if you grab and move the object, you'll get manipulation ended. If you grab affordances you'll be using bounding box which doesn't raise those event.

Suggested workaround is to handle manipulationHandler.ManipulationEnded for translate and two handed rotate/scale, but then boundingbox.RotateStopped/ScaleStopped for rotation and scale manipulations?

That sounds right. Can you think of a better way to design it? For example, should bounding box also just raise ManipulationStarted/Ended events?

I hate to ask, but will RotateStopped/ScaleStopped get called redundantly with ManipulationEnded for two handed rotate/scale, or does two handed rotation not trigger boundingBox.RotateStopped?

No, RotateStopped/ScaleStopped will only be called when you are using the BoundingBox.

Loading

@fast-slow-still
Copy link
Contributor Author

@fast-slow-still fast-slow-still commented Jul 26, 2019

Thanks @julenka , that explanation helps a lot. I'm still trying to get a reasonable solution for my situation, but I think you've given me enough understanding that I should be able to accomplish what I need.

On your design question, I do think that from the user's perspective it would be more convenient and less complex if the various manipulation events were routed through a common handler, and the ManipulationHandler seems perfectly appropriate for that.

Bernie assures me that the BoundingBox has more pressing needs from a refactor, but that might be something to consider during the upcoming refactor.

Thanks again for the clarity!

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants