Phase 3: native cvc::model + Assimp mesh/model loader - #158
Merged
Conversation
added 3 commits
July 26, 2026 16:49
Adds native import for standard mesh/model formats (obj, ply, stl, fbx,
gltf/glb, dae, 3ds, ...) into libcvc, VTK-free, mirroring the Phase-1
image + geometry_file_io registry pattern.
- cvc::model (inc/cvc/model/model.h, src/cvc/model/model.cpp): a small
scene value type — meshes (each a cvc::geometry + material index) plus
cvc::material (PBR-ish: base_color, metallic/roughness, emissive, and a
base-color texture decoded to a cvc::image below the VTK line).
merged() flattens to one geometry (offsetting indices via geometry::merge);
extents() unions the mesh bboxes.
- model_file_io registry (inc/cvc/model/model_file_io.h + .cpp): read_model/
write_model + register_default_model_handlers(), a faithful mirror of
geometry_file_io/image_file_io (extension dispatch, first-handler-wins,
lazy re-entrancy-guarded default registration).
- Assimp handlers (src/cvc/model/assimp_io.cpp, guarded by CVC_ENABLE_ASSIMP):
an assimp_model_io (full multi-mesh scene) + an assimp_geometry_io that
flattens via model::merged() so read_geometry("mesh.obj") works too.
Lowers positions/normals/vertex-colors/UV0/tangents (with reconstructed
handedness w) and materials incl. embedded (*N) and external base-color
textures resolved to cvc::image (texture failures are non-fatal). When the
flag is off, registration is a no-op and read_model raises 'no handler'.
- Build wiring (src/cvc/CMakeLists.txt): CVC_ENABLE_ASSIMP option +
find_package(assimp CONFIG) with graceful fallback + PUBLIC link/define
(mirrors ImageMagick). DEFENSIVE: rewrites assimp's exported ABSOLUTE
build-sandbox zlib path (a cvcpkg assimp-recipe quirk from
-DASSIMP_BUILD_ZLIB=OFF + FindZLIB) to the relocatable ZLIB::ZLIB, or CI
would fail to link assimp::assimp.
- Recipe (cvcpkg/recipes/libcvc/recipe.yaml): + assimp runtime dep (validated).
- Tests (src/cvc/tests/model_test.cpp): 8 gtest cases — merged() index
offsets, extents() union, unsupported-extension throw, registry has obj,
OBJ+UVs+material+texture, read_geometry flatten, STL-no-UVs. All pass
against a pristine assimp prefix; library links; clang-format clean.
Adversarial review of PR #158 confirmed one blocker + two minors; fixed: 1. BLOCKER — model::merged() desynced per-vertex attributes for meshes with heterogeneous attribute presence. geometry::merge appends normals/colors/ uvs/tangents by raw insert WITHOUT padding to the vertex count (it only offsets the appended indices), so flattening a UV'd mesh together with a UV-less one (routine for multi-material OBJ/glTF) left the merged array shorter than points() and misaligned against the offset triangle indices — a consumer indexing uvs[vertexIndex] then reads out of bounds or the wrong vertex. Fixed in model::merged() (Phase-3-scoped, no change to shared geometry::merge): if ANY mesh carries an attribute, pad the meshes lacking it to full length with a neutral default before folding, so every present per-vertex array stays exactly num_points() long and index-aligned in either mesh order. New regression test MergedPadsHeterogeneousAttributes fails without the fix (uvs.size()==3 vs 6) and passes with it. 2. MINOR — embedded-texture temp filename used a plain 'static int counter' (collides across concurrent reads / processes sharing TMPDIR). Now an atomic counter qualified by getpid(). 3. Strengthened ObjWithUvsMaterialTexture: assert the decoded texture's first pixel RGBA == authored (200,100,50,255) (guards channel order / decode, not just dims) and that CalcTangentSpace produced full-length tangents with |w|==1. Not changed: the registry's first-call default-registration guard uses plain static bools (a benign race on concurrent first use) — this mirrors the existing geometry_file_io/image_file_io registries verbatim; making just this one thread-safe would diverge from the established pattern (a proper fix is a shared-registry follow-up). model_test: 9/9 pass; clang-format clean.
fetch-libcvc-deps installs libcvc's recipe dependency closure on EVERY build platform (CI + publish), and assimp currently has no windows bundle (only linux/macos-arm64/freebsd/netbsd). An unconditional 'assimp' dep therefore breaks the windows deps-install — package-windows passes on master, so this would be a regression. Scope the dep with 'platforms: [linux, macos, freebsd, netbsd]' (same mechanism as pthreads4w's [windows]). Where assimp is absent, find_package(assimp CONFIG) misses and CVC_ENABLE_ASSIMP flips OFF, so libcvc still builds (without the native model loader). Add windows once an assimp windows bundle exists.
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
1 participant
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.
Native import for standard mesh/model formats (obj, ply, stl, fbx, gltf/glb, dae, 3ds, …) into libcvc — VTK-free, mirroring the Phase-1
cvc::image+geometry_file_ioregistry pattern. Unblocks Phase 6 (volrover3 dropsvtkGLTFReaderforpycvc.load_model).What's added
cvc::model— a small scene value type: meshes (each acvc::geometry+ material index) +cvc::material(PBR-ish base color / metallic / roughness / emissive + a base-color texture decoded tocvc::imagebelow the VTK line).merged()flattens to one geometry (offsetting indices viageometry::merge);extents()unions bboxes.model_file_ioregistry —read_model/write_model+register_default_model_handlers(), a faithful mirror ofgeometry_file_io/image_file_io.CVC_ENABLE_ASSIMP) —assimp_model_io(full multi-mesh scene) +assimp_geometry_iothat flattens viamodel::merged()soread_geometry("mesh.obj")also works. Lowers positions/normals/vertex-colors/UV0/tangents (reconstructed handednessw) + materials incl. embedded (*N) and external textures →cvc::image(texture failures non-fatal).CVC_ENABLE_ASSIMPoption +find_package(assimp CONFIG)with graceful fallback + PUBLIC link/define (mirrors ImageMagick).+ assimpruntime dep (validated with--recipes-dir).Heads-up: a real cvcpkg assimp-recipe bug, worked around here
The published
assimpbundle's cmake export hardcodes an absolute build-sandbox zlib path (/tmp/cvcpkg-builder/…/libz.so) inINTERFACE_LINK_LIBRARIES— the recipe builds with-DASSIMP_BUILD_ZLIB=OFFand assimp's CMake bakes${ZLIB_LIBRARIES}(a FindZLIB absolute path) into the export instead of the relocatableZLIB::ZLIB. That path exists on no consumer, solink assimp::assimpfails everywhere including CI. This PR defensively rewrites that stale entry toZLIB::ZLIBin libcvc's CMake (libcvc links zlib itself; the rewrite is a forward-compatible no-op once the recipe is fixed). Follow-up: fix the assimp recipe to export zlib via an imported target (libcvc-deps).Verification
Built
libcvc.so+model_testagainst a pristine assimp prefix (symlink workarounds removed) — links cleanly, and all 8 gtest cases pass (merged()index offsets,extents()union, unsupported-extension throw, registry-has-obj, OBJ+UVs+material+texture,read_geometryflatten, STL-no-UVs). clang-format clean; recipe validates.Ships on the next family publish (compute-revisions auto-bumps libcvc's revision).