Skip to content

Add resolve_duplicated_faces binding; fix boundary_loop/vertex_components docs - #305

Merged
alecjacobson merged 1 commit into
mainfrom
resolve-dup-faces-and-doc-fixes
Aug 10, 2026
Merged

Add resolve_duplicated_faces binding; fix boundary_loop/vertex_components docs#305
alecjacobson merged 1 commit into
mainfrom
resolve-dup-faces-and-doc-fixes

Conversation

@alecjacobson

Copy link
Copy Markdown
Contributor

Knocks out the easy items from the recent issue/PR triage.

New binding

  • igl.resolve_duplicated_faces(F1) -> (F2, J) (core). Closes resolve_duplicated_faces is missing in python package #277.
    • The input is copied into a concrete matrix before the call: the libigl
      implementation reuses the input's Derived type for internal plain objects,
      so an Eigen::Ref (what nb::DRef produces) fails to default-construct.
    • The docstring describes the actual Release-build behavior: for a
      non-orientable triangle (|pos − neg| > 1) the C++ "throw" is an
      IGL_ASSERT, which is compiled out in Release, so those faces are simply
      dropped rather than raising.
    • New test_resolve_duplicated_faces covers no-duplicate (identity), the
      cancel rule, the keep-one rule, and the non-orientable/drop case.

Doc fixes

  • boundary_loop.cpp: correct the copy-pasted comment on the boundary_loop
    wrapper — it returns only the longest loop, not all loops (related to boundary_loop_all isn't documented. #286;
    boundary_loop_all already exists for all loops).
  • vertex_components.cpp: the docstring claimed it takes "an adjacency matrix …
    and counts", but the binding takes a face list F and returns only per-vertex
    component ids. Fixed the comment, docstring, and @param/@return.

Testing

Built and ran the full suite locally: 91 passed (was 90).

🤖 Generated with Claude Code

…nents docs

- Bind igl::resolve_duplicated_faces (core), returning (F2, J). Copies the
  input into a concrete matrix first since the implementation reuses the
  input's Derived type for internal plain objects (an Eigen::Ref won't
  default-construct). Docstring reflects actual Release behavior: the
  non-orientable case drops those faces rather than throwing (the rule-4 check
  is an IGL_ASSERT, compiled out in Release). Closes #277.

- Fix the misleading comment on the boundary_loop wrapper (it returns the
  longest loop, not all loops). Related to #286.

- Fix vertex_components docstring/comment: it takes a face list F and returns
  per-vertex component ids, not "an adjacency matrix ... and counts".

Adds test_resolve_duplicated_faces covering the no-duplicate, cancel,
keep-one, and non-orientable cases. Full suite: 91 passed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@alecjacobson
alecjacobson merged commit 99d2c47 into main Aug 10, 2026
39 checks passed
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.

resolve_duplicated_faces is missing in python package

1 participant