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.

Sign up for GitHub

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

Jump to bottom

PMP: Non-rigid mesh registration #8329

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

PMP: Non-rigid mesh registration #8329

wants to merge 32 commits into from

Conversation

soesau
Copy link
Member

@soesau soesau commented Jul 2, 2024 •
edited by sloriot
Loading

Summary of Changes

WIP

Non-rigid mesh registration based on:
https://vgl.ict.usc.edu/Research/NonRigidRegistration/MODERN%20TECHNIQUES%20AND%20APPLICATIONS%20FOR%20REAL-TIME%20NON-RIGID%20REGISTRATION.pdf
https://www.comp.nus.edu.sg/~lowkl/publications/lowk_point-to-plane_icp_techrep.pdf

TODO:

  • check size (for sloriot)

Release Management

  • Affected package(s): PMP
  • Feature/Small Feature (if any): small feature
  • Link to compiled documentation (obligatory for small feature) user manual ref manual
  • License and copyright ownership: Inria/GeometryFactory?

@soesau soesau mentioned this pull request Jul 2, 2024
Closed
@soesau
Copy link
Member Author

soesau commented Aug 5, 2024

/build:v0

@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 5, 2024

There was an error while building the doc:

https://github.com/CGAL/cgal/actions/runs/10250523837

afabri
afabri reviewed Aug 7, 2024
View reviewed changes
Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/non_rigid_mesh_registration.h Outdated
* @param np2 an optional sequence of \ref bgl_namedparameters "Named Parameters" providing a point_map and normal_map for the `PointRange`
* @param correspondences a vector given matching points between the `source` and the `target`
*
* \cgalNamedParamsBegin
Copy link
Member

Choose a reason for hiding this comment

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

All these numbers are opaque. What is a reasonable range? Why the defaults? Do they have any dependence on dimensions of the input (average edge length, bbox,....) ?

@sloriot sloriot added this to the 6.1-beta milestone Oct 10, 2024
@afabri
Copy link
Member

afabri commented Oct 11, 2024

Timouts in I-c-345

sloriot
sloriot reviewed Oct 14, 2024
View reviewed changes
Installation/CHANGES.md Outdated Show resolved Hide resolved
sloriot
sloriot reviewed Oct 14, 2024
View reviewed changes
Polygon_mesh_processing/doc/Polygon_mesh_processing/Polygon_mesh_processing.txt Outdated
- \f$\mathbf{M}\f$ is the intermediate mesh (initially equal to the source mesh).
- \f$N(\mathbf{v}_i)\f$ denotes the set of vertices adjacent to \f$\mathbf{v}_i\f$.

The \f$\mathbf{E_{match}}\f$ energy penalizes the distance between point pairs \f$\mathbf{v}_i\f$ and \f$\mathbf{\tilde{v}}_i\f$. The point pairs are restablished after each iteration. In addition, the method can take fixed point pairs of correspondences between source and target. This greatly improves the quality of the registration and is generally required for non-simple meshes.
Copy link
Member

Choose a reason for hiding this comment

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

non-simple is too vague: what does matter? the number of vertices, the curvature of the mesh, sharp creases, ...?

Polygon_mesh_processing/doc/Polygon_mesh_processing/Polygon_mesh_processing.txt Show resolved Hide resolved
...gon_mesh_processing/examples/Polygon_mesh_processing/non_rigid_mesh_registration_example.cpp Outdated Show resolved Hide resolved
Polygon_mesh_processing/doc/Polygon_mesh_processing/PackageDescription.txt Show resolved Hide resolved
Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/non_rigid_mesh_registration.h
* \cgalParamExtra{The geometric traits class must be compatible with the vertex point type.}
* \cgalParamNEnd
*
* \cgalParamNBegin{vertex_point_map}
Copy link
Member

Choose a reason for hiding this comment

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

don't you also need the vertex normal map for applying the rotation matrix?

Copy link
Member

Choose a reason for hiding this comment

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

ping @soesau

Copy link
Member Author

Choose a reason for hiding this comment

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

I did not manage to handle an optional vertex normal map.

Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/non_rigid_mesh_registration.h
* \cgalNamedParamsEnd
*
* \cgalNamedParamsBegin{Named Parameters 2}
* \cgalParamNBegin{vertex_normal_map}
Copy link
Member

Choose a reason for hiding this comment

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

check the doc here if you want to init the vpm

Copy link
Member

Choose a reason for hiding this comment

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

ping @soesau

Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/non_rigid_mesh_registration.h Show resolved Hide resolved
Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/non_rigid_mesh_registration.h Outdated Show resolved Hide resolved
Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/non_rigid_mesh_registration.h Outdated Show resolved Hide resolved
Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/non_rigid_mesh_registration.h Outdated
* \cgalParamDefault{`50`}
* \cgalParamNEnd
*
* \cgalParamNBegin{point_to_plane_energy}
Copy link
Member

Choose a reason for hiding this comment

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

providing a link to the section in the user manual where parameters are explained would be helpful. The range of possible values for w1, w2, w3 is also needed.

@sloriot sloriot force-pushed the PMP-Non_rigid_registration-GF branch from 5e48786 to 09ca6d5 Compare October 15, 2024 08:16
@afabri
Copy link
Member

afabri commented Oct 17, 2024

warning in Ic-350

@sloriot
Copy link
Member

sloriot commented Nov 8, 2024

Successfully tested in CGAL-6.1-Ic-14

@sloriot sloriot added Tested Pkg::PMP Not yet approved The feature or pull-request has not yet been approved. and removed Under Testing CHANGES.md not updated Ready to be tested labels Nov 8, 2024
sloriot
sloriot reviewed Nov 8, 2024
View reviewed changes
Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/non_rigid_mesh_registration.h
#include <CGAL/boost/graph/Face_filtered_graph.h>

#include <CGAL/Polygon_mesh_processing/compute_normal.h>
#include <CGAL/Surface_mesh_deformation.h>
Copy link
Member

Choose a reason for hiding this comment

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

is this one really needed? Seems you are only using the closest_rotation_traits, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

The ARAP visitor is defined in that header. However, it only uses a few lines of code and the Cotangent_weight. We could remove the dependency to Surface_mesh_deformation.

Copy link
Member

Choose a reason for hiding this comment

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

That was indeed my idea, moving the thing in Solver_interface or Weight package

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@afabri afabri afabri left review comments

@sloriot sloriot sloriot left review comments

Assignees
No one assigned
Labels
Not yet approved The feature or pull-request has not yet been approved. Pkg::PMP Small feature Tested
Projects
None yet
Milestone
6.1-beta
Development

Successfully merging this pull request may close these issues.
4 participants
@soesau @afabri @sloriot @MaelRL