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

DragControls: Add rotate mode. #27689

Merged
merged 10 commits into from
Feb 9, 2024
Merged

DragControls: Add rotate mode. #27689

merged 10 commits into from
Feb 9, 2024

Conversation

lewibs
Copy link
Contributor

@lewibs lewibs commented Feb 5, 2024

Description

I often use three extensions that do not work well with the TranslateControls. I as a result have been using DragControls but when I need to rotate I have been using my own variation RotateControls. I am pushing this so that perhaps others don't need to rewrite the same code.

It intentionally has extremely similar API to DragControls as well so most of this was copy pasted from that code.

The way that it works, is that when the user clicks on an object, a up vector and horizon are created. When the user moves their mouse, the object is rotated around those vectors.

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 6, 2024

Do you mind reverting the deletion of build files? I've have tried to test out your controls with the below link but three.module.js can't be loaded.

https://rawcdn.githack.com/lewibs/three.js/dev/examples/misc_controls_rotate.html

I often use three extensions that do not work well with the TranslateControls.

Do you mean TransformControls? TranslateControls does not exists in this repository.

@lewibs
Copy link
Contributor Author

lewibs commented Feb 6, 2024

Yes sorry about that. Just reverted it. It should contain the build files.

I'm talking about TransformControls.
https://threejs.org/docs/#examples/en/controls/TransformControls

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 6, 2024

Yes sorry about that. Just reverted it. It should contain the build files.

Please make sure to not update build files at all. There should be no diff.

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 7, 2024

The feature is actually quite nice! I wonder if it makes sense to add a separate controls class though.

How about we add a new property to DragControls called mode? The default would be translate (similar to TransformControls) but you can set it to rotate to enable the new interaction presented in this PR.

misc_controls_drag can be modified such that users can choose the transformation mode for better testing.

@lewibs
Copy link
Contributor Author

lewibs commented Feb 7, 2024

I'm curious, what is it about it that you like? I also made a flag for the TransformControls that allows you to use it without the gizmos. It works the same way as the drag and rotate controls combined. However it doesn't change the rotation axis unfortunately it still rotates with the plane instead of about the horizon. I can make two prs if you are interested in seeing the other.

Tbh I would personally recommend the remove gizmo flag for the TransformControls. The reality is that TransformControls is just made extremely well by whoever made it. The main concern I have with the flag is the rotation axis. I haven't had the time to update it when the flag is on and I'm unsure when I will.

@lewibs
Copy link
Contributor Author

lewibs commented Feb 7, 2024

I just finished moving the rotation controls into the DragControls and updated the docs and the demo.

I reverted back to the original build so I'm not sure why its still showing a diff. What would you recommend doing to remove these changes from the history?

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 7, 2024

I suggest you revert all your commits in your branch (via git reset since this will keep the changes), undo changes to the build files, make a single new commit and then force-push. This will correct the history.

@lewibs lewibs reopened this Feb 7, 2024
@lewibs
Copy link
Contributor Author

lewibs commented Feb 7, 2024

Hey just finished that up. It should be good now. Let me know if there are any issues.

Also let me know if modifying TransformControls to have a flag called gizmoless that would make it act like DragControls would be useful.

@lewibs lewibs requested a review from Mugen87 February 7, 2024 21:54
@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 8, 2024

Also let me know if modifying TransformControls to have a flag called gizmoless that would make it act like DragControls would be useful.

Let's not develop TransformControls in this direction. That said, there is the idea to make the gizmos more configurable, see #27627.

@Mugen87 Mugen87 changed the title RotateControls: drag to rotate controls to pair with DragControls to translate objects DragControls: Add rotate mode. Feb 8, 2024
@lewibs lewibs requested a review from Mugen87 February 8, 2024 14:33
@Mugen87 Mugen87 added this to the r162 milestone Feb 8, 2024
@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 9, 2024

Let's give this a try!

@Mugen87 Mugen87 merged commit 8aef016 into mrdoob:dev Feb 9, 2024
11 checks passed
@lewibs
Copy link
Contributor Author

lewibs commented Feb 9, 2024

Awesome! Thanks so much

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

Successfully merging this pull request may close these issues.

2 participants