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

Wideanglecamera equidistant mapping bounding fix #3135

Merged

Conversation

kjeppesen1
Copy link
Contributor

Currently when using a wideanglecamera sensor with a lens type of equidistant, the actual image data gets restricted to a small circular region with the rest of the image having a gray ring around it. See the images below for the image that is produced for an equidistant lens with an HFOV of 1.5708, 2.0, and 3.14159.
equidistant_1p5708
equidistant_2p0
equidistant_3p14159

The issue appears to be in wide_lens_map_fp.glsl on Line 38. The value theta gets calculated with asin(param) driving the equation if fun.x is 1 (which is true for equisolid_angle and orthographic lens types). The way this equation is written becomes problematic for other lens types, though, when the param > 1 since the resulting value of asin(param) becomes undefined.

This PR is to add a check for function type to avoid the undefined asin computation. See the images below for an equidistant lens with an HFOV of 1.5708, 2.0, and 3.14159 after this fix.
equidistant_1p5708_fixed
equidistant_2p0_fixed
equidistant_3p14159_fixed

@kjeppesen1
Copy link
Contributor Author

@scpeters @iche033

@@ -1515,7 +1515,7 @@ fragment_program Gazebo/WideLensMapFS glsl
param_named c2 float 1
param_named c3 float 0
param_named f float 1
param_named fun float3 0 1
param_named fun float3 0 0 1
Copy link
Contributor Author

@kjeppesen1 kjeppesen1 Nov 23, 2021

Choose a reason for hiding this comment

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

This line might not change anything, but it seemed odd to not complete the default float3 values

Copy link
Contributor

Choose a reason for hiding this comment

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

If float3 0 1 was working before, wouldn't Ogre have been interpreting it as float3 0 1 0? This looks like it would change behavior from using atan(param) to using param.

Copy link
Member

Choose a reason for hiding this comment

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

I agree, this is suspicious

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it could be 0 0 1 to match the default fun value of id specified here (which is defined to be UnitZ here.)

It may not matter that much though as the values specified here in the material script are the initial values but should be overriden every frame by the SetUniformVariables call

Copy link
Member

Choose a reason for hiding this comment

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

👍

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.

good catch! looks good to me

@@ -1515,7 +1515,7 @@ fragment_program Gazebo/WideLensMapFS glsl
param_named c2 float 1
param_named c3 float 0
param_named f float 1
param_named fun float3 0 1
param_named fun float3 0 0 1
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it could be 0 0 1 to match the default fun value of id specified here (which is defined to be UnitZ here.)

It may not matter that much though as the values specified here in the material script are the initial values but should be overriden every frame by the SetUniformVariables call

@scpeters scpeters merged commit 68c9d6f into gazebosim:gazebo11 Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants