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

TransformControls v3 #20720

Closed
wants to merge 5 commits into from
Closed

TransformControls v3 #20720

wants to merge 5 commits into from

Conversation

arodic
Copy link
Sponsor Contributor

@arodic arodic commented Nov 20, 2020

Here is a full rewrite of TransformControls with a bunch of improvements. Most notably:

  • Combined modes - Now you can translate, rotate and scale with the same gizmo. Optionally, you can replicate the old behavior using showTranslate, showRotate and showScale properties. mode property still works with a deprecation warning.
  • Inertia now works with TransformControls if you set enableDamping and dampingFactor property.
  • Grazing angle interactions are now much, much better. It is impossible to translate your object into infinity by mistake.
  • Active axis helpers are removed There are no more gray axis lines during interactions (if this is something people really liked I can bring it back).
  • WebXR support (requires Added "controllermove" event to WebXRController #20790)
  • Improved code quality and various other things.

This PR should be labeled as DRAFT.

Any feedback and testing support is welcome!

LIVE DEMO

TODO:

  • Figure out jsm -> js conversion.
  • Get Added "controllermove" event to WebXRController #20790 merged for WebXR support.
  • Manually test all examples and editor for regressions in multiple OS/Browser environments.
  • Add new parameters to documentation.
  • Make dithering optional. Don't dither gl.lines.
  • Remove residual inertia on mouseup.
  • Increase highlight fade speed.
  • Fix safari grid layout.

@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 20, 2020

Nit: You also have to update the examples/js version. Or to be more precise: You edit this version and generate examples/jsm/controls/TransformControls.js via node utils/modularize.js.

@Mugen87 Mugen87 marked this pull request as draft November 20, 2020 09:55
@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 20, 2020

This PR should be labeled as DRAFT.

Then let's make it a Draft PR.

@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 20, 2020

If I open the example and directly scale the mesh (by selecting one of the white helper boxes), it seems I get a mix of camera movement and scaling. This does not happens if I click on the canvas once.

@arodic
Copy link
Sponsor Contributor Author

arodic commented Nov 20, 2020

Nit: You also have to update the examples/js version. Or to be more precise: You edit this version and generate

I can wait until examples/js gets deprecated. I'll need another week or two to finish this PR and get feedback. If I remember correctly, removal was scheduled for this November. Has the plan changed?

If I open the example and directly scale the mesh (by selecting one of the white helper boxes), it seems I get a mix of camera movement and scaling. This does not happens if I click on the canvas once.

Thanks, I'll look into it.

@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 20, 2020

I can wait until examples/js gets deprecated. I'll need another week or two to finish this PR and get feedback. If I remember correctly, removal was scheduled for this November. Has the plan changed?

Yes, this plan has changed. A module-only codebase is now a thing for the future: #20568 (comment)

@arodic
Copy link
Sponsor Contributor Author

arodic commented Nov 20, 2020

That's too bad. Compiling modules from old namespace-style codebase is completely backwards. I'd prefer not to do that. Perhaps if there was a way to generate js from jsm I could live with that.

@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 20, 2020

Perhaps if there was a way to generate js from jsm I could live with that.

It's the plan to do that. There are already PRs for this, see #20527 or #20529.

@mrdoob
Copy link
Owner

mrdoob commented Nov 20, 2020

That's too bad. Compiling modules from old namespace-style codebase is completely backwards. I'd prefer not to do that. Perhaps if there was a way to generate js from jsm I could live with that.

Would you like to create a utils/demodularize.js that converts jsm/controls/TransformControls.js to js/controls/TransformControls.js as a proof of concept?

@mrdoob mrdoob added this to the r124 milestone Nov 20, 2020
@mrdoob
Copy link
Owner

mrdoob commented Nov 20, 2020

Oh wait, I just saw #20529.

@arpu
Copy link

arpu commented Nov 21, 2020

@arodic any idea if this will work with #webxr?

@arodic
Copy link
Sponsor Contributor Author

arodic commented Nov 23, 2020

@arpu that would be awesome. Are there any #webxr controls I can use as an example. I just ordered Oculus Quest 2 so I might as well give it a shot.

@arpu
Copy link

arpu commented Nov 23, 2020

@arodic for testing i think https://github.com/MozillaReality/WebXR-emulator-extension is good

@arodic
Copy link
Sponsor Contributor Author

arodic commented Nov 25, 2020

As alternative to #20527 and #20529 I added a simple demodularize.js prototype to this PR. It is only tested with TransformControls and works fine in examples when imported via THREE namespace.

Converted non-modules still use es6 syntax such as class, let, const, () => {} etc. I hope this is acceptable.

Also added to this PR: Updated examples and editor to use the new API.

@arodic arodic marked this pull request as ready for review November 25, 2020 06:36
@gkjohnson
Copy link
Collaborator

It is only tested with TransformControls and works fine in examples when imported via THREE namespace.

Due to some of the things I've mentioned in #20455 (comment) (specifically dependencies split out into multiple files) I think "demodularizing" files is a more complex task than "modularizing" in the general case. In my opinion it would be best to rely on purpose built and maintained tools to perform this transformation -- I think the first thing that needs to happen for the JSM -> JS transformation pipeline is to settle on what a desirable final file structure is.

@arodic
Copy link
Sponsor Contributor Author

arodic commented Nov 28, 2020

@gkjohnson sounds good. I agree we should use purpose-built tools. Hopefully there will be a conclusion to the jsm > js story.

@arpu I started looking into WebXR support. I completed the first step by adding RayPointer abstraction to the PointerTracker which should be more compatible with XR controllers. However, it seems to me that more work needs to be done on overall WebXR support in three.js - or perhaps I'm just not familiar with it.

@arodic
Copy link
Sponsor Contributor Author

arodic commented Nov 29, 2020

Good news @arpu! I was able to add WebXR support. Check it out here!

It still needs some cleanup and testing but its looking promising. However, it does require some minor changes to WebXRManager and WebXRController. I'll prepare another PR for that.

@obrhaberer
Copy link

Could you please add a "mode" where there is only the rotation fixed to the layer perpendicular to the view-axis activated? The yellow circle in the example. Thank you.

@ManishJu
Copy link
Contributor

i think it would be good to have active axis helpers back. People use it to see the relative position to other objects

@mrdoob mrdoob modified the milestones: r124, r125 Dec 24, 2020
@arodic
Copy link
Sponsor Contributor Author

arodic commented Dec 25, 2020

Could you please add a "mode" where there is only the rotation fixed to the layer perpendicular to the view-axis activated? The yellow circle in the example. Thank you.

Good suggestion @obrhaberer. I added showE property so you can have the desired behavior by setting showX, showY and showZ to false. You can try it here by pressing "x", "y" and "z" keys.

i think it would be good to have active axis helpers back. People use it to see the relative position to other objects

I figured it was useful for someone @ManishJu. I'll add it back.

@marcofugaro
Copy link
Contributor

marcofugaro commented Dec 27, 2020

Some feedback on the look&feel:

  • the damping feels wrong, it's there even when the mouse has stopped completely. In the example I first stop the mouse and then do the mouseup:
damping.mov
  • the opacity transition is too slow, it shouldn't get in the way of the user performing its transforms quick. You should keep the transition under 100ms.
opacity.mov

@arodic
Copy link
Sponsor Contributor Author

arodic commented Dec 28, 2020

Thanks for the feedback @marcofugaro!

the damping feels wrong

What is your OS/Browser and pointer device? I noticed similar behavior on Macbooks when using touchpad with three-finger-drag (enabled as accessibility feature). It appears to be a bug in the OS but I think there is a way to mitigate.

The opacity transition is too slow, it shouldn't get in the way of the user performing its transforms quick. You should keep the transition under 100ms.

Agreed! Both issues should be fixed now. Thanks!

@marcofugaro
Copy link
Contributor

What is your OS/Browser and pointer device? I noticed similar behavior on Macbooks when using touchpad with three-finger-drag (enabled as accessibility feature). It appears to be a bug in the OS but I think there is a way to mitigate.

Chrome on a Macbook pro, I am using the touchpad. Also I have enabled the three finger swipe as you say (I think).


PS the preview link is now broken:

@arodic
Copy link
Sponsor Contributor Author

arodic commented Dec 31, 2020

i think it would be good to have active axis helpers back. People use it to see the relative position to other objects

@ManishJu I added back translate offset and rotation axis visualization. For scale offset I have initial design but it has some issues. This feature can be disabled by setting .showOffset = false.

@marcofugaro the issue with residual inertia on macbook touchpad should be fixed now. Fade in/out speed is somewhat faster but still above 100ms. My issue with faster transition is that it can cause strobing-like effect which I dont like. But if you still like it to be faster, this feature can be adjusted with .dampingFactor parameter.

@mrdoob mrdoob removed this from the r125 milestone Jan 27, 2021
@mrdoob mrdoob added this to the r126 milestone Jan 27, 2021
- Deprecated `mode` property in favor of `showTranslate`, `showRotate` and `showScale`.
- Added support for combined transfomration gizmos.
- Added support for inertia via `enableDamping` and `dampingFactor` properties.
- Improved grazing angle interactions.
- Added support for WebXR (requires #20790)
- Improved code quality.
@mrdoob mrdoob modified the milestones: r126, r127 Feb 23, 2021
@mrdoob mrdoob modified the milestones: r127, r128 Mar 30, 2021
@obrhaberer
Copy link

@arodic there is one test continuously failing.

@mrdoob
Copy link
Owner

mrdoob commented Apr 22, 2021

Figure out jsm -> js conversion.

This should be solved now 👍

@mrdoob mrdoob modified the milestones: r128, r129 Apr 23, 2021
@arodic
Copy link
Sponsor Contributor Author

arodic commented Apr 23, 2021

I re-implemented WebXR support with "move" event from a63ff05 but there are still few small issues to iron out.

Next, I will look into jsm > js conversion.

@obrhaberer thanks for pointing out the failing test. I'll need to update some of the screenshots since the visual appearance of the gizmo has changed.

@obrhaberer
Copy link

@arodic again some hiccups.

@mrdoob mrdoob modified the milestones: r129, r130 May 27, 2021
@mrdoob mrdoob modified the milestones: r130, r131 Jun 30, 2021
@arodic
Copy link
Sponsor Contributor Author

arodic commented Jun 30, 2021

I'm gonna close this pull request because:

  • Current implementation diverged too much and it's going to be nearly impossible to rebase this PR.
  • I have a newborn to take care of and I find it difficult to find time to work on TransformControls in the near future.

I'll be keeping track of progress with TransformControls but I'll give up ownership of the code @mrdoob @Mugen87 @marcofugaro @tschw. In the future, I hope to make a new suite of three.js controls in a separate repository.

@arodic arodic closed this Jun 30, 2021
@Mugen87 Mugen87 removed this from the r131 milestone Jun 30, 2021
@mrdoob
Copy link
Owner

mrdoob commented Jul 1, 2021

I have a newborn to take care of

That's definitely P0! 😍

@arodic arodic deleted the TransformControls_v3 branch April 19, 2022 07:09
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.

None yet

8 participants