kzampog / cilantro

A lean C++ library for working with point cloud data
MIT License
1.02k stars 207 forks source link

Need some favors about adding the ARAP term to the non-rigid energy function #17

Closed etudemin closed 5 years ago

etudemin commented 5 years ago

Hi,

Here is my fork based on your commit db7e511.

We modified the regularization term in the file warp_field_estimation.hpp in order to realize As Rigid As Possible(ARAP) Term described in the paper ” As-Rigid-As-Possible Surface Modeling”, which is also introduced in the well-known paper “ VolumeDeform : Real-time Volumetric Non-Rigid Reconstruction”.

We didn’t get comparable result against the original regularization term at first when the ARAP term was implemented (commit aa77969). 2019-01-13 00-54-25

So, we tried to comment out the offset swapping step (commit 53222e7). Better results are showed below, but some points still drifted backward. 2019-01-13 00-59-09 2019-01-13 01-01-45

After made the following two separate changes, the result turned out to be quite good:

  1. Change the sign of the partial derivative of the rotation of the neighboring control nodes from positive to negative (commit b84e303). 2019-01-13 01-10-22 2019-01-13 01-11-27 Though the partial derivative of the rotation is supposed to be positive, yet better result is achieved by turning the sign from positive to negative.

  2. In non_rigid_icp.cpp, increase the regularization weight from 200 to 2000 (commit fabe65d). 2019-01-13 01-14-48 2019-01-13 01-16-10

We are not pretty sure if our implementation of ARAP term is correct, please give us some advice. Thank you for your help and look forward to your kind reply.

kzampog commented 5 years ago

Hi,

The ARAP regularization implementation looks correct!

I have slightly modified warp_field_estimation.hpp from your fork to ensure that sparse matrix inner indices are sorted (offset swapping in the original version). Eigen's insert makes sure of that, but since we populate the buffers manually (for significant headaches and marginal speed gains), we do this to possibly avoid things breaking.

I also adapted the signs for some Jacobian and (negative) residual terms. As you noted, swapping the signs indeed seems to work better (qualitatively), but, unless I am missing something, is not consistent with the formulation. I have kept the math-consistent version in the attached.

One thing missing was updating the control points in every ICP iteration. I added it to icp_warp_field_combined_metric_sparse.hpp.

With this setup, I was able to get qualitatively good results by slightly increasing the regularization neighborhood sizes (e.g. to 12) and the weight to 500 (non_rigid_icp.cpp example). All my modifications are here:

cilantro_arap.zip

I was also considering adopting this regularization term before going for the transformation parameter-wise one. I remember I had (qualitatively) found the latter to be a bit more effective but did not test a lot. Please let me know if you find any interesting differences in their behavior.

Cheers!

etudemin commented 5 years ago

Hi,

Thank you very much!

The results are good after adopting your modifications (2273119).

We have tested some datasets, and the original transformation parameter-wise one is indeed more effective than the ARAP one.

We are still running some tests, and will inform you if there are some interesting findings.

Thank you for your kind assistance again.

kzampog commented 5 years ago

Awesome -- no problem! Please let me know of any interesting findings or suggestions.