Skip to content

Set simulation time to Rendering #1415

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

Merged
merged 6 commits into from
Apr 20, 2022

Conversation

darksylinc
Copy link
Contributor

See gazebosim/gz-rendering#556
See gazebosim/gz-rendering#584

Signed-off-by: Matias N. Goldberg [email protected]

🦟 Bug fix

There is no ticket filed for this issue, but there is one at ign-rendering#556

Summary

Previously rendering was not being told of the current simulation time.

This is needed for proper particle FXs simulation and some postprocessing effects that rely on time.

This PR also accounts for a hack required only by ign-rendering6 so that SetTime() is honoured (otherwise legacy behavior is used, which uses real time instead of simulation time). Details of this legacy issue can be found in my reply.

This PR should be merged AFTER ign-rendering6's ign-rendering#584

Checklist

  • Signed all commits for DCO
  • Added tests
  • 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 and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@darksylinc darksylinc requested a review from iche033 as a code owner March 27, 2022 00:09
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Mar 27, 2022
@codecov
Copy link

codecov bot commented Mar 27, 2022

Codecov Report

Merging #1415 (ae6211b) into ign-gazebo6 (2938ede) will increase coverage by 29.41%.
The diff coverage is 75.00%.

❗ Current head ae6211b differs from pull request most recent head 3b0b878. Consider uploading reports for the commit 3b0b878 to get more accurate results

@@               Coverage Diff                @@
##           ign-gazebo6    #1415       +/-   ##
================================================
+ Coverage        33.58%   62.99%   +29.41%     
================================================
  Files               44      301      +257     
  Lines             2260    24265    +22005     
================================================
+ Hits               759    15286    +14527     
- Misses            1501     8979     +7478     
Impacted Files Coverage Δ
src/rendering/RenderUtil.cc 36.69% <66.66%> (ø)
src/systems/sensors/Sensors.cc 69.00% <100.00%> (ø)
...ins/component_inspector/qrc_ComponentInspector.cpp
src/gui/plugins/copy_paste/moc_CopyPaste.cpp
...n-gazebo6-gui_autogen/EWIEGA46WW/moc_GuiRunner.cpp
src/gui/plugins/lights/moc_Lights.cpp
...-gui_autogen/EWIEGA46WW/moc_AboutDialogHandler.cpp
...osition_controller/qrc_JointPositionController.cpp
...ion_capabilities/moc_VisualizationCapabilities.cpp
src/msgs/performer_affinity.pb.cc
... and 337 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 2938ede...3b0b878. Read the comment docs.

@chapulina chapulina added the rendering Involves Ignition Rendering label Mar 28, 2022
@darksylinc darksylinc force-pushed the matias-PFX_simulTime branch from c2de743 to c433f92 Compare April 3, 2022 23:34
@iche033 iche033 added the needs upstream release Blocked by a release of an upstream library label Apr 5, 2022
@iche033 iche033 merged commit df62658 into gazebosim:ign-gazebo6 Apr 20, 2022
@iche033 iche033 mentioned this pull request Apr 20, 2022
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress needs upstream release Blocked by a release of an upstream library rendering Involves Ignition Rendering
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants