gazebosim / gazebo-classic

Gazebo classic. For the latest version, see https://github.com/gazebosim/gz-sim
http://classic.gazebosim.org/
Other
1.2k stars 484 forks source link

Fix wide-angle lens flare occlusion lag #3325

Closed audrow closed 1 year ago

audrow commented 1 year ago

Fixes #3310

This forces an update of the wide angle dummy camera, which updates the Ogre camera's view matrix.


One way to check that this fixes the bug, is to use this world file. Open it in Gazebo, and then move the only camera (wide_angle_cameras_occluded_higher) from it's starting height (z=21) to z=11, where it will have a wall between it and the light source. You can then move it back to z=21.

You should see lens flare when the light source is in the image and now lens flare when the light source is occluded.

scpeters commented 1 year ago

this fixes it for me! I've been tinkering with how to add an integration test for it, but if I don't figure it out in my next time box, then I'll just skip it and approve this

scpeters commented 1 year ago

this fixes it for me! I've been tinkering with how to add an integration test for it, but if I don't figure it out in my next time box, then I'll just skip it and approve this

I've gotten a test working in branch scpeters/lens-flare-occlusion-lag. I opened a pull request targeting this PR in https://github.com/audrow/gazebo-classic/pull/3, but it didn't start Jenkins jobs because it's targeting your fork. If the test looks ok, then please merge it into this PR with a merge commit and we can confirm that the tests pass in CI

scpeters commented 1 year ago

this fixes it for me! I've been tinkering with how to add an integration test for it, but if I don't figure it out in my next time box, then I'll just skip it and approve this

I've gotten a test working in branch scpeters/lens-flare-occlusion-lag. I opened a pull request targeting this PR in audrow#3, but it didn't start Jenkins jobs because it's targeting your fork. If the test looks ok, then please merge it into this PR with a merge commit and we can confirm that the tests pass in CI

the new test case passed on Ubuntu but failed in the homebrew build on mac-four.bigsur. I noticed all the GUI tests failed on that run as well, so there may be an issue with the CI machine. I just pushed an improvement to the image waiting logic in https://github.com/gazebosim/gazebo-classic/pull/3325/commits/29dbe5a03994dd7fb17b541ba98a4caf9494c173 that hopefully is more robust. The new CI build is also running on mac-four.bigsur, so we'll see if it helps

scpeters commented 1 year ago

now with https://github.com/gazebosim/gazebo-classic/commit/29dbe5a03994dd7fb17b541ba98a4caf9494c173 it passed on homebrew but failed repeatedly on Ubuntu. I'll keep testing locally to see if I can reproduce the flakiness

scpeters commented 1 year ago

now with 29dbe5a it passed on homebrew but failed repeatedly on Ubuntu. I'll keep testing locally to see if I can reproduce the flakiness

I was able to reproduce the issue locally, and it appears in my local testing that --lockstep ( https://github.com/gazebosim/gazebo-classic/pull/3325/commits/032247ef245b984d4bc4b9230b2382edc0028b4c ) fixes the sensor timing and test flakiness

scpeters commented 1 year ago

test passes in both CI runs; merging