Update Magnum from v2019.10 to latest master

[mosra]

Motivation and Context

Obsoletes facebookresearch/habitat-lab#295 and unblocks #489.

Was much less painful than I expected it to be. The critically important things this brings:

• Removed the ResourceManager singleton that got deprecated in v2019.10 to unbreak multithreaded workflows (mosra/magnum@e7aeaf7)
• Resource no longer attempts to fetch updated data when not needed (mosra/magnum@47e1106)
• TinyGltfImporter now sets shininess to 80.0f by default, instead of 1.0f (mosra/magnum-plugins@ecec794)
• A (partial) workaround for crashy GL::BufferTexture on macOS (ping: @bigbike). There's still one corner case left to fix but it can be easily prevented in the code (see the apple-buffer-texture-unbind-on-buffer-modify workaround)

The seven megatons of other stuff you didn't ask for but get "by the way":

• Growable Arrays that aim to be faster than std::vector (blogpost is in the pipeline!)
• New Utility::copy() utility that works on multi-dimensional strided arrays, plus various other utilities related to strided array views, as those will be central to the new asset pipeline
• Printing file+line with Debug
• KHR_lights_punctual support in TinyGltfImporter, in case you'd want to import light setup from a file instead of having it hardcoded (ping: @matthewjmay)
• TinyGltfImporter now supports interleaved vertex attributes, broadening asset compatibility
• Color support in StanfordImporter, bringing it closer to what we'll need here for PLY files (see also the benchmark below)
• Cursor and window icon management in GlfwAppliction, in case you want to spice up the viewer utility
• Everything else that changed since the v2019.10 tag:
  • Corrade changelog
  • Magnum changelog
  • Magnum Plugins changelog

There are no known regressions, and no compatibility breakages that would affect the use in Habitat. I also removed one now-obsolete CMake workaround for the Basis submodule and updated all Find modules. The asset pipeline rework (sneak peek benchmark below) is fortunately still in a branch, otherwise this relatively straightforward upgrade wouldn't be possible. The bigger update is yet to come :)

How Has This Been Tested

I trust my code ✔️ (and hope the CIs turn green)

[erikwijmans]

Printing file+line with Debug

Oooo, maybe time for swap away from GLOG?

[mosra]

maybe time for swap away from GLOG?

Depends what else from #121 needs to get implemented :)

... the CI is failing on the JS tests and I have no idea what could cause it -- IIRC there wasn't any significant change affecting glTF import that would cause this to break. Is anybody who has the web build set up be able to reproduce this? Or is there a way to get the diff image out of the CI?

[mathfac]

@mosra, looks like something in rendering changes as visual comparison with previously saved frames changed: https://app.circleci.com/jobs/github/facebookresearch/habitat-sim/6779

[mosra]

@mathfac yeah I know, which is why I asked if somebody could retrieve the diff image for me ;)

Managed to reproduce the difference using the viewer utility, investigating locally.

[mosra]

Okay, it was due to an unfortunate breaking change in Magnum's math lib that I reverted after rethinking the consequences (plus hardened the tests in mosra/magnum@c85b537 and mosra/magnum@4a0f843 to make sure these important properties don't get broken again), but the CI still reports a 11% difference (instead of 90%) and the threshold is 10%.

Is anybody able to reproduce this and get the diff image to verify that everything is okay? Or should we just bump the threshold a bit more? It's getting late here and I don't want to block you by this for yet another day -- @matthewjmay @aclegg3 @erikwijmans if you can check or think it's fine to bump the threshold to 12% or 15%, feel free to do this on this branch :)

[erikwijmans]

@apsdehal I believe you wrote this test in JS? Do you mind verifying that JS is working as expected on this branch?

[erikwijmans]

@mosra I checked the other sensor output tests we have and for the scenes we have on the CI, they don't pass. Everything looks fine and I would be totally happy with changing the value here https://github.com/facebookresearch/habitat-sim/blob/magnum-master/tests/test_sensors.py#L126 to 5.0e-2 (which seems like that's what it would take to make it pass). Interestingly, the Matterport3D tests still pass with the current value.

[mosra]

Wait, so this is an additional failure apart from the JS fail? Interesting.

Should I bump those numbers or are we waiting for a 3rd person to confirm?

[mathfac]

@mosra bump those numbers to unblock us here. Or Consider update test frames with new values.
cc @apsdehal

[erikwijmans]

Bump the values IMO, things look completely as expected.

[codecov]

Codecov Report

Merging #496 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #496      +/-   ##
==========================================
- Coverage   59.28%   59.26%   -0.03%     
==========================================
  Files         165      165              
  Lines        7386     7381       -5     
  Branches       84       84              
==========================================
- Hits         4379     4374       -5     
  Misses       3007     3007

Flag	Coverage Δ
#CPP	54.66% <100%> (-0.06%)	⬇️
#JavaScript	10% <ø> (ø)	⬆️
#Python	79.49% <ø> (ø)	⬆️

Impacted Files	Coverage Δ
src/esp/geo/geo.cpp	34.28% <ø> (ø)	⬆️
tests/test_sensors.py	100% <ø> (ø)	⬆️
src/tests/SimTest.cpp	100% <100%> (ø)	⬆️

---

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 9a1fcb2...ac63719. Read the comment docs.

[mosra]

Updated the thresholds to make tests pass.

The slight difference in rendering was bothering me, and I managed to track it down to a commit updating the bundled tiny_gltf (which was needed for the updated light extension) -- mosra/magnum-plugins@8fcbf5d. There's a lot happening so I didn't bother digging further in (and I don't think that's necessary), but all clues point to something happening with how numbers are parsed, leading to transformations being slightly different and thus particular parts of the model positioned slightly differently. That's almost unnoticeable except for aliased polygon edges that get jagged differently, as can be seen in the diff (using https://online-image-comparison.com, thanks @erikwijmans for providing me the before/after images):

To be sure, I audited all other commits happening since the v2019.10 tag and this commit was the single one responsible for the difference (in particular, checking out the magnum-plugins submodule to a parent of this commit makes the tests pass even without updated thresholds). For performance reasons I'll be switching to cgltf for glTF parsing at some point in the future, so I expect a similar minor difference happening again, but with the increased thresholds I don't think it will cause more CI failures.

[mathfac]

Thank you for investigating the difference. Looks good to go for me. We should update the test pictures to compete to new etalon.

[mosra]

The SimTest was failing due to similar reasons and because it tested against a value of a single pixel, which is not nearly enough, I ended up rewriting it using Corrade's TestSuite and Magnum's image comparison features. For comparison, this is what the test prints now in case of failure (the thresholds were increased to 15 and 0.17 since to make the test pass). Note the --save-diagnostic hint at the bottom, which was also used to get the ground truth PNG:

Now everything should be finally passing again :)