Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation of a re-meshing based implicit surface reconstruction #36

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

michaelpantic
Copy link
Member

@michaelpantic michaelpantic commented Sep 6, 2019

Needs to wait until #35 is merged. Ready to review.

Contains yet another offset surface method.

Short summary of this method:

  • We ignore the meshing topology of the original mesh, and only use the original mesh to define an (implicit) meshing domain

  • This implicit meshing domain is evaluated by a function which returns the distance of a point to the offset surface, see

  • A meshing process is started to create a mesh that encloses this meshing domain. See lines following from here:

    cgal::MeshDomain domain = cgal::MeshDomain::create_implicit_mesh_domain(

  • Advantages are: It's quite generic and we have some control over how the resulting mesh is discretized (params in the meshing process). Runtime somewhat independent of original mesh resolution.

  • Disadvantages are: Really slow. Not feature preserving (e.g. sharp boundaries). Both are because we ignore the original mesh topology.

For now good as a intermediate test method to test impact of mesh quality on algorithms further down the line.

9sec solution (facet_size 0.03):
Screenshot_2019-09-06_21-00-23

100sec solution (facet_size 0.01):
Screenshot_2019-09-09_13-52-29

100sec solution (facet_size 0.01) for a larger offset distance:
Screenshot_2019-09-09_14-13-56

45sec solution (facet_size 0.02) for a even larger offset distance to test self-intersections:
larger

@ethzasl-jenkins
Copy link

Test PASSed.

@ethzasl-jenkins
Copy link

Test PASSed.

@ethzasl-jenkins
Copy link

Test PASSed.

@ethzasl-jenkins
Copy link

Test PASSed.

@ethzasl-jenkins
Copy link

Test PASSed.

@ethzasl-jenkins
Copy link

Test PASSed.

@michaelpantic michaelpantic marked this pull request as ready for review September 9, 2019 12:07
@ethzasl-jenkins
Copy link

Test PASSed.

@ethzasl-jenkins
Copy link

Test PASSed.

Copy link
Collaborator

@hermannsblum hermannsblum left a comment

Choose a reason for hiding this comment

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

I have some general questions about structure and how we want to use the MeshModel within the library packages, see comments.

@@ -21,6 +21,9 @@ class MeshModel {
static bool create(const std::string &off_pathm, MeshModel::Ptr *meshmodel_ptr,
bool verbose = false);

MeshModel(Polyhedron &p);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why can't we stick to the ::create syntax?

@@ -35,7 +36,7 @@ class IsoSurfaceManifold : public CollisionManifoldInterface {
const double body_radius_;
bool is_constructed_;
const cgal::PolyhedronPtr original_manifold_;
cgal::Polyhedron collision_manifold_;
std::unique_ptr<cgal::MeshModel> collision_manifold_;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does this have to be a unique_ptr? Seems rather exotic.

CGAL::Bounded_side side = m_is_closed ? m_side_of_ptr->operator()(p) : CGAL::ON_UNBOUNDED_SIDE;
if (side == CGAL::ON_BOUNDARY) return m_offset_distance;

cgal::Point closest_point = m_tree_ptr->closest_point(p);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Following the previous design of cad-percept, we tried to use MeshModel as our 'universal' cgal interface. Operations like distance queries are already part of that. Why not use it?

bool success = offset_constructor_->execute(*original_manifold_, body_radius_, &collision_poly);

// non-explicit constructor of MeshModel takes care of this conversion.
*collision_manifold_ = collision_poly;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't it make more sense to just create a new mesh-model and reset the pointer?

@@ -16,12 +20,14 @@ void IsoSurfaceManifold::setBodyAttitude(const Eigen::Quaterniond& attitude) {
}

double IsoSurfaceManifold::signedDistance(const Eigen::Vector3d& position) {
throw cad_percept::NotImplementedException();
cgal::Point position_cgal = cgal::eigenVectorToCgalPoint(position);
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't we now have our nice implicit conversions for this? ;)

<< std::chrono::duration_cast<std::chrono::nanoseconds>(t_per_call).count() << " [ns]"
<< std::endl;

EXPECT_TRUE(true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

can't we at least test some expectation here? like time > 0?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants