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 31 commits into
base: master
Choose a base branch
from
Open

PMP: Non-rigid mesh registration #8329

wants to merge 31 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,....) ?

@github-actions GitHub Actions
Copy link

The documentation is built. It will be available, after a few minutes, here: https://cgal.github.io/8329/v0/Manual/index.html

@soesau soesau marked this pull request as ready for review September 23, 2024 06:47
@afabri
Copy link
Member

afabri commented Sep 27, 2024 •
edited
Loading

Warnings in Ic-334

Also a compilaton error here

@afabri
Copy link
Member

afabri commented Oct 2, 2024

Warning in I-c-337

@soesau
added apply_non_rigid_transformation to CHANGES.md
b0e5301 
removing warnings (unused variables and signed/unsigned comparison)
switching to ARAP formulation also used in Surface deformation
@soesau
Merge branch 'master' into PMP-Non_rigid_registration-GF
4b14289
@soesau
bug fixes
357d345
@soesau
removed unused variable
ab51f73
@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
<center>
<img src="non-rigid_registration.png" style="max-width:80%;"/>
</center>
\cgalFigureCaptionBegin{registration_results}
Copy link
Member

Choose a reason for hiding this comment

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

it would help understanding to remind that rigidity = w3

...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?

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

Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/non_rigid_mesh_registration.h
/*!
* \ingroup PMP_registration_grp
*
* \brief computes non-rigid transformation of a mesh onto a set of oriented points.
Copy link
Member

Choose a reason for hiding this comment

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

is it clear that it means points + oriented normals?

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've read it before. But in the CGAL repository the only other occurrence is in the poisson reconstruction documentation.

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

@soesau
another pass for review
7a10001 
removed warning
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sloriot sloriot sloriot left review comments

@afabri afabri afabri left review comments

Assignees
No one assigned
Labels
CHANGES.md not updated Ready to be tested Under Testing
Projects
None yet
Milestone
6.1-beta
Development

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