Lower severity level for ogre2 visibility mask msgs and unavailable render passes #830

iche033 · 2023-03-09T00:19:22Z

🦟 Bug fix

Summary

Currently when gazebo is launched with camera sensors in the scene, it gets scary red error msgs:

[Err] [Ogre2Camera.cc:390] Ogre2Camera::SetVisibilityMask: Mask bits c0000000 are set but will be ignored by ogre2 backend.

This should not be an error but a warning as gz-rendering will continue to work and just ignore the reserved mask bits.

To test

Test with gazebo and check the msgs printed in the console:

ign gazebo -v 4 sensors_demo.sdf

The msg should now be an warning istead of error.

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.

Signed-off-by: Ian Chen <[email protected]>

codecov · 2023-03-09T00:40:46Z

Codecov Report

Merging #830 (805c71d) into ign-rendering6 (420f8f1) will increase coverage by 0.00%.
The diff coverage is 33.33%.

❗ Current head 805c71d differs from pull request most recent head 2bc3753. Consider uploading reports for the commit 2bc3753 to get more accurate results

@@               Coverage Diff               @@
##           ign-rendering6     #830   +/-   ##
===============================================
  Coverage           79.61%   79.61%           
===============================================
  Files                 146      146           
  Lines               13444    13445    +1     
===============================================
+ Hits                10703    10704    +1     
  Misses               2741     2741

Impacted Files	Coverage Δ
ogre2/src/Ogre2Camera.cc	`83.51% <0.00%> (-0.47%)`	⬇️
src/RenderPassSystem.cc	`100.00% <100.00%> (ø)`

... and 1 file with indirect coverage changes

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

JaouadROS · 2023-03-09T12:47:28Z

I've got the same error but on gazebo garden and right after that error I have another one related to camera sensor too:

[ruby $(which gz) sim-1] [Err] [Ogre2Camera.cc:404] Ogre2Camera::SetVisibilityMask: Mask bits c0000000 are set but will be ignored by ogre2 backend.
[ruby $(which gz) sim-1] [Err] [RenderPassSystem.cc:54] RenderPass of typeid 'N2gz9rendering2v714DistortionPassE' is not registered

The simulation keeps working normally.

iche033 · 2023-03-09T17:49:20Z

The second error msg is correct and lets users know an actual error. Distortion hasn't been ported to ogre2 yet (default render engine used in gz sim) so any distortions that've been specified in SDF are currently not applied to cameras sensors.

azeey

I've also wondered about the RenderPass of typeid 'N2gz9rendering2v714DistortionPassE' is not registered error. It would be nice if it just said "Distortion is not yet supported in Ogre2" or something to that effect.

azeey · 2023-03-14T21:08:33Z

ogre2/src/Ogre2Camera.cc

-           << " are set but will be ignored by ogre2 backend." << std::endl;
+    ignwarn << "Ogre2Camera::SetVisibilityMask: Mask bits " << std::hex
+            << ~Ogre::VisibilityFlags::RESERVED_VISIBILITY_FLAGS << std::dec
+            << " are set but will be ignored by ogre2 backend." << std::endl;


Is it ignored because we don't support these flags yet or for some other reason? If it's the former, it would be nice to mention that.

updated comment to explain that they are ignored because of conflict with internal reserved bits used by ogre2. 5445026

Signed-off-by: Ian Chen <[email protected]>

iche033 · 2023-03-14T21:54:16Z

I've also wondered about the RenderPass of typeid 'N2gz9rendering2v714DistortionPassE' is not registered error. It would be nice if it just said "Distortion is not yet supported in Ogre2" or something to that effect.

There should be another msg printed immediately afterwards to say that distortion is not supported in ogre2 here:
https://github.com/gazebosim/gz-sensors/blob/gz-sensors7/src/ImageBrownDistortionModel.cc#L112-L113

The difference I see is that one is a warning and the other is an error so the user may not associate that these 2 msgs are related. We could either change this msg in gz-rendering to a warning or the one gz-sensors to an error?

azeey · 2023-03-23T16:14:25Z

I've also wondered about the RenderPass of typeid 'N2gz9rendering2v714DistortionPassE' is not registered error. It would be nice if it just said "Distortion is not yet supported in Ogre2" or something to that effect.

There should be another msg printed immediately afterwards to say that distortion is not supported in ogre2 here: https://github.com/gazebosim/gz-sensors/blob/gz-sensors7/src/ImageBrownDistortionModel.cc#L112-L113

The difference I see is that one is a warning and the other is an error so the user may not associate that these 2 msgs are related. We could either change this msg in gz-rendering to a warning or the one gz-sensors to an error?

Do we even need the message with N2gz9rendering2v714DistortionPassE then? For the uninitiated, it looks like some sort of linking issue and more alarming than necessary.

iche033 · 2023-03-23T17:59:13Z

Do we even need the message with N2gz9rendering2v714DistortionPassE then? For the uninitiated, it looks like some sort of linking issue and more alarming than necessary.

if we remove this msg, it's then a silent failure for users of the gz-rendering library (those who are using it without sensors/sim).

The render passes (distortion, gaussian noise, etc) register themselves using their typeid and that's what gets printed. Unfortunately we can't print a friendly name in the msg because the render pass system does not know about it. It's possible to change the macro to store a name other than typeid but that'll be a behavior change and we'll need to target main

azeey · 2023-03-23T18:20:14Z

if we remove this msg, it's then a silent failure for users of the gz-rendering library (those who are using it without sensors/sim).

I see. Ideally, users should be checking that the returned pointer is null and print their own warning/error, but if you think that would be a breaking change, let's not remove it. Perhaps we can just make it a warning then?

Signed-off-by: Ian Chen <[email protected]>

iche033 · 2023-03-23T19:04:58Z

Perhaps we can just make it a warning then?

sounds good. I've just made the change in this PR.

I'll ticket an issue about printing a more friendly msg.

lower severity level for visibility mask msg

3b0c97a

Signed-off-by: Ian Chen <[email protected]>

github-actions bot added the 🏯 fortress Ignition Fortress label Mar 9, 2023

azeey self-requested a review March 13, 2023 18:56

azeey reviewed Mar 14, 2023

View reviewed changes

update warning msg

5445026

Signed-off-by: Ian Chen <[email protected]>

iche033 added 2 commits March 23, 2023 19:03

change err to warn for missing render passes

44c4bf2

Signed-off-by: Ian Chen <[email protected]>

Merge branch 'ign-rendering6' into visibility_mask_warn

2bc3753

iche033 changed the title ~~Lower severity level for ogre2 visibility mask msgs~~ Lower severity level for ogre2 visibility mask msgs and unavailable render passes Mar 23, 2023

iche033 mentioned this pull request Mar 23, 2023

Print a more descriptive / friendly msg when a render pass is unavailable #837

Open

azeey self-requested a review April 10, 2023 18:36

azeey approved these changes Apr 12, 2023

View reviewed changes

iche033 merged commit 9421565 into ign-rendering6 Apr 12, 2023

iche033 deleted the visibility_mask_warn branch April 12, 2023 21:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lower severity level for ogre2 visibility mask msgs and unavailable render passes #830

Lower severity level for ogre2 visibility mask msgs and unavailable render passes #830

iche033 commented Mar 9, 2023

codecov bot commented Mar 9, 2023 •

edited

Loading

JaouadROS commented Mar 9, 2023

iche033 commented Mar 9, 2023

azeey left a comment

azeey Mar 14, 2023

iche033 Mar 14, 2023

iche033 commented Mar 14, 2023 •

edited

Loading

azeey commented Mar 23, 2023 •

edited

Loading

iche033 commented Mar 23, 2023

azeey commented Mar 23, 2023

iche033 commented Mar 23, 2023

Lower severity level for ogre2 visibility mask msgs and unavailable render passes #830

Lower severity level for ogre2 visibility mask msgs and unavailable render passes #830

Conversation

iche033 commented Mar 9, 2023

🦟 Bug fix

Summary

To test

Checklist

codecov bot commented Mar 9, 2023 • edited Loading

Codecov Report

JaouadROS commented Mar 9, 2023

iche033 commented Mar 9, 2023

azeey left a comment

Choose a reason for hiding this comment

azeey Mar 14, 2023

Choose a reason for hiding this comment

iche033 Mar 14, 2023

Choose a reason for hiding this comment

iche033 commented Mar 14, 2023 • edited Loading

azeey commented Mar 23, 2023 • edited Loading

iche033 commented Mar 23, 2023

azeey commented Mar 23, 2023

iche033 commented Mar 23, 2023

codecov bot commented Mar 9, 2023 •

edited

Loading

iche033 commented Mar 14, 2023 •

edited

Loading

azeey commented Mar 23, 2023 •

edited

Loading