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

Fix RenderTarget::destroy not getting called in both ogre1 & ogre2 #776

Merged

Conversation

darksylinc
Copy link
Contributor

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

🦟 Bug fix

No ticket was filed for this bug.

Summary

During PR #775 I discovered this bug.

That PR will fix that bug in main, this is a backport for Fortress.

A tiny difference is that in main changes were done so that RenderPasses are destroyed cleanly, while in Fortress' backport I can't do that without breaking the ABI.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • 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 December 4, 2022 15:32
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Dec 4, 2022
@codecov
Copy link

codecov bot commented Dec 4, 2022

Codecov Report

Merging #776 (67012b5) into ign-rendering6 (eddc119) will increase coverage by 0.03%.
The diff coverage is 100.00%.

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

@@                Coverage Diff                 @@
##           ign-rendering6     #776      +/-   ##
==================================================
+ Coverage           77.82%   77.85%   +0.03%     
==================================================
  Files                 146      146              
  Lines               13399    13404       +5     
==================================================
+ Hits                10428    10436       +8     
+ Misses               2971     2968       -3     
Impacted Files Coverage Δ
ogre2/src/Ogre2Camera.cc 89.34% <100.00%> (+0.46%) ⬆️
ogre2/src/Ogre2RenderTarget.cc 86.33% <100.00%> (+0.65%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@darksylinc darksylinc marked this pull request as draft December 4, 2022 18:51
@darksylinc darksylinc force-pushed the matias-renderTargetDestroy branch from a8d2097 to 67012b5 Compare December 4, 2022 18:59
@darksylinc darksylinc marked this pull request as ready for review December 4, 2022 18:59
Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

looks good to me

@iche033 iche033 merged commit e6c3b72 into gazebosim:ign-rendering6 Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants