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

Set simulation time to Rendering #1415

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 dark_sylinc@yahoo.com.ar

🦟 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.

@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Mar 27, 2022
@osrf-triage osrf-triage added this to Inbox in Core development 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
@chapulina chapulina moved this from Inbox to In review in Core development Mar 28, 2022
See gazebosim/gz-rendering#556
See gazebosim/gz-rendering#584

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
darksylinc and others added 2 commits April 3, 2022 21:01
Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
@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
Core development automation moved this from In review to Done Apr 20, 2022
@iche033 iche033 mentioned this pull request Apr 20, 2022
7 tasks
@j-rivero j-rivero removed this from Done in Core development May 6, 2022
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.

None yet

3 participants