Helper function to get a scene

[chapulina]

🎉 New feature

Summary

Most applications will only have one rendering engine loaded at a time, and only one scene within that. This helper function gets the first scene that can be found in any rendering engine.

This pattern is being repeated on several ign-gui and ign-gazebo plugins, so it would be nice to reduce duplication. For example:

• Screenshot
• Grid3D
• VisualizeLidar
• etc

This PR could target ign-rendering3, I'm targeting main because I have other PRs that will depend on it. Once this has been reviewed I can rebase and target 3.

Test it

Take a look at the added tests.

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

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

[ahcorde]

LGTM, just one minor comment, what do you think about use another name for the method? getFirstInitializedScene ?

I can see an error in windows

47: �[31m[Err] [C:\Jenkins\workspace\ign_rendering-pr-win\ws\ign-rendering\src\RenderEngineManager.cc:336] �[m�[31mInvalid render-engine index: �[m�[31m1000000�[m�[31m
47: �[m[       OK ] RenderingIfaceTest.RegisterEngine (67 ms)

[chapulina]

what do you think about use another name for the method? getFirstInitializedScene ?

I don't know if that name would be necessarily true for the current implementation. For example:

1. Register engine EA
2. Register engine EB
3. Create scene on SB on EB
4. Create scene on SA on EA

SB was initialized before SA, but we'll return SA. Moreover, if we don't do step 4, we will return null.

[ahcorde]

@osrf-jenkins retest this please

[iche033]

I don't know if that name would be necessarily true for the current implementation.

How about getFirstRenderEngineScene to be more specific?

[chapulina]

How about getFirstRenderEngineScene to be more specific

The trick is whether the "first" applies to the render engine, to the scene, or to both...

I don't have a better suggestion though. I can go with that if you think there isn't a lot of ambiguity. I'd just drop the get.

[iche033]

sceneFromFirstRenderEngine?

I think either one works

[chapulina]

sceneFromFirstRenderEngine?

Ok, I went with that: e19bd73

[chapulina]

Once this has been reviewed I can rebase and target 3.

Done, this is now targeted at v3

[codecov]

Codecov Report

Merging #320 (e19bd73) into ign-rendering3 (e3d56ad) will increase coverage by 6.85%.
The diff coverage is 58.36%.

❗ Current head e19bd73 differs from pull request most recent head 9ed634a. Consider uploading reports for the commit 9ed634a to get more accurate results

@@                Coverage Diff                 @@
##           ign-rendering3     #320      +/-   ##
==================================================
+ Coverage           50.59%   57.44%   +6.85%     
==================================================
  Files                 129      159      +30     
  Lines               11887    15718    +3831     
==================================================
+ Hits                 6014     9029    +3015     
- Misses               5873     6689     +816     

Impacted Files	Coverage Δ
include/ignition/rendering/ArrowVisual.hh	100.00% <ø> (+100.00%)	⬆️
include/ignition/rendering/AxisVisual.hh	100.00% <ø> (+100.00%)	⬆️
include/ignition/rendering/Camera.hh	100.00% <ø> (ø)
include/ignition/rendering/CompositeVisual.hh	100.00% <ø> (+100.00%)	⬆️
include/ignition/rendering/Geometry.hh	100.00% <ø> (ø)
include/ignition/rendering/GpuRays.hh	0.00% <ø> (ø)
include/ignition/rendering/Image.hh	100.00% <ø> (ø)
include/ignition/rendering/Light.hh	100.00% <ø> (ø)
include/ignition/rendering/Material.hh	100.00% <ø> (ø)
include/ignition/rendering/Mesh.hh	100.00% <ø> (ø)
... and 144 more

---

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 e3d56ad...9ed634a. Read the comment docs.

[iche033]

looks good to me.