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

Added winID to fix second windows in OGRE2.2 #292

Merged
merged 3 commits into from
Sep 30, 2021

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Sep 27, 2021

Signed-off-by: ahcorde ahcorde@gmail.com

🦟 Bug fix

Summary

Fix #291

Requires a PR in ign-rendering6

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

Signed-off-by: ahcorde <ahcorde@gmail.com>
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Sep 27, 2021
@chapulina chapulina added the needs upstream release Blocked by a release of an upstream library label Sep 27, 2021
@chapulina chapulina added beta Targeting beta release of upcoming collection bug Something isn't working labels Sep 27, 2021
@codecov
Copy link

codecov bot commented Sep 27, 2021

Codecov Report

Merging #292 (3582f33) into main (653b0fb) will increase coverage by 0.04%.
The diff coverage is 100.00%.

❗ Current head 3582f33 differs from pull request most recent head 10b5e3b. Consider uploading reports for the commit 10b5e3b to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #292      +/-   ##
==========================================
+ Coverage   65.31%   65.35%   +0.04%     
==========================================
  Files          32       32              
  Lines        4621     4627       +6     
==========================================
+ Hits         3018     3024       +6     
  Misses       1603     1603              
Impacted Files Coverage Δ
src/plugins/minimal_scene/MinimalScene.cc 59.71% <100.00%> (+0.25%) ⬆️
src/plugins/scene3d/Scene3D.cc 49.09% <100.00%> (+0.19%) ⬆️

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 653b0fb...10b5e3b. Read the comment docs.

@iche033
Copy link
Contributor

iche033 commented Sep 27, 2021

hmm with this change and gazebosim/gz-rendering#436, I get an empty render window and then a crash on render.

I'm on Ubuntu bionic with Qt 5.9.5

@ahcorde
Copy link
Contributor Author

ahcorde commented Sep 28, 2021

hmm with this change and ignitionrobotics/ign-rendering#436, I get an empty render window and then a crash on render.

I'm on Ubuntu bionic with Qt 5.9.5

you probably used a world with the Gazebo Scene3D here it's the fix gazebosim/gz-sim#1063

Signed-off-by: ahcorde <ahcorde@gmail.com>
@matosinho
Copy link

Issue fixed here for me with this branch.

Signed-off-by: ahcorde <ahcorde@gmail.com>
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Works for me 👍 We just need to wait for the ign-rendering release before merging.

@chapulina
Copy link
Contributor

Actually there's no harm in merging this before an ign-rendering release, so I'll just merge it to check it off

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta Targeting beta release of upcoming collection bug Something isn't working 🏯 fortress Ignition Fortress needs upstream release Blocked by a release of an upstream library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants