Implicit function clipping #16

lachlangrose · 2025-11-04T01:03:01Z

Add option to clip surface with implicit function.
Also change predicate for clipping to exact kernel to avoid infinite hangs

this is interpreted as a signed distance function and could be evaluated on the triangle nodes from an implicit function. Note if the surface is remeshed the property values will need to be re-evaluated on the new nodes.

…p3D/loop-cgal into fix/adding-implicit-function-cutting

Set VCPKG_ROOT environment variable for package build.

…ging ci: change name of tester to testing

Copilot

Pull Request Overview

This PR adds several significant features to the loop_cgal library:

Implements cut_with_implicit_function to cut meshes based on vertex properties
Adds exact kernel support for improved numerical robustness in mesh operations
Introduces comprehensive test coverage for mesh operations
Refactors validation logic into separate utils module

Reviewed Changes

Copilot reviewed 14 out of 17 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
src/mesh.cpp	Adds cut_with_implicit_function implementation and exact kernel support for cutWithSurface
src/mesh.h	Declares new methods and exact kernel types
src/meshutils.cpp	Implements conversion functions between exact and double precision meshes
src/meshutils.h	Declares mesh conversion functions
src/meshenums.h	Defines ImplicitCutMode enum for cut operations
loop_cgal/init.py	Refactors validation logic and adds new helper methods
loop_cgal/utils.py	Extracts validation functions into separate module
loop_cgal/bindings.cpp	Exposes new C++ functionality to Python
tests/test_mesh_operations.py	Adds comprehensive test suite for mesh operations
tests/test_degenerate.py	Refactors degenerate mesh tests into proper pytest format
pyproject.toml	Adds vtk dependency and test optional dependencies
CMakeLists.txt	Updates build configuration for cross-platform support
.github/workflows/ci.yml	Adds CI workflow for automated testing
examples/*.py	Adds example scripts demonstrating new features

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-04T01:05:46Z

src/meshutils.h

                               const std::array<double, 3> &v3);
+Exact_Mesh convert_to_exact(const TriMesh& input);
+TriangleMesh convert_to_double_mesh(const Exact_Mesh& input);
+


Trailing whitespace on line 13 should be removed to maintain consistent code formatting.

Suggested change

Copilot · 2025-11-04T01:05:46Z

src/mesh.cpp

+                             bool preserve_intersection_clipper,
+                            bool use_exact_kernel)
 {
+


Trailing whitespace on line 297 should be removed to maintain consistent code formatting.

Suggested change

Copilot · 2025-11-04T01:05:46Z

loop_cgal/__init__.py

-            degen_idx = np.where(m.sum(axis=0) != 3)[1]
-            raise ValueError(f"Surface contains degenerate triangles: {degen_idx} (each triangle must have exactly 3 vertices)")
-
+        if (not validate_vertices_and_faces(verts, faces)):


[nitpick] Unnecessary parentheses around the boolean expression. Python style guides recommend removing them unless needed for line continuation or grouping.

Suggested change

if (not validate_vertices_and_faces(verts, faces)):

if not validate_vertices_and_faces(verts, faces):

Copilot · 2025-11-04T01:05:46Z

loop_cgal/__init__.py

+            The created TriMesh object.
+        """
+        # Create a temporary PyVista PolyData object for validation
+        if (not validate_vertices_and_faces(vertices, triangles)):


[nitpick] Unnecessary parentheses around the boolean expression. Python style guides recommend removing them unless needed for line continuation or grouping.

Suggested change

if (not validate_vertices_and_faces(vertices, triangles)):

if not validate_vertices_and_faces(vertices, triangles):

Copilot · 2025-11-04T01:05:47Z

pyproject.toml

+optional-dependencies = { test = ['pytest'] }



The TOML syntax for optional-dependencies should use nested table format for better compatibility with TOML parsers. Should be: [project.optional-dependencies]\ntest = ['pytest']

Suggested change

optional-dependencies = { test = ['pytest'] }

[project.optional-dependencies]

test = ['pytest']

Copilot · 2025-11-04T01:05:48Z

loop_cgal/utils.py

+def validate_vertices_and_faces(verts, faces) -> bool:
+    """Validate vertices and faces arrays.
+    Parameters
+    ----------
+    verts : np.ndarray
+
+        An array of shape (n_vertices, 3) containing the vertex coordinates.
+    faces : np.ndarray
+
+        An array of shape (n_faces, 3) containing the triangle vertex indices.
+    Returns
+    -------
+    bool
+        True if valid, False otherwise.


The function's return type annotation claims it returns bool, but the function actually raises ValueError on validation failures and only returns True on success. The docstring correctly documents that it raises ValueError, so the return type should be either removed or the function should be refactored to return bool consistently.

Suggested change

def validate_vertices_and_faces(verts, faces) -> bool:

"""Validate vertices and faces arrays.

Parameters

----------

verts : np.ndarray

An array of shape (n_vertices, 3) containing the vertex coordinates.

faces : np.ndarray

An array of shape (n_faces, 3) containing the triangle vertex indices.

Returns

-------

bool

True if valid, False otherwise.

def validate_vertices_and_faces(verts, faces):

"""Validate vertices and faces arrays.

Parameters

----------

verts : np.ndarray

An array of shape (n_vertices, 3) containing the vertex coordinates.

faces : np.ndarray

An array of shape (n_faces, 3) containing the triangle vertex indices.

Copilot · 2025-11-04T01:05:48Z

tests/test_mesh_operations.py

+
+
+


Trailing whitespace on line 13 should be removed to maintain consistent code formatting.

Suggested change

Copilot · 2025-11-04T01:05:48Z

.github/workflows/ci.yml

+      - if: matrix.os == 'ubuntu-latest'
+        run: |
+          sudo apt-get install -y libgl1-mesa-dev libeigen3-dev libcgal-dev
+      - if: matrix.os == 'macos-latest' || matrix.os == 'macos-14'     


GitHub Actions conditional expressions should use proper YAML syntax. The condition should be: if: matrix.os == 'macos-latest' || matrix.os == 'macos-14' (wrapped in quotes if needed), or better yet, use: if: startsWith(matrix.os, 'macos')

Suggested change

- if: matrix.os == 'macos-latest' || matrix.os == 'macos-14'

- if: ${{ startsWith(matrix.os, 'macos') }}

Copilot · 2025-11-04T01:05:49Z

examples/cut_example.py

@@ -0,0 +1,36 @@
+import numpy as np
+import pyvista as pv


Import of 'pv' is not used.

Suggested change

import pyvista as pv

examples/cut_example.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

lachlangrose · 2025-11-04T01:56:33Z

@copilot open a new pull request to apply changes based on the comments in this thread

Copilot · 2025-11-04T01:56:40Z

@lachlangrose I've opened a new pull request, #17, to work on those changes. Once the pull request is ready, I'll request review from you.

* Initial plan * Address code review comments Co-authored-by: lachlangrose <7371904+lachlangrose@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lachlangrose <7371904+lachlangrose@users.noreply.github.com>

lachlangrose added 18 commits October 23, 2025 11:01

fix: add copy method and vtk method for trimesh object

50b5710

chore: ignore windows build

7f26b75

fix: specify .so for unix and pyd for windows

9eb2876

ci: rename examples and adding test suite + git action

977ee4f

fix: adding option to cut surface with a property

5368ede

this is interpreted as a signed distance function and could be evaluated on the triangle nodes from an implicit function. Note if the surface is remeshed the property values will need to be re-evaluated on the new nodes.

fix: move checks to separate file and add import/export from np arrays

e3ab251

ci: use loop python versions (#15)

404814e

ci: adding windows/mac tests

dd8b3e0

Merge branch 'fix/adding-implicit-function-cutting' of github.com:Loo…

3ee9aec

…p3D/loop-cgal into fix/adding-implicit-function-cutting

ci: use c:: for vcpkg and revert to loop python env var

825dcfb

ci: use env var for mac as well

486cb2e

ci: update windows cmake instructions

1482f1d

Update environment variable for vcpkg toolchain

9a0b0e9

Set VCPKG_ROOT environment variable for package build.

test: update test ci runner to use pypi runner template

e28f2a0

tests: add cgal/eigen to nix

c14f29d

fix: specify python versions

cadf8c8

fix: use exact meshes for clipping, avoid infinite hangs

6fd7900

fix: set default kernel to exact, use double for speed at risk of han…

d9ec82e

…ging ci: change name of tester to testing

lachlangrose requested a review from Copilot November 4, 2025 01:03

Copilot AI reviewed Nov 4, 2025

View reviewed changes

Update examples/cut_example.py

d346db3

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI mentioned this pull request Nov 4, 2025

Address code review feedback from PR #16 #17

Merged

Copilot AI and others added 3 commits November 4, 2025 13:00

fix: add dlls to package for wheel

7eea22e

tests: remove tmp files

234a5b6

lachlangrose merged commit 6cec095 into main Nov 4, 2025
32 checks passed

lachlangrose deleted the fix/adding-implicit-function-cutting branch November 4, 2025 02:41

	if (not validate_vertices_and_faces(verts, faces)):
	if not validate_vertices_and_faces(verts, faces):

	if (not validate_vertices_and_faces(vertices, triangles)):
	if not validate_vertices_and_faces(vertices, triangles):

	- if: matrix.os == 'macos-latest' \|\| matrix.os == 'macos-14'
	- if: ${{ startsWith(matrix.os, 'macos') }}

Implicit function clipping #16

Implicit function clipping #16

Uh oh!

Conversation

lachlangrose commented Nov 4, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lachlangrose commented Nov 4, 2025

Uh oh!

Copilot AI commented Nov 4, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants