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

Introduce an extensible widget system #229

Merged
merged 39 commits into from
Jun 24, 2024
Merged

Introduce an extensible widget system #229

merged 39 commits into from
Jun 24, 2024

Conversation

mxgrey
Copy link
Collaborator

@mxgrey mxgrey commented Jun 18, 2024

This PR migrates the site editor's implementation of egui UI widgets over to an extensible widget system approach. Each widget can define its own parameters in isolation from the rest, and widgets can be defined as trees of other widgets. The implementation takes heavy inspiration from this discussion with the main difference being that we store system states as components associated with entities instead of storing them in a hashmap contained in a resource.

Widget trees are determined by parent/child relationships of entities that contain a Widget component. The parent entity must contain a widget system that traverses its children somehow, otherwise there will be no effect from setting a widget's entity to be the child of another widget.

Root-level widgets should implement PanelWidget component. They will be expected to access the EguiContexts parameter directly. In most cases, users who are creating new widgets will want to add their widget to the PropertiesPanel or the Inspector.

Add a widget to the Inspector if you want it to be grouped with other inspector widgets and have it be told which entity is selected. Implement the WidgetSystem<Inspect> trait for your widget struct and then add it as a plugin using InspectionPlugin::<W>::new() where W is the widget struct.

Add a widget to the PropertiesPanel if you want it to always be rendered on the panel that displays relevant properties of the site. Implement the WidgetSystem<Tile> trait for your widget struct and then add it as a plugin using PropertiesTilePlugin::<W>::new() where W is the widget struct.

A "widget struct" is a struct that derives SystemParam and contains the system parameters that you want your widget to have access to while it renders. Implementing the WidgetSystem trait allows the struct to be used as a widget by other systems in the application.

mxgrey and others added 26 commits June 12, 2024 16:45
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
…s widgets

Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Copy link
Member

@luca-della-vedova luca-della-vedova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not much to say, I tested it on a few different maps, trying again inspector / spawning / menu bar actions and I can't spot any regressions. I can see how the UX of extending the UI improves through this approach.

Apart from the minor inline comments my main feedback would be to add docstrings to the public types. We can probably get away with not adding one for each inspector plugin but since the purpose of this PR is to make the UI extensible by third parties imho it would be good to document the structs and traits that users might have to work with the most, which seem to be mostly defined in src/widgets/mod.rs, such as Widget, WidgetSystem, Panel, Tile, and anything else I might be missing (it's a fair bit of code to digest!)

rmf_site_editor/src/site/fuel_cache.rs Outdated Show resolved Hide resolved
rmf_site_editor/src/widgets/diagnostics.rs Outdated Show resolved Hide resolved
rmf_site_editor/src/widgets/creation.rs Show resolved Hide resolved
rmf_site_editor/src/widgets/inspector/inspect_drawing.rs Outdated Show resolved Hide resolved
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
mxgrey and others added 5 commits June 20, 2024 15:07
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
@mxgrey
Copy link
Collaborator Author

mxgrey commented Jun 20, 2024

@luca-della-vedova Thanks for the feedback; I think I've addressed each item.

@mxgrey
Copy link
Collaborator Author

mxgrey commented Jun 20, 2024

By the way, I recommend running cargo doc and taking a look at the widgets module page to see the new documentation.

Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Copy link
Member

@luca-della-vedova luca-della-vedova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just found a (I believe) broken link but rather than doing another round of review I took the liberty to fix it directly 5fcac9e.

Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
@mxgrey
Copy link
Collaborator Author

mxgrey commented Jun 22, 2024

@luca-della-vedova I fixed some merge conflicts that came up as a result of the headless export PR.

While fixing the merge conflicts I identified some questionable resource hygiene and reorganized where a few resources are located so that we no longer need to add any egui-related plugins at all when running in headless mode.

mxgrey and others added 3 commits June 22, 2024 07:06
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Copy link
Member

@luca-della-vedova luca-della-vedova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Yea I saw the merge conflict and it was a bit nasty

@mxgrey mxgrey merged commit 32b8109 into main Jun 24, 2024
5 checks passed
@mxgrey mxgrey deleted the ui_refactor branch June 24, 2024 06:34
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