pyvista / fast-simplification

Fast Quadratic Mesh Simplification
https://pyvista.github.io/fast-simplification/
MIT License
111 stars 15 forks source link

Add a replay_decimation functionnality #8

Closed Louis-Pujol closed 1 year ago

Louis-Pujol commented 1 year ago

Hello,

Following my last week contribution, I implemented a function to replay a decimation process thanks to the history of collapses.

This functionnality is presented as a function replay_simplification, here is a basic example of usage :

replay_points, replay_triangles, indices_mapping = replay_simplification(points, triangles, collapses)

replay_points and replay_triangles will be the same as the output of simplify, and indice_mapping a '(n_points, )' array of integer encoding the correspondence of vertices from the original mesh to the decimated one.

I see three applications of this function:

Applications 1 and 2 are illustrated in examples/replay.py, 2 points are randomly picked on the mesh and thanks to indice_mapping, we are able, after replaying, to find associations on decimated meshes ReplayDecimation

Regarding the code, I added the following files to the fast_simplification folder : Replay.h, wrapper_replay.h, _replay.pyx and replay.py, following the spirit of the code for simplify. This was a convenient solution for me because the compilation of a second cython module, independent from simplify, prevent from breaking previous code.

I've also added a test file : test_replay.py where I test that replaying decimation leads to the same result as applying simplify, both on toy examples and on Sphere and louis_louvre meshes from pyvista examples gallery.

Let me now if you think it could be added to fast-simplification. Thanks

akaszynski commented 1 year ago

Looks like this test is failing:

@skip_no_vtk
      def test_collapses_sphere(mesh):
          points = mesh.points
          faces = mesh.faces.reshape(-1, 4)[:, 1:]
          reduction = 0.5

          points_out, faces_out, collapses = fast_simplification.simplify(
              points, faces, reduction, return_collapses=True
          )
          n_points_before_simplification = len(points)
          n_points_after_simplification = len(points_out)
          n_collapses = len(collapses)

          (
              replay_points,
              replay_faces,
              indice_mapping,
          ) = fast_simplification.replay_simplification(points, faces, collapses)
  >       assert np.allclose(points_out, replay_points)
  E       assert False
  E        +  where False = <function allclose at 0x7f17f33b2050>(array([[-5.5511151e-17,  0.0000000e+00, -5.0000000e-01],\n       [-4.0411569e-02,  3.8855094e-02, -4.9757284e-01],\n    ....5616201e-01, -3.3193260e-02,  4.7[382](https://github.com/pyvista/fast-simplification/actions/runs/6175131288/job/16761365610?pr=8#step:3:391)659e-01],\n       [-5.1321186e-02, -1.7672617e-02,  4.9707606e-01]], dtype=float32), array([[-5.5511151e-17,  0.0000000e+00, -5.0000000e-01],\n       [-3.5702158e-02,  3.2146368e-02, -4.9706897e-01],\n    ....5616201e-01, -3.3193260e-02,  4.7382659e-01],\n       [-5.1132001e-02, -1.6613794e-02,  4.9706897e-01]], dtype=float32))
  E        +    where <function allclose at 0x7f17f33b2050> = np.allclose
Louis-Pujol commented 1 year ago

I've adjusted the tolerance for this test. Seems to be ok now

akaszynski commented 1 year ago

Need another patch release?

Louis-Pujol commented 1 year ago

Yes maybe to add documentation. I've updated my fork of the repository, added a paragraph at the end of readme.rst to present replay_simplification and made modifications to the doc folder. Do I need to create a new PR or it is possible to reopen this one ?

akaszynski commented 1 year ago

Do I need to create a new PR or it is possible to reopen this one ?

Please create another PR. Sorry, should have asked if you were ready to merge.

Louis-Pujol commented 1 year ago

No worry, I do it right now