open-rmf / rmf_site

Experimental visualizer for dense buildings in RMF
32 stars 13 forks source link

Use workflows for mouse interactions #236

Closed mxgrey closed 2 weeks ago

mxgrey commented 3 weeks ago

This PR removes the InteractionMode state machine and replaces it with a high-level workflow for switching between mouse interaction services. This high-level workflow uses service injection to allow custom mouse interaction services to be plugged in by downstream users of the site editor.

We introduce several different mouse interaction services that can activated by the high-level workflow. Each of these mouse interaction services is implemented as its own workflow:

Some other mouse interaction workflows that we should consider adding in the future (left out of this PR since they were not already available or not well supported):

This PR introduces some major changes to 3D object placement for the workcell editor. Placing a 3D object now involves an optional step of selecting a parent reference frame for the object. The workflow goes like this:

  1. If the user had something selected when they clicked on the spawn model button, that something will automatically be used as the parent.
  2. If nothing was selected when the spawn model button was clicked, we will not start out with any parent.
  3. Pressing Esc while a parent is selected will clear out the parent, leaving the workflow state similar to (2).
  4. Clicking on an object in the scene while no parent is selected will select that object as the parent.
  5. Clicking on a spot on the selected parent will place the object at the intersecting location on the parent, with the z-axis aligned with the normal vector of the chosen spot.
  6. Clicking somewhere that's not on the parent object will place the object in the x-y plane of the parent's frame.
  7. Clicking somewhere that's not on any object will place the object there, whether or not a parent is selected.
  8. Holding shift and clicking will ignore any hovering that is happening, placing the object in the x-y plane of the parent frame, or in the x-y plane of the world if no parent is selected.

All of this is communicated to the user through a combination of cursor-attached previewing and a new cursor-attached tooltip feature.

This PR also introduces some crucial fixes to model loading which had some reliability issues around potentially random system ordering and incomplete property propagation.

luca-della-vedova commented 3 weeks ago

Some other mouse interaction workflows that we should consider adding in the future (left out of this PR since they were not already available or not well supported):

  • Re-parent a 3D object to another reference frame

This feature is necessary for workcell calibration, the sequence is usually the following:

If we cannot reparent a mesh to a different anchor we can't do this calibration workflow, so the functionality is needed.

The most important point however, is in the redesign of the parenting UX.

It is a deliberate design choice in the current workcell editor that the only entity that can be a parent of another entity is a Frame. This is necessary for a few reasons:

I'm attaching a videos of a few of the rough edges that came up while allowing models to be parents of other entities: Screencast from 2024-08-22 12-06-54.webm

For example when spawning a model as a child of a model the model disappears, frames that are children of models seem to be embedded in the model when saving and reloading. TBH I would just only allow frames as parents to avoid all of these and make the URDF interoperability easier.

Additionally, there was a check in place to make sure we only do the "mesh snapping" when hovering over models. This is necessary to avoid snapping to the anchor meshes as the video below (which is probably not something that a normal user would want):

Screencast from 2024-08-22 12-19-04.webm

I need to setup the rest of the infrastructure to test the rest of the workcell editor pipeline, but these are the points I could find immediately

Edit: Ah I just noticed I didn't capture the mouse pointer :| Apologies for that! I hope the videos still make sense

mxgrey commented 3 weeks ago

If we cannot reparent a mesh to a different anchor we can't do this calibration workflow, so the functionality is needed.

I see, the only reason I didn't include it in this PR is because I wasn't sure how to preview it to the user. I thought we'd want to snap the model back to the cursor while selecting the new parent, and that would've been very difficult to propagate properties correctly.

What you've described, where the object remains at the same global transform while we reparent, will be very easy to implement, so I'll go ahead and add that in now.

It is a deliberate design choice in the current workcell editor that the only entity that can be a parent of another entity is a Frame.

This should be very easy to fix. I just need to drill up to the first ancestor frame of the selected parent before inserting the object into the world. I think down the road we should consider whether it's possible to automatically consider every tangible object in the scene to be a frame, but that's obviously outside the scope of this PR.

mxgrey commented 3 weeks ago

The vanishing model issue was fixed with e9a36cf. I also tested a round-trip of saving and reloading, and everything appeared to be working: everything loaded back up, and all the parent-child relationships appeared to be in tact.

I believe this addresses all the current feedback, but please let me know if I missed anything.

luca-della-vedova commented 2 weeks ago

Last nit is that this PR introduces a bunch of extra warnings, like unused variables and imports that increase noise and make it more likely that we will miss important ones in the future. Should make sure to clean them up before merging

mxgrey commented 2 weeks ago

Ah yeah actually quite a few warnings that have been lingering were kept on purpose as reminders for issues that need to be addressed in the old mouse selection state machine, so this is a good opportunity to clean up any remaining warnings.

But I don't look forward to the day we try to apply clippy to this project...

mxgrey commented 2 weeks ago

I think all the latest feedback is addressed.

I also realized that I accidentally got rid of the ability to deselect the current inspected item by pressing the Esc key so I added that back in with ca82bee.