Skip to content

fix(cuda): report surface intersection diagnostics - #458

Merged
MuGdxy merged 3 commits into
spiriMirror:mainfrom
Ligo04:fix/cuda-surface-intersection-diagnostics
May 12, 2026
Merged

fix(cuda): report surface intersection diagnostics#458
MuGdxy merged 3 commits into
spiriMirror:mainfrom
Ligo04:fix/cuda-surface-intersection-diagnostics

Conversation

@Ligo04

@Ligo04 Ligo04 commented May 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add detailed CUDA surface-intersection sanity-check diagnostics for intersecting edge-triangle pairs.
  • Include geometry, instance, object, and self-collision identifiers in the log output so failures are easier to trace.

Test plan

  • clang-format -i src/backends/cuda/sanity_check/simplicial_surface_intersection_check.cu
  • git diff --check main...HEAD

Log edge-triangle intersection details with geometry, instance, object, and self-collision identifiers to make sanity-check failures easier to trace.

Co-authored-by: Cursor <cursoragent@cursor.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the SimplicialSurfaceIntersectionCheck to include more detailed logging for intersection detection. The changes include adding necessary headers, adjusting code formatting, and introducing diagnostic logging for edges and triangles. However, a high-severity issue was identified regarding the indexing of SelfCollision attributes and the inconsistency in using F[1] instead of F[0] for triangle attribute retrieval, which deviates from established patterns in the codebase.

Comment on lines +433 to +442
auto GeoIdR = VGeoIds[F[1]];

auto ObjIdL = VObjectIds[E[0]];
auto ObjIdR = VObjectIds[F[0]];
auto ObjIdR = VObjectIds[F[1]];

auto InstIdL = VInstanceIds[E[0]];
auto InstIdR = VInstanceIds[F[1]];

auto SelfCollL = SelfCollision[InstIdL];
auto SelfCollR = SelfCollision[InstIdR];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The indexing for SelfCollision is incorrect. SelfCollision is a vertex attribute (retrieved from scene_surface.vertices()), so it must be indexed by a vertex index (e.g., E[0] or F[0]), not by an instance ID (InstIdL or InstIdR). Indexing with an instance ID will likely lead to out-of-bounds access or incorrect diagnostic values.

Additionally, the change from F[0] to F[1] for triangle attributes is inconsistent with the edge handling (which uses E[0]) and with the CUDA kernel implementation (which uses F[0] for tri_bids and tri_cids at lines 134-135). Maintaining consistency in debug export functionality is required to ensure uniform semantics across the repository.

            auto GeoIdR = VGeoIds[F[0]];

            auto ObjIdL = VObjectIds[E[0]];
            auto ObjIdR = VObjectIds[F[0]];

            auto InstIdL = VInstanceIds[E[0]];
            auto InstIdR = VInstanceIds[F[0]];

            auto SelfCollL = SelfCollision[E[0]];
            auto SelfCollR = SelfCollision[F[0]];
References
  1. When adding new debug export functionality, ensure its implementation is consistent with existing, similar functionalities to maintain uniform semantics.

Ligo04 and others added 2 commits May 12, 2026 15:53
Use triangle vertex indices consistently for right-side metadata and vertex indices for self-collision diagnostics.

Co-authored-by: Cursor <cursoragent@cursor.com>
@MuGdxy
MuGdxy merged commit 1fd24fe into spiriMirror:main May 12, 2026
12 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.

2 participants