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

New scene manager GUI plugin that works with ign-gui's MinimalScene #813

Merged
merged 11 commits into from
Aug 10, 2021

Conversation

chapulina
Copy link
Contributor

@chapulina chapulina commented May 12, 2021

🎉 New feature

Closes gazebosim/gz-gui#137

Requires gazebosim/gz-gui#221

Summary

This is a proof of concept for how an ign-gazebo GuiSystem can work with ign-gui's MinimalScene. The idea is to split the concerns:

  • ign-gui's MinimalScene instantiates a 3D scene and paints to the window
  • ign-gazebo's GzSceneManager adds, removes and updates visuals on that scene, based on Gazebo's entities and components.

The two plugins synchronize through the Render event, that's called by MinimalScene within the rendering thread.

This initial implementation supports adding entities to the scene and updating their poses. There's lots still to be figured out:

Missing features that could possibly be moved to independent ign-gui plugins:

Missing features that could possibly be moved to independent ign-gazebo plugins:

Test it

To try it out, run:

ign gazebo --gui-config src/ign-gazebo/src/gui/gui.config rolling_shapes.sdf -v 4

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

@chapulina chapulina added GUI Gazebo's graphical interface (not pure Ignition GUI) rendering Involves Ignition Rendering labels May 12, 2021
@chapulina chapulina requested a review from ahcorde May 12, 2021 21:28
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label May 12, 2021
Signed-off-by: Louise Poubel <[email protected]>
@ahcorde
Copy link
Contributor

ahcorde commented May 27, 2021

I was reviewing some of the plugin in ign-gazebo that you mentioned to move them to a independent plugin (inside ign-gazebo). In particular these 3:

  • Select
  • View collisions
  • Transform controls

All of them require the renderUtil class which means that we need to create a renderUtil object and in each one of them we should update it (I think these calls are very costly).

Anyhow I will prototype something

@chapulina chapulina changed the title New scene manager GUI plugin that works with ign-gui's Scene3D New scene manager GUI plugin that works with ign-gui's MinimalScene Jun 19, 2021
Signed-off-by: Louise Poubel <[email protected]>
@ahcorde ahcorde mentioned this pull request Jun 22, 2021
4 tasks
src/gui/plugins/scene_manager/GzSceneManager.cc Outdated Show resolved Hide resolved
src/gui/gui.config Outdated Show resolved Hide resolved
src/rendering/RenderUtil.cc Show resolved Hide resolved
@ahcorde
Copy link
Contributor

ahcorde commented Jul 1, 2021

we should add to the gui.config file when the PR is merged

<plugin filename="MarkerManager" name="Marker manager">
  <ignition-gui>
    <title>Marker Manager</title>
    <property type="bool" key="showTitleBar">false</property>
    <property type="string" key="state">docked</property>
    <property key="resizable" type="bool">false</property>
    <property key="width" type="double">5</property>
    <property key="height" type="double">5</property>
    <property key="state" type="string">floating</property>
    <property key="showTitleBar" type="bool">false</property>
  </ignition-gui>
</plugin>

@ahcorde
Copy link
Contributor

ahcorde commented Jul 1, 2021

I was having a look to the record video plugin but there is a feature to record the video in lockstep mode which means I can create the plugin but instead of including the plugin in ign-gui it should live in ign-gazebo.

@chapulina, is this assumption correct ? or am i missing something ?

@chapulina
Copy link
Contributor Author

@chapulina, is this assumption correct ?

Good point, Ignition GUI doesn't have the concept of time, so it would take some extra efforts if we wanted to move that plugin there. I'm fine keeping it in Ignition Gazebo 👍

@chapulina chapulina marked this pull request as ready for review August 9, 2021 22:22
@chapulina chapulina requested a review from iche033 as a code owner August 9, 2021 22:22
@chapulina chapulina removed the needs upstream release Blocked by a release of an upstream library label Aug 9, 2021
@chapulina
Copy link
Contributor Author

chapulina commented Aug 9, 2021

@ahcorde , I'm opening this PR for review.

I reverted the changes to gui.config, so the current PR doesn't affect any current users.

The new scene manager can be tested with the minimal_scene.sdf example world.

One suggestion for testing is to:

  • Start the example that uses this plugin: ign gazebo minimal_scene.sdf
  • While that's open, open just a client using the default config: ign gazebo -g --gui-config src/ign-gazebo/src/gui/gui.config

Then use the 2nd window to insert, delete and move shapes, and verify that they move on the new plugin too.

Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

Two minor style issues. Otherwise LGTM

examples/worlds/minimal_scene.sdf Outdated Show resolved Hide resolved
src/gui/plugins/scene_manager/GzSceneManager.cc Outdated Show resolved Hide resolved
Signed-off-by: Louise Poubel <[email protected]>
@chapulina chapulina merged commit efa7d44 into main Aug 10, 2021
@chapulina chapulina deleted the chapulina/6/scene_manager branch August 10, 2021 20:51
WilliamLewww pushed a commit to WilliamLewww/ign-gazebo that referenced this pull request Dec 7, 2021
…azebosim#813)

Signed-off-by: Louise Poubel <[email protected]>

Co-authored-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: William Lew <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress GUI Gazebo's graphical interface (not pure Ignition GUI) rendering Involves Ignition Rendering
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants