Allow rendering to be forced externally #1475
Conversation
|
Early push, to get feedback. I'll go implement a custom rendering sensor on top of this now. |
Codecov Report
@@ Coverage Diff @@
## main #1475 +/- ##
=======================================
Coverage 35.01% 35.01%
=======================================
Files 44 44
Lines 2356 2356
=======================================
Hits 825 825
Misses 1531 1531 Continue to review full report at Codecov.
|
chapulina
added
rendering
| @@ -65,6 +65,16 @@ namespace ignition | |||
| /// \endcode | |||
| using PostRender = ignition::common::EventT<void(void), | |||
| struct PostRenderTag>; | |||
|
|
|||
| /// \brief The force render event may be emitted outside the | |||
| /// rendering thread to force rendering calls. | |||
There was a problem hiding this comment.
The approach looks reasonable to me.
I'd just improve the documentation a bit to explain that "forcing" rendering means that when the time comes, a plugin that performs rendering will do so even if it wasn't planning to.
There was a problem hiding this comment.
Done in 010b191, and rebased to deal with the ignition -> gz migration.
hidmic
force-pushed
the
hidmic/force-render-event
branch
from
9113a0d
to
010b191
Compare
Codecov Report
@@ Coverage Diff @@
## main #1475 +/- ##
=======================================
Coverage 63.62% 63.62%
=======================================
Files 330 330
Lines 25837 25846 +9
=======================================
+ Hits 16438 16444 +6
- Misses 9399 9402 +3
Continue to review full report at Codecov.
|
Includes an event during rendering for downstream use. Signed-off-by: Michel Hidalgo <[email protected]>
hidmic
force-pushed
the
hidmic/force-render-event
branch
from
010b191
to
69294dd
Compare
Signed-off-by: Michel Hidalgo <[email protected]>
|
@chapulina @iche033 A couple notes about tests and examples for this patch (which we're lacking). You can see in osrf/lrauv#213 that putting together a plugin to make meaningful use of these new events is very tricky. So I think we have two choices: (a) we defer examples here and ticket a migration of osrf/lrauv#213 into |
|
The changes look fine. One note about going with this approach is that there is no guarantee that the custom rendering sensor's updates are lockstepped with physics. Something to keep in mind in case you are seeing that your sensor is not hitting the target update rate. |
|
@caguero we have 3 checks not passing that either fail to build some sources or fail some tests. In both cases, issues are in other source files. How do we go about it? |
|
Alright, according to @caguero, failing checks are already happening on Moving forward! |
🎉 New feature
Summary
ignition::gazebo::systems::Sensorswill only update its rendering sensors when there's a need for it. While perfectly reasonable, this also means that any custom rendering sensor relying on pre/post render hooks to do its work must wait for a builtin sensor that is known to this system to require an update.This patch works around this by introducing a
ForceRenderevent that other systems can emit to force a rendering pass.Test it
...
Checklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-bymessages.