-
Notifications
You must be signed in to change notification settings - Fork 703
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
Implemented GizmoHelper, ViewportGizmo and ViewcubeGizmo #303
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/pmndrs/drei/fdx9403xz |
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 fbebfda:
|
|
this would be great to have. as for position and heads-up-display, you can take a look at this: https://codesandbox.io/s/react-three-fiber-viewcube-forked-pgm9m def dont use react spring for this, if you want to animate, better lerp it. controls is going to be tough. with drei we could add something to our controls so that they allow movement readout. you could add it to this pr - it would also be useful in many other situations. i just dont know where you'd store that. but if this gizmo had at least the control reference, it could shut it off while moving. the best way i can think of is forking orbit/trackball. we are suffering from these for a long time and threejs is not going to fix them. they have longstanding bugs and are sealed. if we could say "controls, go there" by code, "zoom here", etc, that would make lots of things easier. |
Made a few changes per @drcmda's recommendations (position is now configurable) - horizontal animation seems ok, but vertical seems janky (maybe I need to set the camera up vector during animation) and the distance calculation doesn't seem quite correct either. Demo at https://codesandbox.io/s/bold-browser-i2qr1?file=/src/index.js I would love to collaborate on "CAD controls" (with zoom to mouse functionality and the common presets/animations baked in.) |
Would it be crazy to decouple functionality from presentation? <NavigationGizmo>
{(...whateverIsNecessary) => (<CustomGizmoComponent />)}
</NavigationGizmo> that would allow users to drop in their own Gizmo renderer. This can be done at a later time and is not blocking for the PR, of course |
This could be an interesting starting point for react-based controls:
Of course the alternative is just making them for vanilla and using them as usual, but we've been thinking about trying the react way, for a while 🤔 |
Great suggestion! I split up the code into GizmoHelper (functionality) and two presentation components (BlenderViewportGizmo and a basic ViewCubeGizmo). |
ffd4edf
to
3cc7a56
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left some thoughts, some of them apply to multiple components, but I didn't want to spam and re-write it x times. If you have any questions or want to discuss anything let me know.
8517471
to
b99791e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall it's shaping up really nicely, there's a lot of code to go through! But i think there's a lots to discuss // do after this round to shape it up further.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been thinking more about the use case where some, but not all of the Gizmo's controls reflect the orientation of the main scene, like the rotation buttons in Autodesk's ViewCube (https://youtu.be/UlUssM353Rw?t=116). Would this require adding another (static) camera to the GizmoHelper to use with the "fixed position" Children, or can this be done within the Gizmo component, maybe using the Billboard component? |
Why would this need another camera? Maybe I'm not understanding correctly 🤔 If you think it's worth it, maybe an example impl could be done in a sandbox so we don't mess with this PR before discussion? |
@w3dot0 did you want to change something or could this come later? |
@joshuaellis sorry, been swamped and haven't gotten to creating the example. I think a combination of Billboard and Html components should work for what I was thinking of (no extra camera required), so I'd say let's merge this if @gsimone and @drcmda are ok with the current structure. |
No worries! Yep, I'll merge tomorrow at some point unless they have objections. Thanks again, look forward to pt. 2 |
🎉 This PR is included in version 3.9.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
@w3dot0 does it work with trackballcontrols? i am trying to combine them but it seems to break. could we add this? |
The camera animation logic could use some help. Could this use react-spring?
Integrating it with OrbitControls and other camera controls is rather hard. Using
onUpdate
andonTarget
to return the control target is hacky, but I don't see a way to get a reference to the controls (and to handle the different types). Any help is appreciated!