Skip to content

Conversation

@petrasvestartas
Copy link
Collaborator

@petrasvestartas petrasvestartas commented Mar 31, 2025

This is pull request to change pybind11 build system of C++ to Python binding to nanobind.

Following methods are wrapped:

  • Boundary loops
    Screenshot from 2025-03-31 16-48-14

  • Curvature (gaussian - normals, principal, in-plane vectors)
    image

  • Geodesic Distance
    image

  • Ray/Mesh Intersections
    image

  • Isolines
    image

  • Mesh Mass Matrix
    image

  • Mesh Parametrisations
    image

  • Quad Mesh Planarisation
    image

  • Remeshing from isolines
    image
    image
    image

@tomvanmele
Copy link
Member

the isolines are wrong...

@petrasvestartas
Copy link
Collaborator Author

the isolines are wrong...

Is it related to building process or implementation?

Isolines method has been slightly changed in the recent libigl source code. Now it takes isovalues to retrieve output.
Also a third output is added for grouping. This helps to avoid x, y, z coordinate sorting.

@tomvanmele
Copy link
Member

they are based on the OBJ coordinate direction instead of the orientation of the model

@petrasvestartas
Copy link
Collaborator Author

they are based on the OBJ coordinate direction instead of the orientation of the model

Is it about orientation:

image

@petrasvestartas
Copy link
Collaborator Author

petrasvestartas commented Apr 2, 2025

Building libigl on all os works fine 🎉🥳🙌

Windows problem was this line in compas.h:
#define NOMINMAX

ToDo's before the final merge to align with compas_cgal workflow:

  • documentation examples: images and renaming: rename examples to example_name.py
  • add api for remeshing on isolines
  • add multiple iso remeshing split example from C++
  • check if cibuildwheels work correctly locally
  • bring test files from the methods docstrings to tests folder

@petrasvestartas
Copy link
Collaborator Author

petrasvestartas commented Apr 3, 2025

@tomvanmele I would like to ask for a review.
The package build system is transformed to nanobind structure as in compas_cgal - one target per sub-module.

Multiple files were changed, the simplest way, would be to check the branch: https://github.com/petrasvestartas/compas_libigl/tree/nb

We see in this pull-request the whole history of commit. Should I leave it as is or should I create a new pull-request with the changes only?

Dependencies

C++ dependencies: eigen and libigl are handled via cmake.

Note: Third party dependencies like CGAL, embree, tetgen, Triangle, OpenGL pipeline libraries, are skipped since they are either used in other compas bindings and are only highly specific use cases of libigl that we dont need. None of the binded methods use that, as a result maintaining this package will be much easier.

Github Actions

Github actions are changed to the actions used in compas_cgal.

Tests

Minimal tests were added to the test folder for each binded method. Tests are helpful in github actions.

Docs

Documentation with tested examples:

Aufzeichnung.2025-04-04.014330.mp4

PYPI + Pending Publisher

Locally I tested the PYPI building process on windows and docker for linux:

Screenshot 2025-04-04 012854

For release to pypi we need the pending publisher:

image

@tomvanmele
Copy link
Member

We see in this pull-request the whole history of commit. Should I leave it as is or should I create a new pull-request with the changes only?

perhaps a bit annoying, but i think it would be better...

@tomvanmele
Copy link
Member

For release to pypi we need the pending publisher:

done

********************************************************************************

Coming soon!
COMPAS libigl
Copy link
Member

Choose a reason for hiding this comment

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

perhaps this subtitle is not needed?

Comment on lines +32 to +46
# Create two-color maps for negative and positive curvature
cmap_negative = ColorMap.from_two_colors(Color.blue(), Color.yellow())
cmap_positive = ColorMap.from_two_colors(Color.yellow(), Color.magenta())

# Create vertex colors dictionary
vertex_colors = {}
for i, k in enumerate(gaussian_curvature):
if trimesh.is_vertex_on_boundary(i):
# Set boundary vertices to black
vertex_colors[i] = Color.black()
else:
if k < 0:
vertex_colors[i] = cmap_negative(k, minval=min_gaussian, maxval=0)
else:
vertex_colors[i] = cmap_positive(k, minval=0, maxval=max_gaussian)
Copy link
Member

Choose a reason for hiding this comment

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

not now, but we need to make things like this simpler

Comment on lines +58 to +66
normal_scale = -10
for vertex in trimesh.vertices():
if not trimesh.is_vertex_on_boundary(vertex):
point = Point(*trimesh.vertex_coordinates(vertex))
normal = trimesh.vertex_normal(vertex)
k = gaussian_curvature[vertex]

scaled_normal = [n * normal_scale * k for n in normal]
end_point = Point(*(p + n for p, n in zip(point, scaled_normal)))
Copy link
Member

Choose a reason for hiding this comment

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

@Licini perhaps we can add a few special scene objects for making recurring visualisation patterns easier to execute

  • mesh vertex normals
  • mesh face normals
  • mesh/graph paths
  • ...

@@ -1,10 +1,10 @@
********************************************************************************
Ray/Mesh Intersections
Mesh-ray intersections
Copy link
Member

Choose a reason for hiding this comment

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

it is the ray that intersects the mesh

from compas.colors import ColorMap
from compas.geometry import Rotation
from compas.geometry import Scale
import math
Copy link
Member

Choose a reason for hiding this comment

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

i can't believe this passes ruff linting and formatting

Copy link
Member

Choose a reason for hiding this comment

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

please turn on sorting in the project config

Comment on lines +21 to +27
Several examples use the COMPAS Viewer for visualisation.
To install :mod:`compas_viewer` in the same environment

.. code-block:: bash
git clone --recursive https://github.com/compas-dev/compas_libigl.git
conda activate libigl
pip install compas_viewer
Copy link
Member

Choose a reason for hiding this comment

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

see above

Comment on lines +21 to +27
Curvature Analysis
------------------

V, F = mesh.to_vertices_and_faces()
* :func:`compas_libigl.trimesh_gaussian_curvature` - Compute Gaussian curvature at vertices
* :func:`compas_libigl.trimesh_principal_curvature` - Compute principal curvatures and directions

source = trimesh.vertex_sample(size=1)[0]
distance = compas_libigl.trimesh_geodistance(
(V, F),
source,
method="heat",
)
Geodesic Distances
Copy link
Member

Choose a reason for hiding this comment

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

not sure i would categorize this under analysis. for me this highlights why excessive structuring is not really helpful

Parameters
----------
M : tuple[:class:`list`, :class:`list`]
Copy link
Member

Choose a reason for hiding this comment

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

remove :class:

Parameters
----------
M : tuple[:class:`list`, :class:`list`]
Copy link
Member

Choose a reason for hiding this comment

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

should be tuple[list[list[float]], list[list[int]]]

Parameters
----------
M : tuple[list[list[float]], list[list[int]]] | :class:`compas.datastructures.Mesh`
Copy link
Member

Choose a reason for hiding this comment

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

not sure if it is still true that the mesh unpacks into vertices and faces in current compas versions

@petrasvestartas
Copy link
Collaborator Author

@tomvanmele thank you for the corrections, I fixed them and added a new pull-request to avoid commit history.

#21

Also found the right command for ruff:
ruff check --select I --fix '/Users/petras/brg/2_code/forks/compas_libigl/docs/examples'

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.

4 participants