Add initial support for usd for sfmData#2119
Add initial support for usd for sfmData#2119servantftransperfect wants to merge 1 commit intodevelopfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds initial USD (.usd/.usda/.usdc) import/export support for sfmData, centered around a compact, table-based landmark/observation representation to scale to large reconstructions.
Changes:
- Introduces
sfmData::LandmarkTableandbuildLandmarkTable()to pack landmarks/observations into contiguous arrays (optionally with a view-based inverse index). - Implements USD IO (
saveUSD/loadUSD) and integrates it into the genericsfmDataIOload/save pipeline, including payload sidecar writing for landmark tables. - Updates build/export configuration, Meshroom format options, docs, and adds unit tests for landmark table packing and USD save/load coverage.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/software/export/main_exportMeshUSD.cpp | Makes USDZ support conditional on availability of pxr/usd/usd/zipFile.h. |
| src/cmake/config.hpp.in | Exposes ALICEVISION_HAVE_USD() to C++ via generated config header. |
| src/cmake/AliceVisionConfig.cmake.in | Exports USD availability and attempts to locate USD (pxr) for downstream consumers. |
| src/aliceVision/sfmDataIO/usdIO.hpp | Declares USD save/load entry points + payload helper. |
| src/aliceVision/sfmDataIO/usdIO.cpp | Implements full USD read/write for SfMData + landmark table payload strategy. |
| src/aliceVision/sfmDataIO/sfmDataIO_test.cpp | Adds usda to the save/load format test list when USD is enabled. |
| src/aliceVision/sfmDataIO/sfmDataIO.cpp | Routes .usd/.usda/.usdc extensions through the new USD IO and renames the payload on atomic save. |
| src/aliceVision/sfmDataIO/CMakeLists.txt | Builds usdIO and propagates USD link dependencies (now PUBLIC). |
| src/aliceVision/sfmData/sfmData_test.cpp | Adds unit tests for landmark table packing + optional view inverse index. |
| src/aliceVision/sfmData/README.md | Documents the compact landmark table approach and motivation. |
| src/aliceVision/sfmData/LandmarkTable.hpp | Defines the packed landmark/observation table structure. |
| src/aliceVision/sfmData/LandmarkTable.cpp | Implements packing from map-based SfMData into table arrays. |
| src/aliceVision/sfmData/CMakeLists.txt | Adds LandmarkTable to the sfmData library build. |
| meshroom/aliceVision/ConvertSfMFormat.py | Adds usda as an output format option for format conversion. |
| docs/usd/schema.usda | Provides a proposed USD schema for the SfMData hybrid (prim-per-object + table landmarks) layout. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
servantftransperfect
force-pushed
the
dev/usd
branch
from
7fe9534 to
b44d711
Compare
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
servantftransperfect
force-pushed
the
dev/usd
branch
from
b44d711 to
9a8ea2f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fs::rename(tmpPath, filename); | ||
|
|
||
| #if ALICEVISION_IS_DEFINED(ALICEVISION_HAVE_USD) | ||
| // Rename the payload file | ||
| if (extension == ".usd" || extension == ".usda" || extension == ".usdc") | ||
| { | ||
| const std::string tmpPayload = landmarksPayloadPath(tmpPath); | ||
| if (fs::exists(tmpPayload)) | ||
| fs::rename(tmpPayload, landmarksPayloadPath(filename)); | ||
| } | ||
| #endif |
There was a problem hiding this comment.
The USD payload rename is performed after the main file rename but without error handling; std::filesystem::rename can throw (e.g. destination exists on Windows, permission issues), leaving the main USD renamed but the payload still at the temp name. Consider catching filesystem_error here and either roll back the main rename or surface a clear failure while cleaning up/ensuring consistent outputs.
| fs::rename(tmpPath, filename); | |
| #if ALICEVISION_IS_DEFINED(ALICEVISION_HAVE_USD) | |
| // Rename the payload file | |
| if (extension == ".usd" || extension == ".usda" || extension == ".usdc") | |
| { | |
| const std::string tmpPayload = landmarksPayloadPath(tmpPath); | |
| if (fs::exists(tmpPayload)) | |
| fs::rename(tmpPayload, landmarksPayloadPath(filename)); | |
| } | |
| #endif | |
| bool mainFileRenamed = false; | |
| try | |
| { | |
| fs::rename(tmpPath, filename); | |
| mainFileRenamed = true; | |
| #if ALICEVISION_IS_DEFINED(ALICEVISION_HAVE_USD) | |
| // Rename the payload file | |
| if (extension == ".usd" || extension == ".usda" || extension == ".usdc") | |
| { | |
| const std::string tmpPayload = landmarksPayloadPath(tmpPath); | |
| if (fs::exists(tmpPayload)) | |
| fs::rename(tmpPayload, landmarksPayloadPath(filename)); | |
| } | |
| #endif | |
| } | |
| catch (const fs::filesystem_error& e) | |
| { | |
| ALICEVISION_LOG_ERROR("Cannot finalize save to '" << filename << "': " << e.what()); | |
| if (mainFileRenamed) | |
| { | |
| try | |
| { | |
| fs::rename(filename, tmpPath); | |
| } | |
| catch (const fs::filesystem_error& rollbackError) | |
| { | |
| ALICEVISION_LOG_ERROR("Cannot roll back renamed file from '" << filename << "' to '" << tmpPath << "': " << rollbackError.what()); | |
| } | |
| } | |
| status = false; | |
| } |
| if (!stage->GetRootLayer()->Save()) | ||
| { | ||
| ALICEVISION_LOG_ERROR("USD export failed while saving layer: " << filename); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
If saving the main USD layer fails, the already-written landmarks payload file (created earlier in this function) is left on disk. Consider cleaning up the payload file on failure (or writing both layers to temp names and only moving them into place once both saves succeed) to avoid orphaned sidecar files.
| // Non-Pinhole camera: write the transform only and mark it explicitly. | ||
| usdCamera.GetProjectionAttr().Set(UsdGeomTokens->perspective); |
There was a problem hiding this comment.
The function-level documentation says non-pinhole cameras leave projection attributes at USD defaults, but this branch explicitly sets projection to perspective before continuing. Either update the docstring to match the behavior or drop/adjust the projection assignment so the implementation matches the documented intent.
| // Non-Pinhole camera: write the transform only and mark it explicitly. | |
| usdCamera.GetProjectionAttr().Set(UsdGeomTokens->perspective); | |
| // Non-Pinhole camera: write the transform only and leave projection-related | |
| // USD camera attributes at their defaults. |
| if (saveConstraints2d) | ||
| { | ||
| writeConstraints2D(stage, sfmData, paths); | ||
| writeConstraintPoints(stage, sfmData, paths); | ||
| } |
There was a problem hiding this comment.
USD IO adds serialization for constraints/constraint points (and related new schema parts), but the existing save/load round-trip test scene does not populate these structures. Adding a targeted unit test that writes and reads back at least one Constraint2D/ConstraintPoint (and ideally RotationPrior/SurveyPoint/Rig) would help prevent schema regressions.
| if (buildViewIndex) | ||
| { | ||
| viewToObservations.clear(); | ||
| } |
There was a problem hiding this comment.
viewToObservations is default-constructed empty; calling clear() inside the buildViewIndex branch is redundant and can be removed to simplify the logic slightly.
| if (buildViewIndex) | |
| { | |
| viewToObservations.clear(); | |
| } |
servantftransperfect
force-pushed
the
dev/usd
branch
from
9a8ea2f to
74c0378
Compare
servantftransperfect
force-pushed
the
dev/usd
branch
from
74c0378 to
7e94020
Compare
|
This pull request introduces support for a compact, table-based USD schema for AliceVision SfMData, enabling efficient serialization and export of large-scale structure-from-motion data. The main changes include the addition of a new
LandmarkTabledata structure and utility, updates to the build system, new tests, documentation, and integration of the USD format into the IO pipeline.USD schema and compact table export:
docs/usd/schema.usda) for AliceVision SfMData, using a hybrid approach: table storage for landmarks/observations and one prim per object for other entities.LandmarkTable(src/aliceVision/sfmData/LandmarkTable.hpp,.cpp), a structure and utility function to pack landmarks and observations into contiguous arrays for efficient export, with support for optional view-based inverse indexing. [1] [2]src/aliceVision/sfmData/README.md.Build system and integration:
LandmarkTablein the build and to add new USD IO files (usdIO.hpp,.cpp) for reading/writing USD files. Also changed USD-related targets to link libraries as PUBLIC. [1] [2] [3] [4]usda) as an export option in the Meshroom node for format conversion..usd,.usda,.usdc). [1] [2]Testing:
sfmData_test.cpp. [1] [2]These changes lay the groundwork for scalable, efficient SfMData export and import with USD, especially for large datasets.