Enabling Global Illumination (GI VCT) for sensors in SDF #2550

athenaz2 · 2024-08-27T17:30:23Z

🎉 New feature

Summary

Global illumination (GI) has been enabled for the GUI with #1597, but not for the camera sensor view. This PR allows for GI to be enabled for sensors in the SDF as a <global_illumination> element within the sensor_system plugin. Currently supports the VCT technique, in the future can consider adding support for CI VCT.

Sensor view (left) and GUI (right) without GI VCT:

With GI VCT:

Test it

gi_for_sensors_demo.zip

Modify the <global_illumination> element in the SDF. This element is within the sensor_system plugin. The format is as follows:

<global_illumination type="vct">
    <enabled>true</enabled>
    <resolution>16 16 16</resolution>
    <octant_count>1 1 1</octant_count>
    <bounce_count>6</bounce_count>
    <high_quality>true</high_quality>
    <anisotropic>true</anisotropic>
    <thin_wall_counter>1.0</thin_wall_counter>
    <conserve_memory>true</conserve_memory>
    <debug_vis_mode>none</debug_vis_mode>
</global_illumination>

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.

Signed-off-by: Athena Z <[email protected]>

iche033

this generally looks good, added some suggestions

iche033 · 2024-08-27T20:22:14Z

src/systems/sensors/Sensors.cc

@@ -521,6 +628,52 @@ Sensors::~Sensors()
  this->dataPtr->Stop();
 }

+//TODO: why does math::Vector3d work but not math::Vector3i?


looks like math::Vector3i is not one of the supported types: https://github.com/gazebosim/sdformat/blob/ba1bf8fedc0b6063d4312ad312e515e28f71cfb9/include/sdf/Param.hh#L529-L537

iche033 · 2024-08-27T20:30:51Z

src/systems/sensors/Sensors.cc

+      std::string giType = giElem->GetAttribute("type")->GetAsString();
+      if (giType == "vct")
+      {
+        this->dataPtr->giVctParameters.enabled = giElem->Get<bool>("enabled", this->dataPtr->giVctParameters.enabled).first;


wrap lines to 80char

Checked style with codecheck in f82c68e

src/systems/sensors/Sensors.cc

Co-authored-by: Ian Chen <[email protected]> Signed-off-by: Athena Z. <[email protected]>

… clean with codecheck Signed-off-by: Athena Z <[email protected]>

Signed-off-by: Athena Z <[email protected]>

iche033 · 2024-08-30T01:20:15Z

looks like there's an issue with GI on metal. Output from homebrew CI build:

281: [ RUN      ] CameraSensorGlobalIlluminationTest.GlobalIlluminationEnabled
281: libc++abi: terminating with uncaught exception of type Ogre::FileNotFoundException: OGRE EXCEPTION(6:FileNotFoundException): Cannot locate resource CrossPlatformSettings_piece_all.metal in resource group General or any other group. in ResourceGroupManager::openResource at /tmp/ogre2.3-20230918-86352-14va4s/ogre-next-2.3.1/OgreMain/src/OgreResourceGroupManager.cpp (line 793)

you can disable the test on mac using this macro. Can you ticket an issue on this in gz-rendering?

…S :( Signed-off-by: Athena Z <[email protected]>

athenaz2 · 2024-08-30T18:31:05Z

looks like there's an issue with GI on metal. Output from homebrew CI build:

281: [ RUN      ] CameraSensorGlobalIlluminationTest.GlobalIlluminationEnabled
281: libc++abi: terminating with uncaught exception of type Ogre::FileNotFoundException: OGRE EXCEPTION(6:FileNotFoundException): Cannot locate resource CrossPlatformSettings_piece_all.metal in resource group General or any other group. in ResourceGroupManager::openResource at /tmp/ogre2.3-20230918-86352-14va4s/ogre-next-2.3.1/OgreMain/src/OgreResourceGroupManager.cpp (line 793)

you can disable the test on mac using this macro. Can you ticket an issue on this in gz-rendering?

Okay, created an issue - gazebosim/gz-rendering#1048. I also noted it in the issue description but the GI unit test also skips testing for Apple platform.

azeey · 2024-08-30T21:44:43Z

The test is still segfaulting. I'm wondering if it's an issue with having two rendering tests in the same file. Can you try commenting out the "GiNotEnabled" version to see if just running the test with GI enabled still segfaults?

Signed-off-by: Athena Z <[email protected]>

…gz-sim into athenaz/gi_for_sensors

test/worlds/camera_sensor_gi_enabled_true.sdf

iche033 · 2024-09-06T21:54:24Z

test/worlds/camera_sensor_gi_enabled_true.sdf

+    <!-- <plugin
+      filename="gz-sim-scene-broadcaster-system"
+      name="gz::sim::systems::SceneBroadcaster">
+    </plugin> -->


remove if not used.

iche033 · 2024-09-06T21:55:14Z

test/worlds/camera_sensor_gi_enabled_true.sdf

+              <property key="showTitleBar" type="bool">false</property>
+          </gz-gui>
+      </plugin>
+    </gui>


I think you can remove the whole <gui> block because we don't need it for the test right?

same for the camera_sensor_gi_enabled_false.sdf test world

Whoops, was from creating SDF scene and ensuring cam lines up for test case. 95530a6

test/integration/camera_sensor_global_illumination.cc

iche033 · 2024-09-06T23:15:27Z

tutorials/global_illumination.md

+#### Example usage with VCT
+
+We will demonstrate how to enable VCT for the sensor with the SDF file below. (The finished SDF file can be viewed [here](
+https://github.com/gazebosim/gz-sim/blob/main/examples/worlds/global_illumination.sdf).)


change link to point to the gz-sim8 branch

iche033 · 2024-09-06T23:15:34Z

tutorials/global_illumination.md

+#### Example usage for CI VCT
+
+1) Open the [global_illumination.sdf](
+https://github.com/gazebosim/gz-sim/blob/main/examples/worlds/global_illumination.sdf) world using Vulkan with


change link to point to the gz-sim8 branch

tutorials/global_illumination.md

iche033 · 2024-09-06T23:25:40Z

tutorials/global_illumination.md

+#### Example usage for VCT
+
+1) Open the [global_illumination.sdf](
+https://github.com/gazebosim/gz-sim/blob/main/examples/worlds/global_illumination.sdf) world with


update link to point to gz-sim8 branch

Co-authored-by: Ian Chen <[email protected]> Signed-off-by: Athena Z. <[email protected]>

…cause unused, redo links Signed-off-by: Athena Z <[email protected]>

Signed-off-by: Ian Chen <[email protected]>

iche033 · 2024-09-09T20:18:11Z

I made a couple of minor changes in f03476b (removed white space), and 966f48c (hide gui plugins). This now looks good to me.

azeey · 2024-09-11T17:31:18Z

This shouldn't have been merged since we are in code freeze. Let's make sure we don't forward port this to gz-sim9. Ideally, we won't need to forward port in the next few weeks, but if we do, we should just cherry-pick any necessary changes.

athenaz2 added 2 commits August 22, 2024 16:05

Hack to enable GI for sensors

4afb41b

Signed-off-by: Athena Z <[email protected]>

Parse and set SDF parameters for GI VCT

e36edfd

Signed-off-by: Athena Z <[email protected]>

athenaz2 requested a review from iche033 as a code owner August 27, 2024 17:30

github-actions bot added the 🎵 harmonic Gazebo Harmonic label Aug 27, 2024

athenaz2 marked this pull request as draft August 27, 2024 17:31

Add doxygen comments and cleaned up code a bit

bd93e33

Signed-off-by: Athena Z <[email protected]>

iche033 reviewed Aug 27, 2024

View reviewed changes

azeey reviewed Aug 27, 2024

View reviewed changes

src/systems/sensors/Sensors.cc Show resolved Hide resolved

athenaz2 and others added 4 commits August 27, 2024 16:12

Update src/systems/sensors/Sensors.cc

f401172

Co-authored-by: Ian Chen <[email protected]> Signed-off-by: Athena Z. <[email protected]>

Add comments documenting helper funcs as well as clarifying comments;…

f82c68e

… clean with codecheck Signed-off-by: Athena Z <[email protected]>

Add GI demo SDF as example

5a1d4ee

Signed-off-by: Athena Z <[email protected]>

Add test scene SDFs and test file

bb70589

Signed-off-by: Athena Z <[email protected]>

athenaz2 marked this pull request as ready for review August 29, 2024 01:00

athenaz2 requested a review from mjcarroll as a code owner August 29, 2024 01:00

athenaz2 and others added 4 commits August 28, 2024 18:20

Minor codecheck modification

f725194

Signed-off-by: Athena Z <[email protected]>

Minor var rename

843c0c8

Signed-off-by: Athena Z <[email protected]>

Merge branch 'gz-sim8' into athenaz/gi_for_sensors

407ab1c

Add surrounding box in SDF, set test expectations to be more stark

0ab686a

Signed-off-by: Athena Z <[email protected]>

Minor codecheck modification for SDF, disable GIEnabled test for macO…

8738398

…S :( Signed-off-by: Athena Z <[email protected]>

athenaz2 mentioned this pull request Aug 30, 2024

Supporting global illumination for macOS/metal gazebosim/gz-rendering#1048

Open

athenaz2 added 3 commits August 30, 2024 15:26

Comment out GINotEnabled test to check segfault issue

56b841b

Signed-off-by: Athena Z <[email protected]>

Disable test if MESA_GL_VERSION_OVERRIDE env var is set

0293b5f

Signed-off-by: Athena Z <[email protected]>

Add GI tutorial

db226c2

Signed-off-by: Athena Z <[email protected]>

athenaz2 requested a review from mabelzhang as a code owner September 3, 2024 16:56

athenaz2 added 3 commits September 4, 2024 16:03

At end of RenderThread, reset giVct shared ptr

e310454

Signed-off-by: Athena Z <[email protected]>

Merge branch 'gz-sim8' into athenaz/gi_for_sensors

420231d

Trim trailing whitespace

3f64dd0

Signed-off-by: Athena Z <[email protected]>

athenaz2 added 2 commits September 5, 2024 09:37

Merge branch 'athenaz/gi_for_sensors' of https://github.com/athenaz2/…

6a7a6c2

…gz-sim into athenaz/gi_for_sensors

Merge branch 'gz-sim8' into athenaz/gi_for_sensors

10f1b37

iche033 reviewed Sep 6, 2024

View reviewed changes

athenaz2 and others added 5 commits September 6, 2024 16:49

Apply suggestions from code review

7f49293

Co-authored-by: Ian Chen <[email protected]> Signed-off-by: Athena Z. <[email protected]>

Apply suggestions from code review

f04599f

Co-authored-by: Ian Chen <[email protected]> Signed-off-by: Athena Z. <[email protected]>

Remove commented out sections in SDFs, remove GUI plugins from SDF be…

95530a6

…cause unused, redo links Signed-off-by: Athena Z <[email protected]>

remove whitespace

f03476b

Signed-off-by: Ian Chen <[email protected]>

hide scene manager and view control gui plugins

966f48c

Signed-off-by: Ian Chen <[email protected]>

iche033 approved these changes Sep 9, 2024

View reviewed changes

ahcorde approved these changes Sep 10, 2024

View reviewed changes

ahcorde merged commit a167312 into gazebosim:gz-sim8 Sep 10, 2024
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enabling Global Illumination (GI VCT) for sensors in SDF #2550

Enabling Global Illumination (GI VCT) for sensors in SDF #2550

athenaz2 commented Aug 27, 2024 •

edited

Loading

iche033 left a comment

iche033 Aug 27, 2024

iche033 Aug 27, 2024

athenaz2 Aug 28, 2024

iche033 commented Aug 30, 2024

athenaz2 commented Aug 30, 2024

azeey commented Aug 30, 2024

iche033 Sep 6, 2024

athenaz2 Sep 7, 2024

iche033 Sep 6, 2024

iche033 Sep 6, 2024

athenaz2 Sep 7, 2024

iche033 Sep 6, 2024

athenaz2 Sep 7, 2024

iche033 Sep 6, 2024

athenaz2 Sep 7, 2024

iche033 Sep 6, 2024

athenaz2 Sep 7, 2024

iche033 commented Sep 9, 2024

azeey commented Sep 11, 2024

Enabling Global Illumination (GI VCT) for sensors in SDF #2550

Enabling Global Illumination (GI VCT) for sensors in SDF #2550

Conversation

athenaz2 commented Aug 27, 2024 • edited Loading

🎉 New feature

Summary

Test it

Checklist

iche033 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iche033 commented Aug 30, 2024

athenaz2 commented Aug 30, 2024

azeey commented Aug 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iche033 commented Sep 9, 2024

azeey commented Sep 11, 2024

athenaz2 commented Aug 27, 2024 •

edited

Loading