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.

Sign up for GitHub

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

Jump to bottom

ObjectManipulator's ManipulationLogic observes XRSocketInteractor #567

Merged
merged 2 commits into from
Jan 19, 2024
Merged

ObjectManipulator's ManipulationLogic observes XRSocketInteractor #567

merged 2 commits into from
Jan 19, 2024

Conversation

whebertML
Copy link
Contributor

  • Modifies ManipulationLogic for position and rotation to properly observe XRSocketInteractor.
  • Turns off object smoothing when selected by XRSocketInteractor.
  • Also observes XRRayInteractor forceGrab property for position, moving the object to the ray interactor's force grab attach transform.

Resolves #492

@whebertML whebertML self-assigned this Nov 29, 2023
@whebertML whebertML mentioned this pull request Nov 29, 2023
Closed
david-c-kline
david-c-kline previously approved these changes Dec 6, 2023
View reviewed changes
Copy link
Contributor

@david-c-kline david-c-kline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved pending unit test success 😄

AMollis
AMollis previously approved these changes Dec 7, 2023
View reviewed changes
@AMollis AMollis added Type: Feature Request A request for a new feature that can be included with the next minor version release. Package: Spatial Manipulation The Project spatial manipulation package is impacted by this issue. labels Dec 7, 2023
@whebertML
ObjectManipulator's ManipulationLogic observes XRSocketInteractor
0d81ae1 
* Modifies ManipulationLogic for position and rotation to properly
  observe XRSocketInteractor.
* Turns off object smoothing when selected by XRSocketInteractor.
* Also observes XRRayInteractor forceGrab property for position, moving
  the object to the ray interactor's force grab attach transform.
* Added unit tests.
@whebertML whebertML dismissed stale reviews from AMollis and david-c-kline via 0d81ae1 January 4, 2024 18:47
@whebertML
Copy link
Contributor Author

whebertML commented Jan 4, 2024 •
edited
Loading

Unit tests added. @AMollis @david-c-kline

@whebertML whebertML merged commit 0bf1b5c into MixedRealityToolkit:main Jan 19, 2024
4 checks passed
@anonymous2585
Copy link
Contributor

anonymous2585 commented Jan 23, 2024 •
edited
Loading

@whebertML thank you for this improvement!

I tested it and it works but the option Hover Socket Snapping of the XRSocketInteractor is not observed by the ObjectManipulator when it is set to true: the object doesn't snap to the socket attach position before user release select.

Can you check if you have the same behaviour and open another issue if confirmed?

@whebertML
Copy link
Contributor Author

whebertML commented Jan 23, 2024 •
edited
Loading

@anonymous2585 Thank you for testing the feature!

I believe the MRTK3 project itself is still only dependent on XRI version 2.3.0, which doesn't yet contain the Hover Socket Snapping property of XRSocketInteractor. Not sure when that was introduced (XRI 2.5.2?), but such a feature could be added when the project upgrades it's dependency on a newer XRI I would think.

Just curious, did you modify your project to update to XRI 2.5.2 or later?

@anonymous2585
Copy link
Contributor

@whebertML You are right! I didn't noticed Hover Socket Snapping was added only in XRI 2.5.0

Added snap transformations to XRSocketInteractor on hover.

Yes, we use XRI 2.5.2.

Maybe it's feasible to support it when the installed XRI package is >= 2.5.0?

@whebertML
Copy link
Contributor Author

@anonymous2585 Definitely technical possibly of course. Please feel free to add a feature request and it will be discussed.

@anonymous2585 anonymous2585 mentioned this pull request Jan 24, 2024
Open
@whebertML whebertML deleted the issue492 branch May 8, 2024 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AMollis AMollis AMollis approved these changes

@david-c-kline david-c-kline david-c-kline left review comments

@marlenaklein-msft marlenaklein-msft Awaiting requested review from marlenaklein-msft marlenaklein-msft is a code owner

@shaynie shaynie Awaiting requested review from shaynie shaynie is a code owner

Assignees

@whebertML whebertML

Labels
Package: Spatial Manipulation The Project spatial manipulation package is impacted by this issue. Type: Feature Request A request for a new feature that can be included with the next minor version release.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEATURE REQUEST] ObjectManipulator observes XRSocketInteractor selection as expected.
4 participants
@whebertML @anonymous2585 @david-c-kline @AMollis