[esp/scene] Added support of object's centers and bounding boxes to Gibson semantic scenes #407

mathfac · 2019-12-28T01:06:27Z

Motivation and Context

To leverage 3dscenegraph semantic annotation spatial information added support of object's bounding boxes to Gibson semantic scenes. Related to issue #374 and depends on PR #406.

How Has This Been Tested

Added the integration test and run it locally with *.scn data:

Running main() from /private/home/maksymets/habitat-sim/src/deps/googletest/googletest/src/gtest_main.cc
[==========] Running 2 tests from 2 test cases.
[----------] Global test environment set-up.
[----------] 1 test from GibsonSceneTest
[ RUN      ] GibsonSceneTest.Basic
[       OK ] GibsonSceneTest.Basic (2 ms)
[----------] 1 test from GibsonSceneTest (2 ms total)

[----------] 1 test from GibsonSemanticSimTest
[ RUN      ] GibsonSemanticSimTest.Basic
[       OK ] GibsonSemanticSimTest.Basic (5451 ms)
[----------] 1 test from GibsonSemanticSimTest (5451 ms total)

[----------] Global test environment tear-down
[==========] 2 tests from 2 test cases ran. (5453 ms total)
[  PASSED  ] 2 tests.

Types of changes

New feature (non-breaking change which adds functionality)

…is missing

…tegration test

src/esp/scene/GibsonSemanticScene.cpp

aclegg3

LGTM. Minor comment on placement of json parsing function.

src/esp/scene/GibsonSemanticScene.cpp

msbaines · 2020-01-07T17:25:45Z

src/esp/gfx/Simulator.cpp

@@ -57,6 +57,10 @@ void Simulator::reconfigure(const SimulatorConfiguration& cfg) {
    houseFilename = cfg.scene.filepaths.at("house");
  }

+  if (!io::exists(houseFilename)) {


You can avoid this by adding a "house" entry to your configuration in cfg.scene.filepaths (see line 56)

Thank you, missed that logic.

…tegration test

msbaines · 2020-01-10T22:41:09Z

The .obj mesh used for main gibson (what 3Dscenegraph is built from) has a rotated coordinate system when compared to the gibson gltf meshes we are using for rgb in Habitat. My utility rotates the coordinate system to match habitat and outputs to .ply semantic mesh. I transformed the bounding box info from the 3Dscenegraph .npz dataset in the same way. My goal was to have a consistent coordinated system. I verified the semantic meshes using the mesh highlighting logic we have in WebGL but did not have time before holidays to verify the bounding boxes were correct.

We have coordinate flipping logic in Asset.cpp that may be relevant here:

AssetInfo AssetInfo::fromPath(const std::string& path) {
  using Corrade::Utility::String::endsWith;
  AssetInfo info{AssetType::UNKNOWN, path};

  if (endsWith(path, "_semantic.ply")) {
    info.type = AssetType::INSTANCE_MESH;
  } else if (endsWith(path, "mesh.ply")) {
    info.type = AssetType::FRL_PTEX_MESH;
    info.frame = {quatf::FromTwoVectors(geo::ESP_GRAVITY, -vec3f::UnitZ())};
  } else if (endsWith(path, "house.json")) {
    info.type = AssetType::SUNCG_SCENE;
  } else if (endsWith(path, ".glb")) {
    // assumes MP3D glb with gravity = -Z
    info.type = AssetType::MP3D_MESH;
    // Create a coordinate for the mesh by rotating the default ESP
    // coordinate frame to -Z gravity
    info.frame = {quatf::FromTwoVectors(geo::ESP_GRAVITY, -vec3f::UnitZ())};
  }

  return info;
}

I have not studied the above code so don't know exactly what it does.

codecov-io · 2020-01-17T10:30:53Z

Codecov Report

Merging #407 into master will increase coverage by 1.37%.
The diff coverage is 95.54%.

@@            Coverage Diff            @@
##           master    #407      +/-   ##
=========================================
+ Coverage   56.02%   57.4%   +1.37%     
=========================================
  Files         171     175       +4     
  Lines        7994    8170     +176     
  Branches       84      84              
=========================================
+ Hits         4479    4690     +211     
+ Misses       3515    3480      -35

Flag	Coverage Δ
#CPP	`53.44% <89.32%> (+0.95%)`	⬆️
#JavaScript	`10% <ø> (ø)`	⬆️
#Python	`78.82% <100%> (+1.4%)`	⬆️

Impacted Files	Coverage Δ
src/esp/assets/GltfMeshData.h	`60% <ø> (ø)`	⬆️
src/esp/assets/CollisionMeshData.h	`100% <ø> (ø)`	⬆️
src/esp/scene/SemanticScene.h	`16.32% <ø> (ø)`	⬆️
src/esp/bindings/GfxBindings.cpp	`93.65% <ø> (-0.2%)`	⬇️
src/esp/assets/ResourceManager.h	`100% <ø> (ø)`	⬆️
src/esp/assets/MeshMetaData.h	`81.81% <ø> (ø)`	⬆️
src/esp/assets/BaseMesh.h	`57.14% <ø> (ø)`	⬆️
src/esp/physics/PhysicsManager.h	`51.85% <ø> (ø)`	⬆️
src/esp/scene/Mp3dSemanticScene.cpp	`0.85% <0%> (ø)`	⬆️
src/esp/scene/test/GibsonSceneTest.cpp	`60.6% <0%> (-1.16%)`	⬇️
... and 25 more

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 c79a854...1ec7f68. Read the comment docs.

msbaines · 2020-01-17T18:19:14Z

src/esp/scene/GibsonSemanticScene.cpp

@@ -19,6 +19,18 @@ namespace scene {

 constexpr int kMaxIds = 10000; /* We shouldn't every need more than this. */

+namespace {
+vec3f jsonToVec3f(const esp::io::JsonGenericValue& jsonObject) {


Should this be removed?

msbaines · 2020-01-17T18:21:39Z

src/esp/scene/GibsonSemanticScene.cpp

-    // TODO(msb) add support for aabb
+
+    const auto& jsonCenter = jsonObject["location"];
+    if (!jsonCenter.IsNull()) {


Code might be simpler if you checked center AND size at the top.

I want to create OBB only if at least center is available, while size can be optional.

src/esp/gfx/Simulator.cpp

msbaines · 2020-01-17T18:23:35Z

src/esp/io/json.h

@@ -7,6 +7,7 @@
 #include <cstdint>
 #define RAPIDJSON_NO_INT64DEFINE
 #include <rapidjson/document.h>
+#include "esp/core/esp.h"


Why is this include needed?

msbaines · 2020-01-17T18:25:05Z

src/esp/gfx/Simulator.cpp

@@ -53,6 +53,9 @@ void Simulator::reconfigure(const SimulatorConfiguration& cfg) {
  }

  std::string houseFilename = io::changeExtension(sceneFilename, ".house");
+  if (!io::exists(houseFilename)) {


Is this still needed? You can avoid this by adding a "house" entry to your configuration in cfg.scene.filepaths

@msbaines by default, we are not filling a "house" entry in cfg.scene.filepaths. To support same behavior for `".house" (line 55) and ".scn" that make sense to leave it. Otherwise, wider refactoring needed and there will be no that seamless experience loading MP3D with semantic scenes.

msbaines

Compile issue and one nit. Otherwise, LGTM.

msbaines · 2020-01-17T21:23:58Z

src/esp/io/json.h

@@ -26,6 +27,9 @@ JsonDocument parseJsonString(const std::string& jsonString);
 //! Return string representation of given JsonDocument
 std::string jsonToString(const JsonDocument& d);

+//! Return Vec3f coordinates representation of given JsonObject of array type
+esp::vec3f jsonToVec3f(const JsonGenericValue& jsonArray);


Looks like you need to include esp/core/esp.h for esp::vec3f

msbaines · 2020-01-17T21:24:55Z

src/esp/gfx/Simulator.cpp

+          scene::SemanticScene::loadGibsonHouse(
+              houseFilename, *semanticScene_,
+              // Default World rotation for Gibson semantic data
+              quatf::FromTwoVectors(geo::ESP_GRAVITY, -vec3f::UnitZ()));


mp3d does this as a default argument in the header file. Not sure which is better but we should try to be consistent

@msbaines, that's great suggestion, thank you!

…arch#407) Fixed path to data directory for DDPPO objectnav config

…ibson semantic scenes (facebookresearch#407) To leverage 3dscenegraph semantic annotation spatial information added support of object's bounding boxes to Gibson semantic scenes. Related to issue facebookresearch#374 and depends on PR facebookresearch#406.

mathfac requested review from bigbike, aclegg3, erikwijmans and msbaines December 28, 2019 01:06

facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Dec 28, 2019

[esp/scene] Added loading of Gibson Semantics scene if MP3D semantic …

34ccff4

…is missing

mathfac changed the title ~~[esp/scene] Added support of object's bounding boxes to Gibson semantic scenes~~ [esp/scene] Added support of object's centers and bounding boxes to Gibson semantic scenes Dec 28, 2019

Added support of object's bounding boxes to Gibson semantic scenes in…

13b58dc

…tegration test

mathfac force-pushed the obb_gibson_support branch from 871ba10 to 13b58dc Compare December 28, 2019 02:36

mathfac self-assigned this Dec 28, 2019

erikwijmans reviewed Jan 2, 2020

View reviewed changes

src/esp/scene/GibsonSemanticScene.cpp Show resolved Hide resolved

aclegg3 reviewed Jan 2, 2020

View reviewed changes

src/esp/scene/GibsonSemanticScene.cpp Outdated Show resolved Hide resolved

msbaines reviewed Jan 6, 2020

View reviewed changes

src/esp/scene/GibsonSemanticScene.cpp Show resolved Hide resolved

msbaines reviewed Jan 7, 2020

View reviewed changes

Added support of object's bounding boxes to Gibson semantic scenes in…

c011165

…tegration test

Moved json_vec function to io. Added world rotation.

c79a854

Merge branch 'master' into obb_gibson_support

227762a

msbaines reviewed Jan 17, 2020

View reviewed changes

Removed unused code, added comments about world rotation

9c1ba9f

msbaines approved these changes Jan 17, 2020

View reviewed changes

mathfac added 2 commits January 17, 2020 16:21

Added transformation as default value for reading Gibson scene.

ff94137

Fixed naming between headers and cpp and fixed test.

1ec7f68

mathfac merged commit a008cde into master Jan 18, 2020

mathfac deleted the obb_gibson_support branch January 18, 2020 23:10

eundersander pushed a commit to eundersander/habitat-sim that referenced this pull request Aug 6, 2020

Fixed path to data directory for DDPPO objectnav config (facebookrese…

d01a0e5

…arch#407) Fixed path to data directory for DDPPO objectnav config

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[esp/scene] Added support of object's centers and bounding boxes to Gibson semantic scenes #407

[esp/scene] Added support of object's centers and bounding boxes to Gibson semantic scenes #407

mathfac commented Dec 28, 2019 •

edited

Loading

aclegg3 left a comment

msbaines Jan 7, 2020

mathfac Jan 15, 2020

msbaines commented Jan 10, 2020

codecov-io commented Jan 17, 2020 •

edited

Loading

msbaines Jan 17, 2020

msbaines Jan 17, 2020

mathfac Jan 17, 2020

msbaines Jan 17, 2020

msbaines Jan 17, 2020

mathfac Jan 17, 2020

msbaines left a comment

msbaines Jan 17, 2020

msbaines Jan 17, 2020

mathfac Jan 18, 2020

[esp/scene] Added support of object's centers and bounding boxes to Gibson semantic scenes #407

[esp/scene] Added support of object's centers and bounding boxes to Gibson semantic scenes #407

Conversation

mathfac commented Dec 28, 2019 • edited Loading

Motivation and Context

How Has This Been Tested

Types of changes

aclegg3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msbaines commented Jan 10, 2020

codecov-io commented Jan 17, 2020 • edited Loading

Codecov Report

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

msbaines 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

mathfac commented Dec 28, 2019 •

edited

Loading

codecov-io commented Jan 17, 2020 •

edited

Loading