habitat-sim
habitat-sim copied to clipboard
Sharing of rendering results
Motivation and Context
Habitat-sim currently has the ability to draw RGB, Depth, and Semantic observations all on the same render pass, however, this isn't currently leveraged! This PR adds the ability for sensor to "borrow" eachother's render results so that if you draw for an RGB sensor and have a Depth sensor that is configured identically, the Depth sensor won't invoke another draw but will instead just read the Depth Buffer from the RGB draw. For Replica scenes, we can share between RGB, Depth, and Semantic!
One question for reviewers: I also tried an implementation that copies between RenderTargets instead of sharing them. While this seems safer, that copy obviously has a cost and grows with resolution. We currently don't make any assumptions that would be violated by this direct sharing so maybe it is fine? If we do want this copy, my testing says that the cost of doing the copy in draw_observation
is pretty high (performance with GPU2GPU drops to bellow without for 256x256 for instance). Doing the copy in get_observation
is less costly.
@mosra I left the code to copy between framebuffers in the PR to see if you knew if there was a way to do it faster.
How Has This Been Tested
New tests.
I re-ran the benchmark on the normal MP3D test scene and got
====== FPS (512 x 512, depth_only): 5624.3 =============
================ Performance (FPS) NPROC=1 ========
Resolution rgb rgbd depth_only
128 x 128 3151.7 1510.4 2361.9
256 x 256 2750.8 1137.1 1893.8
512 x 512 1607.2 697.9 1315.9
====================================================
================ Performance (FPS) NPROC=3 =============
Resolution rgb rgbd depth_only
128 x 128 7684.1 3423.9 5935.4
256 x 256 6339.5 2885.4 4878.6
512 x 512 3817.1 1929.5 3610.2
======================================================
================ Performance (FPS) NPROC=5 ===============
Resolution rgb rgbd depth_only
128 x 128 10651.7 4819.9 9146.4
256 x 256 8940.7 4056.9 7848.4
512 x 512 6674.5 2939.8 5624.3
=====================================================
Comparing to the existing results, this maybe matters for higher resolutions, but otherwise it surprisingly doesn't. However, 17DRP5sb8fy
is a reasonably lightweight scene.
1LXtFkjw3qL
on the other-hand is much heavier and there it matters. Before the PR:
================ Performance (FPS) NPROC=1 ==========
Resolution rgb rgbd depth_only
128 x 128 1227.5 557.1 1129.8
256 x 256 1128.7 501.7 918.3
512 x 512 833.3 415.0 802.9
=================================================
After:
================ Performance (FPS) NPROC=1 ========
Resolution rgb rgbd depth_only
128 x 128 1289.9 868.7 1073.2
256 x 256 1270.8 855.0 1044.9
512 x 512 994.9 566.6 843.0
===============================================
Then with GPU2GPU transfer enabled we get close to zero-cost RGB-D
================ Performance (FPS) NPROC=1 ======
Resolution rgb rgbd depth_only
128 x 128 1319.1 909.5 1219.3
256 x 256 1426.3 924.4 1207.0
512 x 512 1364.3 959.6 1125.2
==========================================
Types of changes
New feature (non-breaking change which adds functionality)
This looks like it will help speed up the simulator quite a bit for many of the challenge tracks. Well done.
Codecov Report
Merging #626 into master will increase coverage by
0.18%
. The diff coverage is97.82%
.
@@ Coverage Diff @@
## master #626 +/- ##
==========================================
+ Coverage 58.30% 58.48% +0.18%
==========================================
Files 130 130
Lines 5905 5931 +26
Branches 84 84
==========================================
+ Hits 3443 3469 +26
Misses 2462 2462
Flag | Coverage Δ | |
---|---|---|
#CPP | 54.48% <100.00%> (+0.10%) |
:arrow_up: |
#JavaScript | 10.00% <ø> (ø) |
|
#Python | 76.52% <97.43%> (+0.19%) |
:arrow_up: |
Impacted Files | Coverage Δ | |
---|---|---|
src/esp/gfx/RenderTarget.h | 100.00% <ø> (ø) |
|
src/esp/sensor/Sensor.h | 85.71% <ø> (ø) |
|
src/esp/sim/Simulator.h | 100.00% <ø> (ø) |
|
habitat_sim/agent/agent.py | 98.21% <90.00%> (+0.01%) |
:arrow_up: |
habitat_sim/sensors/sensor_suite.py | 100.00% <100.00%> (ø) |
|
habitat_sim/simulator.py | 96.45% <100.00%> (+0.20%) |
:arrow_up: |
src/esp/sensor/Sensor.cpp | 87.17% <100.00%> (+2.80%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update bfc7623...27a4f54. Read the comment docs.
@erikwijmans Any reason this still hasn't been merged? Are we blocked by something?
The speedups aren't high enough to justify the ugliness that is this solution IMO. @mosra and I had a chat when I originally made this and the result of that (after some initial resistance from me) is that a better design would be to have a VisualSensor that inherently supports rendering multiple modalities (this would also reduce the ugliness that is making an agent with RGB-D + Sem agent currently), but that is a larger change that I haven't had time to make yet.