gz-sim icon indicating copy to clipboard operation
gz-sim copied to clipboard

GUI for Global Illumination (VCT / CI VCT)

Open darksylinc opened this issue 1 year ago • 5 comments

🎉 New feature

Related to gazebosim/gz-rendering#675 Related to gazebosim/gz-rendering#435

Summary

This PR, which depends on PR gazebosim/gz-rendering#675 to work, adds a GUI to enable and configure the new Global Illumination features added to gz-rendering

The GUI is fully functional, but there is more features planned for the future. GI is important for sensors, and right now server-side GI cannot be controlled.

This can be more complex to solve because GUI & Server should be in sync; at least in settings. And it's important the server is deterministic.

CI VCT will require attention to some specifics because the GI quality is concentric to a given camera location; hence this needs to be accounted properly when there's multiple sensors.

Realtime GI is a difficult problem to solve hence this PR introduces users to the feature. We expect to be a few bumps and a lot of feedback to fix the problems we all (users and us) encounter.

All graphics engines are in a similar situation.

Checklist

  • [x] Signed all commits for DCO
  • [ ] Added tests
  • [ ] Updated documentation (as needed)
  • [ ] Updated migration guide (as needed)
  • [ ] Consider updating Python bindings (if the library has them)
  • [x] codecheck passed (See contributing)
  • [x] All tests passed (See test coverage)
  • [x] 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.

darksylinc avatar Jul 18 '22 00:07 darksylinc

Thanks for the plugin! I only looked at the code superficially, and it looks to me that it doesn't have any gz-sim specific functionality, is that the case? If so, this plugin could be in gz-gui instead, so applications other than the simulator can leverage it.

chapulina avatar Jul 18 '22 16:07 chapulina

it looks to me that it doesn't have any gz-sim specific functionality, is that the case? If so, this plugin could be in gz-gui instead

Right now it doesn't, but it is expected to in the future.

As explained in the PR description right now the plugin will only enable GI in the preview; so you can see it "pretty" on the GUI. But it is expected in the future that this plugin will control GI server-side as well.

This is a bit complex to debate because there are different issues to address in how server & gui client are synchronized:

  • Option 1: Server & Client sync their settings. Client & Server may see different things due to various differences (like camera placement for the case of CI VCT, when was the VCT internal structure build)
  • Option 2: Server & Client sync VCT's internal structures. This may be BW intensive and too hard to do; but Server & Client would see exactly the same
  • Option 3: Very little sync; only a few settings. In this case what both see may diverge a lot

Our aim is at option 1; but right now this PR only introduces GUI-only GI.

Update: Longer answer

The problem about real time GI is that there is a baking part. Global Illumination can be:

  • Completely Static. Such as with lightmaps. The objects cannot move, baking takes a long time. Only the camera can move freely
  • Semi Dynamic. This is where we are now. Static objects are baked into acceleration structures. Non-static objects can receive Global Illumination, but they do not participate of the bouncing process. If all objects are dynamic, then GI cannot work
    • VCT doesn't take a lot of time to bake (i.e. it's not measured in minutes like lightmaps), but baking every frame would cause framerate to go into 1 fps.
  • Fully Dynamic. All objects can move freely. No one in the industry is here yet. Not even with Raytracing Hardware (because there is a BLAS and TLAS acceleration structures; one of them is for baking static objects with lots vertices) unless you have a low polygon count (such as SM64 RT project).
    • CIVCT is close to this goal, but not quite there yet.

Because Gazebo doesn't have the notion of static objects (thus it doesn't have the GUI widgets necessary to easily classify objects into dynamic & static), this GI plugin cheats a little by treating all objects as static, baking the structures when it is enabled, then ignoring any object movement.

This means that if a yellow cube is baked, it will reflect off yellow indirect light. But if that yellow cube moves or disappears, there will be a phantom indirect light left unless the GI is rebaked.

As a result a fully matured plugin would expose:

  • A way to tag objects as static / dynamic (ideally this info is provided during creation to speed up performance)
  • A way to mark when or why to trigger a VCT re-bake
  • A way to speed up re-bakes (i.e. VCT can start from scratch which means it has to prepare a lot of vertex buffers; but if meshes are promised that they vertex data hasn't changed, we can reuse some part that has already been baked)

And when a VCT re-bake is triggered, the GUI & servers should be in sync (whether it was the server who requested it, or the GUI)

darksylinc avatar Jul 18 '22 16:07 darksylinc

in the future that this plugin will control GI server-side as well.

Got it, makes sense. Thinking a bit more broadly, I could see this functionality being part of a GUI widget that exposes all scene properties. It could use the user command pattern to send commands to the UserCommands server-side system, which takes care of propagating the changes to the ECM.

  • https://github.com/gazebosim/gz-sim/issues/32

chapulina avatar Jul 18 '22 16:07 chapulina

I've updated my comment with more info (see "Longer answer)

darksylinc avatar Jul 18 '22 16:07 darksylinc

The PR this depends on, https://github.com/gazebosim/gz-rendering/pull/675, didn't get into Garden. I'll remove the label. This PR can be merged into main, which will become gz-sim8 / Gazebo H.

chapulina avatar Aug 26 '22 22:08 chapulina

waiting on version bumps

azeey avatar Nov 14 '22 19:11 azeey

@osrf-jenkins run tests please

iche033 avatar Jan 05 '23 00:01 iche033

Codecov Report

Merging #1597 (c51de35) into main (5fe83a3) will not change coverage. The diff coverage is n/a.

:exclamation: Current head c51de35 differs from pull request most recent head f788bf5. Consider uploading reports for the commit f788bf5 to get more accurate results

@@           Coverage Diff           @@
##             main    #1597   +/-   ##
=======================================
  Coverage   64.49%   64.49%           
=======================================
  Files         341      341           
  Lines       27098    27098           
=======================================
  Hits        17477    17477           
  Misses       9621     9621           
Impacted Files Coverage Δ
src/rendering/RenderUtil.cc 39.56% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Jan 05 '23 02:01 codecov[bot]