Repository navigation
Fix review findings: crashes, USDZ export, performance, deps and CI - #7
Merged
Merged
Conversation
- Reject faceless meshes, zero-extent meshes and invalid --export-precision instead of segfaulting or aborting; catch exceptions in main. - Parse option files on any whitespace (indent, tabs, CRLF), reject unknown keys and malformed values, and keep the tuned defaults for omitted keys. - Reject NaN/Inf parameters; accept MeshUpdateDisplacementEps <= 0 as documented (disables early stop). - Merge coincident vertices for filtering (seams, triangle soups) and copy results back to the input's vertices. - Apply USD node world transforms and bounds-check face indices. - Poll a cancellation hook between solver iterations. - Print mesh-update parameters; fix a vector<bool> data race under OpenMP. - Cut memory: 32-bit neighborhood indices, drop an unused matrix copy, parallel order-preserving neighbor search and guidance-normal pass, array-backed glTF vertex cache. Output is unchanged.
- Read and write ModelIO float3 attributes as three floats; SIMD3<Float> is 16 bytes and clobbered the next attribute or ran past the buffer. Keep mapped buffers alive while their pointers are used. - Denoise a position-welded copy so seams and flat-shaded assets are not open boundaries; fall back to the input topology if welding fails. - Cancel between solver iterations via a new md_cancel_fn; catch C++ exceptions at the C boundary; reject zero-extent meshes. - USDZ export: take the root layer from the first archive entry, match meshes by position instead of order, recompute primvars:normals (vertex, faceVarying, uniform, indexed) while keeping authored hard edges, patch a flattened stage when meshes live in other layers, and parse USDA as bytes for large files. - Remove per-vertex/per-face allocations in coincident-vertex averaging and MeshPreprocessor; avoid Int64 traps on extreme coordinates. - Locate test executables next to the test bundle.
- Accumulate area, distance and displacement sums in Double; Float sums inflated the rescaled lambda by 22% at 360K faces. - isGPUAvailable reports whether the Metal pipeline was actually created. - Parallel, deterministic CPU filter over exact-size CSR rows (5.6x at 89K faces). - Vertex update: one three-column Jacobi-preconditioned CG solve with warm start and reused buffers (4-8x). - GPU: write rows and weights straight into Metal buffers (peak memory roughly halved) and round the reduction width to a power of two.
…ries - Pin the OpenMesh 11.0 tarball with URL_HASH and replace the deprecated FetchContent_Populate/CMP0169 OLD setup with FetchContent_MakeAvailable. - Link OpenMesh statically (with OM_STATIC_BUILD) so the CLI is a single self-contained executable; CI verifies it has no OpenMesh dylib. - Make -march=native opt-in (MESHDENOISER_NATIVE_ARCH) so released binaries run on any CPU of the target architecture. - Read-only workflow token; only a separate release job can write. - Update actions to checkout v7, setup-python v7, upload-artifact v7, download-artifact v8 and action-gh-release v3; drop unneeded installs. - Stop ignoring cmake/*.cmake; remove the unused vcpkg.json.
The CLI output on a 327K-face mesh is byte-identical to Eigen 3.4.1 and all CLI and Swift package tests pass.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 454bb177e3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A mesh whose points were a subset of another prim's could claim that prim when ModelIO listed it first, leaving both prims with the wrong geometry. Every point referenced by the prim's faces must now receive a result; coincident USD points share one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the bugs, USDZ export issues, performance problems, dependency and CI items from a full code review. Each commit is one area and builds on its own.
Bugs fixed
SIMD3<Float>, which clobbered the next attribute (normal.x / UV.u) and wrote past packed buffers.--export-precisionaborted; zero-extent meshes divided by zero. The C bridge now catches C++ exceptions.isGPUAvailablereflects whether the Metal pipeline was created; latent non-power-of-two reduction bug.USDZ export
points(curves) are left alone.primvars:normalsrecomputed (vertex / faceVarying / uniform, indexed or not) while keeping authored hard edges.Performance
Dependencies and CI
URL_HASH, linked statically; deprecatedFetchContent_Populateremoved.-march=nativeis opt-in (MESHDENOISER_NATIVE_ARCH), so release binaries run on any CPU.contents: write. Actions bumped to current majors..gitignoreno longer hidescmake/*.cmake; unusedvcpkg.jsonremoved.Behaviour changes
md_denoisegained a cancel callback (internal target, no public product change).--export-precisionaccepts 1–17.Verification (macOS, Apple Silicon)
ctest) 14/14 with AppleClang, with AppleClang + libomp (OpenMP, CHOLMOD auto-detected), and against Eigen 5.0.1.Not changed: the
MeshUpdateDisplacementEps 0.1default still stops the mesh update after one iteration. Changing it alters the tuned output and the golden fixture, so it needs a separate decision.