feat: add support for multiple active modes to TransformControls (#927)

[Glavin001]

Why

Enable multiple/all modes at the same time for TransformControls. See #927

What

Demo: https://drei-git-fork-glavin001-feat-927-all-transformcontrols-pmndrs.vercel.app/?path=/story/controls-transformcontrols--transform-controls-all-modes-story

Modes:

🆕 All	Translate	Rotate	Scale

Checklist

• [x] Get 👍 from @sketchpunk it's OK to add to Drei
• [x] Merge in dependent Pull Request
  • https://github.com/sketchpunklabs/manipulator3d/pull/1
  • [x] manipulator3d is published to npm
• [x] Add support for other TransformControls features (i.e. from transformOnlyPropNames)
  • [x] Works by default
  • Snap
    • ~~Translation Snap~~
    • ~~Rotation Snap~~
  • ~~Show axis~~
  • Events
    • [x] change
    • ~~mouseDown~~
    • ~~mouseUp~~
    • [x] objectChange
  • ~~Methods~~
  • ~~Properties~~
• [x] Documentation updated
• [x] Storybook entry added
• [x] Ready to be merged

For future Pull Requests:

• [ ] Add support for other TransformControls features (i.e. from transformOnlyPropNames)
  • Snap
    • [ ] Translation Snap
    • [ ] Rotation Snap
  • [ ] Show axis
  • Events
    • [ ] mouseDown
    • [ ] mouseUp
  • [ ] Methods
  • [ ] Properties

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
drei	✅ Ready (Inspect)	Visit Preview	Jul 10, 2022 at 3:33AM (UTC)

[codesandbox-ci[bot]]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 37524514155c1b8e26fcb039514ac4e53077c307:

Sandbox	Source
suspicious-cannon-emymk0	Configuration
Ground reflections and video textures	Configuration
arc-x-pmndrs-colors	Configuration

[FarazzShaikh]

I think the ability to enable only selected modes and any combination of modes would be a great feature to have.

Let’s fork the original into drei/sec/helpers and add this to the code. It should be fairly simple. This will also get rid of the ugly git import.

also, if we implement the above feature, we can completely replace out the original TransfromControls “UI” with this Manipulator3D one, it’s much better looking. Thoughts?

[sketchpunk]

https://twitter.com/SketchpunkLabs/status/1537265991599669250 :)

[Glavin001]

Wow, great news! Thanks! I hope to have some time this weekend to polish this MR up. 🎉

[Glavin001]

Switched to the npm package and receiving an error now. I don't have much time to debug it right now, will investigate later.

Pushed so you both can see and hopefully have ideas for how to fix: https://github.com/pmndrs/drei/runs/6977979092?check_suite_focus=true#step:6:3063

ERR! => Failed to build the preview
ERR! Module not found: Error: Can't resolve 'manipulator3d' in '/home/runner/work/drei/drei/src/core'
ERR! ModuleNotFoundError: Module not found: Error: Can't resolve 'manipulator3d' in '/home/runner/work/drei/drei/src/core'
ERR!     at /home/runner/work/drei/drei/node_modules/webpack/lib/Compilation.js:925:10
...
ERR!  ModuleNotFoundError: Module not found: Error: Can't resolve 'manipulator3d' in '/home/runner/work/drei/drei/src/core'
...
ERR! resolve 'manipulator3d' in '/home/runner/work/drei/drei/src/core'
ERR!   Parsed request is a module
ERR!   using description file: /home/runner/work/drei/drei/package.json (relative path: ./src/core)
ERR!     Field 'browser' doesn't contain a valid alias configuration
ERR!     resolve as module
ERR!       /home/runner/work/drei/drei/src/core/node_modules doesn't exist or is not a directory
ERR!       /home/runner/work/drei/drei/src/node_modules doesn't exist or is not a directory
ERR!       /home/runner/work/drei/node_modules doesn't exist or is not a directory
ERR!       /home/runner/work/node_modules doesn't exist or is not a directory
ERR!       /home/runner/node_modules doesn't exist or is not a directory
ERR!       /home/node_modules doesn't exist or is not a directory
ERR!       /node_modules doesn't exist or is not a directory
ERR!       looking for modules in /home/runner/work/drei/drei/node_modules
ERR!         using description file: /home/runner/work/drei/drei/package.json (relative path: ./node_modules)
ERR!           Field 'browser' doesn't contain a valid alias configuration
ERR!           using description file: /home/runner/work/drei/drei/node_modules/manipulator3d/package.json (relative path: .)
ERR!             no extension
ERR!               Field 'browser' doesn't contain a valid alias configuration
ERR!               /home/runner/work/drei/drei/node_modules/manipulator3d is not a file
ERR!             .mjs
ERR!               Field 'browser' doesn't contain a valid alias configuration
ERR!               /home/runner/work/drei/drei/node_modules/manipulator3d.mjs doesn't exist
ERR!             .js
ERR!               Field 'browser' doesn't contain a valid alias configuration
ERR!               /home/runner/work/drei/drei/node_modules/manipulator3d.js doesn't exist

[Glavin001]

@sketchpunk Any ideas on the error message above? I've pushed the code so it can be pulled down.

[sketchpunk]

Googling the error about browser not having an alias seems to be varied. Saw this link, maybe it'll help you trouble shoot how your referencing the module. Beyond that I'm not sure what the problem is. I setup a blank product and npm installed the module & my test ran as expected.

I notice there might be 2 version of threejs being loaded, not sure if thats part of the problem.

https://candid.technology/field-browser-doesnt-contain-a-valid-alias-configuration/

[sketchpunk]

ERR!                     using description file: /home/runner/work/drei/drei/node_modules/manipulator3d/package.json (relative path: ./dist/Manipulator3D.es.js)
ERR!                       no extension
ERR!                         Field 'browser' doesn't contain a valid alias configuration
ERR!                         /home/runner/work/drei/drei/node_modules/manipulator3d/dist/Manipulator3D.es.js doesn't exist

Think I might see the problem. Package.json marks the entry module file as ./dist/Manipulator3D.es.js but in the dist folder the file name is ./dist/manipulator3d.es.js. For me this isn't an issue since windows file names / paths are case insensitive but if your on macs or linux, this can result in not found errors. Is there way for you to modify the package.json file in your node_modules folder to make the main/module and export property have the file names in all lowercase to confirm that this is indeed the problem which I'll then fix it on my end & update npm with a new version.

[drcmda]

ERR!                     using description file: /home/runner/work/drei/drei/node_modules/manipulator3d/package.json (relative path: ./dist/Manipulator3D.es.js)
ERR!                       no extension
ERR!                         Field 'browser' doesn't contain a valid alias configuration
ERR!                         /home/runner/work/drei/drei/node_modules/manipulator3d/dist/Manipulator3D.es.js doesn't exist

Think I might see the problem. Package.json marks the entry module file as ./dist/Manipulator3D.es.js but in the dist folder the file name is ./dist/manipulator3d.es.js. For me this isn't an issue since windows file names / paths are case insensitive but if your on macs or linux, this can result in not found errors. Is there way for you to modify the package.json file in your node_modules folder to make the main/module and export property have the file names in all lowercase to confirm that this is indeed the problem which I'll then fix it on my end & update npm with a new version.

the package is indeed broken, like you said it won't work on anything non-windows. the cause is 100% certain camelcase. even remote tools like https://unpkg.com/manipulator3d fail, i guess cdn's would fail also.

the manipulator is beautiful btw!

[sketchpunk]

updated npm with case fix in package.json. Hopefully the module will now build correctly for you.

[Glavin001]

Thank you both!

Latest version works 👍 https://drei-git-fork-glavin001-feat-927-all-transformcontrols-pmndrs.vercel.app/?path=/story/controls-transformcontrols--transform-controls-all-modes-story

I'm travelling this week & next week. Will try to get back to this when I can. Feel free to continue with this Pull Request while I'm away if someone else has bandwidth.

[Glavin001]

Subset of active modes

Examples using https://drei-git-fork-glavin001-feat-927-all-transformcontrols-pmndrs.vercel.app/?path=/story/controls-transformcontrols--transform-controls-select-object-story

All

Translate only

Translate + Scale

Rotate + Scale

[Glavin001]

@FarazzShaikh : Ready for review! There are minor TransformControls features which are not yet supported (see above), I think the important features are now though.

[FarazzShaikh]

On first read te code looks good to me. Few notes:

• The "hit boxes" of the transform gizmo is a bit small making it hard to target
• The translation gizmo does not allow translation in all 3 axes at once
• The scale gizmo does not allow scaling in 2 axes at once
• It just doesnt feel as smooth as three/jsm/controls/TransformControls especially when zooming in, can we make the gizmo's resizing when zooming more responsive?

Also, do we want to keep the current TransfromControls gizmo or replace it with this? I think we can convert TransformControls into something like GizmoHelper where the Gizmo can be swapped out with the default or this one. Not sure how complicated that would be or would it even be worth it.

[Glavin001]

do we want to keep the current TransformControls gizmo or replace it with this?

I was thinking we keep the original for single modes and if developers want multiple modes activated then they can easily opt-in to the new controls. Original TransformControls is more battle-tested. After we've received and implemented more feedback then could deprecate the old in favour of the new.

Few notes:

I'll leave these for @sketchpunk to comment.

[iuriiiurevich]

• The "hit boxes" of the transform gizmo is a bit small making it hard to target

Yes, it is not really convenient to use on touch devices.

By the way, thanks to all of you guys! This is a great feature!

[Glavin001]

@sketchpunk @FarazzShaikh : Can you both discuss the manipulator3d feedback (1, 2) to come to an agreement on what changes manipulator3d will support and when, so we can proceed with this PR for drei? Thanks!

Let me know if there are changes within the scope of this PR for drei I should make. This PR now achieves what I intended, happy to make edits though. FYI I'm back from travelling, hosting visiting family, etc -- July has been crazy -- and will be able to get into coding again soon/this week. 🎉 😃 Looking forward to merging and sharing with the community!

[drcmda]

@Glavin001 @iuriiiurevich @FarazzShaikh to give you all an update, sorry it's been taking so long. i've been playing with manipulator and imo it behaves odd in some situations. i get race conditions, scaling issues, and, like transform controls, it's global and doesn't rotate with the content it transforms. some can probably be fixed upstream but the rest is just how vanilla rolls (mutation and traversal) and that will always be an issue imo.

we (team buerli) have started to re-implement one from scratch: https://codesandbox.io/s/object-gizmo-controls-om2ff8?file=/src/App.js

it

• doesn't mutate or take over the scene, add gizmo stuff via scene.add etc
• works with any other control like orbitcontrols automatically
• rotates with its children, behaves as a real pivot
• offsets with position={[x,y,z]} rotation={[x,y,z]} which does not change contents but just sets the pivot outset
• can be used as a -1/0/1 bbanchor via anchor={[1,1,1]} which uses the childrens boundary box (this was modelled after drei/bbanchor
• outputs the matrix itself without actually turning children or as a standalone so it can be used for more complex usecases
• can activate and de-activate specific axes
• shows labels (rotate it)
• can round transforms for clean positioning (press shift while rotating it)

i think it would make more sense to go with this one since we have full control over it and can extend it.

we would not merge it with transform controls but add it as <ObjectControls>

@Glavin001 would you want to hop in and add stuff? there is so much we could do. real heads up display for angles and coords, view flip so that all axes face the camera, etc.

[FarazzShaikh]

Fair points, ObjectControls looks to be more robust. It needs some QOL improvements like scaling as the camera zooms but it looks solid. I like the labels, another QOL improvement could be snapping when Shift is held.

[drcmda]

fair points, ObjectControls looks to be more robust. It needs some QOL improvements like scaling as the camera zooms but it looks solid. I like the labels, another QOL improvement could be snapping when Shift is held.

it has that, i think sizeAttenuation or worldUnits not sure what the prop was called, but it's line2 based and it can scale for sure, somehow :-P

we add types now and then quickly move forward, add a story etc. this could then be adjusted over time.

[iuriiiurevich]

@drcmda Yes, it sounds great, thank you!

A couple of feature requests:

• flag to keep the same size when zooming the camera (as in TransformControls)
• same for rotation: flag to keep the same default angle (not rotate with children or camera)
• large hit areas (unusable on touch devices otherwise, manipulator3d has the same problem)

Maybe I can find the time to do some of that.

[drcmda]

flag to keep the same size when zooming the camera (as in TransformControls)

done, with the "fixed" prop

same for rotation: flag to keep the same default angle (not rotate with children or camera)

not sure what this means but pivot controls can be a controlled component, like a react form, ondrag feeds matrix, matrix feed pivot. it doesn't have to wrap children and the user can decide what to do with these matrixes.

large hit areas (unusable on touch devices otherwise, https://github.com/sketchpunklabs/manipulator3d/issues/8

i guess this would need work. no idea how to do that rn.

[drcmda]

@iuriiiurevich i am trying to add scale to it https://codesandbox.io/s/object-gizmo-controls-om2ff8?file=/src/App.js but struggle a lot with the math, would you be able to help with that?

[iuriiiurevich]

@drcmda I'd love to, but struggled with the same things when trying to add it quickly a couple of months ago. I definitely don't have free time in December, but I might return to this feature early next year if it hasn't been added yet.