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

Lens distortion shader #2145

Merged
merged 16 commits into from
Oct 3, 2019
Merged

Lens distortion shader #2145

merged 16 commits into from
Oct 3, 2019

Conversation

marcgpuig
Copy link
Contributor

@marcgpuig marcgpuig commented Sep 30, 2019

Description

Now all the camera-based sensors are provided with and additional lens distortion shader.
You can see the effect of just increasing the lens_circle_multiplier value to 1.0 - 2.0 or so.

camera_bp.set_attribute('lens_circle_falloff', str(5.0))
camera_bp.set_attribute('lens_circle_multiplier', str(0.0))
camera_bp.set_attribute('lens_k', str(-1.0))
camera_bp.set_attribute('lens_kcube', str(0.0))
camera_bp.set_attribute('lens_x_size', str(0.08))
camera_bp.set_attribute('lens_y_size', str(0.08))

Also, exposed chromatic aberration for sensor.camera.rgb:

rgb_camera_bp.set_attribute('chromatic_aberration_intensity', str(0.5)) # 0.0 by default
rgb_camera_bp.set_attribute('chromatic_aberration_offset', str(0.0))

Changed manual_control.py so you can visualize a lens distortion example pressing 8.

Where has this been tested?

  • Platform(s): Ubuntu 18.04
  • Python version(s): 2 & 3
  • Unreal Engine version(s): 4.22

Possible Drawbacks

Slightly more GPU consumption since this shader is applied now by default.


This change is Reviewable

@marcgpuig marcgpuig self-assigned this Sep 30, 2019
@update-docs
Copy link

update-docs bot commented Sep 30, 2019

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would update our CHANGELOG.md based on your changes.

@marcgpuig marcgpuig force-pushed the marcgpuig/camera_lens branch from a57ba21 to 9a35ba5 Compare September 30, 2019 13:20
Copy link
Contributor

@jackbart94 jackbart94 left a comment

Choose a reason for hiding this comment

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

Reviewed 14 of 15 files at r1, 2 of 3 files at r3.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @bernatx and @marcgpuig)


Docs/bp_library.md, line 13 at r3 (raw file):

       print('  - {}'.format(attr))

Trailing white spaces. Actually, a lot of white spaces are present at the end of some sentences, are they supposed to be there?


Docs/bp_library.md, line 101 at r3 (raw file):

        - `upper_fov` (_Float_)<sub>_ – Modifiable_</sub>
- **<font color="#498efc">sensor.other.collision</font>**  
    - **Attributes:**

This and the next two sensors used to have "role_name" as an attribute. Now it's empty, but the others have kept the "role_name" attribute. Is it normal?


Docs/cameras_and_sensors.md, line 121 at r1 (raw file):

| `black_clip` | float | 0.0 | This will set where the crossover happens where black's start to cut off their value. In general, this value should NOT be adjusted. Range: [0.0, 1.0] |
| `white_clip` | float | 0.04 | This will set where the crossover happens where white's start to cut off their values. This will appear as a subtle change in most cases. Range: [0.0, 1.0] |

For the first sentence in both lines, grammar seems wrong. Maybe "This will set where the crossover happens when black/white tones start to cut off their value"


Docs/cameras_and_sensors.md, line 122 at r1 (raw file):

 When the light temperature and this one match, the light will appear white

Docs/cameras_and_sensors.md, line 126 at r3 (raw file):

Whether the post-process effect in the scene affect the image 

affects

Copy link
Contributor Author

@marcgpuig marcgpuig left a comment

Choose a reason for hiding this comment

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

Reviewable status: 15 of 16 files reviewed, 5 unresolved discussions (waiting on @bernatx, @jackbart94, and @marcgpuig)


Docs/bp_library.md, line 13 at r3 (raw file):

Previously, jackbart94 (Jacopo Bartiromo) wrote…

Trailing white spaces. Actually, a lot of white spaces are present at the end of some sentences, are they supposed to be there?

Yes! is a markdown feature to jump to the next line :)


Docs/cameras_and_sensors.md, line 122 at r1 (raw file):

Previously, jackbart94 (Jacopo Bartiromo) wrote…
 When the light temperature and this one match, the light will appear white

Done.


Docs/cameras_and_sensors.md, line 126 at r3 (raw file):

Previously, jackbart94 (Jacopo Bartiromo) wrote…
Whether the post-process effect in the scene affect the image 

affects

Done.

jackbart94
jackbart94 previously approved these changes Oct 1, 2019
Copy link
Contributor

@jackbart94 jackbart94 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r4.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @bernatx)

Copy link
Contributor

@bernatx bernatx left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @marcgpuig)


Unreal/CarlaUE4/Plugins/Carla/Source/Carla/Sensor/SceneCaptureCamera.h, line 9 at r4 (raw file):

#pragma once

#include "Carla/Sensor/ShaderBasedSensor.h"

We could sort this include moving it down a bit :)

Copy link
Contributor Author

@marcgpuig marcgpuig left a comment

Choose a reason for hiding this comment

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

Reviewable status: 15 of 16 files reviewed, 1 unresolved discussion (waiting on @bernatx and @jackbart94)


Unreal/CarlaUE4/Plugins/Carla/Source/Carla/Sensor/SceneCaptureCamera.h, line 9 at r4 (raw file):

Previously, bernatx (bernat) wrote…

We could sort this include moving it down a bit :)

Not only that but sorted alphabetically!

jackbart94
jackbart94 previously approved these changes Oct 1, 2019
Copy link
Contributor

@jackbart94 jackbart94 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r5.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

bernatx
bernatx previously approved these changes Oct 1, 2019
@marcgpuig marcgpuig dismissed stale reviews from bernatx and jackbart94 via 752a258 October 2, 2019 12:46
@marcgpuig marcgpuig force-pushed the marcgpuig/camera_lens branch 16 times, most recently from 5f091d8 to 968f59f Compare October 3, 2019 14:22
jackbart94
jackbart94 previously approved these changes Oct 3, 2019
Copy link
Contributor

@jackbart94 jackbart94 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r7.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@marcgpuig marcgpuig force-pushed the marcgpuig/camera_lens branch from 1f2352a to 8207963 Compare October 3, 2019 16:08
Copy link
Contributor

@jackbart94 jackbart94 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r8.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Contributor

@bernatx bernatx left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@marcgpuig marcgpuig merged commit 3b625c2 into master Oct 3, 2019
@delete-merged-branch delete-merged-branch bot deleted the marcgpuig/camera_lens branch October 3, 2019 17:08
@marcgpuig marcgpuig mentioned this pull request Oct 4, 2019
@Solonets
Copy link

@marcgpuig what is the configuration for undistorted images (pinhole projection)?

@syoels
Copy link

syoels commented Feb 9, 2020

@marcgpuig what is the configuration for undistorted images (pinhole projection)?

@marcgpuig Joining Sergei on this one. Now that you've exposed these, I'd expect the default values in the docs to represent a pinhole model. Is that the case?

@marcgpuig
Copy link
Contributor Author

@Solonets the values in the description of this PR.
@syoels Yes!

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.

6 participants