Skip to content

Added handle types for 2d/3d translation #711

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Added handle types for 2d/3d translation #711

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
765b942
added handle types Translation2D and Translation3D
Daniel4144 Apr 2, 2024
74199ca
fixed enum values
Daniel4144 Apr 2, 2024
ea75af1
Merge branch 'main' into added-handletype-translation2d
keveleigh Apr 10, 2024
9ed906e
Merge branch 'main' into added-handletype-translation2d
Daniel4144 Jun 25, 2024
49a37a9
Merge branch 'main' into added-handletype-translation2d
keveleigh Jul 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions org.mixedrealitytoolkit.spatialmanipulation/BoundsControl/BoundsControl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -915,6 +915,56 @@ private void TransformTarget()
// transformUpdated = elasticsManager.ApplyTargetTransform(constraintTranslate, TransformFlags.Move);
// }

if (!transformUpdated.IsMaskSet(TransformFlags.Move))
{
Target.transform.position = smoothingActive ?
Smoothing.SmoothTo(Target.transform.position, constraintTranslate.Position, translateLerpTime, Time.deltaTime) :
constraintTranslate.Position;
}
}
else if (currentHandle.HandleType == HandleType.Translation2D)
{
Vector3 translateVectorOnPlane = Vector3.ProjectOnPlane(currentGrabPoint - initialGrabPoint, currentHandle.transform.forward);

var goal = initialTransformOnGrabStart.Position + translateVectorOnPlane;
MixedRealityTransform constraintTranslate = MixedRealityTransform.NewTranslate(goal);
if (EnableConstraints && constraintsManager != null)
{
constraintsManager.ApplyTranslationConstraints(ref constraintTranslate, true, currentHandle.IsGrabSelected);
}

// TODO: Elastics integration (soon!)

// if (elasticsManager != null)
// {
// transformUpdated = elasticsManager.ApplyTargetTransform(constraintTranslate, TransformFlags.Move);
// }
Comment on lines +936 to +941
Copy link
Contributor

@ms-RistoRK ms-RistoRK Jun 25, 2024

Choose a reason for hiding this comment

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

Thank you for your contribution, this change looks great. We are trying to keep the codebase as clean as possible so we encourage contributors to avoid leaving commented code out of main branch. Could you please remove the commented code?


if (!transformUpdated.IsMaskSet(TransformFlags.Move))
{
Target.transform.position = smoothingActive ?
Smoothing.SmoothTo(Target.transform.position, constraintTranslate.Position, translateLerpTime, Time.deltaTime) :
constraintTranslate.Position;
}
}
else if (currentHandle.HandleType == HandleType.Translation3D)
{
Vector3 translateVector = currentGrabPoint - initialGrabPoint;

var goal = initialTransformOnGrabStart.Position + translateVector;
MixedRealityTransform constraintTranslate = MixedRealityTransform.NewTranslate(goal);
if (EnableConstraints && constraintsManager != null)
{
constraintsManager.ApplyTranslationConstraints(ref constraintTranslate, true, currentHandle.IsGrabSelected);
}

// TODO: Elastics integration (soon!)

// if (elasticsManager != null)
// {
// transformUpdated = elasticsManager.ApplyTargetTransform(constraintTranslate, TransformFlags.Move);
// }
Comment on lines +961 to +966
Copy link
Contributor

@ms-RistoRK ms-RistoRK Jun 25, 2024

Choose a reason for hiding this comment

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

Thank you for your contribution, this change looks great. We are trying to keep the codebase as clean as possible so we encourage contributors to avoid leaving commented code out of main branch. Could you please remove the commented code?

Copy link
Contributor Author

@Daniel4144 Daniel4144 Jun 26, 2024

Choose a reason for hiding this comment

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

The same commented code is in all other if-branches, so I decided to keep it in the two branches I added. I think removing the comments only here would cause more headache, in case that code is still relevant in the future.
In fact, the new branches are just copies of the existing branch for HandleType.Translation where only the first line of the branch changed, which is arguably not the ideal solution. Alternatively, I could merge the three branches for the translation types and add another if-statement inside to calculate the correct translation vector.
Let me know what you think about it.

Copy link
Contributor

@ms-RistoRK ms-RistoRK Jun 26, 2024

Choose a reason for hiding this comment

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

I understand your point of view, I too leave commented sections in my personal branches. However, since this is an open-source project I think it is best to leave the code as clean as possible because not everyone in the community may have the same context as the author. @MixedRealityToolkit/mixedrealitytoolkit-unity-maintainers What are your thoughts on this?


if (!transformUpdated.IsMaskSet(TransformFlags.Move))
{
Target.transform.position = smoothingActive ?
Expand Down
12 changes: 11 additions & 1 deletion org.mixedrealitytoolkit.spatialmanipulation/BoundsControl/BoundsControlTypes.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,22 @@ public enum HandleType
Scale = 1 << 1,

/// <summary>
/// A handle that is mounted to the face of a <see cref="BoundsControl"/>, and can move the object.
/// A handle that is mounted to the face of a <see cref="BoundsControl"/>, and can move the object along the forward axis.
/// </summary>
/// <remarks>
/// Handles of this type are currently not supported.
/// </remarks>
Translation = 1 << 2,

/// <summary>
/// A handle that is mounted to the face of a <see cref="BoundsControl"/>, and can move the object normal to the forward axis.
/// </summary>
Translation2D = 1 << 3,

/// <summary>
/// A handle that is mounted to the face of a <see cref="BoundsControl"/>, and can move the object in all three dimensions.
/// </summary>
Translation3D = 1 << 4,
}

/// <summary>
Expand Down