-
Notifications
You must be signed in to change notification settings - Fork 44
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
Expose shadow texture size for directional lighting in SDF #633
Conversation
Only recently realized that my PR breaks ABI, so PR is for EDIT - my changes in both gz-gui, gz-rendering work in |
Signed-off-by: Athena Z <athenaz@google.com>
Signed-off-by: Athena Z <athenaz@google.com>
Signed-off-by: Athena Z <athenaz@google.com>
Signed-off-by: Athena Z <athenaz@google.com>
Signed-off-by: Athena Z <athenaz@google.com>
Signed-off-by: Athena Z <athenaz@google.com>
Signed-off-by: Athena Z <athenaz@google.com>
db34426
to
acea8e3
Compare
…false; update names of enum vals used Signed-off-by: Athena Z <athenaz@google.com>
Signed-off-by: Athena Z <athenaz@google.com>
@iche033 if you have some bandwidth, would you be able to review this since you already have some context with gazebosim/gz-rendering#1034? |
yep will do |
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.
just some minor comments on error msgs
@@ -238,6 +239,8 @@ namespace gz::gui::plugins | |||
/// \brief True if sky is enabled; | |||
public: bool skyEnable = false; | |||
|
|||
public: unsigned int directionalLightTextureSize = 2048u; |
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.
add doxygen comment
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.
Oops. Added here 0bc87f8
@@ -368,6 +371,12 @@ namespace gz::gui::plugins | |||
/// \param[in] _sky True to enable the sky, false otherwise. | |||
public: void SetSkyEnabled(const bool &_sky); | |||
|
|||
/// @brief \brief Set the shadow texture size for the given light type. | |||
/// @param _lightType Light type that creates the shadow | |||
/// @param _textureSize Shadow texture size |
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.
nit: replace @
with \
for consistency
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.
Co-authored-by: Ian Chen <ichen@openrobotics.org> Signed-off-by: Athena Z. <athenaz@google.com>
Signed-off-by: Athena Z <athenaz@google.com>
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.
Looks good to me
Just a note that this works for the 3D scene in the GUI. In the future, we may want to extend this functionality to shadows on the server side for sensors.
🎉 New feature
Related to gazebosim/gz-rendering#1034, view that for details.
Summary
Test it
Checklist
codecheck
passed (See contributing)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.