Expose lighting setup and individual object lighting manipulation

[matthewjmay]

Motivation and Context

This PR builds on #489 and allows manipulation of light setups through the simulator.

Fixes #302.
Contributes to #332, #361, #319, #315.

Through the simulator, the user can now:

• select a light setup to use when adding an object
• change the light setup a single object uses
• create new light setups
• change an existing light setup, updating all current objects using that setup

For changing the setup for an individual entity, we currently have to traverse the subtree of that object and dynamic cast to the Drawable feature. There are ways to avoid this (ie. store a set of Drawable features) which requires larger code changes and changes to the physics objects memory management, which I can revisit later if wanted.

How Has This Been Tested

• many C++ screenshot tests which compare different lighting configurations, manipulation of lighting configurations, and more!
• Python sanity tests which update lighting configurations
• Visual tests using viewer trying out different lighting setups

Types of changes

• Docs change / refactoring / dependency upgrade
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have completed my CLA (see CONTRIBUTING)
• I have added tests to cover my changes.
• All new and existing tests passed.

[matthewjmay]

There's an annoying issue where a const char[] can't be used as a default parameter to a Magnum::ResourceKey on certain compilers (it worked locally) since default parameters undergo array to pointer decay, and the ResourceKey ctor requires array size info. To mitigate this I've changed default parameter types to strings.

[codecov]

Codecov Report

Merging #500 into master will decrease coverage by 0.02%.
The diff coverage is 76.19%.

@@            Coverage Diff             @@
##           master     #500      +/-   ##
==========================================
- Coverage   60.41%   60.38%   -0.03%     
==========================================
  Files         159      159              
  Lines        7199     7207       +8     
  Branches       84       84              
==========================================
+ Hits         4349     4352       +3     
- Misses       2850     2855       +5

Flag	Coverage Δ
#CPP	56.05% <37.5%> (-0.17%)	⬇️
#JavaScript	10% <ø> (ø)	⬆️
#Python	79.76% <100%> (+0.09%)	⬆️

Impacted Files	Coverage Δ
src/esp/geo/geo.cpp	30% <0%> (-4.29%)	⬇️
src/esp/geo/OBB.cpp	31.42% <0%> (ø)	⬆️
tests/test_simulator.py	100% <100%> (ø)	⬆️
habitat_sim/geo.py	100% <100%> (ø)	⬆️
src/tests/CullingTest.cpp	97.75% <100%> (-0.23%)	⬇️
src/esp/nav/PathFinder.cpp	77.32% <100%> (+0.04%)	⬆️

---

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 ef2cd97...44b66a6. Read the comment docs.

[mosra]

There's an annoying issue where a const char[] can't be used as a default parameter to a Magnum::ResourceKey on certain compilers

ResourceKey keyParam = ResourceKey{"bla"} should work I think (used that back in GCC 4.4 days), no need to go back to std::string. I'll check this, I think the constructor could accept a plain pointer also.

[matthewjmay]

ResourceKey keyParam = ResourceKey{"bla"} should work I think (used that back in GCC 4.4 days), no need to go back to std::string. I'll check this, I think the constructor could accept a plain pointer also.

Right, the issue is that I want to be able to pass in ResourceManager::DEFAULT_LIGHTING_KEY instead of the literal "bla" so I don't have to hardcode strings everywhere. I could always use your suggestion to just use an empty string to do this though.

[mosra]

Yes, I mean ResourceKey param = ResourceKey{ResourceManager::DEFAULT_LIGHTING_KEY} should work too, just that the implicit conversion decays the array for some reason.

[mosra]

Oh and please let me know if the = ResourceKey(...) default param values work.

[mosra]

Yay, tests! :)

I'd make the test images smaller, for example just 128x128. While that might be too small for a human eye, I'd say it's more than good enough for a computer. This would otherwise add over half a megabyte to the repo size, and I assume these are far from being the only test images we'll have.

What I'm also doing before commiting PNGs is running pngcrush -ow file.png on each, it can shrink the files quite a lot compared to what libPNG alone is capable of.

[aclegg3]

Looks good to me. Thanks for working on this! 👍