Skip to content

Replica OBB - #503

Merged
erikwijmans merged 7 commits into
masterfrom
replica-obb
Mar 2, 2020
Merged

Replica OBB#503
erikwijmans merged 7 commits into
masterfrom
replica-obb

Conversation

@erikwijmans

Copy link
Copy Markdown
Contributor

Motivation and Context

Add Replica OBB parsing.

How Has This Been Tested

Via the test

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Feb 22, 2020
Comment thread src/esp/scene/ReplicaSemanticScene.cpp Outdated
#include "ReplicaSemanticScene.h"
#include "SemanticScene.h"

#include <Sophus/se3.hpp>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fatal error: Sophus/se3.hpp: No such file or directory

@mathfac mathfac left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thank you fro adding this.

Can we add e2e test as we have the test for Gibson dataset?
I noticed that even functions for reading metadata are merged, there can be other code preventing from loading data correctly. For example, logic how we choose annotation data files.

@codecov

codecov Bot commented Feb 24, 2020

Copy link
Copy Markdown

Codecov Report

Merging #503 into master will decrease coverage by 0.35%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #503      +/-   ##
==========================================
- Coverage   59.81%   59.45%   -0.36%     
==========================================
  Files         158      160       +2     
  Lines        7104     7167      +63     
  Branches       84       84              
==========================================
+ Hits         4249     4261      +12     
- Misses       2855     2906      +51     
Flag Coverage Δ
#CPP 54.63% <21.42%> (-0.56%) ⬇️
#JavaScript 10.00% <ø> (ø) ⬆️
#Python 79.59% <ø> (ø) ⬆️
Impacted Files Coverage Δ
src/esp/io/json.h 70.00% <0.00%> (-30.00%) ⬇️
src/esp/assets/BaseMesh.h 0.00% <0.00%> (ø) ⬆️
src/esp/assets/GenericInstanceMeshData.h 0.00% <0.00%> (ø) ⬆️
src/esp/assets/CollisionMeshData.h 0.00% <0.00%> (ø)
src/esp/scene/test/ReplicaSceneTest.cpp 27.27% <0.00%> (ø)

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 eb0d196...456a2d1. Read the comment docs.

return cpu_ibo_;
}

const std::vector<uint16_t> getObjectIdsBufferObjectCPU() const {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you mean to return a ref here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I did, thank you!

quadCenter += vbo[ibo[fid + i]] / 6;
}

CORRADE_VERIFY(obj->obb().contains(quadCenter, 5e-2));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With #505 merged, you can now use CORRADE_ITERATION() to have an actually usable diagnostic when something fails. For example:

  for (const auto& obj : scene.objects()) {
    if (obj == nullptr)
      continue;

    const auto& stringId = obj->id();
    const int id = std::stoi(stringId.substr(stringId.find_last_of("_") + 1));
    CORRADE_ITERATION(stringId);

    for (uint64_t fid = 0; fid < ibo.size(); fid += 6) {
      CORRADE_ITERATION(fid);
      …

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooo yay. I wanting to use that but wasn't in our repo yet.

@erikwijmans
erikwijmans merged commit f261f07 into master Mar 2, 2020
@erikwijmans
erikwijmans deleted the replica-obb branch March 2, 2020 16:15
Ram81 pushed a commit to Ram81/habitat-web-sim that referenced this pull request Dec 10, 2020
* Add replica OBB loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Do not delete this pull request or issue due to inactivity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants