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

Examples and tutorial on using rendering API from plugins #596

Merged
merged 8 commits into from
Feb 12, 2021

Conversation

chapulina
Copy link
Contributor

Builds on top of #595


Add 2 example plugins and a tutorial explaining how they work. See tutorial for more context.

I had to link to some Dome APIs because we don't have Edifice APIs deployed yet.

Here are the example plugins at work, the server one sets a random color every 2 simulation seconds, and the GUI one sets a new random color when the button is clicked.

rendering_plugins

@chapulina chapulina added documentation Improvements or additions to documentation GUI Gazebo's graphical interface (not pure Ignition GUI) rendering Involves Ignition Rendering 🏢 edifice Ignition Edifice labels Jan 29, 2021
@adlarkin adlarkin self-requested a review January 29, 2021 17:57
@chapulina
Copy link
Contributor Author

@osrf-jenkins run tests please

Copy link
Contributor

@adlarkin adlarkin left a comment

Choose a reason for hiding this comment

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

The tutorial itself is great, thanks for doing this! One main question/concern I have is that the tutorial doesn't seem to discuss when or why one should use server/GUI rendering plugins instead of adding to SceneManagerRenderUtils, or Scene3D. Perhaps we can add comments about this to the tutorial? I am assuming that the reasons for using server/GUI rendering plugins are along the lines of the following:

  • Performance (users only use what they need)
  • Maintenance/code cleanliness/testability

examples/plugin/rendering_plugins/README.md Outdated Show resolved Hide resolved
examples/plugin/rendering_plugins/README.md Outdated Show resolved Hide resolved
examples/plugin/rendering_plugins/README.md Outdated Show resolved Hide resolved
tutorials/rendering_plugins.md Show resolved Hide resolved
tutorials/rendering_plugins.md Show resolved Hide resolved
tutorials/rendering_plugins.md Show resolved Hide resolved
tutorials/rendering_plugins.md Outdated Show resolved Hide resolved
tutorials/rendering_plugins.md Show resolved Hide resolved
tutorials/rendering_plugins.md Outdated Show resolved Hide resolved
examples/plugin/rendering_plugins/RenderingGuiPlugin.hh Outdated Show resolved Hide resolved
Base automatically changed from chapulina/5/guievents to main February 9, 2021 17:55
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
@chapulina
Copy link
Contributor Author

the tutorial doesn't seem to discuss when or why one should use server/GUI rendering plugins instead of adding to SceneManager, RenderUtils, or Scene3D

I'm giving this some thought.

The short answer is that we don't need to mention this to external users, because they can't modify those classes from their code.

We've been doing this as maintainers, but that doesn't scale, because we can't support every use case in the same plugin at the same time. That's why we have a plugin architecture, so users can implement functionality that's specific to their needs without modifying the core code. This to me is the biggest reason to keep everything in separate plugins, a lot of the functionality we ship should be as optional as user's plugins.

@adlarkin
Copy link
Contributor

This to me is the biggest reason to keep everything in separate plugins, a lot of the functionality we ship should be as optional as user's plugins.

Agreed; perhaps we should consider refactoring SceneManager, RenderUtils, and Scene3D in the future to make better use of plugins. I am not sure how much work this would require, though (I'm guessing it would be a large task).

Signed-off-by: Louise Poubel <[email protected]>
@codecov
Copy link

codecov bot commented Feb 11, 2021

Codecov Report

Merging #596 (2856793) into main (8f1f44f) will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #596      +/-   ##
==========================================
- Coverage   77.43%   77.38%   -0.05%     
==========================================
  Files         212      212              
  Lines       12016    12016              
==========================================
- Hits         9304     9299       -5     
- Misses       2712     2717       +5     
Impacted Files Coverage Δ
src/SimulationRunner.cc 92.76% <0.00%> (-1.09%) ⬇️
src/Server.cc 83.43% <0.00%> (+0.63%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f1f44f...2856793. Read the comment docs.

@chapulina chapulina merged commit c2fb4f2 into main Feb 12, 2021
@chapulina chapulina deleted the chapulina/5/rendering_plugins branch February 12, 2021 00:59
chapulina added a commit that referenced this pull request Jul 30, 2021
chapulina added a commit that referenced this pull request Jul 30, 2021
Signed-off-by: Louise Poubel <[email protected]>

Edifice to Citadel (#945)

Signed-off-by: Louise Poubel <[email protected]>
chapulina added a commit that referenced this pull request Jul 30, 2021
Signed-off-by: Louise Poubel <[email protected]>

Edifice to Citadel (#945)

Signed-off-by: Louise Poubel <[email protected]>
chapulina added a commit that referenced this pull request Aug 4, 2021
Signed-off-by: Louise Poubel <[email protected]>

Edifice to Citadel (#945)

Signed-off-by: Louise Poubel <[email protected]>
chapulina added a commit that referenced this pull request Aug 4, 2021
Signed-off-by: Louise Poubel <[email protected]>

Edifice to Citadel (#945)

Signed-off-by: Louise Poubel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation 🏢 edifice Ignition Edifice 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