Skip to content

Add MeshDenoiserKit Swift package for iOS/macOS#4

Merged
lanxinger merged 12 commits into
mainfrom
meshdenoiserkit
Jun 12, 2026
Merged

Add MeshDenoiserKit Swift package for iOS/macOS#4
lanxinger merged 12 commits into
mainfrom
meshdenoiserkit

Conversation

@lanxinger

Copy link
Copy Markdown
Owner

Summary

  • Wraps the SD-filter denoiser in a SwiftPM package (MeshDenoiserKit, iOS 17+ / macOS 14+) with a buffers-in/buffers-out C API bridge — vertex count/order preserved so UVs/materials survive
  • Async Swift API with per-outer-iteration progress reporting and Task cancellation (new callback hook in the C++ core; CLI behavior unchanged)
  • Vendors Eigen 3.4.0 + OpenMesh 11 Core via pinned scripts/vendor_dependencies.sh; tinygltf/tinyusdz/CLI excluded from the package
  • Golden-file parity test pins package output to CLI numerics (max error < 1e-6 on a 162-vertex noisy icosphere fixture, deterministic mode)

Design spec: docs/superpowers/specs/2026-06-12-meshdenoiserkit-swift-package-design.md

Test Plan

  • swift test — 8 tests: contract (count/order, NaN/index/params errors, cancellation, progress) + golden parity
  • CMake CLI still builds and denoises with unchanged output
  • CI swift-package job passes on macos-15

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e31fb9fa5b

ℹ️ 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".

Comment thread Package.swift Outdated
Comment on lines +13 to +14
path: "Sources/OpenMeshCore",
publicHeadersPath: "include",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid exposing OpenMesh headers as a SwiftPM module

With this target exported through publicHeadersPath, SwiftPM builds OpenMesh as a Clang module; compiling CMeshDenoiserCore then fails because OpenMesh/Core/IO/SR_binary_spec.hh includes another OpenMesh header while namespace OpenMesh::IO is open, which Clang diagnoses as redundant #include of module 'OpenMeshCore' appears within namespace 'OpenMesh::IO'. I verified swift test fails before any tests run, so the new package cannot currently build until the OpenMesh headers are made module-safe or are consumed without this module boundary.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in fd637b8. The OpenMesh headers are no longer exposed through publicHeadersPath — they moved to a Headers directory consumed via textual -I search paths only, so no Clang module is generated over them. A comment-only shim header satisfies SwiftPM's public-headers requirement for the target. Could not reproduce the module diagnostic on Xcode 27 beta (either build system), but the construct was toolchain-fragile and unnecessary regardless — nothing should import OpenMeshCore.

Comment thread Package.swift
name: "MeshDenoiserKitTests",
dependencies: ["MeshDenoiserKit"],
path: "Tests/MeshDenoiserKitTests",
resources: [.copy("Fixtures")]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Add the declared golden-test fixtures

The test target declares Fixtures as a copied resource, but the committed tree under Tests/MeshDenoiserKitTests contains only the Swift test/helper files and no Fixtures directory; swift test already warns Invalid Resource 'Fixtures': File not found. Once the package compiles, GoldenParityTests force-loads noisy_icosphere.obj and golden_denoised.obj from that resource directory, so the package test suite will fail unless those OBJ fixtures are committed or the resource/test is adjusted.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in fd637b8. Root cause: the repo's pre-existing *.obj gitignore rule (intended for MSVC object files) silently excluded the Wavefront fixtures from git add. Added a negation for Tests/MeshDenoiserKitTests/Fixtures/*.obj and committed both OBJ files; swift test passes 8/8 from a clean checkout state.

…s a module

- *.obj gitignore rule (meant for MSVC object files) silently excluded the
  Wavefront test fixtures; negate it for the Fixtures directory and commit them.
- Move OpenMesh headers out of SwiftPM's public-headers path so they are
  consumed as textual includes, never built as a Clang module (their internal
  includes inside open namespaces are not module-safe). A comment-only shim
  header satisfies SwiftPM's public-headers requirement.
@lanxinger lanxinger merged commit 236ad05 into main Jun 12, 2026
4 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.

1 participant