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

Basic Lattice/FFD Implementation #44

Merged
merged 15 commits into from
Feb 3, 2021

Conversation

sabresaurus
Copy link
Contributor

Simple Lattice deformer implementation, this uses a lattice with a resolution from 2 to 32 per axis. Vertices are reprojected using the lattice by finding the neighbouring cell in the source lattice (when its a source grid), finding its local position within the cell then interpolating it against the modified cell corner positions. For vertices outside the lattice it will clamp the interpolation to the nearest control points. Control points can be edited by selecting them on the lattice and moving them with the translate handle.

Feedback is very welcome, happy to make changes to fix any issues or bring it in line with any particular code style.

Some ideas for future areas of improvements (outside the scope of this PR):

  1. Support non-linear interpolation modes
  2. Find a way of getting marquee select working with the control points, also could consider edge select
  3. Find a way of making it work with the Unity animation system
  4. Add support for non-grid lattices (e.g. custom meshes)
Screen.Recording.2021-01-31.at.20.08.58.mov

keenanwoodall and others added 14 commits December 2, 2020 18:04
…y written, Process needs a better way of turning serialised arrays into native arrays, doesn't work with meshes bigger than unit bounds, no scene view editing support yet, no subdivision support yet
…ustom transform), means a lattice source projection can now be easily mapped to any bounds
…ice are also deformed (doesn't seem quite right so needs more work) * When changing the subdivision now resamples using the last lattice * Cap max subdivisions at 32 per axis * Unrelated to lattice - cleaning all deformables also cleans GroupDeformers
@sabresaurus sabresaurus changed the title Basic Lattice/FFD Implementation (#2) Basic Lattice/FFD Implementation Feb 1, 2021
@keenanwoodall
Copy link
Owner

This is really wonderful, excellent job!

Vertices are reprojected using the lattice by finding the neighbouring cell in the source lattice (when its a source grid), finding its local position within the cell then interpolating it against the modified cell corner positions.

Great! I didn't even consider how important this is, thank you for getting the resolution control to work so well.

or bring it in line with any particular code style.

Looks like how I'd write it myself (editor boilerplate and all! 😀) Beautiful!

I think this is basically ready to be merged, but here's some additional notes/ideas that are mostly nitpicky and definitely don't need to be done now, but I wanted to get them written down so I don't forget.

Ideas - The control points array length can be changed in the inspector which could break stuff

- The control points array can be accessed in code and have its length changed. Perhaps it would be safer to provide a `GetControlPoint` function/indexer and `GetControlPointCount` function and make the array private.

- Now that I think about it, do we even need to draw all the control points in the inspector? Perhaps just showing the selected points positions would be best.

- Once control points are selected it's hard to get back to the GameObject's regular transform handle without deselecting and reselecting the deformer. Not sure how to do it yet, but ideally clicking off of any points should deselect all points (without deselecting the GameObject). Would also be nice if selecting a tool (Grab/Move/Rotate/Scale) deselected all control points instead of constantly setting the tool to none. Perhaps editing control points could be ported to an EditorTool?

- This might be tricksy since a deformer can be applied to multiple Deformables, but a button to fit the lattice to the bounds of a mesh would be nice. Perhaps this could be done with a scriptable wizard in which you assign the GameObjects whose bounds the lattice should fit? Or a Fit Parent button might be easier.

- When you select multiple control points and the transform gizmo mode is Pivot, it should be located on the last selected point rather than the first (when selecting GameObjects in pivot mode, the last selected GameObject has the handle - which I'd like to mimic)

- How can we handle selecting multiple lattices/multiple points across multiple lattices? Right now if you select a point, a transform gizmo will be drawn at the same index on other selected lattices. (can throw an error if you select a point that doesn't exist on another selected lattice) Ideally all points would be editable together intuitively regardless of the lattice they belong to.

- In addition to moving pivot points, it'd be great to rotate/scale them

- Random idea, but it might be cool to draw lighter lines where solid lines fail depth test so you can always see the grid lines, but can see where they penetrate the mesh.

- Undo/redo control point selection changes

- Ctrl-A to select all control points

- Right now ctrl+click always deselects a control point and shift+select always selects a control point, my intuition is that they should toggle the selection.

- This is unbelievably nitpicky and implementation might be unrealistic, but would be cool if scrubbing the lattice resolution updated the grid in realtime. Perhaps a more reasonable change would be to add an increment/decrement buttons for the resolution. I'd have to actually implement it to see how it feels, maybe how it is is fine!

Absolutely no pressure to implement any of this stuff - you've really overshot my expectations quality-wise - but I'd love to hear your thoughts :)

…rolPoint methods * Removed the ControlPoints field on the inspector as it's not that useful with large counts and error prone * Changed behaviour of modifier keys when selecting control points so they toggle * Tool is now only reset when you click the first control point, changing tool with a control point selected will reset the selection so you can easily change mode back
@sabresaurus
Copy link
Contributor Author

Hey, thanks for all the feedback. I've implemented some of it:

  • Control Points and Resolution made readonly
  • Added Get and SetControlPoint methods
  • Removed the ControlPoints field on the inspector as it's not that useful with large counts and error prone
  • Changed behaviour of modifier keys when selecting control points so they toggle
  • Tool is now only reset when you click the first control point, changing tool with a control point selected will reset the selection so you can easily change mode back

Beyond that I think they're all good ideas but as you say they can probably come later in future PRs. A couple of thoughts on two of them:

When you select multiple control points and the transform gizmo mode is Pivot, it should be located on the last selected point rather than the first (when selecting GameObjects in pivot mode, the last selected GameObject has the handle - which I'd like to mimic)

This is not the behaviour I see when selecting game objects in Unity in pivot mode, for me the first object is the one that remains the pivot. I believe the existing implementation is consistent with this. Can you confirm if you get a different behaviour in Unity?

This is unbelievably nitpicky and implementation might be unrealistic, but would be cool if scrubbing the lattice resolution updated the grid in realtime. Perhaps a more reasonable change would be to add an increment/decrement buttons for the resolution. I'd have to actually implement it to see how it feels, maybe how it is is fine!

I agree on this, I did consider it but I was worried about compound floating point issues when users scroll the resolution. I decided to leave it as a button press for the initial implementation.

Thanks again for your detailed review and Deform in general, it's great to contribute a part to it!

@keenanwoodall keenanwoodall merged commit c1591d8 into keenanwoodall:develop Feb 3, 2021
@keenanwoodall
Copy link
Owner

Awesome! thanks a bunch - merge complete :)

@keenanwoodall
Copy link
Owner

I believe the existing implementation is consistent with this. Can you confirm if you get a different behaviour in Unity?

Haha odd, when I ctrl+click in pivot mode the last selected game object has the move gizmo. Perhaps there's an editor preference buried somewhere I'm unaware of that would cause this 🤔

QVglJUMV8g

@sabresaurus
Copy link
Contributor Author

Ah I see, looks like the behaviour in Unity is different if you select the objects in the scene view or the hierarchy. Selecting in the scene view has the pivot as the last object, selecting in the hierarchy has the pivot as the first. It probably makes more sense to use the scene view behaviour as you say

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