-
Notifications
You must be signed in to change notification settings - Fork 486
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
LensFlare - allow inheritance #2965
LensFlare - allow inheritance #2965
Conversation
…stener defintion to header
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the ABI checker job failed; I'll see if I can get it running again
diff without whitespace: |
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
our jenkins builds are not properly reporting style errors (gazebo-tooling/release-tools#446), so I took the liberty of shortening some line lengths to 80 characters and cleaning up some whitespace in 48837d1 |
* Add LensFlarePrivate setter methods and move LensFlareCompositorListener defintion to header * Modified LensFlareCompositorListener members to follow PIMPL * Reduce line length and fix whitespace Signed-off-by: Steve Peters <scpeters@openrobotics.org> Co-authored-by: kbjeppes <kaden.b.jeppesen@nasa.gov> Co-authored-by: Steve Peters <scpeters@openrobotics.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A little late. Changes look good. Just a minor comment about function name
|
||
/// \brief Set camera stored in LensFlarePrivate | ||
/// \param[in] _camera Camera to use in sensor. | ||
protected: void SetCameraSensor(CameraPtr _camera); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note that _camera
can be either a camera sensor or a user camera but this name suggests that it needs to be a camera sensor.
Maybe something like SetCameraImp
? We've used the the *Imp suffix in other places for internal functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice catch! updating to LensFlare::SetCameraImpl
in #2975
This is an adjustment to API added in gazebosim#2965, which is allowed because the API has not yet been released. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
This is an adjustment to API added in #2965, which is allowed because the API has not yet been released. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
* Add LensFlarePrivate setter methods and move LensFlareCompositorListener defintion to header * Modified LensFlareCompositorListener members to follow PIMPL * Reduce line length and fix whitespace Signed-off-by: Steve Peters <scpeters@openrobotics.org> Co-authored-by: kbjeppes <kaden.b.jeppesen@nasa.gov> Co-authored-by: Steve Peters <scpeters@openrobotics.org>
This is an adjustment to API added in gazebosim#2965, which is allowed because the API has not yet been released. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
* Add LensFlarePrivate setter methods and move LensFlareCompositorListener defintion to header * Modified LensFlareCompositorListener members to follow PIMPL * Reduce line length and fix whitespace Signed-off-by: Steve Peters <scpeters@openrobotics.org> Co-authored-by: kbjeppes <kaden.b.jeppesen@nasa.gov> Co-authored-by: Steve Peters <scpeters@openrobotics.org>
This is an adjustment to API added in #2965, which is allowed because the API has not yet been released. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Added LensFlarePrivate setter methods and moved LensFlareCompositor definition to header