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

Custom shadow caster materials #3048

Conversation

WilliamLewww
Copy link
Contributor

I added a new SDF parameter ignition:shadow_caster_material_name to specify what shadow caster material should be used to render shadows.

<scene>
  <ignition:shadow_caster_material_name>Gazebo/shadow_caster_ignore_heightmap</ignition:shadow_caster_material_name>
  <background>0.0 0.0 0.0 1</background>
</scene>

Currently, the scene is hard-coded to always use "Gazebo/shadow_caster" for all shadow caster materials.

Adding custom shadow casters could also allow the usage of custom uniform parameters without clumping up "Gazebo/shadow_caster".

Related pull request: WilliamLewww#5

WilliamLewww and others added 19 commits May 21, 2021 11:38
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: Steven Peters <scpeters@openrobotics.org>
Signed-off-by: Steven Peters <scpeters@openrobotics.org>
Copy link
Member

@scpeters scpeters left a comment

Choose a reason for hiding this comment

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

I managed to remove the input parameter to the service in scpeters@18092a2; please consider merging it in

@WilliamLewww
Copy link
Contributor Author

The changes work great! I just merged in your branch.

@WilliamLewww
Copy link
Contributor Author

WilliamLewww commented Jul 22, 2021

the CustomShadowCaster tests are failing on macOS:

* https://build.osrfoundation.org/job/gazebo-ci-pr_any-homebrew-amd64/2493/testReport/junit/(root)/CustomShadowCasterTest/DefaultShadowCaster/

* https://build.osrfoundation.org/job/gazebo-ci-pr_any-homebrew-amd64/2493/testReport/junit/(root)/CustomShadowCasterTest/CustomShadowCaster/

I think it's ok to disable these tests if there is not an easy fix

These tests I created use approximating for validation and can be unpredictable.
I think the build not failing is a good test, but I'll fix up the tests for completion :)

Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
@WilliamLewww
Copy link
Contributor Author

The test is still failing and I'll try to update it again before I just remove it.

@scpeters
Copy link
Member

I did notice an issue with PCF on macOS in #3036 (review), but I'm not sure if that's related

@WilliamLewww
Copy link
Contributor Author

#3036 uses separate shaders so that might not be the issue. I removed the integration test to prevent future error output.

Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
@WilliamLewww WilliamLewww force-pushed the wlew/spotlight_shadows_ignore_sun_heightmap branch from 4f4c801 to 1096936 Compare July 26, 2021 21:42
@scpeters
Copy link
Member

scpeters commented Aug 4, 2021

#3036 uses separate shaders so that might not be the issue. I removed the integration test to prevent future error output.

the test is working on Ubuntu, so I would add it back in but disable it for macOS. For example, here is some cmake logic that skip tests depending on the operating system:

Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
@scpeters scpeters merged commit 42f7e48 into gazebosim:gazebo11 Aug 12, 2021
scpeters added a commit to gazebosim/gz-msgs that referenced this pull request Aug 16, 2021
This field was desired by gazebosim/gazebo-classic#3048, though it couldn't
be added due to ABI concerns. So add it to fortress for future use.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
scpeters added a commit to scpeters/gazebo that referenced this pull request Aug 17, 2021
The shadow caster name in the Scene is empty if the ignition
transport service call fails, so initialize it to the default
value.

Follow up to gazebosim#3048.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
scpeters added a commit that referenced this pull request Aug 17, 2021
The shadow caster name in the Scene is empty if the ignition
transport service call fails, so initialize it to the default
value.

Follow up to #3048.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@Tobias-Fischer
Copy link
Contributor

Hi @WilliamLewww @scpeters,

We received a bug report in the conda-forge repo (conda-forge/gazebo-feedstock#90) that Gazebo 11.8 (where this PR first appears) does not start in Windows, with this error:

[Err] [..\gazebo\gui\main.cc:37] Ogre Error:ItemIdentityException: Resource with the name shadow_caster_vp_glsl already exists. in ResourceManager::add at ..\OgreMain\src\OgreResourceManager.cpp (line 158)
[Err] [..\gazebo\server_main.cc:57] Ogre Error:ItemIdentityException: Resource with the name shadow_caster_vp_glsl already exists. in ResourceManager::add at ..\OgreMain\src\OgreResourceManager.cpp (line 158)
[Err] [..\gazebo\transport\Connection.cc:273] The I/O operation has been aborted because of either a thread exit or an application requestValue[995]

I think that this PR is the culprit. Do you have any idea how to fix the issue? I am not sure whether the problem occurs outside of our conda-forge build.

/cc @johnwason @wolfv @traversaro @seanyen

@WilliamLewww
Copy link
Contributor Author

WilliamLewww commented Aug 19, 2021

Hi @WilliamLewww @scpeters,

We received a bug report in the conda-forge repo (conda-forge/gazebo-feedstock#90) that Gazebo 11.8 (where this PR first appears) does not start in Windows, with this error:

[Err] [..\gazebo\gui\main.cc:37] Ogre Error:ItemIdentityException: Resource with the name shadow_caster_vp_glsl already exists. in ResourceManager::add at ..\OgreMain\src\OgreResourceManager.cpp (line 158)
[Err] [..\gazebo\server_main.cc:57] Ogre Error:ItemIdentityException: Resource with the name shadow_caster_vp_glsl already exists. in ResourceManager::add at ..\OgreMain\src\OgreResourceManager.cpp (line 158)
[Err] [..\gazebo\transport\Connection.cc:273] The I/O operation has been aborted because of either a thread exit or an application requestValue[995]

I think that this PR is the culprit. Do you have any idea how to fix the issue? I am not sure whether the problem occurs outside of our conda-forge build.

/cc @johnwason @wolfv @traversaro @seanyen

I think I see the issue where shadow_caster_vp_glsl is defined both in:

  • gazebo/media/materials/scripts/shadow_caster.program
  • gazebo/media/materials/scripts/shadow_caster_ignore_heightmap.program

A temporary fix could be to:

  1. Delete the following lines
    https://github.com/osrf/gazebo/blob/b502f17729302fa0da8e2a00ffbd69112bb7e6aa/media/materials/scripts/shadow_caster_ignore_heightmap.program#L1-L10

If that doesn't work then
2. Change
https://github.com/osrf/gazebo/blob/b502f17729302fa0da8e2a00ffbd69112bb7e6aa/media/materials/scripts/shadow_caster_ignore_heightmap.program#L32
to vertex_program_ref DeferredShading/vs

I'm currently checking out the issue and rebuilding my workspace, but I'll send a follow up if the solution works on my end.

@Tobias-Fischer
Copy link
Contributor

Thanks for looking into it so quickly @WilliamLewww. It would be great if you let us know which of your two suggestions ends up being the better one, and we can patch it on conda-forge until a new release comes out.

@WilliamLewww
Copy link
Contributor Author

Thanks for looking into it so quickly @WilliamLewww. It would be great if you let us know which of your two suggestions ends up being the better one, and we can patch it on conda-forge until a new release comes out.

No worries! I was able to fix the issue on my end with the following changes: WilliamLewww@8d50434.

I will be opening a pull request soon to add the changes. Let me know if this fixes the issue you ran into.

scpeters added a commit to scpeters/ign-msgs that referenced this pull request Sep 8, 2021
Motivated by gazebosim/gazebo-classic#3048.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
chapulina added a commit to gazebosim/gz-msgs that referenced this pull request Sep 17, 2021
* scene.proto: add shadow_caster_material_name

Motivated by gazebosim/gazebo-classic#3048.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Co-authored-by: Louise Poubel <louise@openrobotics.org>
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.

3 participants