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

Add Lens Flare System #1933

Merged
merged 6 commits into from
Mar 17, 2023
Merged

Conversation

jasmeet0915
Copy link
Contributor

🎉 New feature

Closes #1909

Summary

  • Added Lens Flare System that adds Lens Flare Render Pass to the camera and allows users to enable lens flare in camera images. The lens flares can be configured using <scale>, <color>, and <occlusion_steps> tags in the SDF.
  • Added an example camera_lens_flare.sdf file for testing which contains 2 cameras(wide angle and normal) with different configurations for the lens flare (one default and other custom).

ezgif com-video-to-gif (1)

Test it

  • Make sure to use the gz-sim8 and compatible versions of other packages especially gz-rendering8 since it contains the Lens Flare Pass.
  • Launch the examples/worlds/camera_lens_flare.sdf file from the gz-sim repo using:
gz sim camera_lens_flare.sdf

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • 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.

🎈 Release

Preparation for <X.Y.Z> release.

Comparison to <x.y.z>: https://github.com/gazebosim//compare/<LATEST_TAG_BRANCH>...<RELEASE_BRANCH>

Needed by <PR(s)>

Checklist

  • Asked team if this is a good time for a release
  • There are no changes to be ported from the previous major version
  • No PRs targeted at this major version are close to getting in
  • Bumped minor for new features, patch for bug fixes
  • Updated changelog
  • Updated migration guide (as needed)
  • Link to PR updating dependency versions in appropriate repository in gazebo-release (as needed):

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.

@jasmeet0915 jasmeet0915 requested a review from mjcarroll as a code owner March 16, 2023 16:14
@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Mar 16, 2023
@jasmeet0915 jasmeet0915 force-pushed the jasmeet/lens_flare_system branch 3 times, most recently from 3693e6f to 433e4ea Compare March 16, 2023 16:24
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.

nice, this works well for me! I left a few comments, mostly coding style changes.

examples/worlds/camera_lens_flare.sdf Outdated Show resolved Hide resolved
@@ -0,0 +1,244 @@
/*
* Copyright (C) 2020 Open Source Robotics Foundation
Copy link
Contributor

Choose a reason for hiding this comment

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

2020 -> 2023

@@ -0,0 +1,73 @@
/*
* Copyright (C) 2020 Open Source Robotics Foundation
Copy link
Contributor

Choose a reason for hiding this comment

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

2020 -> 2023

@@ -0,0 +1,103 @@
/*
* Copyright (C) 2022 Open Source Robotics Foundation
Copy link
Contributor

Choose a reason for hiding this comment

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

2022 -> 2023

}



Copy link
Contributor

Choose a reason for hiding this comment

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

remove a couple of extra lines here

<far>100</far>
</clip>
<lens>
<type>gnomonical</type>
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm just noticed that there could an issue with the gnomonical type on certain graphics drivers, e.g. on mesa + llvmpipe the camera does not render properly but works fine on Nvidia drivers. Something to be investigated later. In the mean time, can you change this to equidistant? and maybe make the the horizontal_fov larger to make it obvious it's a wide angle camera, e.g. 3.14159

// call function that connects to post render event
// using a separate function because unique_ptr (this->dataPtr)
// is not supported as a argument of std::bind
this->dataPtr->ConnectToPostRender();
Copy link
Contributor

Choose a reason for hiding this comment

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

you should be able to do:

    this->dataPtr->postRenderConn =
        this->dataPtr->eventMgr->Connect<events::PostRender>(
        std::bind(&LensFlarePrivate::OnPostRender, this->dataPtr.get()));

Copy link
Contributor

Choose a reason for hiding this comment

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

to be consistent with gazebo coding style, can you change indentation from 4 spaces to 2 spaces in this file?

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 you missed this one. Can you update the indentation?

Copy link
Contributor

Choose a reason for hiding this comment

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

can you add this test to CMakeLists.txt here?

@iche033
Copy link
Contributor

iche033 commented Mar 16, 2023

can you also sign your commits?

Signed-off-by: Jasmeet Singh <[email protected]>
 - Adds doxygen comments in LensFlare.hh

Signed-off-by: Jasmeet Singh <[email protected]>
Signed-off-by: Jasmeet Singh <[email protected]>
@jasmeet0915 jasmeet0915 force-pushed the jasmeet/lens_flare_system branch from 433e4ea to 4a6a9c9 Compare March 17, 2023 05:45
Signed-off-by: Jasmeet Singh <[email protected]>
@jasmeet0915
Copy link
Contributor Author

@iche033 I have made all the suggested changes in the latest commit and have signed all previous commits as well ✔️

@@ -11,6 +11,7 @@ set(tests
breadcrumbs.cc
buoyancy.cc
buoyancy_engine.cc
camera_lens_flare.cc
Copy link
Contributor

Choose a reason for hiding this comment

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

can you move this line to the tests_needing_display list below?

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 you missed this one. Can you update the indentation?

 - Updated LensFlare.cc indentation to 2 spaces

Signed-off-by: Jasmeet Singh <[email protected]>
@jasmeet0915
Copy link
Contributor Author

@iche033 Not sure how I missed that one 😅. Anyways, I have made the required changes.

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.

awesome, thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants