From 31314aab585918e442037973a709bd3ff8f3c398 Mon Sep 17 00:00:00 2001 From: snkmcb Date: Sat, 12 Sep 2026 13:19:36 +0900 Subject: [PATCH 1/2] feat: make generated payload ownership explicit Tiled FileFormat reads wrote their payloads with no owner, so the same layer could not be read a second time: the payloads from the first read refused the write (LAS016, COPC007). Layer authoring also built content in scratch stages that loaded every payload and borrowed file identities to anchor them, which made tiled COPC reads warn "Could not open asset". - Build layer content in in-memory stages opened with LoadNone and export payloads from in-memory layers; drop the layer identifier rewriting. - Add a layer-level AuthorPointCloudTiledAssetWithPayloads overload with typed diagnostics and move the COPC adapter onto it. - Record payload ownership per layer identity in payload-owner-.manifest: a read replaces the payloads its layer generated before, removes superseded ones, and refuses any other file. Ownership is claimed before the first write and restored on failure. - Apply the same rule when materializing a generated-cache hit, taking over files that already hold the cached bytes. - Report the reason behind tiled author failures in LAS, LAZ, and COPC. - Fix the stream benchmark build on macOS. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 38 +++ docs/architecture/FILE_FORMAT_ARGUMENTS.md | 37 +- docs/architecture/PLUGIN_ADAPTER.md | 5 + docs/architecture/WORKSPACE.md | 2 +- docs/compatibility/OPENUSD.md | 4 +- docs/reference/CAPABILITY_MATRIX.md | 1 + docs/roadmap/implementation-status.md | 24 ++ docs/roadmap/streaming-and-tiling.md | 11 +- libs/usd-pointcloud-authoring/CMakeLists.txt | 1 + libs/usd-pointcloud-authoring/README.md | 28 +- .../benchmarks/stream_benchmark.cpp | 2 +- .../include/usdgeo/PointCloudLayer.h | 24 ++ .../src/GeneratedPayloadSet.cpp | 320 ++++++++++++++++++ .../src/GeneratedPayloadSet.h | 72 ++++ .../src/PointCloudCache.cpp | 210 ++++++++---- .../src/PointCloudLayer.cpp | 272 +++++++++------ .../src/PointCloudStreaming.cpp | 58 ++-- .../src/TiledPayloadAuthoring.h | 36 ++ .../tests/test_pointcloud_cache.cpp | 71 +++- .../tests/test_usd_authoring.cpp | 310 +++++++++++++++++ plugins/pointcloud-copc/README.md | 5 +- .../src/UsdGeoCopcFileFormat.cpp | 23 +- .../tests/test_pointcloud_copc.cpp | 63 ++++ plugins/pointcloud-las/README.md | 2 +- .../src/UsdGeoLasFileFormat.cpp | 10 +- .../tests/test_pointcloud_las.cpp | 35 +- plugins/pointcloud-laz/README.md | 2 +- .../src/UsdGeoLazFileFormat.cpp | 10 +- plugins/pointcloud-ply/README.md | 4 +- .../src/UsdGeoPlyFileFormat.cpp | 2 + 30 files changed, 1439 insertions(+), 243 deletions(-) create mode 100644 libs/usd-pointcloud-authoring/src/GeneratedPayloadSet.cpp create mode 100644 libs/usd-pointcloud-authoring/src/GeneratedPayloadSet.h create mode 100644 libs/usd-pointcloud-authoring/src/TiledPayloadAuthoring.h diff --git a/CHANGELOG.md b/CHANGELOG.md index f927b65..1dc2658 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,44 @@ All notable changes to this project are documented here. - Local `.copc` and `.copc.laz` inputs in `usd-pointcloud-convert`, including deterministic generated-cache publication and reuse. +- `PointCloudPayloadOptions::owner`, `PointCloudPayloadOwner`, and a + layer-level `AuthorPointCloudTiledAssetWithPayloads` overload that reports + typed diagnostics. A payload directory records which layer owns the files it + generated in `payload-owner-.manifest`, keyed by a hash of the layer + identity. The rule is in the + [file-format argument contract](docs/architecture/FILE_FORMAT_ARGUMENTS.md#generated-payload-ownership). + +### Changed + +- Layer authoring builds content in an in-memory stage that loads nothing, and + payload files are exported from in-memory layers. No authoring call renames + a caller's layer or gives a scratch layer a file identity any more. +- The COPC adapter authors native-hierarchy tiles through the shared + layer-level entry point instead of creating and transferring its own stage. +- A tiled LAS, LAZ, or COPC read that cannot author reports the reason, such + as the payload file it refused to replace, alongside `LAS016`, `LAZ007`, or + `COPC007` instead of the bare code. PLY already did so under `PLY007`. + +### Fixed + +- Reading a tiled LAS, LAZ, PLY, or COPC layer again, for example in a new + process or after the first layer was released, no longer fails because the + payloads the first read generated already exist. The layer replaces the + payloads it owns and still refuses any file it did not generate. +- Tiled COPC reads no longer warn `Could not open asset` for their own payloads + when the working directory differs from the source directory. +- Materializing a generated-cache hit replaces a stale payload the layer owns + instead of keeping it, and never overwrites a file someone else placed. +- The authoring stream benchmark builds on macOS, where `std::uintmax_t` and + `std::uint64_t` are distinct types. + +### Compatibility + +- Payload directories written by earlier tiled reads carry no ownership + record, so a regeneration treats their payloads as foreign and refuses them, + as the earlier release already did. Delete such a directory once and the + next read generates and owns it. Payloads a cache hit materialized earlier + are taken over when they still hold the cached bytes. ## [0.11.0] - 2026-08-27 diff --git a/docs/architecture/FILE_FORMAT_ARGUMENTS.md b/docs/architecture/FILE_FORMAT_ARGUMENTS.md index 8889b6d..6e268fe 100644 --- a/docs/architecture/FILE_FORMAT_ARGUMENTS.md +++ b/docs/architecture/FILE_FORMAT_ARGUMENTS.md @@ -96,7 +96,7 @@ The tiled surface also exposes: | `tile` | `true` when tiling is enabled | | `tileSize` | Positive source-coordinate tile size | | `tileMemoryLimit` | Positive total spool working-set limit in bytes | -| `payloadDirectory` | Payload output directory | +| `payloadDirectory` | Payload output directory; see [generated payload ownership](#generated-payload-ownership) | The explicit `usd-pointcloud-convert` tool additionally accepts the generic point-budget arguments `maxPointsPerTile`, `minPointsPerTile`, and `maxDepth`. @@ -139,6 +139,38 @@ descriptor identity. When a committed entry exists for the source, normalized arguments, and reader metadata, the LAS, LAZ, or COPC adapter loads it before point delivery; a miss follows the normal reader and authoring path. +## Generated Payload Ownership + +A tiled read writes its payloads into `payloadDirectory` and authors a root +that references them by relative path. The root lives only in the opened +layer, so every read of the layer writes the payloads again, by generating +them or by materializing a cache hit. Ownership of the files in the directory +is therefore explicit: + +- The owner of a payload set is the layer's identity: the resolved source and + its normalized arguments. The directory records it in + `payload-owner-.manifest`, where `` is a 64-bit hash of that + identity. The record lists relative payload names only; no source path, + resolver identifier, or validation token is written. +- Reading the same layer again replaces the payloads that layer generated + before and removes those it no longer generates. This is what lets a host + reopen or reload a tiled layer. +- A file the layer did not generate is never replaced. When another source, + other arguments, or a user file already holds a payload name, the read fails + with the plugin's author-failure code, names the file, and leaves the + directory unchanged. +- Ownership is recorded before the first payload is written, so an + interrupted read leaves payloads the next read of the same layer replaces. A + read that fails or is cancelled removes the payloads it wrote, restores the + previous record, and removes a directory it created. +- Materializing a committed `USDGEO_CACHE_ROOT` entry into `payloadDirectory` + follows the same rule. An existing file that already holds the cached bytes + is taken over, so payloads materialized before ownership was recorded keep + resolving. + +`usd-pointcloud-convert` does not share this rule. Its output root and payload +directory must not exist yet, and its transaction marker owns recovery. + ## Rules 1. **Validate everything.** An unknown key, an unparsable value, or an @@ -189,3 +221,6 @@ should not silently change authored output. - Conflicting combinations are rejected rather than resolved by precedence. - A default-valued argument and an absent argument produce identical layers. - LAS and LAZ accept the same arguments with the same meaning. +- A tiled layer read a second time regenerates over its own payloads, never + replaces a file another layer or a user placed, and restores its ownership + record when the read fails. diff --git a/docs/architecture/PLUGIN_ADAPTER.md b/docs/architecture/PLUGIN_ADAPTER.md index d587ae4..b45710b 100644 --- a/docs/architecture/PLUGIN_ADAPTER.md +++ b/docs/architecture/PLUGIN_ADAPTER.md @@ -62,6 +62,11 @@ Both plugins now pass reader output through the shared `usdlas` point-data and asset builders, then call the layer-level `usdgeo::AuthorPointCloudAsset` entry point. The plugins no longer own point fan-out, chunk schema construction, CRS or bounds conversion, stage metrics, or layer transfer. +COPC native-hierarchy tiles reach the layer-level +`usdgeo::AuthorPointCloudTiledAssetWithPayloads` entry point the same way, so +no adapter creates a stage or transfers layer content. Every tiled adapter +names its layer as the owner of the payloads it generates through +`usdgeo::PointCloudPayloadOwner`. ### Consequence diff --git a/docs/architecture/WORKSPACE.md b/docs/architecture/WORKSPACE.md index 139bd40..570c84c 100644 --- a/docs/architecture/WORKSPACE.md +++ b/docs/architecture/WORKSPACE.md @@ -23,7 +23,7 @@ modules implement is fixed in | `usdPointCloudCore` | `libs/usd-pointcloud-core` | plain CMake/OpenStrata static library | implemented | Format-independent point-cloud contracts: point attribute definitions and types, `PointChunk` / `PointData` / `PointCloudAsset`, point validation, attribute selection, range and chunk read options, deterministic sampling, and the shared LOD hierarchy and tile value types. | | `usdLas` | `libs/usd-las` | plain CMake/OpenStrata static library | implemented | LAS parsing and decoding: 1.2-1.4 headers, VLR/EVLR, point formats 0-10, waveform packet metadata, GeoTIFF key records, scalar and vector Extra Bytes, metadata-only reads, and chunked and range-based reads behind `LasReader`. | | `usdLaz` | `libs/usd-laz` | plain CMake/OpenStrata static library | implemented | LAZ decompression through the vendored laz-perf codec, converted into the shared LAS and point-cloud contracts, with metadata-only reads, chunked reads, and stable diagnostics for unsupported compressed formats. Owns the vendored laz-perf subset. | -| `usdPointCloudAuthoring` | `libs/usd-pointcloud-authoring` | plain CMake/OpenStrata static library | implemented | Shared OpenUSD authoring: `UsdGeomPoints` and geospatial metadata authoring, point-attribute authoring, `usdLod` hierarchy authoring, tiled point-cloud authoring, payload-backed tile assets, root layer generation, and stage/layer validation. | +| `usdPointCloudAuthoring` | `libs/usd-pointcloud-authoring` | plain CMake/OpenStrata static library | implemented | Shared OpenUSD authoring: `UsdGeomPoints` and geospatial metadata authoring, point-attribute authoring, `usdLod` hierarchy authoring, tiled point-cloud authoring, payload-backed tile assets and the ownership of generated payload files, root layer generation, and stage/layer validation. | | `pointcloud-las` | `plugins/pointcloud-las` | OpenStrata plugin bundle (`usd-fileformat`) | implemented | OpenUSD `SdfFileFormat` adapter for `.las`: plugin registration, argument normalization, `LasReader` construction, and authoring through the shared library. Owns its `LASxxx` diagnostic codes. | | `pointcloud-laz` | `plugins/pointcloud-laz` | OpenStrata plugin bundle (`usd-fileformat`) | implemented | The same adapter shape for `.laz`, using `LazReader` and the laz-perf codec integration. Owns its `LAZxxx` diagnostic codes. | | `usdCopc` | `libs/usd-copc` | plain CMake/OpenStrata static library | implemented (source-backed) | COPC Info and hierarchy validation, project-owned random-access source integration, selective point-data range decoding through the shared LAZ chunk decoder, and native hierarchy streaming. It remains OpenUSD- and transport-independent. | diff --git a/docs/compatibility/OPENUSD.md b/docs/compatibility/OPENUSD.md index 8aad9d9..f76f266 100644 --- a/docs/compatibility/OPENUSD.md +++ b/docs/compatibility/OPENUSD.md @@ -49,7 +49,9 @@ the plugin bundles require OpenUSD. The plugins depend on a small, stable part of the API: - `SdfFileFormat`, `SdfLayer`, and `SdfFileFormat::FindByExtension("usda")` -- `UsdStage::Open` on an anonymous layer, then `SdfLayer::TransferContent` +- `UsdStage::CreateInMemory` with `UsdStage::LoadNone`, then + `SdfLayer::TransferContent` into the layer being read; payload files are + built in in-memory layers and written with `SdfLayer::Export` - `UsdGeomPoints`, `UsdGeomSetStageUpAxis`, `UsdGeomSetStageMetersPerUnit` - `TfType` registration through `TF_REGISTRY_FUNCTION` and `SDF_DEFINE_FILE_FORMAT` diff --git a/docs/reference/CAPABILITY_MATRIX.md b/docs/reference/CAPABILITY_MATRIX.md index a02a27f..0e3bcdf 100644 --- a/docs/reference/CAPABILITY_MATRIX.md +++ b/docs/reference/CAPABILITY_MATRIX.md @@ -198,6 +198,7 @@ reported separately, because they differ. | Bounded-memory generation during file open | Implemented for LAS, LAZ, COPC, and PLY tiled reads; generated-corpus benchmark available | Source streams, spool buffers, and one-tile reconstruction are bounded; generated-corpus and checked-in real-data measurements are available | | LOD file-format arguments | n/a | `lod=off\|preview\|balanced\|quality` | | Spatial tile file-format arguments | n/a | `tile=true`, `tileSize`, `tileMemoryLimit`, `payloadDirectory` for LAS, LAZ, COPC, and PLY | +| Reopening a tiled read over its own payloads | Supported when the caller names a payload owner | Supported: a read replaces the payloads its layer generated before and refuses any file it did not generate; see [payload ownership](../architecture/FILE_FORMAT_ARGUMENTS.md#generated-payload-ownership) | With `lod=off` every read authors one `UsdGeomPoints` prim at `/PointCloud`. The other profiles author a single non-tiled `usdLod` root with fixed-stride diff --git a/docs/roadmap/implementation-status.md b/docs/roadmap/implementation-status.md index 28a3c7f..37ffb8d 100644 --- a/docs/roadmap/implementation-status.md +++ b/docs/roadmap/implementation-status.md @@ -401,6 +401,30 @@ Phase 6 — validation and baselines: Stable resolver identities publish and reuse generated entries; unstable and unavailable identities convert without cache publication. +#### Generated-layer ownership + +The [ecosystem strategy](../design/ECOSYSTEM_STRATEGY.md) asks that temporary +generated USD be reduced where in-memory layers are practical, and that the +ownership, paths, crash recovery, and test isolation of generated files that +remain be explicit. + +- [x] Build layer content in in-memory stages that load nothing, so a read + never resolves the payloads it is generating, and stop renaming caller + or scratch layers to anchor payload paths. Tiled COPC reads no longer + report `Could not open asset` for their own payloads. +- [x] Move COPC native-hierarchy tiled authoring onto a layer-level shared + entry point, so no adapter creates a stage or transfers layer content +- [x] Record ownership of FileFormat-generated payloads by layer identity. + Reading a tiled layer again regenerates its own payloads instead of + failing on them, a file it did not generate still refuses the write, and + an interrupted read leaves files the next read replaces; see + [payload ownership](../architecture/FILE_FORMAT_ARGUMENTS.md#generated-payload-ownership) +- [x] Apply the same ownership to generated-cache materialization, replacing + stale owned payloads instead of keeping whatever file is already present +- [ ] Give tile spools an owned, recoverable working location instead of + timestamped directories in the system temporary directory, which an + interrupted process leaves behind + #### Research - runtime streaming (no release gate) - [x] Add an opt-in COPC runtime-streaming probe that takes a requested node diff --git a/docs/roadmap/streaming-and-tiling.md b/docs/roadmap/streaming-and-tiling.md index 904871e..912a0f9 100644 --- a/docs/roadmap/streaming-and-tiling.md +++ b/docs/roadmap/streaming-and-tiling.md @@ -270,7 +270,10 @@ The payload writer produces: - root `usdLod` metadata; - a source fingerprint and the generation settings; - tile bounds and point counts; -- no partially published root asset after failure. +- no partially published root asset after failure; +- explicit ownership of the payload files, so a FileFormat read regenerates + its own payloads when the layer is read again and never replaces a file it + did not generate. The conversion tool writes the versioned tile manifest only after all payloads have been authored successfully. Entries are sorted by tile ID and LOD, and @@ -278,6 +281,12 @@ portable payload paths, duplicate tile/LOD entries, bounds, and point counts are validated before publication. Cache materialization preserves the tile manifest beside the cached payloads. +FileFormat reads record which layer owns the payloads in the directory before +writing the first one, so an interrupted read leaves files the next read of +the same layer replaces; the rule is in the +[file-format argument contract](../architecture/FILE_FORMAT_ARGUMENTS.md#generated-payload-ownership). +The plugins never write the root layer of a FileFormat read to disk. + Final output is committed atomically where the filesystem allows it. The authored representation stays inside the existing [tile and LOD contract](../architecture/LOD.md); streaming changes how assets diff --git a/libs/usd-pointcloud-authoring/CMakeLists.txt b/libs/usd-pointcloud-authoring/CMakeLists.txt index 4c6fbaf..f8a1845 100644 --- a/libs/usd-pointcloud-authoring/CMakeLists.txt +++ b/libs/usd-pointcloud-authoring/CMakeLists.txt @@ -9,6 +9,7 @@ find_package(pxr CONFIG REQUIRED) add_library(usdPointCloudAuthoring STATIC src/ArAssetRandomAccessSource.cpp + src/GeneratedPayloadSet.cpp src/PointCloudCache.cpp src/PointCloudLayer.cpp src/PointCloudStreaming.cpp) diff --git a/libs/usd-pointcloud-authoring/README.md b/libs/usd-pointcloud-authoring/README.md index 0b143cf..9f9d67e 100644 --- a/libs/usd-pointcloud-authoring/README.md +++ b/libs/usd-pointcloud-authoring/README.md @@ -30,7 +30,8 @@ namespace `usdgeo`. index. - Tiled authoring: one deterministic `usdLod` root per tile. - Payload-backed tile assets: one USDC payload per tile and LOD level, with - portable relative asset paths. + portable relative asset paths, and ownership of the generated payload set + when the caller names an owner. - Layer- and stage-level validation, and the typed `PointCloudAuthorFailure` a caller maps onto its own codes. @@ -61,7 +62,7 @@ usdgeo/PointCloudCache.h | Metadata only | `AuthorPointCloudMetadata`, `PointCloudSourceMetadata` | | LOD | `AuthorPointCloudLodAsset` | | Tiling | `PointCloudTileAsset`, `AuthorPointCloudTiledAsset` | -| Payloads | `PointCloudPayloadOptions`, `AuthorPointCloudTiledAssetWithPayloads` | +| Payloads | `PointCloudPayloadOptions`, `AuthorPointCloudTiledAssetWithPayloads` (stage and layer overloads), `PointCloudPayloadOwner` | | Generated cache | `TryBuildPointCloudCacheLayout`, `TryLoadPointCloudCache`, `TryBuildResolverSourceIdentity` | | Failure kinds | `PointCloudAuthorFailure` (`InvalidLayer`, `StageCreation`, `StageMetrics`, `PointCloud`) | @@ -139,6 +140,25 @@ One caller-side note that belongs to the bundles rather than to this module: thread-local state, so a bundle that authors a detached stage and transfers its content must do so on the same thread that will hand the result back. +The layer overloads build content in an in-memory stage opened with +`UsdStage::LoadNone` and transfer it into the caller's layer. Nothing is +loaded, so a payload arc is never resolved or opened while its layer is being +built, and no layer is renamed to anchor one: the caller's layer keeps its +identifier throughout, and the detached stage never takes a file identity. +Each payload file is built in an in-memory layer and written with +`SdfLayer::Export`, so writing it never registers a layer under the payload +path. + +`PointCloudPayloadOptions::owner` decides who owns the files written into the +payload directory. Left empty, the directory is exclusive and any existing +payload path refuses the write, which is what `usd-pointcloud-convert` relies +on. Set, typically to `PointCloudPayloadOwner(resolvedPath, request)`, the +generation replaces the payloads that owner generated before, removes those it +no longer generates, and refuses every other existing file. Ownership is +recorded in the directory before the first payload is written and restored if +the generation fails. The rule is stated in the +[file-format argument contract](../../docs/architecture/FILE_FORMAT_ARGUMENTS.md#generated-payload-ownership). + ## Coordinate-space assumptions Input positions are **source-space** `usdgeo::Vec3d`. This module applies @@ -179,6 +199,10 @@ ctest --test-dir build/cy2026-windows-x86_64-py313-usd -C Release ` - Sampling is fixed-stride, inherited from `usdPointCloudCore`. - Payload working-set behavior is unmeasured: the library emits payloads, but no claim is made that a non-selected LOD child's payload stays unloaded. +- Payloads are replaced by writing a new file and renaming it over the old + one. A platform that refuses to rename over an open file, such as Windows, + fails a regeneration while another stage in the process still holds that + payload open. - `AuthorPointCloudTiledAssetFromStream` consumes a pull stream, spools points by source tile, and reconstructs one tile at a time before payload authoring. The router overload also accepts a planned `PointBudgetTileRouter`, allowing diff --git a/libs/usd-pointcloud-authoring/benchmarks/stream_benchmark.cpp b/libs/usd-pointcloud-authoring/benchmarks/stream_benchmark.cpp index cb2b4dc..87fb6c0 100644 --- a/libs/usd-pointcloud-authoring/benchmarks/stream_benchmark.cpp +++ b/libs/usd-pointcloud-authoring/benchmarks/stream_benchmark.cpp @@ -632,7 +632,7 @@ int main(int argc, char** argv) { if (entry.lod != 0) continue; const auto entryPayloadPath = outputDirectory / entry.payloadPath; std::error_code payloadError; - const auto entryPayloadBytes = std::filesystem::file_size( + const std::uint64_t entryPayloadBytes = std::filesystem::file_size( entryPayloadPath, payloadError); if (payloadError) continue; ++tileManifestCount; diff --git a/libs/usd-pointcloud-authoring/include/usdgeo/PointCloudLayer.h b/libs/usd-pointcloud-authoring/include/usdgeo/PointCloudLayer.h index e146111..653f3a9 100644 --- a/libs/usd-pointcloud-authoring/include/usdgeo/PointCloudLayer.h +++ b/libs/usd-pointcloud-authoring/include/usdgeo/PointCloudLayer.h @@ -1,6 +1,7 @@ #pragma once #include "usdgeo/GeoReference.h" +#include "usdpointcloud/FileFormatArguments.h" #include "usdpointcloud/PointCloud.h" #include "usdpointcloud/Lod.h" #include "usdpointcloud/Spool.h" @@ -131,8 +132,21 @@ struct PointCloudPayloadOptions { std::vector* tileManifestEntries = nullptr; usdpointcloud::SpoolIoStats* spoolIoStats = nullptr; + // Identity of the layer the generated payloads belong to. When empty the + // payload directory is exclusive and any existing payload path refuses + // the write. When set, payloads this owner generated before are replaced + // and removed as the new generation supersedes them, while any other + // existing file still refuses the write. The directory records ownership + // under a hash of this value; the value itself is never persisted. + std::string owner; }; +// The payload owner for a FileFormat read: the resolved source and its +// normalized arguments, which together are the layer's identity. +std::string PointCloudPayloadOwner( + const std::string& resolvedPath, + const usdpointcloud::PointReadRequest& request); + bool AuthorPointCloudTiledAssetFromStream( pxr::SdfLayer* layer, const std::string& primPath, @@ -174,4 +188,14 @@ bool AuthorPointCloudTiledAssetWithPayloads( const PointCloudPayloadOptions& options, std::vector& generatedPayloads); +// Authors the payload-backed tiles into a detached stage and transfers the +// result into `layer`. On failure the layer is unchanged, the payloads this +// call wrote are removed, and `diagnostics` explains why. +bool AuthorPointCloudTiledAssetWithPayloads( + pxr::SdfLayer* layer, + const std::string& primPath, + const std::vector& tiles, + const PointCloudPayloadOptions& options, + std::vector& diagnostics); + } // namespace usdgeo \ No newline at end of file diff --git a/libs/usd-pointcloud-authoring/src/GeneratedPayloadSet.cpp b/libs/usd-pointcloud-authoring/src/GeneratedPayloadSet.cpp new file mode 100644 index 0000000..c6f8869 --- /dev/null +++ b/libs/usd-pointcloud-authoring/src/GeneratedPayloadSet.cpp @@ -0,0 +1,320 @@ +#include "GeneratedPayloadSet.h" + +#include "usdgeo/CacheKey.h" + +#include +#include +#include +#include +#include +#include + +namespace usdgeo::detail { +namespace { + +constexpr const char* kRecordFormat = "usd-pointcloud-payload-owner-v1"; + +// A recorded name is only ever joined onto the payload directory, and a +// commit removes names the owner no longer generates, so a name that could +// escape the directory is rejected rather than trusted. +bool IsSafeRelativeName(const std::string& value) { + if (value.empty() || value.front() == '/' || + value.find_first_of("\\:\r\n") != std::string::npos) { + return false; + } + std::size_t start = 0; + for (;;) { + const auto separator = value.find('/', start); + const auto end = + separator == std::string::npos ? value.size() : separator; + const auto component = value.substr(start, end - start); + if (component.empty() || component == "." || component == "..") { + return false; + } + if (separator == std::string::npos) { + return true; + } + start = separator + 1; + } +} + +std::string RelativeName(const std::filesystem::path& directory, + const std::filesystem::path& payload) { + const auto name = payload.lexically_normal() + .lexically_relative(directory.lexically_normal()) + .generic_string(); + return IsSafeRelativeName(name) ? name : std::string(); +} + +bool ReadRecord(const std::filesystem::path& path, + std::set& names, + std::string& error) { + names.clear(); + const auto invalid = [&]() { + names.clear(); + error = "payload ownership record " + path.filename().string() + + " is invalid"; + return false; + }; + std::ifstream input(path, std::ios::binary); + if (!input) { + error = "unable to read payload ownership record " + + path.filename().string(); + return false; + } + + std::optional count; + std::uint64_t lineIndex = 0; + std::string line; + while (std::getline(input, line)) { + const auto separator = line.find('='); + if (separator == std::string::npos) { + return invalid(); + } + const auto key = line.substr(0, separator); + const auto value = line.substr(separator + 1); + if (lineIndex == 0) { + if (key != "format" || value != kRecordFormat) { + return invalid(); + } + } else if (lineIndex == 1) { + std::uint64_t parsed = 0; + const auto* first = value.data(); + const auto* last = value.data() + value.size(); + const auto result = std::from_chars(first, last, parsed); + if (key != "payload.count" || value.empty() || + result.ec != std::errc() || result.ptr != last) { + return invalid(); + } + count = parsed; + } else if (key != "payload." + std::to_string(lineIndex - 2) || + !IsSafeRelativeName(value) || + !names.insert(value).second) { + return invalid(); + } + ++lineIndex; + } + if (input.bad() || !count || names.size() != *count) { + return invalid(); + } + return true; +} + +} // namespace + +GeneratedPayloadSet::GeneratedPayloadSet(std::filesystem::path directory, + std::string owner) + : directory_(directory.lexically_normal()), owner_(std::move(owner)) { + // A trailing separator would leave an empty final element that makes + // payload names relative to it ambiguous. + if (!directory_.has_filename() && directory_.has_relative_path()) { + directory_ = directory_.parent_path(); + } + if (!owner_.empty()) { + record_ = RecordPath(directory_, owner_); + } +} + +GeneratedPayloadSet::~GeneratedPayloadSet() { + Rollback(); +} + +std::filesystem::path GeneratedPayloadSet::RecordPath( + const std::filesystem::path& directory, const std::string& owner) { + return directory / ("payload-owner-" + + StableCacheKey({{"payloadOwner", owner}}) + + ".manifest"); +} + +bool GeneratedPayloadSet::Claim( + const std::vector& payloads, std::string& error, + const AdoptPredicate& adopt) { + if (active_) { + error = "payload set is already claimed"; + return false; + } + std::error_code status; + const auto directoryExists = std::filesystem::exists(directory_, status); + if (!status && directoryExists && + !std::filesystem::is_directory(directory_, status)) { + status = std::make_error_code(std::errc::not_a_directory); + } + if (status) { + error = "payload directory is not a usable directory"; + return false; + } + + std::set names; + for (const auto& payload : payloads) { + const auto name = RelativeName(directory_, payload); + if (name.empty() || !names.insert(name).second) { + error = "payload paths must be unique and inside the payload " + "directory"; + return false; + } + } + + previous_.clear(); + if (directoryExists) { + if (!record_.empty()) { + const auto recordExists = std::filesystem::exists(record_, status); + if (status) { + error = "unable to inspect the payload ownership record"; + return false; + } + if (recordExists && !ReadRecord(record_, previous_, error)) { + return false; + } + } + for (const auto& name : names) { + const auto path = directory_ / std::filesystem::path(name); + const auto exists = std::filesystem::exists(path, status); + if (status) { + error = "unable to inspect payload " + name; + return false; + } + if (!exists) { + continue; + } + if (!std::filesystem::is_regular_file(path, status) || status) { + error = "payload directory entry " + name + + " is not a regular file"; + return false; + } + if (previous_.count(name) == 0 && !(adopt && adopt(path))) { + error = "payload directory already contains " + name + + ", which this layer did not generate"; + return false; + } + } + } else { + std::filesystem::create_directories(directory_, status); + if (status) { + error = "unable to create payload directory"; + return false; + } + directoryCreated_ = true; + } + + claimed_ = std::move(names); + written_.clear(); + active_ = true; + if (!record_.empty()) { + auto claim = previous_; + claim.insert(claimed_.begin(), claimed_.end()); + if (!WriteRecord(claim, error)) { + Rollback(); + return false; + } + recordWritten_ = true; + } + return true; +} + +void GeneratedPayloadSet::MarkWritten(const std::filesystem::path& payload) { + written_.push_back(payload); +} + +bool GeneratedPayloadSet::Commit(std::string& error) { + if (!active_) { + error = "payload set was not claimed"; + return false; + } + if (!record_.empty()) { + // Superseded files go before the record shrinks, so a crash in + // between leaves them listed and still replaceable. + auto record = claimed_; + for (const auto& name : previous_) { + if (claimed_.count(name) != 0) { + continue; + } + std::error_code removal; + std::filesystem::remove(directory_ / std::filesystem::path(name), + removal); + if (removal) { + record.insert(name); + } + } + if (!WriteRecord(record, error)) { + return false; + } + } + active_ = false; + recordWritten_ = false; + directoryCreated_ = false; + written_.clear(); + return true; +} + +void GeneratedPayloadSet::Rollback() { + if (!active_) { + return; + } + active_ = false; + + std::set retained; + for (const auto& payload : written_) { + std::error_code removal; + std::filesystem::remove(payload, removal); + if (removal) { + retained.insert(RelativeName(directory_, payload)); + } + } + written_.clear(); + + if (recordWritten_) { + auto restored = previous_; + restored.insert(retained.begin(), retained.end()); + std::error_code removal; + std::string ignored; + if (restored.empty()) { + std::filesystem::remove(record_, removal); + } else { + WriteRecord(restored, ignored); + } + recordWritten_ = false; + } + + if (directoryCreated_) { + std::error_code status; + if (std::filesystem::is_empty(directory_, status) && !status) { + std::filesystem::remove(directory_, status); + } + directoryCreated_ = false; + } +} + +bool GeneratedPayloadSet::WriteRecord(const std::set& names, + std::string& error) const { + std::ostringstream text; + text << "format=" << kRecordFormat << '\n' + << "payload.count=" << names.size() << '\n'; + std::size_t index = 0; + for (const auto& name : names) { + text << "payload." << index++ << '=' << name << '\n'; + } + + const std::filesystem::path temporary(record_.string() + ".tmp"); + { + std::ofstream output(temporary, std::ios::binary | std::ios::trunc); + output << text.str(); + output.close(); + if (!output) { + std::error_code removal; + std::filesystem::remove(temporary, removal); + error = "unable to write payload ownership record"; + return false; + } + } + std::error_code rename; + std::filesystem::rename(temporary, record_, rename); + if (rename) { + std::error_code removal; + std::filesystem::remove(temporary, removal); + error = "unable to publish payload ownership record"; + return false; + } + return true; +} + +} // namespace usdgeo::detail diff --git a/libs/usd-pointcloud-authoring/src/GeneratedPayloadSet.h b/libs/usd-pointcloud-authoring/src/GeneratedPayloadSet.h new file mode 100644 index 0000000..9220b35 --- /dev/null +++ b/libs/usd-pointcloud-authoring/src/GeneratedPayloadSet.h @@ -0,0 +1,72 @@ +#pragma once + +#include +#include +#include +#include +#include + +namespace usdgeo::detail { + +// The payload files one generation writes into a payload directory, and the +// rule that decides which existing files it may replace. +// +// Without an owner the directory is exclusive: any existing target path is a +// conflict. With an owner, the generation may replace exactly the files the +// same owner generated before, as listed in that owner's record inside the +// directory; any other existing file is still a conflict and is never +// touched. Ownership is recorded before the first write, so an interrupted +// generation leaves files the next generation for the same owner replaces. +// +// The record is named by a hash of the owner and lists relative payload +// names only, so neither the owner string nor a source identifier is +// persisted. +class GeneratedPayloadSet final { +public: + GeneratedPayloadSet(std::filesystem::path directory, std::string owner); + ~GeneratedPayloadSet(); + + GeneratedPayloadSet(const GeneratedPayloadSet&) = delete; + GeneratedPayloadSet& operator=(const GeneratedPayloadSet&) = delete; + + using AdoptPredicate = std::function; + + // Creates the directory when needed, refuses any planned path that + // exists without belonging to this owner, and records the claim. An + // existing file the owner does not hold is claimed anyway only when + // `adopt` accepts it, which a caller does when it can prove the file + // already holds exactly the bytes it would write. + bool Claim(const std::vector& payloads, + std::string& error, const AdoptPredicate& adopt = {}); + + // Notes that this generation wrote one claimed payload. + void MarkWritten(const std::filesystem::path& payload); + + // Removes files the owner generated before but this generation did not, + // then records exactly the claimed payloads. + bool Commit(std::string& error); + + // Removes what this generation wrote, restores the owner's previous + // record, and removes the directory when this generation created it and + // left it empty. Does nothing after Commit. + void Rollback(); + + static std::filesystem::path RecordPath( + const std::filesystem::path& directory, const std::string& owner); + +private: + bool WriteRecord(const std::set& names, + std::string& error) const; + + std::filesystem::path directory_; + std::string owner_; + std::filesystem::path record_; + std::set previous_; + std::set claimed_; + std::vector written_; + bool directoryCreated_ = false; + bool recordWritten_ = false; + bool active_ = false; +}; + +} // namespace usdgeo::detail diff --git a/libs/usd-pointcloud-authoring/src/PointCloudCache.cpp b/libs/usd-pointcloud-authoring/src/PointCloudCache.cpp index 71bfef8..c1fd1fb 100644 --- a/libs/usd-pointcloud-authoring/src/PointCloudCache.cpp +++ b/libs/usd-pointcloud-authoring/src/PointCloudCache.cpp @@ -1,13 +1,19 @@ #include "usdgeo/PointCloudCache.h" +#include "usdgeo/PointCloudLayer.h" #include "usdpointcloud/Tiling.h" +#include "GeneratedPayloadSet.h" + #include #include +#include +#include #include #include +#include #include -#include +#include #include #include @@ -162,14 +168,73 @@ bool ValidateCachedPayloads( return valid; } +bool SameFileContent(const std::filesystem::path& left, + const std::filesystem::path& right) { + std::error_code error; + const auto leftSize = std::filesystem::file_size(left, error); + if (error) { + return false; + } + const auto rightSize = std::filesystem::file_size(right, error); + if (error || leftSize != rightSize) { + return false; + } + std::ifstream leftStream(left, std::ios::binary); + std::ifstream rightStream(right, std::ios::binary); + if (!leftStream || !rightStream) { + return false; + } + std::array leftBuffer{}; + std::array rightBuffer{}; + while (leftStream && rightStream) { + leftStream.read(leftBuffer.data(), leftBuffer.size()); + rightStream.read(rightBuffer.data(), rightBuffer.size()); + const auto count = leftStream.gcount(); + if (count != rightStream.gcount() || + !std::equal(leftBuffer.data(), leftBuffer.data() + count, + rightBuffer.data())) { + return false; + } + } + return leftStream.eof() && rightStream.eof(); +} + +// Copies beside the target and renames over it, so a payload another stage +// still has open keeps the bytes it opened instead of being truncated. +bool ReplaceWithCopy(const std::filesystem::path& source, + const std::filesystem::path& target) { + const std::filesystem::path temporary(target.string() + ".tmp"); + std::error_code error; + if (!std::filesystem::copy_file( + source, temporary, + std::filesystem::copy_options::overwrite_existing, error) || + error) { + std::error_code removal; + std::filesystem::remove(temporary, removal); + return false; + } + std::filesystem::rename(temporary, target, error); + if (error) { + std::error_code removal; + std::filesystem::remove(temporary, removal); + return false; + } + return true; +} + +// Copies the cached payloads into the layer's payload directory under the +// same ownership rule as generation: files the owner placed before are +// replaced when their bytes differ, and a file someone else placed is only +// taken over when it already holds the cached bytes. bool MaterializePayloads( const pxr::SdfLayerHandle& layer, const usdgeo::cache::Layout& layout, - const std::filesystem::path& targetDirectory) { + const std::filesystem::path& targetDirectory, + const std::string& owner) { const auto sourceDirectory = layout.payloadDirectory.lexically_normal(); const auto normalizedTarget = targetDirectory.lexically_normal(); - std::vector> files; + std::map files; bool valid = true; layer->Traverse(pxr::SdfPath::AbsoluteRootPath(), [&](const pxr::SdfPath& path) { @@ -201,7 +266,7 @@ bool MaterializePayloads( valid = false; return; } - files.emplace_back(sourcePath, targetPath); + files.emplace(targetPath, sourcePath); } }; collect(payloads.GetExplicitItems()); @@ -213,37 +278,39 @@ bool MaterializePayloads( return false; } - std::set createdFiles; - std::error_code error; - for (const auto& [sourcePath, targetPath] : files) { - static_cast(sourcePath); - const auto targetExists = std::filesystem::exists(targetPath, error); - if (error || (targetExists && - !std::filesystem::is_regular_file(targetPath, error)) || - error) { - valid = false; - break; + std::vector targets; + targets.reserve(files.size()); + for (const auto& entry : files) { + targets.push_back(entry.first); + } + detail::GeneratedPayloadSet materialized(normalizedTarget, owner); + std::string error; + const auto holdsCachedBytes = [&files](const std::filesystem::path& target) { + const auto found = files.find(target.lexically_normal()); + return found != files.end() && SameFileContent(found->second, target); + }; + if (!materialized.Claim(targets, error, holdsCachedBytes)) { + return false; + } + for (const auto& [targetPath, sourcePath] : files) { + std::error_code status; + const auto targetExists = std::filesystem::exists(targetPath, status); + if (status) { + return false; } - if (targetExists) { + if (targetExists && SameFileContent(sourcePath, targetPath)) { continue; } - std::filesystem::create_directories(targetPath.parent_path(), error); - if (error || !std::filesystem::copy_file( - sourcePath, targetPath, - std::filesystem::copy_options::none, error) || - error) { - valid = false; - break; + std::filesystem::create_directories(targetPath.parent_path(), status); + if (status) { + return false; } - createdFiles.insert(targetPath); - } - if (!valid) { - for (const auto& path : createdFiles) { - std::filesystem::remove(path, error); - error.clear(); + materialized.MarkWritten(targetPath); + if (!ReplaceWithCopy(sourcePath, targetPath)) { + return false; } } - return valid; + return materialized.Commit(error); } void RebasePayloads(pxr::SdfLayer* layer, @@ -345,38 +412,15 @@ bool TryBuildPointCloudCacheLayout( return true; } -bool TryLoadPointCloudCache( - pxr::SdfLayer* layer, - const std::filesystem::path& sourcePath, - const GeoReference& reference, - const usdpointcloud::PointReadRequest& request, - const std::string& parserVersion, - bool& hit, - std::string& errorMessage, - cache::CacheDecision* decision) { - hit = false; - if (!layer) { - errorMessage = "cache lookup requires a writable layer"; - return false; - } - if (PointCloudCacheRootFromEnvironment().empty()) { - return true; - } - - usdgeo::cache::SourceIdentity sourceIdentity; - if (!usdgeo::cache::TryBuildLocalSourceIdentity( - sourcePath, sourceIdentity, errorMessage)) { - return false; - } - return TryLoadPointCloudCache( - layer, sourceIdentity, sourcePath.parent_path(), reference, request, - parserVersion, hit, errorMessage, decision); -} +namespace { -bool TryLoadPointCloudCache( +// `payloadOwner` is the identity materialized payloads are recorded under, so +// a later generation for the same layer can replace them. +bool LoadPointCloudCache( pxr::SdfLayer* layer, const cache::SourceIdentity& sourceIdentity, const std::filesystem::path& payloadBaseDirectory, + const std::string& payloadOwner, const GeoReference& reference, const usdpointcloud::PointReadRequest& request, const std::string& parserVersion, @@ -446,8 +490,9 @@ bool TryLoadPointCloudCache( std::error_code error; targetPayloadDirectory = std::filesystem::absolute(targetPayloadDirectory, error); - if (error || !MaterializePayloads( - cachedLayer, layout, targetPayloadDirectory)) { + if (error || !MaterializePayloads(cachedLayer, layout, + targetPayloadDirectory, + payloadOwner)) { return true; } } @@ -464,4 +509,51 @@ bool TryLoadPointCloudCache( return true; } +} // namespace + +bool TryLoadPointCloudCache( + pxr::SdfLayer* layer, + const std::filesystem::path& sourcePath, + const GeoReference& reference, + const usdpointcloud::PointReadRequest& request, + const std::string& parserVersion, + bool& hit, + std::string& errorMessage, + cache::CacheDecision* decision) { + hit = false; + if (!layer) { + errorMessage = "cache lookup requires a writable layer"; + return false; + } + if (PointCloudCacheRootFromEnvironment().empty()) { + return true; + } + + usdgeo::cache::SourceIdentity sourceIdentity; + if (!usdgeo::cache::TryBuildLocalSourceIdentity( + sourcePath, sourceIdentity, errorMessage)) { + return false; + } + return LoadPointCloudCache( + layer, sourceIdentity, sourcePath.parent_path(), + PointCloudPayloadOwner(sourcePath.string(), request), reference, + request, parserVersion, hit, errorMessage, decision); +} + +bool TryLoadPointCloudCache( + pxr::SdfLayer* layer, + const cache::SourceIdentity& sourceIdentity, + const std::filesystem::path& payloadBaseDirectory, + const GeoReference& reference, + const usdpointcloud::PointReadRequest& request, + const std::string& parserVersion, + bool& hit, + std::string& errorMessage, + cache::CacheDecision* decision) { + return LoadPointCloudCache( + layer, sourceIdentity, payloadBaseDirectory, + PointCloudPayloadOwner(sourceIdentity.identifier, request), reference, + request, parserVersion, hit, errorMessage, decision); +} + } // namespace usdgeo diff --git a/libs/usd-pointcloud-authoring/src/PointCloudLayer.cpp b/libs/usd-pointcloud-authoring/src/PointCloudLayer.cpp index f7c3055..3fbdcf2 100644 --- a/libs/usd-pointcloud-authoring/src/PointCloudLayer.cpp +++ b/libs/usd-pointcloud-authoring/src/PointCloudLayer.cpp @@ -1,7 +1,8 @@ #include "usdgeo/PointCloudLayer.h" +#include "TiledPayloadAuthoring.h" + #include -#include #include #include #include @@ -31,37 +32,6 @@ bool IsValidPrimPath(const std::string& primPath) { return !primPath.empty() && primPath.front() == '/'; } -class ScopedLayerIdentifier final { -public: - ScopedLayerIdentifier(const pxr::SdfLayerHandle& layer, - const std::string& identifier) - : layer_(layer), - originalIdentifier_(layer ? layer->GetIdentifier() : std::string()), - changed_(layer_ && !layer_->IsAnonymous()) { - if (changed_) { - const auto temporaryIdentifier = - identifier + ".usdgeo-authoring-" + - std::to_string(++sequence_); - layer_->SetIdentifier(temporaryIdentifier); - } - } - - ~ScopedLayerIdentifier() { - if (changed_) { - layer_->SetIdentifier(originalIdentifier_); - } - } - - ScopedLayerIdentifier(const ScopedLayerIdentifier&) = delete; - ScopedLayerIdentifier& operator=(const ScopedLayerIdentifier&) = delete; - -private: - pxr::SdfLayerHandle layer_; - std::string originalIdentifier_; - bool changed_ = false; - inline static std::atomic_uint64_t sequence_{0}; -}; - bool SameBounds(const usdgeo::SpatialBounds& left, const usdgeo::SpatialBounds& right) { return left.minimum.x == right.minimum.x && @@ -144,14 +114,21 @@ std::string TilePrimName(const usdpointcloud::PointTileId& id) { "_" + axisName(id.y) + "_" + axisName(id.z); } +// Where a payload-backed LOD root writes its payload files. Every path it +// writes has already been claimed by `payloads`. +struct PayloadTarget { + const std::filesystem::path& directory; + const std::filesystem::path& rootLayerPath; + detail::GeneratedPayloadSet& payloads; +}; + bool AuthorLodRoot( const pxr::UsdStageRefPtr& stage, const std::string& primPath, const std::vector& levels, const usdpointcloud::PointLodHierarchy& hierarchy, const pxr::SdfPath& heuristicPath, - const std::filesystem::path* payloadDirectory = nullptr, - const std::filesystem::path* rootLayerPath = nullptr) { + const PayloadTarget* payloadTarget = nullptr) { if (!stage || !IsValidPrimPath(primPath) || levels.size() != hierarchy.items.size()) { return false; @@ -174,7 +151,7 @@ bool AuthorLodRoot( const auto& level = levels[index]; const auto lodName = "LOD" + std::to_string(index); const auto lodPath = primPath + "/" + lodName; - if (payloadDirectory == nullptr) { + if (payloadTarget == nullptr) { if (!AuthorPointCloudAsset(stage, lodPath, level.reference, level.bounds, level.chunk, level.data)) { return false; @@ -182,23 +159,31 @@ bool AuthorLodRoot( continue; } - const auto payloadPath = *payloadDirectory / + const auto payloadPath = payloadTarget->directory / (primPath.substr(primPath.find_last_of('/') + 1) + "_" + lodName + ".usdc"); std::error_code relativeError; const auto payloadIdentifier = std::filesystem::relative( - payloadPath, rootLayerPath->parent_path(), relativeError); + payloadPath, payloadTarget->rootLayerPath.parent_path(), + relativeError); if (relativeError || payloadIdentifier.empty() || payloadIdentifier.is_absolute()) { return false; } - const auto payloadStage = pxr::UsdStage::CreateNew(payloadPath.string()); + // The payload is built in memory and exported, so writing it never + // registers a layer under the payload path. A regeneration can then + // replace a payload this process still has open. + const auto payloadStage = + pxr::UsdStage::CreateInMemory("payload.usdc"); if (!payloadStage || !pxr::UsdGeomXform::Define( payloadStage, pxr::SdfPath("/" + lodName)) || !AuthorPointCloudAsset(payloadStage, "/" + lodName + "/Points", level.reference, level.bounds, level.chunk, - level.data) || - !payloadStage->GetRootLayer()->Save()) { + level.data)) { + return false; + } + payloadTarget->payloads.MarkWritten(payloadPath); + if (!payloadStage->GetRootLayer()->Export(payloadPath.string())) { return false; } const auto lodPrim = pxr::UsdGeomXform::Define( @@ -254,6 +239,39 @@ bool AuthorScreenSizeHeuristic( } // namespace +namespace detail { + +pxr::UsdStageRefPtr CreateDetachedStage() { + return pxr::UsdStage::CreateInMemory(pxr::UsdStage::LoadNone); +} + +std::filesystem::path TilePayloadPath(const std::filesystem::path& directory, + const usdpointcloud::PointTileId& id, + std::size_t lodIndex) { + return directory / + (TilePrimName(id) + "_LOD" + std::to_string(lodIndex) + ".usdc"); +} + +std::vector TilePayloadPaths( + const std::filesystem::path& directory, + const std::vector& tiles) { + std::vector paths; + for (const auto& tile : tiles) { + for (std::size_t index = 0; index < tile.levels.size(); ++index) { + paths.push_back(TilePayloadPath(directory, tile.tile.id, index)); + } + } + return paths; +} + +} // namespace detail + +std::string PointCloudPayloadOwner( + const std::string& resolvedPath, + const usdpointcloud::PointReadRequest& request) { + return resolvedPath + "?" + request.normalizedArguments; +} + pxr::UsdStageRefPtr PointCloudLayer::CreateStage() { return pxr::UsdStage::CreateInMemory(); } @@ -323,7 +341,7 @@ bool AuthorPointCloudAsset( return false; } - const auto stage = PointCloudLayer::CreateStage(); + const auto stage = detail::CreateDetachedStage(); if (!stage) { failure = PointCloudAuthorFailure::StageCreation; return false; @@ -636,7 +654,7 @@ bool AuthorPointCloudMetadata( return false; } - const auto stage = PointCloudLayer::CreateStage(); + const auto stage = detail::CreateDetachedStage(); if (!stage || !pxr::UsdGeomSetStageUpAxis( stage, pxr::TfToken(reference.stageUpAxis)) || !pxr::UsdGeomSetStageMetersPerUnit(stage, 1.0)) { @@ -750,7 +768,7 @@ bool AuthorPointCloudLodAsset( if (!layer || levels.empty() || !levels.front().reference.IsValid()) { return false; } - const auto stage = PointCloudLayer::CreateStage(); + const auto stage = detail::CreateDetachedStage(); if (!stage || !pxr::UsdGeomSetStageUpAxis( stage, pxr::TfToken(levels.front().reference.stageUpAxis)) || !pxr::UsdGeomSetStageMetersPerUnit(stage, 1.0) || @@ -824,12 +842,13 @@ bool AuthorPointCloudTiledAssetWithPayloads( stage, primPath, tiles, options, generatedPayloads); } -bool AuthorPointCloudTiledAssetWithPayloads( - const pxr::UsdStageRefPtr& stage, - const std::string& primPath, - const std::vector& tiles, - const PointCloudPayloadOptions& options, - std::vector& generatedPayloads) { +namespace detail { + +bool AuthorClaimedTilePayloads(const pxr::UsdStageRefPtr& stage, + const std::string& primPath, + const std::vector& tiles, + const PointCloudPayloadOptions& options, + GeneratedPayloadSet& payloads) { if (!stage || !IsValidPrimPath(primPath) || tiles.empty() || options.directory.empty() || options.rootLayerPath.empty()) { return false; @@ -837,59 +856,13 @@ bool AuthorPointCloudTiledAssetWithPayloads( const std::filesystem::path payloadDirectory(options.directory); const std::filesystem::path rootLayerPath(options.rootLayerPath); - const auto rootLayer = stage->GetRootLayer(); - if (!rootLayer) { - return false; - } - std::vector payloadPaths; std::vector manifestEntries; - std::error_code error; - const auto payloadDirectoryExisted = - std::filesystem::exists(payloadDirectory, error); - if (error) { - return false; - } - std::filesystem::create_directories(payloadDirectory, error); - if (error) { - return false; - } - - const auto cleanup = [&payloadPaths, &payloadDirectory, - payloadDirectoryExisted]() { - std::error_code cleanupError; - for (const auto& payloadPath : payloadPaths) { - std::filesystem::remove(payloadPath, cleanupError); - cleanupError.clear(); - } - if (!payloadDirectoryExisted) { - const auto empty = std::filesystem::is_empty( - payloadDirectory, cleanupError); - if (empty && !cleanupError) { - std::filesystem::remove(payloadDirectory, cleanupError); - } - } - }; - for (const auto& tile : tiles) { - const auto tilePath = primPath + "/Tiles/" + TilePrimName(tile.tile.id); - for (std::size_t index = 0; index < tile.levels.size(); ++index) { - const auto payloadPath = payloadDirectory / - (tilePath.substr(tilePath.find_last_of('/') + 1) + "_LOD" + - std::to_string(index) + ".usdc"); - if (std::filesystem::exists(payloadPath)) { - cleanup(); - return false; - } - payloadPaths.push_back(payloadPath); - } - } - std::set tileNames; for (const auto& tile : tiles) { std::vector diagnostics; if (!usdpointcloud::ValidatePointTile(tile.tile, diagnostics) || tile.levels.size() != tile.tile.lod.items.size() || !tileNames.insert(tile.tile.id.ToString()).second) { - cleanup(); return false; } for (std::size_t index = 0; index < tile.levels.size(); ++index) { @@ -897,15 +870,13 @@ bool AuthorPointCloudTiledAssetWithPayloads( const auto& item = tile.tile.lod.items[index]; if (!level.IsValid() || level.chunk.pointCount != item.pointCount || !SameBounds(level.bounds, item.bounds)) { - cleanup(); return false; } - const auto payloadPath = payloadPaths[ - manifestEntries.size()]; + std::error_code error; const auto relativePayloadPath = std::filesystem::relative( - payloadPath, rootLayerPath.parent_path(), error); + TilePayloadPath(payloadDirectory, tile.tile.id, index), + rootLayerPath.parent_path(), error); if (error || relativePayloadPath.empty()) { - cleanup(); return false; } manifestEntries.push_back({ @@ -921,22 +892,20 @@ bool AuthorPointCloudTiledAssetWithPayloads( const usdpointcloud::PointTileManifest tileManifest{manifestEntries}; if (!usdpointcloud::ValidatePointTileManifest( tileManifest, manifestDiagnostics)) { - cleanup(); return false; } } - const ScopedLayerIdentifier scopedRootIdentifier( - rootLayer, rootLayerPath.generic_string()); if (!pxr::UsdGeomXform::Define(stage, pxr::SdfPath(primPath)) || !pxr::UsdGeomScope::Define( stage, pxr::SdfPath(primPath + "/LodHeuristics")) || !pxr::UsdGeomScope::Define( stage, pxr::SdfPath(primPath + "/Tiles"))) { - cleanup(); return false; } + const PayloadTarget payloadTarget{payloadDirectory, rootLayerPath, + payloads}; const auto heuristicPath = pxr::SdfPath(primPath + "/LodHeuristics"); for (const auto& tile : tiles) { @@ -949,18 +918,13 @@ bool AuthorPointCloudTiledAssetWithPayloads( !AuthorScreenSizeHeuristic( stage, tileHeuristicPath, tile.levels.front().reference, tile.tile.bounds, tile.tile.lod.screenSizeThresholds)) { - cleanup(); return false; } if (!AuthorLodRoot(stage, tilePath.GetString(), tile.levels, - tile.tile.lod, tileHeuristicPath, - &payloadDirectory, &rootLayerPath)) { - cleanup(); + tile.tile.lod, tileHeuristicPath, &payloadTarget)) { return false; } } - generatedPayloads.insert(generatedPayloads.end(), payloadPaths.begin(), - payloadPaths.end()); if (options.tileManifestEntries) { options.tileManifestEntries->insert( options.tileManifestEntries->end(), manifestEntries.begin(), @@ -969,6 +933,92 @@ bool AuthorPointCloudTiledAssetWithPayloads( return true; } +} // namespace detail + +namespace { + +bool AuthorGeneratedTilePayloads( + const pxr::UsdStageRefPtr& stage, + const std::string& primPath, + const std::vector& tiles, + const PointCloudPayloadOptions& options, + std::vector& generatedPayloads, + std::string& error) { + if (!stage || !IsValidPrimPath(primPath) || tiles.empty() || + options.directory.empty() || options.rootLayerPath.empty()) { + error = "invalid tiled point-cloud payload request"; + return false; + } + const std::filesystem::path payloadDirectory(options.directory); + const auto payloadPaths = detail::TilePayloadPaths(payloadDirectory, tiles); + detail::GeneratedPayloadSet payloads(payloadDirectory, options.owner); + if (!payloads.Claim(payloadPaths, error)) { + return false; + } + if (!detail::AuthorClaimedTilePayloads(stage, primPath, tiles, options, + payloads)) { + error = "unable to author tiled point-cloud payloads"; + payloads.Rollback(); + return false; + } + if (!payloads.Commit(error)) { + payloads.Rollback(); + return false; + } + generatedPayloads.insert(generatedPayloads.end(), payloadPaths.begin(), + payloadPaths.end()); + return true; +} + +} // namespace + +bool AuthorPointCloudTiledAssetWithPayloads( + const pxr::UsdStageRefPtr& stage, + const std::string& primPath, + const std::vector& tiles, + const PointCloudPayloadOptions& options, + std::vector& generatedPayloads) { + std::string error; + return AuthorGeneratedTilePayloads(stage, primPath, tiles, options, + generatedPayloads, error); +} + +bool AuthorPointCloudTiledAssetWithPayloads( + pxr::SdfLayer* layer, + const std::string& primPath, + const std::vector& tiles, + const PointCloudPayloadOptions& options, + std::vector& diagnostics) { + diagnostics.clear(); + const auto fail = [&diagnostics](DiagnosticCode code, + const std::string& message) { + diagnostics.push_back( + {code, Severity::Error, message, std::nullopt, std::nullopt}); + return false; + }; + if (!layer || tiles.empty() || tiles.front().levels.empty() || + !tiles.front().levels.front().reference.IsValid()) { + return fail(DiagnosticCode::InvalidPointTile, + "invalid tiled point-cloud authoring request"); + } + const auto stage = detail::CreateDetachedStage(); + if (!stage || !pxr::UsdGeomSetStageUpAxis( + stage, pxr::TfToken( + tiles.front().levels.front().reference.stageUpAxis)) || + !pxr::UsdGeomSetStageMetersPerUnit(stage, 1.0)) { + return fail(DiagnosticCode::DecodeFailure, + "unable to create tiled point-cloud stage"); + } + std::vector generatedPayloads; + std::string error; + if (!AuthorGeneratedTilePayloads(stage, primPath, tiles, options, + generatedPayloads, error)) { + return fail(DiagnosticCode::DecodeFailure, error); + } + layer->TransferContent(stage->GetRootLayer()); + return true; +} + bool AuthorPointCloudTiledAsset( pxr::SdfLayer* layer, const std::string& primPath, @@ -977,7 +1027,7 @@ bool AuthorPointCloudTiledAsset( !tiles.front().levels.front().reference.IsValid()) { return false; } - const auto stage = PointCloudLayer::CreateStage(); + const auto stage = detail::CreateDetachedStage(); if (!stage || !pxr::UsdGeomSetStageUpAxis( stage, pxr::TfToken( tiles.front().levels.front().reference.stageUpAxis)) || diff --git a/libs/usd-pointcloud-authoring/src/PointCloudStreaming.cpp b/libs/usd-pointcloud-authoring/src/PointCloudStreaming.cpp index af2371d..947b729 100644 --- a/libs/usd-pointcloud-authoring/src/PointCloudStreaming.cpp +++ b/libs/usd-pointcloud-authoring/src/PointCloudStreaming.cpp @@ -1,8 +1,9 @@ #include "usdgeo/PointCloudLayer.h" +#include "TiledPayloadAuthoring.h" + #include #include -#include #include #include #include @@ -232,8 +233,6 @@ struct TileSpool { std::unique_ptr writer; }; -std::atomic_uint64_t streamLayerSequence{0}; - } // namespace bool AuthorPointCloudTiledAssetFromStream( @@ -257,16 +256,12 @@ bool AuthorPointCloudTiledAssetFromStream( if (spoolDirectory.empty()) return false; std::map spools; std::size_t bufferedBytes = 0; - std::vector generatedPayloads; + detail::GeneratedPayloadSet payloads(options.directory, options.owner); const auto cleanup = [&]() { for (auto& entry : spools) entry.second.writer.reset(); std::vector cleanupDiagnostics; usdpointcloud::RemoveSpoolDirectory(spoolDirectory, cleanupDiagnostics); - std::error_code cleanupError; - for (const auto& payloadPath : generatedPayloads) { - std::filesystem::remove(payloadPath, cleanupError); - cleanupError.clear(); - } + payloads.Rollback(); }; const auto isCancelled = [&]() { return options.isCancelled && options.isCancelled(); @@ -408,8 +403,29 @@ bool AuthorPointCloudTiledAssetFromStream( return false; } } + if (spools.empty()) { + AddError(diagnostics, DiagnosticCode::DecodeFailure, + "point stream did not produce any points"); + cleanup(); + return false; + } + + // The tile set is known once the stream is spooled, so every payload + // path is claimed before the first payload is written. + std::vector payloadPaths; + payloadPaths.reserve(spools.size()); + for (const auto& entry : spools) { + payloadPaths.push_back( + detail::TilePayloadPath(options.directory, entry.second.id, 0)); + } + std::string payloadError; + if (!payloads.Claim(payloadPaths, payloadError)) { + AddError(diagnostics, DiagnosticCode::DecodeFailure, payloadError); + cleanup(); + return false; + } - const auto stage = PointCloudLayer::CreateStage(); + const auto stage = detail::CreateDetachedStage(); if (!stage || !pxr::UsdGeomSetStageUpAxis( stage, pxr::TfToken(reference.stageUpAxis)) || !pxr::UsdGeomSetStageMetersPerUnit(stage, 1.0)) { @@ -418,12 +434,7 @@ bool AuthorPointCloudTiledAssetFromStream( cleanup(); return false; } - const auto streamLayerIdentifier = - options.rootLayerPath + ".usdgeo-stream-" + - std::to_string(++streamLayerSequence); - stage->GetRootLayer()->SetIdentifier(streamLayerIdentifier); - std::size_t tileCount = 0; for (const auto& entry : spools) { if (isCancelled()) { AddError(diagnostics, DiagnosticCode::DecodeFailure, @@ -441,6 +452,7 @@ bool AuthorPointCloudTiledAssetFromStream( usdpointcloud::SpoolSchema tileSchema; if (!reader.Open(entry.second.path, tileId, tileSchema, diagnostics, options.spoolIoStats) || + tileId.ToString() != entry.first || !SameSchema(schema, tileSchema)) { AddError(diagnostics, DiagnosticCode::DecodeFailure, "unable to reopen point tile spool"); @@ -497,8 +509,8 @@ bool AuthorPointCloudTiledAssetFromStream( tile.levels.push_back(std::move(asset)); std::vector singleTile; singleTile.push_back(std::move(tile)); - if (!AuthorPointCloudTiledAssetWithPayloads( - stage, primPath, singleTile, options, generatedPayloads)) { + if (!detail::AuthorClaimedTilePayloads( + stage, primPath, singleTile, options, payloads)) { AddError(diagnostics, DiagnosticCode::DecodeFailure, "unable to author tiled point-cloud payloads"); return failTile(); @@ -508,13 +520,6 @@ bool AuthorPointCloudTiledAssetFromStream( "point-cloud authoring cancelled"); return failTile(); } - ++tileCount; - } - if (tileCount == 0) { - AddError(diagnostics, DiagnosticCode::DecodeFailure, - "point stream did not produce any points"); - cleanup(); - return false; } std::vector cleanupDiagnostics; if (!usdpointcloud::RemoveSpoolDirectory(spoolDirectory, @@ -524,6 +529,11 @@ bool AuthorPointCloudTiledAssetFromStream( cleanup(); return false; } + if (!payloads.Commit(payloadError)) { + AddError(diagnostics, DiagnosticCode::DecodeFailure, payloadError); + cleanup(); + return false; + } layer->TransferContent(stage->GetRootLayer()); return true; } diff --git a/libs/usd-pointcloud-authoring/src/TiledPayloadAuthoring.h b/libs/usd-pointcloud-authoring/src/TiledPayloadAuthoring.h new file mode 100644 index 0000000..7bcc428 --- /dev/null +++ b/libs/usd-pointcloud-authoring/src/TiledPayloadAuthoring.h @@ -0,0 +1,36 @@ +#pragma once + +#include "GeneratedPayloadSet.h" + +#include "usdgeo/PointCloudLayer.h" + +#include +#include +#include +#include + +namespace usdgeo::detail { + +// An in-memory stage whose content is built for transfer into a caller's +// layer. It loads nothing, so a payload arc authored into it is never +// resolved or opened while the content is built, and it never needs a file +// identity to anchor one. +pxr::UsdStageRefPtr CreateDetachedStage(); + +std::filesystem::path TilePayloadPath(const std::filesystem::path& directory, + const usdpointcloud::PointTileId& id, + std::size_t lodIndex); + +std::vector TilePayloadPaths( + const std::filesystem::path& directory, + const std::vector& tiles); + +// Authors payload-backed tiles whose payload paths `payloads` has already +// claimed, writing each payload file and marking it written. +bool AuthorClaimedTilePayloads(const pxr::UsdStageRefPtr& stage, + const std::string& primPath, + const std::vector& tiles, + const PointCloudPayloadOptions& options, + GeneratedPayloadSet& payloads); + +} // namespace usdgeo::detail diff --git a/libs/usd-pointcloud-authoring/tests/test_pointcloud_cache.cpp b/libs/usd-pointcloud-authoring/tests/test_pointcloud_cache.cpp index c1b2e1c..b130950 100644 --- a/libs/usd-pointcloud-authoring/tests/test_pointcloud_cache.cpp +++ b/libs/usd-pointcloud-authoring/tests/test_pointcloud_cache.cpp @@ -1,5 +1,7 @@ #include "usdgeo/PointCloudCache.h" +#include "usdgeo/CacheKey.h" +#include "usdgeo/PointCloudLayer.h" #include "usdgeo/cache/Cache.h" #include "usdpointcloud/FileFormatArguments.h" @@ -30,6 +32,13 @@ void Check(bool condition) { } } +std::string ReadBytes(const std::filesystem::path& path) { + std::ifstream input(path, std::ios::binary); + Check(input.good()); + return {std::istreambuf_iterator(input), + std::istreambuf_iterator()}; +} + std::string FormatDouble(double value) { std::ostringstream result; result << std::setprecision(17) << value; @@ -208,15 +217,59 @@ void TestPointCloudCacheMissAndMaterialization() { Check(payloadItems.front().GetAssetPath() == "../requested_payloads/tile.usdc"); - const auto requestedStage = pxr::UsdStage::Open(requestedLayer); - Check(requestedStage); - const auto requestedPoints = pxr::UsdGeomPoints::Get( - requestedStage, pxr::SdfPath("/PointCloud/Tile/Points")); - Check(requestedPoints.GetPrim().IsValid()); - pxr::VtVec3fArray requestedPositions; - Check(requestedPoints.GetPointsAttr().Get(&requestedPositions)); - Check(requestedPositions.size() == 1 && - requestedPositions[0] == pxr::GfVec3f(1.0f, 2.0f, 3.0f)); + { + const auto requestedStage = pxr::UsdStage::Open(requestedLayer); + Check(requestedStage); + const auto requestedPoints = pxr::UsdGeomPoints::Get( + requestedStage, pxr::SdfPath("/PointCloud/Tile/Points")); + Check(requestedPoints.GetPrim().IsValid()); + pxr::VtVec3fArray requestedPositions; + Check(requestedPoints.GetPointsAttr().Get(&requestedPositions)); + Check(requestedPositions.size() == 1 && + requestedPositions[0] == pxr::GfVec3f(1.0f, 2.0f, 3.0f)); + } + + // Materialized payloads belong to the layer they were materialized for: + // a later hit replaces them when they are stale, and never overwrites a + // file someone else placed. + const auto requestedOwnerRecord = + requestedPayload.parent_path() / + ("payload-owner-" + + usdgeo::StableCacheKey( + {{"payloadOwner", usdgeo::PointCloudPayloadOwner( + resolverIdentity.identifier, request)}}) + + ".manifest"); + Check(std::filesystem::exists(requestedOwnerRecord)); + const auto cachedPayloadBytes = ReadBytes(payloadPath); + const auto loadRequested = [&]() { + const auto layer = + pxr::SdfLayer::CreateAnonymous("requested-again.usda"); + bool loadedHit = false; + std::string loadError; + Check(usdgeo::TryLoadPointCloudCache( + layer.operator->(), resolverIdentity, sourcePath.parent_path(), + reference, request, "las-reader-1", loadedHit, loadError)); + Check(loadError.empty()); + return loadedHit; + }; + std::ofstream(requestedPayload, std::ios::binary | std::ios::trunc) + << "stale payload"; + Check(loadRequested()); + Check(ReadBytes(requestedPayload) == cachedPayloadBytes); + + std::filesystem::remove(requestedOwnerRecord); + std::ofstream(requestedPayload, std::ios::binary | std::ios::trunc) + << "user payload"; + Check(!loadRequested()); + Check(ReadBytes(requestedPayload) == "user payload"); + + // A file that already holds the cached bytes is taken over, so payloads + // materialized before ownership was recorded keep resolving. + Check(std::filesystem::copy_file( + payloadPath, requestedPayload, + std::filesystem::copy_options::overwrite_existing)); + Check(loadRequested()); + Check(std::filesystem::exists(requestedOwnerRecord)); std::error_code error; std::filesystem::remove(layout.manifest, error); diff --git a/libs/usd-pointcloud-authoring/tests/test_usd_authoring.cpp b/libs/usd-pointcloud-authoring/tests/test_usd_authoring.cpp index 1a318d6..8e34d3d 100644 --- a/libs/usd-pointcloud-authoring/tests/test_usd_authoring.cpp +++ b/libs/usd-pointcloud-authoring/tests/test_usd_authoring.cpp @@ -1,8 +1,10 @@ +#include "usdgeo/CacheKey.h" #include "usdgeo/PointCloudLayer.h" #include "usdcopc/Copc.h" #include #include +#include #include #include #include @@ -13,10 +15,13 @@ #include #include +#include +#include #include #include #include #include +#include #include #include #include @@ -36,6 +41,62 @@ std::string ReadFile(const std::filesystem::path& path) { std::istreambuf_iterator()}; } +void WriteFile(const std::filesystem::path& path, const std::string& text) { + std::ofstream output(path, std::ios::binary | std::ios::trunc); + output << text; + Check(output.good()); +} + +// A directory no other test or earlier run shares, so payload ownership +// state never leaks between cases. +std::filesystem::path UniqueTestDirectory(const std::string& name) { + const auto stamp = + std::chrono::steady_clock::now().time_since_epoch().count(); + const auto directory = std::filesystem::temp_directory_path() / + (name + "_" + std::to_string(stamp)); + std::filesystem::remove_all(directory); + return directory; +} + +std::filesystem::path OwnerRecordPath(const std::filesystem::path& directory, + const std::string& owner) { + return directory / ("payload-owner-" + + usdgeo::StableCacheKey({{"payloadOwner", owner}}) + + ".manifest"); +} + +std::string OwnerRecord(const std::vector& payloads) { + std::string record = "format=usd-pointcloud-payload-owner-v1\n" + "payload.count=" + + std::to_string(payloads.size()) + "\n"; + for (std::size_t index = 0; index < payloads.size(); ++index) { + record += "payload." + std::to_string(index) + "=" + + payloads[index] + "\n"; + } + return record; +} + +class WarningCounter final : public pxr::TfDiagnosticMgr::Delegate { +public: + WarningCounter() { pxr::TfDiagnosticMgr::GetInstance().AddDelegate(this); } + ~WarningCounter() override { + pxr::TfDiagnosticMgr::GetInstance().RemoveDelegate(this); + } + + void IssueError(const pxr::TfError&) override {} + void IssueFatalError(const pxr::TfCallContext&, + const std::string&) override { + _UnhandledAbort(); + } + void IssueStatus(const pxr::TfStatus&) override {} + void IssueWarning(const pxr::TfWarning&) override { ++count_; } + + int Count() const noexcept { return count_; } + +private: + std::atomic_int count_{0}; +}; + std::map ReadDirectoryFiles( const std::filesystem::path& directory) { std::map files; @@ -1169,6 +1230,251 @@ void TestStreamPayloadFailureRollsBackGeneratedPayloads() { std::filesystem::remove_all(payloadDirectory); } +std::pair MakeTestChunk( + const std::vector& positions) { + usdpointcloud::PointData data; + data.positions = positions; + usdgeo::SpatialBounds bounds = usdgeo::SpatialBounds::Empty(); + for (const auto& position : positions) bounds.Expand(position); + return {usdpointcloud::MakePointChunk(data, bounds), std::move(data)}; +} + +// Streams `positions` into unit fixed-grid tiles as `owner`'s payloads. +bool AuthorOwnedStream(pxr::SdfLayer* layer, + const std::filesystem::path& directory, + const std::string& owner, + const std::vector& positions, + std::vector& diagnostics, + std::function isCancelled = {}) { + usdgeo::GeoReference reference; + reference.epsgCode = 26910; + TestPointStream stream({MakeTestChunk(positions)}); + usdgeo::PointCloudPayloadOptions options{ + directory.string(), (directory / "PointCloud.usda").string(), 1, + std::move(isCancelled)}; + options.owner = owner; + return usdgeo::AuthorPointCloudTiledAssetFromStream( + layer, "/PointCloud", stream, reference, {1.0, 0}, options, + diagnostics); +} + +void TestOwnedPayloadsAreRegeneratedByTheirOwner() { + const auto directory = UniqueTestDirectory("usd_geo_owned_payloads"); + const auto first = directory / "Tile_L0_p0_p0_p0_LOD0.usdc"; + const auto second = directory / "Tile_L0_p1_p0_p0_LOD0.usdc"; + const auto record = OwnerRecordPath(directory, "layer-a"); + std::vector diagnostics; + + const auto layer = pxr::SdfLayer::CreateAnonymous("owned.usda"); + Check(AuthorOwnedStream(layer.operator->(), directory, "layer-a", + {{0.25, 0.0, 0.0}, {1.25, 0.0, 0.0}}, + diagnostics)); + Check(diagnostics.empty()); + Check(std::filesystem::exists(first) && std::filesystem::exists(second)); + Check(ReadFile(record) == + OwnerRecord({"Tile_L0_p0_p0_p0_LOD0.usdc", + "Tile_L0_p1_p0_p0_LOD0.usdc"})); + + // Reopening the same layer regenerates in place: the payload it still + // produces is replaced, and the one it no longer produces is removed. + const auto reopened = pxr::SdfLayer::CreateAnonymous("reopened.usda"); + Check(AuthorOwnedStream(reopened.operator->(), directory, "layer-a", + {{0.25, 0.0, 0.0}, {0.75, 0.0, 0.0}}, + diagnostics)); + Check(diagnostics.empty()); + Check(std::filesystem::exists(first) && !std::filesystem::exists(second)); + Check(ReadFile(record) == OwnerRecord({"Tile_L0_p0_p0_p0_LOD0.usdc"})); + { + const auto rootLayerPath = directory / "PointCloud.usda"; + Check(reopened->Export(rootLayerPath.string())); + const auto stage = pxr::UsdStage::Open(rootLayerPath.string()); + Check(stage); + const auto points = pxr::UsdGeomPoints::Get( + stage, + pxr::SdfPath("/PointCloud/Tiles/Tile_L0_p0_p0_p0/LOD0/Points")); + pxr::VtVec3fArray positions; + Check(points.GetPointsAttr().Get(&positions)); + Check(positions.size() == 2); + std::filesystem::remove(rootLayerPath); + } + + // A different layer never replaces them, and neither does a generation + // without an owner. + const auto authoredPayload = ReadFile(first); + const auto other = pxr::SdfLayer::CreateAnonymous("other.usda"); + Check(!AuthorOwnedStream(other.operator->(), directory, "layer-b", + {{0.25, 0.0, 0.0}}, diagnostics)); + Check(!diagnostics.empty() && + diagnostics.front().message.find("did not generate") != + std::string::npos); + Check(other->GetPrimAtPath(pxr::SdfPath("/PointCloud")) == nullptr); + Check(!std::filesystem::exists(OwnerRecordPath(directory, "layer-b"))); + Check(!AuthorOwnedStream(other.operator->(), directory, "", + {{0.25, 0.0, 0.0}}, diagnostics)); + Check(ReadFile(first) == authoredPayload); + Check(ReadFile(record) == OwnerRecord({"Tile_L0_p0_p0_p0_LOD0.usdc"})); + + // A file the owner never generated is not claimed on the owner's behalf. + WriteFile(second, "user file"); + Check(!AuthorOwnedStream(reopened.operator->(), directory, "layer-a", + {{0.25, 0.0, 0.0}, {1.25, 0.0, 0.0}}, + diagnostics)); + Check(ReadFile(second) == "user file"); + Check(ReadFile(first) == authoredPayload); + Check(ReadFile(record) == OwnerRecord({"Tile_L0_p0_p0_p0_LOD0.usdc"})); + std::filesystem::remove_all(directory); +} + +void TestInterruptedOwnedGenerationIsReplaceable() { + const auto directory = + UniqueTestDirectory("usd_geo_interrupted_owned_payloads"); + const auto payload = directory / "Tile_L0_p0_p0_p0_LOD0.usdc"; + const auto record = OwnerRecordPath(directory, "layer-a"); + std::filesystem::create_directories(directory); + // What an interrupted generation leaves behind: its claim, recorded + // before the first write, and a payload no root ever referenced. + WriteFile(payload, "partial payload"); + WriteFile(record, OwnerRecord({"Tile_L0_p0_p0_p0_LOD0.usdc", + "Tile_L0_p3_p0_p0_LOD0.usdc"})); + + std::vector diagnostics; + const auto layer = pxr::SdfLayer::CreateAnonymous("recovered.usda"); + Check(AuthorOwnedStream(layer.operator->(), directory, "layer-a", + {{0.25, 0.0, 0.0}}, diagnostics)); + Check(diagnostics.empty()); + Check(ReadFile(payload) != "partial payload"); + Check(ReadFile(record) == OwnerRecord({"Tile_L0_p0_p0_p0_LOD0.usdc"})); + + // A record naming a path outside the directory is refused as a whole + // rather than trusted with a removal. + WriteFile(record, OwnerRecord({"../Tile_L0_p0_p0_p0_LOD0.usdc"})); + const auto authoredPayload = ReadFile(payload); + Check(!AuthorOwnedStream(layer.operator->(), directory, "layer-a", + {{0.25, 0.0, 0.0}}, diagnostics)); + Check(!diagnostics.empty() && + diagnostics.front().message.find("is invalid") != std::string::npos); + Check(ReadFile(payload) == authoredPayload); + std::filesystem::remove_all(directory); +} + +void TestFailedOwnedGenerationRestoresOwnership() { + const auto directory = + UniqueTestDirectory("usd_geo_failed_owned_payloads"); + const auto first = directory / "Tile_L0_p0_p0_p0_LOD0.usdc"; + const auto second = directory / "Tile_L0_p1_p0_p0_LOD0.usdc"; + const auto record = OwnerRecordPath(directory, "layer-a"); + std::vector diagnostics; + + const auto layer = pxr::SdfLayer::CreateAnonymous("failed_owned.usda"); + Check(AuthorOwnedStream(layer.operator->(), directory, "layer-a", + {{0.25, 0.0, 0.0}, {1.25, 0.0, 0.0}}, + diagnostics)); + const auto ownership = ReadFile(record); + const auto untouched = ReadFile(second); + const auto previous = ReadFile(first); + + // Cancel as soon as the regeneration has replaced its first payload. + const auto replacedFirst = [&first, &previous]() { + std::ifstream input(first, std::ios::binary); + const std::string bytes{std::istreambuf_iterator(input), + std::istreambuf_iterator()}; + return bytes != previous; + }; + const auto regenerated = pxr::SdfLayer::CreateAnonymous("cancelled.usda"); + Check(!AuthorOwnedStream(regenerated.operator->(), directory, "layer-a", + {{0.5, 0.0, 0.0}, {1.25, 0.0, 0.0}}, + diagnostics, replacedFirst)); + Check(!diagnostics.empty()); + Check(regenerated->GetPrimAtPath(pxr::SdfPath("/PointCloud")) == nullptr); + Check(!std::filesystem::exists(first)); + Check(ReadFile(second) == untouched); + Check(ReadFile(record) == ownership); + Check(AuthorOwnedStream(regenerated.operator->(), directory, "layer-a", + {{0.25, 0.0, 0.0}, {1.25, 0.0, 0.0}}, + diagnostics)); + std::filesystem::remove_all(directory); + + // A cancelled first generation removes the directory it created, + // ownership record included. + const auto created = UniqueTestDirectory("usd_geo_cancelled_owned_payloads"); + const auto createdPayload = created / "Tile_L0_p0_p0_p0_LOD0.usdc"; + const auto fresh = pxr::SdfLayer::CreateAnonymous("fresh.usda"); + Check(!AuthorOwnedStream( + fresh.operator->(), created, "layer-a", + {{0.25, 0.0, 0.0}, {1.25, 0.0, 0.0}}, diagnostics, + [&createdPayload]() { return std::filesystem::exists(createdPayload); })); + Check(!std::filesystem::exists(created)); +} + +void TestDetachedAuthoringDoesNotResolvePayloads() { + const auto directory = UniqueTestDirectory("usd_geo_detached_payloads"); + const auto payload = directory / "Tile_L0_p0_p0_p0_LOD0.usdc"; + usdgeo::GeoReference reference; + reference.epsgCode = 26910; + + usdgeo::PointCloudTileAsset tile; + tile.tile.id = {0, 0, 0, 0}; + tile.tile.bounds.Expand({0.0, 0.0, 0.0}); + tile.tile.lod.bounds = tile.tile.bounds; + tile.tile.lod.items = {{0, 1, tile.tile.bounds, {0, 1}}}; + usdpointcloud::PointCloudAsset level; + level.reference = reference; + level.bounds = tile.tile.bounds; + level.data.positions = {{0.0, 0.0, 0.0}}; + level.chunk = usdpointcloud::MakePointChunk(level.data, level.bounds); + tile.levels.push_back(std::move(level)); + + // The payload arcs are relative to a root that is not the working + // directory. Building the layer must not try to resolve them: resolving + // from a detached stage is what reported "Could not open asset". + const auto layer = pxr::SdfLayer::CreateAnonymous("detached.usda"); + std::vector diagnostics; + usdgeo::PointCloudPayloadOptions options{ + directory.string(), (directory / "PointCloud.usda").string()}; + options.owner = "detached-layer"; + { + const WarningCounter warnings; + Check(usdgeo::AuthorPointCloudTiledAssetWithPayloads( + layer.operator->(), "/PointCloud", {tile}, options, diagnostics)); + Check(warnings.Count() == 0); + } + Check(diagnostics.empty()); + Check(layer->GetPrimAtPath(pxr::SdfPath( + "/PointCloud/Tiles/Tile_L0_p0_p0_p0/LOD0")) != nullptr); + Check(std::filesystem::exists(payload)); + Check(!pxr::SdfLayer::Find(payload.string())); + + // The same layer regenerates over its own payload. + Check(usdgeo::AuthorPointCloudTiledAssetWithPayloads( + layer.operator->(), "/PointCloud", {tile}, options, diagnostics)); + + // A failure reports why and leaves the layer as it was. + const auto untouched = pxr::SdfLayer::CreateAnonymous("untouched.usda"); + options.owner = "another-layer"; + Check(!usdgeo::AuthorPointCloudTiledAssetWithPayloads( + untouched.operator->(), "/PointCloud", {tile}, options, diagnostics)); + Check(!diagnostics.empty() && + diagnostics.front().message.find("did not generate") != + std::string::npos); + Check(untouched->GetPrimAtPath(pxr::SdfPath("/PointCloud")) == nullptr); + std::filesystem::remove_all(directory); + + const auto streamDirectory = + UniqueTestDirectory("usd_geo_detached_stream_payloads"); + const auto streamLayer = pxr::SdfLayer::CreateAnonymous("stream.usda"); + { + const WarningCounter warnings; + Check(AuthorOwnedStream(streamLayer.operator->(), streamDirectory, + "detached-stream", + {{0.25, 0.0, 0.0}, {1.25, 0.0, 0.0}}, + diagnostics)); + Check(warnings.Count() == 0); + } + Check(!pxr::SdfLayer::Find( + (streamDirectory / "Tile_L0_p0_p0_p0_LOD0.usdc").string())); + std::filesystem::remove_all(streamDirectory); +} + void TestInvalidExtraByteNameDoesNotAuthor() { const auto stage = usdgeo::PointCloudLayer::CreateStage(); usdgeo::GeoReference reference; @@ -1215,5 +1521,9 @@ int main() { TestStreamCancellationDuringSpoolReadCleansSpools(); TestStreamFailureDoesNotMutateLayer(); TestStreamPayloadFailureRollsBackGeneratedPayloads(); + TestOwnedPayloadsAreRegeneratedByTheirOwner(); + TestInterruptedOwnedGenerationIsReplaceable(); + TestFailedOwnedGenerationRestoresOwnership(); + TestDetachedAuthoringDoesNotResolvePayloads(); return 0; } \ No newline at end of file diff --git a/plugins/pointcloud-copc/README.md b/plugins/pointcloud-copc/README.md index 49673f9..c667a82 100644 --- a/plugins/pointcloud-copc/README.md +++ b/plugins/pointcloud-copc/README.md @@ -27,7 +27,10 @@ while decoded points pass through a bounded buffer. Remote COPC is supported only when the active resolver supplies an asset with efficient random-access reads, such as HTTP byte-range support. The plugin does not implement an HTTP client, transport retries, or a network cache. Remote tiled reads require an -absolute local `payloadDirectory`. +absolute local `payloadDirectory`. Reading a tiled layer again replaces the +payloads it generated there and refuses any other file; the directory records +ownership under a hash, so no resolver identifier is written. See +[payload ownership](../../docs/architecture/FILE_FORMAT_ARGUMENTS.md#generated-payload-ownership). Generated-USDC cache lookup is enabled for a stable local filesystem identity and for a `Stable` resolver identity. `Unstable` and `Unavailable` identity diff --git a/plugins/pointcloud-copc/src/UsdGeoCopcFileFormat.cpp b/plugins/pointcloud-copc/src/UsdGeoCopcFileFormat.cpp index 627ed02..5b660f9 100644 --- a/plugins/pointcloud-copc/src/UsdGeoCopcFileFormat.cpp +++ b/plugins/pointcloud-copc/src/UsdGeoCopcFileFormat.cpp @@ -16,7 +16,6 @@ #include #include #include -#include #include #include @@ -572,13 +571,6 @@ bool UsdGeoCopcFileFormat::Read(SdfLayer* layer, return false; } - auto stage = usdgeo::PointCloudLayer::CreateStage(); - if (!stage || !pxr::UsdGeomSetStageUpAxis( - stage, pxr::TfToken(reference.stageUpAxis)) || - !pxr::UsdGeomSetStageMetersPerUnit(stage, 1.0)) { - TF_RUNTIME_ERROR("%s", usdgeocopc::diagnostics::PointCloudAuthorFailed); - return false; - } std::filesystem::path payloadDirectory(request.payloadDirectory); if (payloadDirectory.is_relative()) { if (!IsLocalFileSource(resolvedPath)) { @@ -594,15 +586,22 @@ bool UsdGeoCopcFileFormat::Read(SdfLayer* layer, std::filesystem::path(resolvedPath).parent_path() / payloadDirectory; } - const usdgeo::PointCloudPayloadOptions payloadOptions{ + usdgeo::PointCloudPayloadOptions payloadOptions{ payloadDirectory.string(), resolvedPath, request.tileMemoryLimitBytes}; + payloadOptions.owner = + usdgeo::PointCloudPayloadOwner(resolvedPath, request); if (!usdgeo::AuthorPointCloudTiledAssetWithPayloads( - stage, "/PointCloud", tiles, payloadOptions)) { - TF_RUNTIME_ERROR("%s", usdgeocopc::diagnostics::PointCloudAuthorFailed); + layer, "/PointCloud", tiles, payloadOptions, diagnostics)) { + TF_RUNTIME_ERROR("%s", usdgeocopc::diagnostics::Message( + usdgeocopc::diagnostics::PointCloudAuthorFailed, + "Unable to author tiled COPC point cloud: " + + DiagnosticDetail( + diagnostics, + "tiled authoring failed")) + .c_str()); return false; } - layer->TransferContent(stage->GetRootLayer()); return true; } diff --git a/plugins/pointcloud-copc/tests/test_pointcloud_copc.cpp b/plugins/pointcloud-copc/tests/test_pointcloud_copc.cpp index 3aab50c..6192784 100644 --- a/plugins/pointcloud-copc/tests/test_pointcloud_copc.cpp +++ b/plugins/pointcloud-copc/tests/test_pointcloud_copc.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -22,6 +23,8 @@ #include #include +#include +#include #include #include #include @@ -680,6 +683,65 @@ void TestAuthoredLodEquivalence() { std::filesystem::remove(copcPath, error); } +class WarningCounter final : public pxr::TfDiagnosticMgr::Delegate { +public: + WarningCounter() { pxr::TfDiagnosticMgr::GetInstance().AddDelegate(this); } + ~WarningCounter() override { + pxr::TfDiagnosticMgr::GetInstance().RemoveDelegate(this); + } + + void IssueError(const pxr::TfError&) override {} + void IssueFatalError(const pxr::TfCallContext&, + const std::string&) override { + _UnhandledAbort(); + } + void IssueStatus(const pxr::TfStatus&) override {} + void IssueWarning(const pxr::TfWarning&) override { ++count_; } + + int Count() const noexcept { return count_; } + +private: + std::atomic_int count_{0}; +}; + +void TestTiledReadReopens() { + const auto directory = + std::filesystem::temp_directory_path() / + ("usd_copc_tiled_reopen_" + + std::to_string( + std::chrono::steady_clock::now().time_since_epoch().count())); + std::filesystem::remove_all(directory); + std::filesystem::create_directories(directory); + const auto source = directory / "conformance.copc"; + Check(std::filesystem::copy_file( + std::filesystem::path(USDGEOCOPC_SOURCE_DIR) / "tests" / + "fixtures" / "conformance.copc", + source), + "copy COPC fixture"); + + // The second open reads the layer again once the first is released, over + // the payloads the first open generated. Neither open may resolve those + // payloads while the layer is being built. + const pxr::SdfLayer::FileFormatArguments arguments = { + {"payloadDirectory", "payloads"}, {"tile", "true"}, {"tileSize", "1"}}; + for (int open = 0; open < 2; ++open) { + const WarningCounter warnings; + const auto layer = pxr::SdfLayer::FindOrOpen(source.string(), arguments); + Check(static_cast(layer), "tiled COPC layer opens"); + Check(layer->GetPrimAtPath(pxr::SdfPath("/PointCloud/Tiles")) != nullptr, + "tiled COPC layer authors tiles"); + Check(warnings.Count() == 0, + "tiled COPC read resolved payloads while authoring"); + } + Check(!pxr::SdfLayer::Find(source.string(), arguments), + "tiled COPC layer was released"); + Check(std::filesystem::exists(directory / "payloads" / + "Tile_L0_p0_p0_p0_LOD0.usdc"), + "tiled COPC payload exists"); + std::error_code error; + std::filesystem::remove_all(directory, error); +} + void TestResolverBackedRead() { const auto records = MakeEquivalentRecords(); const auto lazPath = std::filesystem::temp_directory_path() / @@ -981,6 +1043,7 @@ int main() { "plugInfo.json"); TestMetadataIntegration(); TestAuthoredLodEquivalence(); + TestTiledReadReopens(); TestResolverBackedRead(); return 0; } diff --git a/plugins/pointcloud-las/README.md b/plugins/pointcloud-las/README.md index a81908a..e5167af 100644 --- a/plugins/pointcloud-las/README.md +++ b/plugins/pointcloud-las/README.md @@ -63,7 +63,7 @@ canonical map that participates in layer identity. | `tile` | `true` | Routes the pull stream into source-coordinate tile payloads | | `tileSize` | positive source units | Fixed-grid tile width and depth | | `tileMemoryLimit` | positive bytes | Per-tile spool buffer limit | -| `payloadDirectory` | path | Directory for generated USDC payloads | +| `payloadDirectory` | path | Directory for generated USDC payloads; reading the layer again replaces the payloads it generated there and refuses any other file; see [payload ownership](../../docs/architecture/FILE_FORMAT_ARGUMENTS.md#generated-payload-ownership) | ```bash usdcat "sample.las:SDF_FORMAT_ARGS:lod=balanced&attributes=xyz,rgb" diff --git a/plugins/pointcloud-las/src/UsdGeoLasFileFormat.cpp b/plugins/pointcloud-las/src/UsdGeoLasFileFormat.cpp index b8755b4..651c7e9 100644 --- a/plugins/pointcloud-las/src/UsdGeoLasFileFormat.cpp +++ b/plugins/pointcloud-las/src/UsdGeoLasFileFormat.cpp @@ -299,10 +299,18 @@ bool UsdGeoLasFileFormat::Read(SdfLayer* layer, usdgeo::PointCloudPayloadOptions payloadOptions{ payloadDirectory.string(), rootLayerPath, request.tileMemoryLimitBytes}; + payloadOptions.owner = + usdgeo::PointCloudPayloadOwner(resolvedPath, request); if (!usdgeo::AuthorPointCloudTiledAssetFromStream( layer, "/PointCloud", selected, reference, {request.tileSize, 0}, payloadOptions, diagnostics)) { - TF_RUNTIME_ERROR("%s", usdgeolas::diagnostics::PointCloudAuthorFailed); + TF_RUNTIME_ERROR("%s", usdgeolas::diagnostics::Message( + usdgeolas::diagnostics::PointCloudAuthorFailed, + "Unable to author tiled LAS point cloud: " + + DiagnosticDetail( + diagnostics, + "tiled authoring failed")) + .c_str()); return false; } return true; diff --git a/plugins/pointcloud-las/tests/test_pointcloud_las.cpp b/plugins/pointcloud-las/tests/test_pointcloud_las.cpp index 54ab60e..14a700c 100644 --- a/plugins/pointcloud-las/tests/test_pointcloud_las.cpp +++ b/plugins/pointcloud-las/tests/test_pointcloud_las.cpp @@ -203,21 +203,26 @@ void TestFileFormatIntegration() { {"tile", "true"}, {"tileMemoryLimit", "1"}, {"tileSize", "1"}}; - const auto tiledLayer = pxr::SdfLayer::FindOrOpen( - path.string(), tiledArguments); - Check(tiledLayer); - const auto tiledStage = pxr::UsdStage::Open(tiledLayer); - Check(tiledStage); - Check(tiledStage->GetPrimAtPath(pxr::SdfPath( - "/PointCloud/Tiles/Tile_L0_p1000_p2000_p0/LOD0")) - .IsValid()); - Check(tiledStage->GetPrimAtPath(pxr::SdfPath( - "/PointCloud/Tiles/Tile_L0_p1001_p2001_p0/LOD0")) - .IsValid()); - Check(std::filesystem::exists( - tiledPayloadDirectory / "Tile_L0_p1000_p2000_p0_LOD0.usdc")); - Check(std::filesystem::exists( - tiledPayloadDirectory / "Tile_L0_p1001_p2001_p0_LOD0.usdc")); + // The second open reads the layer again once the first is released, over + // the payloads the first open generated. + for (int open = 0; open < 2; ++open) { + const auto tiledLayer = pxr::SdfLayer::FindOrOpen( + path.string(), tiledArguments); + Check(tiledLayer); + const auto tiledStage = pxr::UsdStage::Open(tiledLayer); + Check(tiledStage); + Check(tiledStage->GetPrimAtPath(pxr::SdfPath( + "/PointCloud/Tiles/Tile_L0_p1000_p2000_p0/LOD0")) + .IsValid()); + Check(tiledStage->GetPrimAtPath(pxr::SdfPath( + "/PointCloud/Tiles/Tile_L0_p1001_p2001_p0/LOD0")) + .IsValid()); + Check(std::filesystem::exists( + tiledPayloadDirectory / "Tile_L0_p1000_p2000_p0_LOD0.usdc")); + Check(std::filesystem::exists( + tiledPayloadDirectory / "Tile_L0_p1001_p2001_p0_LOD0.usdc")); + } + Check(!pxr::SdfLayer::Find(path.string(), tiledArguments)); std::filesystem::remove_all(tiledPayloadDirectory); const auto metadataLayer = diff --git a/plugins/pointcloud-laz/README.md b/plugins/pointcloud-laz/README.md index b0c2c6c..cc44bc4 100644 --- a/plugins/pointcloud-laz/README.md +++ b/plugins/pointcloud-laz/README.md @@ -66,7 +66,7 @@ layer-identity participation. | `tile` | `true` | Routes the pull stream into source-coordinate tile payloads | | `tileSize` | positive source units | Fixed-grid tile width and depth | | `tileMemoryLimit` | positive bytes | Per-tile spool buffer limit | -| `payloadDirectory` | path | Directory for generated USDC payloads | +| `payloadDirectory` | path | Directory for generated USDC payloads; reading the layer again replaces the payloads it generated there and refuses any other file; see [payload ownership](../../docs/architecture/FILE_FORMAT_ARGUMENTS.md#generated-payload-ownership) | ```bash usdcat "sample.laz:SDF_FORMAT_ARGS:lod=preview" diff --git a/plugins/pointcloud-laz/src/UsdGeoLazFileFormat.cpp b/plugins/pointcloud-laz/src/UsdGeoLazFileFormat.cpp index 32d7668..40e9ef6 100644 --- a/plugins/pointcloud-laz/src/UsdGeoLazFileFormat.cpp +++ b/plugins/pointcloud-laz/src/UsdGeoLazFileFormat.cpp @@ -279,10 +279,18 @@ bool UsdGeoLazFileFormat::Read(SdfLayer* layer, usdgeo::PointCloudPayloadOptions payloadOptions{ payloadDirectory.string(), rootLayerPath, request.tileMemoryLimitBytes}; + payloadOptions.owner = + usdgeo::PointCloudPayloadOwner(resolvedPath, request); if (!usdgeo::AuthorPointCloudTiledAssetFromStream( layer, "/PointCloud", selected, reference, {request.tileSize, 0}, payloadOptions, diagnostics)) { - TF_RUNTIME_ERROR("%s", usdgeolaz::diagnostics::PointCloudAuthorFailed); + TF_RUNTIME_ERROR("%s", usdgeolaz::diagnostics::Message( + usdgeolaz::diagnostics::PointCloudAuthorFailed, + "Unable to author tiled LAZ point cloud: " + + DiagnosticDetail( + diagnostics, + "tiled authoring failed")) + .c_str()); return false; } return true; diff --git a/plugins/pointcloud-ply/README.md b/plugins/pointcloud-ply/README.md index 14f7dea..f1a7838 100644 --- a/plugins/pointcloud-ply/README.md +++ b/plugins/pointcloud-ply/README.md @@ -15,7 +15,9 @@ contracts, and requires an explicit CRS because PLY has no embedded CRS model. The `epsg` file-format argument is required. `linearUnit`, `sourceUpAxis`, and `stageUpAxis` are optional. Tiled reads use `tileSize`, `tileMemoryLimit`, and -`payloadDirectory`. +`payloadDirectory`; reading the layer again replaces the payloads it generated +there and refuses any other file. See +[payload ownership](../../docs/architecture/FILE_FORMAT_ARGUMENTS.md#generated-payload-ownership). Faces, mesh authoring, PLY writing, and metadata-only reads are out of scope. See the [capability matrix](../../docs/reference/CAPABILITY_MATRIX.md) and diff --git a/plugins/pointcloud-ply/src/UsdGeoPlyFileFormat.cpp b/plugins/pointcloud-ply/src/UsdGeoPlyFileFormat.cpp index 77aa183..4f20f91 100644 --- a/plugins/pointcloud-ply/src/UsdGeoPlyFileFormat.cpp +++ b/plugins/pointcloud-ply/src/UsdGeoPlyFileFormat.cpp @@ -207,6 +207,8 @@ bool UsdGeoPlyFileFormat::Read(SdfLayer* layer, usdgeo::PointCloudPayloadOptions payloadOptions{ payloadDirectory.string(), resolvedPath, request.tileMemoryLimitBytes}; + payloadOptions.owner = + usdgeo::PointCloudPayloadOwner(resolvedPath, request); if (!usdgeo::AuthorPointCloudTiledAssetFromStream( layer, "/PointCloud", selected, reference, {request.tileSize, 0}, payloadOptions, diagnostics)) { From 0080edd60aaeb97a97345b6f78b54544724778ab Mon Sep 17 00:00:00 2001 From: snkmcb Date: Sat, 12 Sep 2026 19:22:15 +0900 Subject: [PATCH 2/2] fix: give each layer its own payload generations The review of the ownership record found that sharing one flat set of payload names between layers let a failed re-read delete payloads a root still referenced, let concurrent reads and stale or adopted records reach another layer's files, collapsed near-equal arguments onto one owner, and required writes on every cache hit. - Publish owned payloads in ///. The owner hashes the source, relative to the payload directory for local files, and the layer's exact file-format arguments, so layers never meet and a relocated project keeps its payloads. - Stage each generation in a unique private directory and publish it by rename. A failure removes only the staging directory, identical content reuses the published generation (g-), and changed content is published beside it before the superseded one is removed. Stale staging left by an interrupted process is swept after an hour. - Materialize cache hits as c-, reusing an intact copy without writing and replacing a damaged one. - Drop the ownership record, adoption, byte comparison, and in-place replacement; derive payload names in one place and append manifest entries only after the generation is published. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 34 +- docs/architecture/FILE_FORMAT_ARGUMENTS.md | 74 +-- docs/reference/CAPABILITY_MATRIX.md | 2 +- docs/roadmap/implementation-status.md | 17 +- docs/roadmap/streaming-and-tiling.md | 13 +- libs/usd-pointcloud-authoring/CMakeLists.txt | 2 +- libs/usd-pointcloud-authoring/README.md | 29 +- .../include/usdgeo/PointCloudLayer.h | 29 +- .../src/GeneratedPayloadSet.cpp | 320 ------------- .../src/GeneratedPayloadSet.h | 72 --- .../src/PayloadGeneration.cpp | 422 ++++++++++++++++++ .../src/PayloadGeneration.h | 84 ++++ .../src/PointCloudCache.cpp | 175 +++----- .../src/PointCloudLayer.cpp | 211 ++++++--- .../src/PointCloudStreaming.cpp | 27 +- .../src/TiledPayloadAuthoring.h | 43 +- .../tests/test_pointcloud_cache.cpp | 73 +-- .../tests/test_usd_authoring.cpp | 250 ++++++----- plugins/pointcloud-copc/README.md | 6 +- .../src/UsdGeoCopcFileFormat.cpp | 4 +- .../tests/test_pointcloud_copc.cpp | 10 +- plugins/pointcloud-las/README.md | 2 +- .../src/UsdGeoLasFileFormat.cpp | 4 +- .../tests/test_pointcloud_las.cpp | 22 +- plugins/pointcloud-laz/README.md | 2 +- .../src/UsdGeoLazFileFormat.cpp | 4 +- plugins/pointcloud-ply/README.md | 4 +- .../src/UsdGeoPlyFileFormat.cpp | 4 +- .../tests/test_pointcloud_ply.cpp | 11 +- 29 files changed, 1134 insertions(+), 816 deletions(-) delete mode 100644 libs/usd-pointcloud-authoring/src/GeneratedPayloadSet.cpp delete mode 100644 libs/usd-pointcloud-authoring/src/GeneratedPayloadSet.h create mode 100644 libs/usd-pointcloud-authoring/src/PayloadGeneration.cpp create mode 100644 libs/usd-pointcloud-authoring/src/PayloadGeneration.h diff --git a/CHANGELOG.md b/CHANGELOG.md index 1dc2658..4808f67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,9 +10,10 @@ All notable changes to this project are documented here. deterministic generated-cache publication and reuse. - `PointCloudPayloadOptions::owner`, `PointCloudPayloadOwner`, and a layer-level `AuthorPointCloudTiledAssetWithPayloads` overload that reports - typed diagnostics. A payload directory records which layer owns the files it - generated in `payload-owner-.manifest`, keyed by a hash of the layer - identity. The rule is in the + typed diagnostics. A tiled FileFormat read publishes its payloads in + `///`, where the owner is a hash of the + source (relative to the payload directory for local files) and the layer's + exact file-format arguments. The layout is in the [file-format argument contract](docs/architecture/FILE_FORMAT_ARGUMENTS.md#generated-payload-ownership). ### Changed @@ -20,32 +21,35 @@ All notable changes to this project are documented here. - Layer authoring builds content in an in-memory stage that loads nothing, and payload files are exported from in-memory layers. No authoring call renames a caller's layer or gives a scratch layer a file identity any more. +- Tiled FileFormat payloads move from `payloadDirectory/Tile_*.usdc` into the + layer's own generation directory below it. Generations are staged privately + and published by rename: identical content reuses the published generation, + changed content is published beside it, and the superseded one is removed. - The COPC adapter authors native-hierarchy tiles through the shared layer-level entry point instead of creating and transferring its own stage. -- A tiled LAS, LAZ, or COPC read that cannot author reports the reason, such - as the payload file it refused to replace, alongside `LAS016`, `LAZ007`, or - `COPC007` instead of the bare code. PLY already did so under `PLY007`. +- A tiled LAS, LAZ, or COPC read that cannot author reports the reason + alongside `LAS016`, `LAZ007`, or `COPC007` instead of the bare code. PLY + already did so under `PLY007`. ### Fixed - Reading a tiled LAS, LAZ, PLY, or COPC layer again, for example in a new process or after the first layer was released, no longer fails because the - payloads the first read generated already exist. The layer replaces the - payloads it owns and still refuses any file it did not generate. + payloads the first read generated already exist. - Tiled COPC reads no longer warn `Could not open asset` for their own payloads when the working directory differs from the source directory. -- Materializing a generated-cache hit replaces a stale payload the layer owns - instead of keeping it, and never overwrites a file someone else placed. +- A generated-cache hit copies its payloads into a generation named by the + entry, reusing an intact copy without writing and replacing a damaged one, + instead of keeping whatever file already sat at the payload path. - The authoring stream benchmark builds on macOS, where `std::uintmax_t` and `std::uint64_t` are distinct types. ### Compatibility -- Payload directories written by earlier tiled reads carry no ownership - record, so a regeneration treats their payloads as foreign and refuses them, - as the earlier release already did. Delete such a directory once and the - next read generates and owns it. Payloads a cache hit materialized earlier - are taken over when they still hold the cached bytes. +- Payload files written by earlier tiled reads directly into + `payloadDirectory` are no longer used or touched; a read publishes a new + generation below them. Delete the old `Tile_*.usdc` files to reclaim the + space. ## [0.11.0] - 2026-08-27 diff --git a/docs/architecture/FILE_FORMAT_ARGUMENTS.md b/docs/architecture/FILE_FORMAT_ARGUMENTS.md index 6e268fe..1396dd1 100644 --- a/docs/architecture/FILE_FORMAT_ARGUMENTS.md +++ b/docs/architecture/FILE_FORMAT_ARGUMENTS.md @@ -141,35 +141,49 @@ point delivery; a miss follows the normal reader and authoring path. ## Generated Payload Ownership -A tiled read writes its payloads into `payloadDirectory` and authors a root +A tiled read writes its payloads below `payloadDirectory` and authors a root that references them by relative path. The root lives only in the opened layer, so every read of the layer writes the payloads again, by generating -them or by materializing a cache hit. Ownership of the files in the directory -is therefore explicit: - -- The owner of a payload set is the layer's identity: the resolved source and - its normalized arguments. The directory records it in - `payload-owner-.manifest`, where `` is a 64-bit hash of that - identity. The record lists relative payload names only; no source path, - resolver identifier, or validation token is written. -- Reading the same layer again replaces the payloads that layer generated - before and removes those it no longer generates. This is what lets a host - reopen or reload a tiled layer. -- A file the layer did not generate is never replaced. When another source, - other arguments, or a user file already holds a payload name, the read fails - with the plugin's author-failure code, names the file, and leaves the - directory unchanged. -- Ownership is recorded before the first payload is written, so an - interrupted read leaves payloads the next read of the same layer replaces. A - read that fails or is cancelled removes the payloads it wrote, restores the - previous record, and removes a directory it created. -- Materializing a committed `USDGEO_CACHE_ROOT` entry into `payloadDirectory` - follows the same rule. An existing file that already holds the cached bytes - is taken over, so payloads materialized before ownership was recorded keep - resolving. - -`usd-pointcloud-convert` does not share this rule. Its output root and payload -directory must not exist yet, and its transaction marker owns recovery. +them or by materializing a cache hit. Each layer therefore owns a directory of +its own: + +```text +/ + / one per layer + g-/ a generated set + c-/ a set copied from a USDGEO_CACHE_ROOT entry + .tmp-/ a set still being written +``` + +- `` is a 64-bit hash of the layer's identity: the source and the + layer's file-format arguments exactly as it holds them. A local source is + named by its path relative to `payloadDirectory`, both canonicalized, so a + project that is moved or mounted elsewhere as a whole keeps its payloads; + any other source is named by its resolved identifier. Only the hash is + written, so no source path, resolver identifier, or validation token + appears on disk. +- Layers never share or replace each other's payloads, and files already in + `payloadDirectory` are never touched. Different sources, arguments, or + spellings of arguments simply get different owners. +- A read writes into a private staging directory and publishes it by renaming + it into place, so a published generation is never modified. A read that + fails or is cancelled removes only its staging directory and leaves the + published generation, and every root referencing it, intact. +- Reading the layer again with the same content finds that generation already + published and keeps it, so nothing that has those payloads open sees them + replaced. Changed content is published as a new generation, and the root + references the new paths. Publishing removes the owner's superseded + generations; a process that still shows a superseded generation keeps what + it has loaded but cannot load more of it. +- A staging directory left by an interrupted process is removed by the next + read of the same layer once it has gone an hour without being written. +- A cache hit whose copy is already published under the entry's key reuses it + without writing anything, so a published payload directory may be + read-only. A damaged copy is replaced by a fresh one. + +`usd-pointcloud-convert` does not share this layout. Its output root and +payload directory must not exist yet, its payloads are written straight into +that directory, and its transaction marker owns recovery. ## Rules @@ -221,6 +235,6 @@ should not silently change authored output. - Conflicting combinations are rejected rather than resolved by precedence. - A default-valued argument and an absent argument produce identical layers. - LAS and LAZ accept the same arguments with the same meaning. -- A tiled layer read a second time regenerates over its own payloads, never - replaces a file another layer or a user placed, and restores its ownership - record when the read fails. +- A tiled layer read a second time reuses or supersedes its own published + payloads, never touches another layer's or a user's files, and leaves its + published payloads intact when the read fails. diff --git a/docs/reference/CAPABILITY_MATRIX.md b/docs/reference/CAPABILITY_MATRIX.md index 0e3bcdf..3df4d72 100644 --- a/docs/reference/CAPABILITY_MATRIX.md +++ b/docs/reference/CAPABILITY_MATRIX.md @@ -198,7 +198,7 @@ reported separately, because they differ. | Bounded-memory generation during file open | Implemented for LAS, LAZ, COPC, and PLY tiled reads; generated-corpus benchmark available | Source streams, spool buffers, and one-tile reconstruction are bounded; generated-corpus and checked-in real-data measurements are available | | LOD file-format arguments | n/a | `lod=off\|preview\|balanced\|quality` | | Spatial tile file-format arguments | n/a | `tile=true`, `tileSize`, `tileMemoryLimit`, `payloadDirectory` for LAS, LAZ, COPC, and PLY | -| Reopening a tiled read over its own payloads | Supported when the caller names a payload owner | Supported: a read replaces the payloads its layer generated before and refuses any file it did not generate; see [payload ownership](../architecture/FILE_FORMAT_ARGUMENTS.md#generated-payload-ownership) | +| Reopening a tiled read over its own payloads | Supported when the caller names a payload owner | Supported: each layer publishes immutable payload generations in a directory of its own, reusing identical content; see [payload ownership](../architecture/FILE_FORMAT_ARGUMENTS.md#generated-payload-ownership) | With `lod=off` every read authors one `UsdGeomPoints` prim at `/PointCloud`. The other profiles author a single non-tiled `usdLod` root with fixed-stride diff --git a/docs/roadmap/implementation-status.md b/docs/roadmap/implementation-status.md index 37ffb8d..abc74f0 100644 --- a/docs/roadmap/implementation-status.md +++ b/docs/roadmap/implementation-status.md @@ -414,13 +414,18 @@ remain be explicit. report `Could not open asset` for their own payloads. - [x] Move COPC native-hierarchy tiled authoring onto a layer-level shared entry point, so no adapter creates a stage or transfers layer content -- [x] Record ownership of FileFormat-generated payloads by layer identity. - Reading a tiled layer again regenerates its own payloads instead of - failing on them, a file it did not generate still refuses the write, and - an interrupted read leaves files the next read replaces; see +- [x] Give every FileFormat-generated payload set a directory owned by its + layer, keyed by the source relative to the payload directory and the + exact file-format arguments. Reading a tiled layer again no longer fails + on its earlier payloads, and never touches another layer's or a user's + files; see [payload ownership](../architecture/FILE_FORMAT_ARGUMENTS.md#generated-payload-ownership) -- [x] Apply the same ownership to generated-cache materialization, replacing - stale owned payloads instead of keeping whatever file is already present +- [x] Stage each payload generation privately and publish it by renaming it + into place, reusing identical content, so a failed, cancelled, or + interrupted read never modifies payloads a root already references +- [x] Materialize generated-cache hits as a generation named by the cache + entry, reusing an intact copy without writing and replacing a damaged + one - [ ] Give tile spools an owned, recoverable working location instead of timestamped directories in the system temporary directory, which an interrupted process leaves behind diff --git a/docs/roadmap/streaming-and-tiling.md b/docs/roadmap/streaming-and-tiling.md index 912a0f9..bbebb2a 100644 --- a/docs/roadmap/streaming-and-tiling.md +++ b/docs/roadmap/streaming-and-tiling.md @@ -271,9 +271,9 @@ The payload writer produces: - a source fingerprint and the generation settings; - tile bounds and point counts; - no partially published root asset after failure; -- explicit ownership of the payload files, so a FileFormat read regenerates - its own payloads when the layer is read again and never replaces a file it - did not generate. +- explicit ownership of the payload files, so a FileFormat read can read the + layer again without colliding with its earlier payloads, another layer's, or + a user's files. The conversion tool writes the versioned tile manifest only after all payloads have been authored successfully. Entries are sorted by tile ID and LOD, and @@ -281,9 +281,10 @@ portable payload paths, duplicate tile/LOD entries, bounds, and point counts are validated before publication. Cache materialization preserves the tile manifest beside the cached payloads. -FileFormat reads record which layer owns the payloads in the directory before -writing the first one, so an interrupted read leaves files the next read of -the same layer replaces; the rule is in the +FileFormat reads publish their payloads in a directory owned by the layer, as +a generation that is staged privately and renamed into place, so a failed or +interrupted read never modifies payloads a root already references; the +layout is in the [file-format argument contract](../architecture/FILE_FORMAT_ARGUMENTS.md#generated-payload-ownership). The plugins never write the root layer of a FileFormat read to disk. diff --git a/libs/usd-pointcloud-authoring/CMakeLists.txt b/libs/usd-pointcloud-authoring/CMakeLists.txt index f8a1845..09e35c9 100644 --- a/libs/usd-pointcloud-authoring/CMakeLists.txt +++ b/libs/usd-pointcloud-authoring/CMakeLists.txt @@ -9,7 +9,7 @@ find_package(pxr CONFIG REQUIRED) add_library(usdPointCloudAuthoring STATIC src/ArAssetRandomAccessSource.cpp - src/GeneratedPayloadSet.cpp + src/PayloadGeneration.cpp src/PointCloudCache.cpp src/PointCloudLayer.cpp src/PointCloudStreaming.cpp) diff --git a/libs/usd-pointcloud-authoring/README.md b/libs/usd-pointcloud-authoring/README.md index 9f9d67e..6d03102 100644 --- a/libs/usd-pointcloud-authoring/README.md +++ b/libs/usd-pointcloud-authoring/README.md @@ -149,14 +149,17 @@ Each payload file is built in an in-memory layer and written with `SdfLayer::Export`, so writing it never registers a layer under the payload path. -`PointCloudPayloadOptions::owner` decides who owns the files written into the -payload directory. Left empty, the directory is exclusive and any existing -payload path refuses the write, which is what `usd-pointcloud-convert` relies -on. Set, typically to `PointCloudPayloadOwner(resolvedPath, request)`, the -generation replaces the payloads that owner generated before, removes those it -no longer generates, and refuses every other existing file. Ownership is -recorded in the directory before the first payload is written and restored if -the generation fails. The rule is stated in the +`PointCloudPayloadOptions::owner` decides where payload files go. Left empty, +the payload directory is exclusive: payloads are written straight into it and +any existing payload path refuses the write, which is what +`usd-pointcloud-convert` relies on. Set, typically to +`PointCloudPayloadOwner(resolvedPath, payloadDirectory, arguments)`, payloads +go to `///`. A generation is staged in a +private directory and published by renaming it into place, so a published one +is never modified: a failure removes only the staging directory, identical +content reuses the published generation, and changed content is published +beside it before the superseded one is removed. Nothing outside the owner's +directory is touched. The layout is stated in the [file-format argument contract](../../docs/architecture/FILE_FORMAT_ARGUMENTS.md#generated-payload-ownership). ## Coordinate-space assumptions @@ -199,10 +202,12 @@ ctest --test-dir build/cy2026-windows-x86_64-py313-usd -C Release ` - Sampling is fixed-stride, inherited from `usdPointCloudCore`. - Payload working-set behavior is unmeasured: the library emits payloads, but no claim is made that a non-selected LOD child's payload stays unloaded. -- Payloads are replaced by writing a new file and renaming it over the old - one. A platform that refuses to rename over an open file, such as Windows, - fails a regeneration while another stage in the process still holds that - payload open. +- A superseded generation is removed on a best-effort basis. On a platform + that refuses to delete an open file, such as Windows, files another stage + still holds stay until a later read of the same layer removes them. +- Generating payloads needs write access to the payload directory even when + the content turns out unchanged; only a cache hit whose copy is already + published reads a directory without writing to it. - `AuthorPointCloudTiledAssetFromStream` consumes a pull stream, spools points by source tile, and reconstructs one tile at a time before payload authoring. The router overload also accepts a planned `PointBudgetTileRouter`, allowing diff --git a/libs/usd-pointcloud-authoring/include/usdgeo/PointCloudLayer.h b/libs/usd-pointcloud-authoring/include/usdgeo/PointCloudLayer.h index 653f3a9..ad1f053 100644 --- a/libs/usd-pointcloud-authoring/include/usdgeo/PointCloudLayer.h +++ b/libs/usd-pointcloud-authoring/include/usdgeo/PointCloudLayer.h @@ -1,7 +1,6 @@ #pragma once #include "usdgeo/GeoReference.h" -#include "usdpointcloud/FileFormatArguments.h" #include "usdpointcloud/PointCloud.h" #include "usdpointcloud/Lod.h" #include "usdpointcloud/Spool.h" @@ -133,19 +132,25 @@ struct PointCloudPayloadOptions { nullptr; usdpointcloud::SpoolIoStats* spoolIoStats = nullptr; // Identity of the layer the generated payloads belong to. When empty the - // payload directory is exclusive and any existing payload path refuses - // the write. When set, payloads this owner generated before are replaced - // and removed as the new generation supersedes them, while any other - // existing file still refuses the write. The directory records ownership - // under a hash of this value; the value itself is never persisted. + // payload directory is exclusive: payloads are written straight into it + // and any existing payload path refuses the write. When set, payloads + // live in `directory///`, where the owner key is a + // hash of this value: each generation is staged privately and published + // whole, identical content reuses the published generation, and + // superseded generations of the same owner are removed. Nothing outside + // the owner's directory is touched, and the value is never persisted. std::string owner; }; -// The payload owner for a FileFormat read: the resolved source and its -// normalized arguments, which together are the layer's identity. +// The payload owner for a FileFormat read: which source the layer reads and +// the layer's file-format arguments exactly as it holds them. A local source +// is named relative to the payload directory, so a project that moves as a +// whole keeps its owner; any other source is named by its resolved +// identifier. std::string PointCloudPayloadOwner( - const std::string& resolvedPath, - const usdpointcloud::PointReadRequest& request); + const std::string& source, + const std::filesystem::path& payloadDirectory, + const pxr::SdfLayer::FileFormatArguments& arguments); bool AuthorPointCloudTiledAssetFromStream( pxr::SdfLayer* layer, @@ -189,8 +194,8 @@ bool AuthorPointCloudTiledAssetWithPayloads( std::vector& generatedPayloads); // Authors the payload-backed tiles into a detached stage and transfers the -// result into `layer`. On failure the layer is unchanged, the payloads this -// call wrote are removed, and `diagnostics` explains why. +// result into `layer`. On failure the layer is unchanged, nothing this call +// wrote remains, and `diagnostics` explains why. bool AuthorPointCloudTiledAssetWithPayloads( pxr::SdfLayer* layer, const std::string& primPath, diff --git a/libs/usd-pointcloud-authoring/src/GeneratedPayloadSet.cpp b/libs/usd-pointcloud-authoring/src/GeneratedPayloadSet.cpp deleted file mode 100644 index c6f8869..0000000 --- a/libs/usd-pointcloud-authoring/src/GeneratedPayloadSet.cpp +++ /dev/null @@ -1,320 +0,0 @@ -#include "GeneratedPayloadSet.h" - -#include "usdgeo/CacheKey.h" - -#include -#include -#include -#include -#include -#include - -namespace usdgeo::detail { -namespace { - -constexpr const char* kRecordFormat = "usd-pointcloud-payload-owner-v1"; - -// A recorded name is only ever joined onto the payload directory, and a -// commit removes names the owner no longer generates, so a name that could -// escape the directory is rejected rather than trusted. -bool IsSafeRelativeName(const std::string& value) { - if (value.empty() || value.front() == '/' || - value.find_first_of("\\:\r\n") != std::string::npos) { - return false; - } - std::size_t start = 0; - for (;;) { - const auto separator = value.find('/', start); - const auto end = - separator == std::string::npos ? value.size() : separator; - const auto component = value.substr(start, end - start); - if (component.empty() || component == "." || component == "..") { - return false; - } - if (separator == std::string::npos) { - return true; - } - start = separator + 1; - } -} - -std::string RelativeName(const std::filesystem::path& directory, - const std::filesystem::path& payload) { - const auto name = payload.lexically_normal() - .lexically_relative(directory.lexically_normal()) - .generic_string(); - return IsSafeRelativeName(name) ? name : std::string(); -} - -bool ReadRecord(const std::filesystem::path& path, - std::set& names, - std::string& error) { - names.clear(); - const auto invalid = [&]() { - names.clear(); - error = "payload ownership record " + path.filename().string() + - " is invalid"; - return false; - }; - std::ifstream input(path, std::ios::binary); - if (!input) { - error = "unable to read payload ownership record " + - path.filename().string(); - return false; - } - - std::optional count; - std::uint64_t lineIndex = 0; - std::string line; - while (std::getline(input, line)) { - const auto separator = line.find('='); - if (separator == std::string::npos) { - return invalid(); - } - const auto key = line.substr(0, separator); - const auto value = line.substr(separator + 1); - if (lineIndex == 0) { - if (key != "format" || value != kRecordFormat) { - return invalid(); - } - } else if (lineIndex == 1) { - std::uint64_t parsed = 0; - const auto* first = value.data(); - const auto* last = value.data() + value.size(); - const auto result = std::from_chars(first, last, parsed); - if (key != "payload.count" || value.empty() || - result.ec != std::errc() || result.ptr != last) { - return invalid(); - } - count = parsed; - } else if (key != "payload." + std::to_string(lineIndex - 2) || - !IsSafeRelativeName(value) || - !names.insert(value).second) { - return invalid(); - } - ++lineIndex; - } - if (input.bad() || !count || names.size() != *count) { - return invalid(); - } - return true; -} - -} // namespace - -GeneratedPayloadSet::GeneratedPayloadSet(std::filesystem::path directory, - std::string owner) - : directory_(directory.lexically_normal()), owner_(std::move(owner)) { - // A trailing separator would leave an empty final element that makes - // payload names relative to it ambiguous. - if (!directory_.has_filename() && directory_.has_relative_path()) { - directory_ = directory_.parent_path(); - } - if (!owner_.empty()) { - record_ = RecordPath(directory_, owner_); - } -} - -GeneratedPayloadSet::~GeneratedPayloadSet() { - Rollback(); -} - -std::filesystem::path GeneratedPayloadSet::RecordPath( - const std::filesystem::path& directory, const std::string& owner) { - return directory / ("payload-owner-" + - StableCacheKey({{"payloadOwner", owner}}) + - ".manifest"); -} - -bool GeneratedPayloadSet::Claim( - const std::vector& payloads, std::string& error, - const AdoptPredicate& adopt) { - if (active_) { - error = "payload set is already claimed"; - return false; - } - std::error_code status; - const auto directoryExists = std::filesystem::exists(directory_, status); - if (!status && directoryExists && - !std::filesystem::is_directory(directory_, status)) { - status = std::make_error_code(std::errc::not_a_directory); - } - if (status) { - error = "payload directory is not a usable directory"; - return false; - } - - std::set names; - for (const auto& payload : payloads) { - const auto name = RelativeName(directory_, payload); - if (name.empty() || !names.insert(name).second) { - error = "payload paths must be unique and inside the payload " - "directory"; - return false; - } - } - - previous_.clear(); - if (directoryExists) { - if (!record_.empty()) { - const auto recordExists = std::filesystem::exists(record_, status); - if (status) { - error = "unable to inspect the payload ownership record"; - return false; - } - if (recordExists && !ReadRecord(record_, previous_, error)) { - return false; - } - } - for (const auto& name : names) { - const auto path = directory_ / std::filesystem::path(name); - const auto exists = std::filesystem::exists(path, status); - if (status) { - error = "unable to inspect payload " + name; - return false; - } - if (!exists) { - continue; - } - if (!std::filesystem::is_regular_file(path, status) || status) { - error = "payload directory entry " + name + - " is not a regular file"; - return false; - } - if (previous_.count(name) == 0 && !(adopt && adopt(path))) { - error = "payload directory already contains " + name + - ", which this layer did not generate"; - return false; - } - } - } else { - std::filesystem::create_directories(directory_, status); - if (status) { - error = "unable to create payload directory"; - return false; - } - directoryCreated_ = true; - } - - claimed_ = std::move(names); - written_.clear(); - active_ = true; - if (!record_.empty()) { - auto claim = previous_; - claim.insert(claimed_.begin(), claimed_.end()); - if (!WriteRecord(claim, error)) { - Rollback(); - return false; - } - recordWritten_ = true; - } - return true; -} - -void GeneratedPayloadSet::MarkWritten(const std::filesystem::path& payload) { - written_.push_back(payload); -} - -bool GeneratedPayloadSet::Commit(std::string& error) { - if (!active_) { - error = "payload set was not claimed"; - return false; - } - if (!record_.empty()) { - // Superseded files go before the record shrinks, so a crash in - // between leaves them listed and still replaceable. - auto record = claimed_; - for (const auto& name : previous_) { - if (claimed_.count(name) != 0) { - continue; - } - std::error_code removal; - std::filesystem::remove(directory_ / std::filesystem::path(name), - removal); - if (removal) { - record.insert(name); - } - } - if (!WriteRecord(record, error)) { - return false; - } - } - active_ = false; - recordWritten_ = false; - directoryCreated_ = false; - written_.clear(); - return true; -} - -void GeneratedPayloadSet::Rollback() { - if (!active_) { - return; - } - active_ = false; - - std::set retained; - for (const auto& payload : written_) { - std::error_code removal; - std::filesystem::remove(payload, removal); - if (removal) { - retained.insert(RelativeName(directory_, payload)); - } - } - written_.clear(); - - if (recordWritten_) { - auto restored = previous_; - restored.insert(retained.begin(), retained.end()); - std::error_code removal; - std::string ignored; - if (restored.empty()) { - std::filesystem::remove(record_, removal); - } else { - WriteRecord(restored, ignored); - } - recordWritten_ = false; - } - - if (directoryCreated_) { - std::error_code status; - if (std::filesystem::is_empty(directory_, status) && !status) { - std::filesystem::remove(directory_, status); - } - directoryCreated_ = false; - } -} - -bool GeneratedPayloadSet::WriteRecord(const std::set& names, - std::string& error) const { - std::ostringstream text; - text << "format=" << kRecordFormat << '\n' - << "payload.count=" << names.size() << '\n'; - std::size_t index = 0; - for (const auto& name : names) { - text << "payload." << index++ << '=' << name << '\n'; - } - - const std::filesystem::path temporary(record_.string() + ".tmp"); - { - std::ofstream output(temporary, std::ios::binary | std::ios::trunc); - output << text.str(); - output.close(); - if (!output) { - std::error_code removal; - std::filesystem::remove(temporary, removal); - error = "unable to write payload ownership record"; - return false; - } - } - std::error_code rename; - std::filesystem::rename(temporary, record_, rename); - if (rename) { - std::error_code removal; - std::filesystem::remove(temporary, removal); - error = "unable to publish payload ownership record"; - return false; - } - return true; -} - -} // namespace usdgeo::detail diff --git a/libs/usd-pointcloud-authoring/src/GeneratedPayloadSet.h b/libs/usd-pointcloud-authoring/src/GeneratedPayloadSet.h deleted file mode 100644 index 9220b35..0000000 --- a/libs/usd-pointcloud-authoring/src/GeneratedPayloadSet.h +++ /dev/null @@ -1,72 +0,0 @@ -#pragma once - -#include -#include -#include -#include -#include - -namespace usdgeo::detail { - -// The payload files one generation writes into a payload directory, and the -// rule that decides which existing files it may replace. -// -// Without an owner the directory is exclusive: any existing target path is a -// conflict. With an owner, the generation may replace exactly the files the -// same owner generated before, as listed in that owner's record inside the -// directory; any other existing file is still a conflict and is never -// touched. Ownership is recorded before the first write, so an interrupted -// generation leaves files the next generation for the same owner replaces. -// -// The record is named by a hash of the owner and lists relative payload -// names only, so neither the owner string nor a source identifier is -// persisted. -class GeneratedPayloadSet final { -public: - GeneratedPayloadSet(std::filesystem::path directory, std::string owner); - ~GeneratedPayloadSet(); - - GeneratedPayloadSet(const GeneratedPayloadSet&) = delete; - GeneratedPayloadSet& operator=(const GeneratedPayloadSet&) = delete; - - using AdoptPredicate = std::function; - - // Creates the directory when needed, refuses any planned path that - // exists without belonging to this owner, and records the claim. An - // existing file the owner does not hold is claimed anyway only when - // `adopt` accepts it, which a caller does when it can prove the file - // already holds exactly the bytes it would write. - bool Claim(const std::vector& payloads, - std::string& error, const AdoptPredicate& adopt = {}); - - // Notes that this generation wrote one claimed payload. - void MarkWritten(const std::filesystem::path& payload); - - // Removes files the owner generated before but this generation did not, - // then records exactly the claimed payloads. - bool Commit(std::string& error); - - // Removes what this generation wrote, restores the owner's previous - // record, and removes the directory when this generation created it and - // left it empty. Does nothing after Commit. - void Rollback(); - - static std::filesystem::path RecordPath( - const std::filesystem::path& directory, const std::string& owner); - -private: - bool WriteRecord(const std::set& names, - std::string& error) const; - - std::filesystem::path directory_; - std::string owner_; - std::filesystem::path record_; - std::set previous_; - std::set claimed_; - std::vector written_; - bool directoryCreated_ = false; - bool recordWritten_ = false; - bool active_ = false; -}; - -} // namespace usdgeo::detail diff --git a/libs/usd-pointcloud-authoring/src/PayloadGeneration.cpp b/libs/usd-pointcloud-authoring/src/PayloadGeneration.cpp new file mode 100644 index 0000000..722ac4b --- /dev/null +++ b/libs/usd-pointcloud-authoring/src/PayloadGeneration.cpp @@ -0,0 +1,422 @@ +#include "PayloadGeneration.h" + +#include "usdgeo/CacheKey.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace usdgeo::detail { +namespace { + +constexpr const char* kStagingPrefix = ".tmp-"; +constexpr const char* kGeneratedPrefix = "g-"; +constexpr const char* kCachedPrefix = "c-"; + +// A live generation touches its staging directory with every payload it +// writes, so one left untouched this long belongs to an interrupted process. +constexpr auto kStaleStagingAge = std::chrono::hours(1); + +bool StartsWith(const std::string& value, const char* prefix) { + return value.rfind(prefix, 0) == 0; +} + +// Payload names are joined onto the directory the generation owns, so a name +// that could leave it is rejected rather than trusted. +bool IsSafeRelativeName(const std::string& value) { + if (value.empty() || value.front() == '/' || + value.find_first_of("\\:\r\n") != std::string::npos) { + return false; + } + std::size_t start = 0; + for (;;) { + const auto separator = value.find('/', start); + const auto end = + separator == std::string::npos ? value.size() : separator; + const auto component = value.substr(start, end - start); + if (component.empty() || component == "." || component == "..") { + return false; + } + if (separator == std::string::npos) { + return true; + } + start = separator + 1; + } +} + +bool IsGenerationName(const std::string& value) { + return (StartsWith(value, kGeneratedPrefix) || + StartsWith(value, kCachedPrefix)) && + value.find('/') == std::string::npos; +} + +std::string Hex(std::uint64_t value) { + std::ostringstream text; + text << std::hex << std::setfill('0') << std::setw(16) << value; + return text.str(); +} + +std::string NewStagingName() { + static std::atomic_uint64_t sequence{0}; + std::random_device device; + const auto clock = static_cast( + std::chrono::steady_clock::now().time_since_epoch().count()); + const auto value = (static_cast(device()) << 32) ^ + device() ^ clock ^ + (++sequence * 0x9e3779b97f4a7c15ull); + return kStagingPrefix + Hex(value); +} + +void RemoveStaleStaging(const std::filesystem::path& ownerDirectory) { + std::vector stale; + const auto now = std::filesystem::file_time_type::clock::now(); + std::error_code error; + for (std::filesystem::directory_iterator entry(ownerDirectory, error), end; + !error && entry != end; entry.increment(error)) { + std::error_code status; + if (!StartsWith(entry->path().filename().string(), kStagingPrefix) || + !entry->is_directory(status) || status) { + continue; + } + const auto modified = + std::filesystem::last_write_time(entry->path(), status); + if (!status && now - modified > kStaleStagingAge) { + stale.push_back(entry->path()); + } + } + for (const auto& path : stale) { + std::error_code removal; + std::filesystem::remove_all(path, removal); + } +} + +// Names the published generation by what it contains, so regenerating the +// same payloads finds the directory already published. +bool HashPayloads(const std::filesystem::path& directory, + const std::set& names, + std::string& hash, + std::string& error) { + constexpr std::uint64_t offsetBasis = 14695981039346656037ull; + constexpr std::uint64_t prime = 1099511628211ull; + std::uint64_t value = offsetBasis; + const auto mix = [&value](const char* data, std::size_t size) { + for (std::size_t index = 0; index < size; ++index) { + value ^= static_cast(data[index]); + value *= prime; + } + }; + std::array buffer{}; + for (const auto& name : names) { + mix(name.c_str(), name.size() + 1); + std::ifstream input(directory / std::filesystem::path(name), + std::ios::binary); + if (!input) { + error = "unable to read generated payload " + name; + return false; + } + std::uint64_t size = 0; + while (input) { + input.read(buffer.data(), + static_cast(buffer.size())); + const auto count = static_cast(input.gcount()); + mix(buffer.data(), count); + size += count; + } + if (!input.eof()) { + error = "unable to read generated payload " + name; + return false; + } + const auto sizeText = std::to_string(size); + mix(sizeText.c_str(), sizeText.size() + 1); + } + hash = Hex(value); + return true; +} + +} // namespace + +std::string PayloadOwnerKey(const std::string& owner) { + return StableCacheKey({{"payloadOwner", owner}}); +} + +std::string CachedGenerationName(const std::string& entryKey) { + return kCachedPrefix + entryKey; +} + +std::filesystem::path PayloadGeneration::OwnerDirectory( + const std::filesystem::path& directory, const std::string& owner) { + return directory / PayloadOwnerKey(owner); +} + +PayloadGeneration::PayloadGeneration(std::filesystem::path directory, + std::string owner) + : directory_(directory.lexically_normal()), owner_(std::move(owner)) { + // A trailing separator would leave an empty final element that makes the + // payload paths below it ambiguous. + if (!directory_.has_filename() && directory_.has_relative_path()) { + directory_ = directory_.parent_path(); + } + if (!owner_.empty()) { + ownerDirectory_ = OwnerDirectory(directory_, owner_); + } +} + +PayloadGeneration::~PayloadGeneration() { + Rollback(); +} + +bool PayloadGeneration::Begin(const std::vector& names, + std::string& error) { + if (active_) { + error = "payload generation has already begun"; + return false; + } + std::set declared; + for (const auto& name : names) { + if (!IsSafeRelativeName(name) || !declared.insert(name).second) { + error = "payload names must be unique relative paths"; + return false; + } + } + + std::error_code status; + const auto directoryExists = std::filesystem::exists(directory_, status); + if (!status && directoryExists && + !std::filesystem::is_directory(directory_, status)) { + status = std::make_error_code(std::errc::not_a_directory); + } + if (status) { + error = "payload directory is not a usable directory"; + return false; + } + if (owner_.empty() && directoryExists) { + for (const auto& name : declared) { + const auto present = std::filesystem::exists( + directory_ / std::filesystem::path(name), status); + if (status) { + error = "unable to inspect payload " + name; + return false; + } + if (present) { + error = "payload directory already contains " + name; + return false; + } + } + } + + names_ = std::move(declared); + active_ = true; + if (!directoryExists) { + std::filesystem::create_directories(directory_, status); + if (status) { + error = "unable to create payload directory"; + Rollback(); + return false; + } + directoryCreated_ = true; + } + if (owner_.empty()) { + return true; + } + + const auto ownerExists = std::filesystem::exists(ownerDirectory_, status); + if (status) { + error = "unable to inspect the payload owner directory"; + Rollback(); + return false; + } + if (ownerExists) { + RemoveStaleStaging(ownerDirectory_); + } else { + std::filesystem::create_directory(ownerDirectory_, status); + if (status) { + error = "unable to create the payload owner directory"; + Rollback(); + return false; + } + ownerDirectoryCreated_ = true; + } + for (int attempt = 0; attempt < 16 && stagingDirectory_.empty(); + ++attempt) { + const auto name = NewStagingName(); + if (std::filesystem::create_directory(ownerDirectory_ / name, status)) { + stagingName_ = name; + stagingDirectory_ = ownerDirectory_ / name; + } + } + if (stagingDirectory_.empty()) { + error = "unable to create a payload staging directory"; + Rollback(); + return false; + } + return true; +} + +std::filesystem::path PayloadGeneration::PathFor(const std::string& name) const { + if (!active_ || names_.count(name) == 0) { + return {}; + } + const auto& base = owner_.empty() ? directory_ : stagingDirectory_; + return base / std::filesystem::path(name); +} + +std::filesystem::path PayloadGeneration::PublishedPathFor( + const std::string& name) const { + if (owner_.empty()) { + return directory_ / std::filesystem::path(name); + } + if (publishedName_.empty()) { + return {}; + } + return ownerDirectory_ / publishedName_ / std::filesystem::path(name); +} + +bool PayloadGeneration::Commit(std::string& error) { + if (!active_) { + error = "payload generation has not begun"; + return false; + } + if (owner_.empty()) { + active_ = false; + directoryCreated_ = false; + return true; + } + for (const auto& name : names_) { + std::error_code status; + if (!std::filesystem::is_regular_file( + stagingDirectory_ / std::filesystem::path(name), status)) { + error = "generated payload " + name + " was not written"; + return false; + } + } + std::string hash; + if (!HashPayloads(stagingDirectory_, names_, hash, error)) { + return false; + } + return Publish(kGeneratedPrefix + hash, error); +} + +bool PayloadGeneration::CommitAs(const std::string& name, std::string& error) { + if (!active_ || owner_.empty()) { + error = "only an owned payload generation can be published by name"; + return false; + } + if (!IsGenerationName(name) || !IsSafeRelativeName(name)) { + error = "invalid payload generation name"; + return false; + } + return Publish(name, error); +} + +bool PayloadGeneration::IsComplete( + const std::filesystem::path& generation) const { + for (const auto& name : names_) { + std::error_code status; + const auto published = generation / std::filesystem::path(name); + if (!std::filesystem::is_regular_file(published, status) || status) { + return false; + } + const auto publishedSize = std::filesystem::file_size(published, status); + if (status) { + return false; + } + const auto stagedSize = std::filesystem::file_size( + stagingDirectory_ / std::filesystem::path(name), status); + if (status || stagedSize != publishedSize) { + return false; + } + } + return true; +} + +bool PayloadGeneration::Publish(const std::string& name, std::string& error) { + const auto published = ownerDirectory_ / name; + std::error_code status; + if (std::filesystem::exists(published, status) && !IsComplete(published)) { + // A damaged generation is replaced rather than trusted. + std::filesystem::remove_all(published, status); + if (status) { + error = "unable to replace incomplete payload generation " + name; + return false; + } + } + if (std::filesystem::exists(published, status)) { + // The same payloads are already published; keep them in place so + // nothing that has them open sees them replaced. + std::filesystem::remove_all(stagingDirectory_, status); + } else { + std::filesystem::rename(stagingDirectory_, published, status); + if (status) { + // A concurrent generation of the same payloads may have + // published first. + std::error_code check; + if (!std::filesystem::exists(published, check) || + !IsComplete(published)) { + error = "unable to publish payload generation " + name; + return false; + } + std::filesystem::remove_all(stagingDirectory_, check); + } + } + publishedName_ = name; + stagingDirectory_.clear(); + active_ = false; + directoryCreated_ = false; + ownerDirectoryCreated_ = false; + + std::vector superseded; + std::error_code iteration; + for (std::filesystem::directory_iterator entry(ownerDirectory_, iteration), + end; + !iteration && entry != end; entry.increment(iteration)) { + const auto entryName = entry->path().filename().string(); + std::error_code entryStatus; + if (entryName != name && IsGenerationName(entryName) && + entry->is_directory(entryStatus) && !entryStatus) { + superseded.push_back(entry->path()); + } + } + for (const auto& path : superseded) { + // Best effort: a file another process still holds open stays until a + // later generation removes it. + std::error_code removal; + std::filesystem::remove_all(path, removal); + } + return true; +} + +void PayloadGeneration::Rollback() { + if (!active_) { + return; + } + active_ = false; + std::error_code status; + if (owner_.empty()) { + // Begin proved none of these existed, so any that do now were + // written by this generation. + for (const auto& name : names_) { + std::filesystem::remove(directory_ / std::filesystem::path(name), + status); + } + } else if (!stagingDirectory_.empty()) { + std::filesystem::remove_all(stagingDirectory_, status); + stagingDirectory_.clear(); + } + // Removing a directory succeeds only while it is empty. + if (ownerDirectoryCreated_) { + std::filesystem::remove(ownerDirectory_, status); + ownerDirectoryCreated_ = false; + } + if (directoryCreated_) { + std::filesystem::remove(directory_, status); + directoryCreated_ = false; + } +} + +} // namespace usdgeo::detail diff --git a/libs/usd-pointcloud-authoring/src/PayloadGeneration.h b/libs/usd-pointcloud-authoring/src/PayloadGeneration.h new file mode 100644 index 0000000..0332c7a --- /dev/null +++ b/libs/usd-pointcloud-authoring/src/PayloadGeneration.h @@ -0,0 +1,84 @@ +#pragma once + +#include +#include +#include +#include + +namespace usdgeo::detail { + +// The directory name under which one owner's payload generations live. +std::string PayloadOwnerKey(const std::string& owner); + +// The generation name for payloads copied out of a generated-cache entry, +// which the entry's key already identifies. +std::string CachedGenerationName(const std::string& entryKey); + +// One set of payload files written into a payload directory. +// +// Without an owner the directory is exclusive: payloads are written straight +// into it, and any payload name that already exists refuses the generation. +// +// With an owner, payloads live in `///`. +// They are written into a private staging directory and published at Commit +// by renaming it into place, so an existing generation is never modified: a +// failed or cancelled generation leaves the published one untouched, and a +// generation with identical content reuses the published directory instead +// of replacing it. Publishing removes the owner's superseded generations. +// Nothing outside the owner's directory is ever written or removed. +class PayloadGeneration final { +public: + PayloadGeneration(std::filesystem::path directory, std::string owner); + ~PayloadGeneration(); + + PayloadGeneration(const PayloadGeneration&) = delete; + PayloadGeneration& operator=(const PayloadGeneration&) = delete; + + // Declares the payload names this generation writes, relative to where it + // publishes them, and prepares the directory they are written into. + bool Begin(const std::vector& names, std::string& error); + + // Where a declared payload is written before Commit. Empty for a name + // that was not declared. + std::filesystem::path PathFor(const std::string& name) const; + + // Publishes the generation under a name derived from its content. + bool Commit(std::string& error); + + // Publishes the generation under `name`, for content whose identity is + // already known. + bool CommitAs(const std::string& name, std::string& error); + + // Removes what this generation wrote, and the directories it created when + // they are left empty. Does nothing after a commit. + void Rollback(); + + // The staging directory name that payload paths contain until Commit, and + // the generation name that replaces it. Both are empty without an owner. + const std::string& StagingName() const noexcept { return stagingName_; } + const std::string& PublishedName() const noexcept { return publishedName_; } + + // Where a declared payload is after Commit. + std::filesystem::path PublishedPathFor(const std::string& name) const; + + // The directory holding the owner's generations. + static std::filesystem::path OwnerDirectory( + const std::filesystem::path& directory, const std::string& owner); + +private: + bool Publish(const std::string& name, std::string& error); + bool IsComplete(const std::filesystem::path& generation) const; + + std::filesystem::path directory_; + std::string owner_; + std::filesystem::path ownerDirectory_; + std::filesystem::path stagingDirectory_; + std::string stagingName_; + std::string publishedName_; + std::set names_; + bool directoryCreated_ = false; + bool ownerDirectoryCreated_ = false; + bool active_ = false; +}; + +} // namespace usdgeo::detail diff --git a/libs/usd-pointcloud-authoring/src/PointCloudCache.cpp b/libs/usd-pointcloud-authoring/src/PointCloudCache.cpp index c1fd1fb..2bc1e41 100644 --- a/libs/usd-pointcloud-authoring/src/PointCloudCache.cpp +++ b/libs/usd-pointcloud-authoring/src/PointCloudCache.cpp @@ -1,17 +1,15 @@ #include "usdgeo/PointCloudCache.h" +#include "usdgeo/CacheKey.h" #include "usdgeo/PointCloudLayer.h" #include "usdpointcloud/Tiling.h" -#include "GeneratedPayloadSet.h" +#include "PayloadGeneration.h" #include #include -#include -#include #include #include -#include #include #include #include @@ -168,73 +166,42 @@ bool ValidateCachedPayloads( return valid; } -bool SameFileContent(const std::filesystem::path& left, - const std::filesystem::path& right) { - std::error_code error; - const auto leftSize = std::filesystem::file_size(left, error); - if (error) { - return false; - } - const auto rightSize = std::filesystem::file_size(right, error); - if (error || leftSize != rightSize) { - return false; - } - std::ifstream leftStream(left, std::ios::binary); - std::ifstream rightStream(right, std::ios::binary); - if (!leftStream || !rightStream) { - return false; - } - std::array leftBuffer{}; - std::array rightBuffer{}; - while (leftStream && rightStream) { - leftStream.read(leftBuffer.data(), leftBuffer.size()); - rightStream.read(rightBuffer.data(), rightBuffer.size()); - const auto count = leftStream.gcount(); - if (count != rightStream.gcount() || - !std::equal(leftBuffer.data(), leftBuffer.data() + count, - rightBuffer.data())) { +// A copy published under an entry's key holds that entry's bytes, so it is +// reused as long as every payload is still there at its cached size. +bool HoldsCachedCopy( + const std::filesystem::path& generation, + const std::map& files) { + for (const auto& [name, sourcePath] : files) { + std::error_code status; + const auto target = generation / std::filesystem::path(name); + if (!std::filesystem::is_regular_file(target, status) || status) { + return false; + } + const auto targetSize = std::filesystem::file_size(target, status); + if (status) { + return false; + } + const auto sourceSize = std::filesystem::file_size(sourcePath, status); + if (status || targetSize != sourceSize) { return false; } - } - return leftStream.eof() && rightStream.eof(); -} - -// Copies beside the target and renames over it, so a payload another stage -// still has open keeps the bytes it opened instead of being truncated. -bool ReplaceWithCopy(const std::filesystem::path& source, - const std::filesystem::path& target) { - const std::filesystem::path temporary(target.string() + ".tmp"); - std::error_code error; - if (!std::filesystem::copy_file( - source, temporary, - std::filesystem::copy_options::overwrite_existing, error) || - error) { - std::error_code removal; - std::filesystem::remove(temporary, removal); - return false; - } - std::filesystem::rename(temporary, target, error); - if (error) { - std::error_code removal; - std::filesystem::remove(temporary, removal); - return false; } return true; } -// Copies the cached payloads into the layer's payload directory under the -// same ownership rule as generation: files the owner placed before are -// replaced when their bytes differ, and a file someone else placed is only -// taken over when it already holds the cached bytes. +// Copies the cached payloads into the owner's directory under a generation +// named by the cache entry. An intact copy from an earlier hit is reused +// without writing anything, and a new copy is staged and published whole, +// so no payload another stage has open is ever modified. bool MaterializePayloads( const pxr::SdfLayerHandle& layer, const usdgeo::cache::Layout& layout, const std::filesystem::path& targetDirectory, - const std::string& owner) { + const std::string& owner, + std::filesystem::path& materializedDirectory) { const auto sourceDirectory = layout.payloadDirectory.lexically_normal(); - const auto normalizedTarget = targetDirectory.lexically_normal(); - std::map files; + std::map files; bool valid = true; layer->Traverse(pxr::SdfPath::AbsoluteRootPath(), [&](const pxr::SdfPath& path) { @@ -253,20 +220,15 @@ bool MaterializePayloads( const std::filesystem::path sourcePath = (layout.entryDirectory / assetPath).lexically_normal(); if (std::filesystem::path(assetPath).is_absolute() || - !IsWithin(sourcePath, sourceDirectory)) { - valid = false; - return; - } - const auto relative = - sourcePath.lexically_relative(sourceDirectory); - const auto targetPath = - (normalizedTarget / relative).lexically_normal(); - if (!IsValidCachedPayloadPath(sourcePath, - sourceDirectory)) { + !IsWithin(sourcePath, sourceDirectory) || + !IsValidCachedPayloadPath(sourcePath, sourceDirectory)) { valid = false; return; } - files.emplace(targetPath, sourcePath); + files.emplace( + sourcePath.lexically_relative(sourceDirectory) + .generic_string(), + sourcePath); } }; collect(payloads.GetExplicitItems()); @@ -278,39 +240,43 @@ bool MaterializePayloads( return false; } - std::vector targets; - targets.reserve(files.size()); + const auto generationName = detail::CachedGenerationName(StableCacheKey( + {{"cacheEntry", + layout.entryDirectory.parent_path().filename().generic_string() + + "/" + layout.entryDirectory.filename().generic_string()}})); + const auto published = + detail::PayloadGeneration::OwnerDirectory( + targetDirectory.lexically_normal(), owner) / + generationName; + if (HoldsCachedCopy(published, files)) { + materializedDirectory = published; + return true; + } + + std::vector names; + names.reserve(files.size()); for (const auto& entry : files) { - targets.push_back(entry.first); + names.push_back(entry.first); } - detail::GeneratedPayloadSet materialized(normalizedTarget, owner); + detail::PayloadGeneration generation(targetDirectory, owner); std::string error; - const auto holdsCachedBytes = [&files](const std::filesystem::path& target) { - const auto found = files.find(target.lexically_normal()); - return found != files.end() && SameFileContent(found->second, target); - }; - if (!materialized.Claim(targets, error, holdsCachedBytes)) { + if (!generation.Begin(names, error)) { return false; } - for (const auto& [targetPath, sourcePath] : files) { + for (const auto& [name, sourcePath] : files) { + const auto target = generation.PathFor(name); std::error_code status; - const auto targetExists = std::filesystem::exists(targetPath, status); - if (status) { - return false; - } - if (targetExists && SameFileContent(sourcePath, targetPath)) { - continue; - } - std::filesystem::create_directories(targetPath.parent_path(), status); - if (status) { - return false; - } - materialized.MarkWritten(targetPath); - if (!ReplaceWithCopy(sourcePath, targetPath)) { + std::filesystem::create_directories(target.parent_path(), status); + if (status || !std::filesystem::copy_file(sourcePath, target, status) || + status) { return false; } } - return materialized.Commit(error); + if (!generation.CommitAs(generationName, error)) { + return false; + } + materializedDirectory = published; + return true; } void RebasePayloads(pxr::SdfLayer* layer, @@ -414,13 +380,13 @@ bool TryBuildPointCloudCacheLayout( namespace { -// `payloadOwner` is the identity materialized payloads are recorded under, so -// a later generation for the same layer can replace them. +// `ownerSource` names the source the way the FileFormat read that generates +// payloads does, so materialized payloads land in that layer's own directory. bool LoadPointCloudCache( pxr::SdfLayer* layer, const cache::SourceIdentity& sourceIdentity, const std::filesystem::path& payloadBaseDirectory, - const std::string& payloadOwner, + const std::string& ownerSource, const GeoReference& reference, const usdpointcloud::PointReadRequest& request, const std::string& parserVersion, @@ -490,11 +456,16 @@ bool LoadPointCloudCache( std::error_code error; targetPayloadDirectory = std::filesystem::absolute(targetPayloadDirectory, error); - if (error || !MaterializePayloads(cachedLayer, layout, - targetPayloadDirectory, - payloadOwner)) { + std::filesystem::path materializedDirectory; + if (error || + !MaterializePayloads( + cachedLayer, layout, targetPayloadDirectory, + PointCloudPayloadOwner(ownerSource, targetPayloadDirectory, + layer->GetFileFormatArguments()), + materializedDirectory)) { return true; } + targetPayloadDirectory = materializedDirectory; } layer->TransferContent(cachedLayer); const auto layerBaseDirectory = @@ -536,7 +507,7 @@ bool TryLoadPointCloudCache( } return LoadPointCloudCache( layer, sourceIdentity, sourcePath.parent_path(), - PointCloudPayloadOwner(sourcePath.string(), request), reference, + sourcePath.string(), reference, request, parserVersion, hit, errorMessage, decision); } @@ -552,7 +523,7 @@ bool TryLoadPointCloudCache( cache::CacheDecision* decision) { return LoadPointCloudCache( layer, sourceIdentity, payloadBaseDirectory, - PointCloudPayloadOwner(sourceIdentity.identifier, request), reference, + sourceIdentity.identifier, reference, request, parserVersion, hit, errorMessage, decision); } diff --git a/libs/usd-pointcloud-authoring/src/PointCloudLayer.cpp b/libs/usd-pointcloud-authoring/src/PointCloudLayer.cpp index 3fbdcf2..092bea0 100644 --- a/libs/usd-pointcloud-authoring/src/PointCloudLayer.cpp +++ b/libs/usd-pointcloud-authoring/src/PointCloudLayer.cpp @@ -114,12 +114,12 @@ std::string TilePrimName(const usdpointcloud::PointTileId& id) { "_" + axisName(id.y) + "_" + axisName(id.z); } -// Where a payload-backed LOD root writes its payload files. Every path it -// writes has already been claimed by `payloads`. +// Which tile a payload-backed LOD root belongs to, and the generation that +// decides where its payload files are written. struct PayloadTarget { - const std::filesystem::path& directory; + const usdpointcloud::PointTileId& tileId; const std::filesystem::path& rootLayerPath; - detail::GeneratedPayloadSet& payloads; + const detail::PayloadGeneration& payloads; }; bool AuthorLodRoot( @@ -159,9 +159,11 @@ bool AuthorLodRoot( continue; } - const auto payloadPath = payloadTarget->directory / - (primPath.substr(primPath.find_last_of('/') + 1) + "_" + - lodName + ".usdc"); + const auto payloadPath = payloadTarget->payloads.PathFor( + detail::TilePayloadName(payloadTarget->tileId, index)); + if (payloadPath.empty()) { + return false; + } std::error_code relativeError; const auto payloadIdentifier = std::filesystem::relative( payloadPath, payloadTarget->rootLayerPath.parent_path(), @@ -171,19 +173,15 @@ bool AuthorLodRoot( return false; } // The payload is built in memory and exported, so writing it never - // registers a layer under the payload path. A regeneration can then - // replace a payload this process still has open. + // registers a layer under the payload path. const auto payloadStage = pxr::UsdStage::CreateInMemory("payload.usdc"); if (!payloadStage || !pxr::UsdGeomXform::Define( payloadStage, pxr::SdfPath("/" + lodName)) || !AuthorPointCloudAsset(payloadStage, "/" + lodName + "/Points", level.reference, level.bounds, level.chunk, - level.data)) { - return false; - } - payloadTarget->payloads.MarkWritten(payloadPath); - if (!payloadStage->GetRootLayer()->Export(payloadPath.string())) { + level.data) || + !payloadStage->GetRootLayer()->Export(payloadPath.string())) { return false; } const auto lodPrim = pxr::UsdGeomXform::Define( @@ -245,31 +243,127 @@ pxr::UsdStageRefPtr CreateDetachedStage() { return pxr::UsdStage::CreateInMemory(pxr::UsdStage::LoadNone); } -std::filesystem::path TilePayloadPath(const std::filesystem::path& directory, - const usdpointcloud::PointTileId& id, - std::size_t lodIndex) { - return directory / - (TilePrimName(id) + "_LOD" + std::to_string(lodIndex) + ".usdc"); +std::string TilePayloadName(const usdpointcloud::PointTileId& id, + std::size_t lodIndex) { + return TilePrimName(id) + "_LOD" + std::to_string(lodIndex) + ".usdc"; } -std::vector TilePayloadPaths( - const std::filesystem::path& directory, +std::vector TilePayloadNames( const std::vector& tiles) { - std::vector paths; + std::vector names; for (const auto& tile : tiles) { for (std::size_t index = 0; index < tile.levels.size(); ++index) { - paths.push_back(TilePayloadPath(directory, tile.tile.id, index)); + names.push_back(TilePayloadName(tile.tile.id, index)); } } - return paths; + return names; +} + +namespace { + +std::string ReplacePathComponent(const std::string& path, + const std::string& from, + const std::string& to) { + std::string result; + std::size_t start = 0; + for (;;) { + const auto separator = path.find('/', start); + const auto component = path.substr( + start, separator == std::string::npos ? std::string::npos + : separator - start); + result += component == from ? to : component; + if (separator == std::string::npos) { + return result; + } + result += '/'; + start = separator + 1; + } +} + +} // namespace + +void RepointPayloads(const pxr::SdfLayerHandle& layer, + const std::string& fromComponent, + const std::string& toComponent) { + layer->Traverse(pxr::SdfPath::AbsoluteRootPath(), + [&](const pxr::SdfPath& path) { + const auto prim = layer->GetPrimAtPath(path); + if (!prim || !prim->HasPayloads()) { + return; + } + const auto payloads = prim->GetPayloadList(); + const auto repoint = [&](auto items) { + for (const auto& item : items) { + const pxr::SdfPayload payload = item; + const auto assetPath = ReplacePathComponent( + payload.GetAssetPath(), fromComponent, toComponent); + if (assetPath != payload.GetAssetPath()) { + auto repointed = payload; + repointed.SetAssetPath(assetPath); + items.Replace(payload, repointed); + } + } + }; + repoint(payloads.GetExplicitItems()); + repoint(payloads.GetAddedItems()); + repoint(payloads.GetPrependedItems()); + repoint(payloads.GetAppendedItems()); + }); +} + +bool CommitTilePayloads( + const pxr::SdfLayerHandle& layer, + PayloadGeneration& payloads, + std::vector& manifestEntries, + std::string& error) { + if (!payloads.Commit(error)) { + return false; + } + if (payloads.StagingName().empty()) { + return true; + } + RepointPayloads(layer, payloads.StagingName(), payloads.PublishedName()); + for (auto& entry : manifestEntries) { + entry.payloadPath = ReplacePathComponent( + entry.payloadPath, payloads.StagingName(), payloads.PublishedName()); + } + return true; } } // namespace detail std::string PointCloudPayloadOwner( - const std::string& resolvedPath, - const usdpointcloud::PointReadRequest& request) { - return resolvedPath + "?" + request.normalizedArguments; + const std::string& source, + const std::filesystem::path& payloadDirectory, + const pxr::SdfLayer::FileFormatArguments& arguments) { + // A local source is named relative to the payload directory, both + // canonicalized, so a project that moves or is mounted elsewhere as a + // whole keeps its owner. Any other source keeps its resolved identifier; + // the owner is only ever hashed, never written. + std::string owner; + std::error_code error; + if (std::filesystem::is_regular_file(source, error) && !error) { + const auto canonicalSource = + std::filesystem::weakly_canonical(source, error); + std::error_code directoryError; + const auto canonicalDirectory = + std::filesystem::weakly_canonical(payloadDirectory, directoryError); + if (!error && !directoryError) { + owner = "local:" + canonicalSource + .lexically_relative(canonicalDirectory) + .generic_string(); + } + } + if (owner.empty()) { + owner = "resolved:" + source; + } + // The arguments exactly as the layer holds them, which is its identity: + // two layers never share an owner because normalization rounded them + // together. + for (const auto& [key, value] : arguments) { + owner += "\n" + key + "=" + value; + } + return owner; } pxr::UsdStageRefPtr PointCloudLayer::CreateStage() { @@ -844,19 +938,20 @@ bool AuthorPointCloudTiledAssetWithPayloads( namespace detail { -bool AuthorClaimedTilePayloads(const pxr::UsdStageRefPtr& stage, - const std::string& primPath, - const std::vector& tiles, - const PointCloudPayloadOptions& options, - GeneratedPayloadSet& payloads) { +bool AuthorClaimedTilePayloads( + const pxr::UsdStageRefPtr& stage, + const std::string& primPath, + const std::vector& tiles, + const PointCloudPayloadOptions& options, + PayloadGeneration& payloads, + std::vector& manifestEntries) { if (!stage || !IsValidPrimPath(primPath) || tiles.empty() || options.directory.empty() || options.rootLayerPath.empty()) { return false; } - const std::filesystem::path payloadDirectory(options.directory); const std::filesystem::path rootLayerPath(options.rootLayerPath); - std::vector manifestEntries; + std::vector tileEntries; std::set tileNames; for (const auto& tile : tiles) { std::vector diagnostics; @@ -872,14 +967,18 @@ bool AuthorClaimedTilePayloads(const pxr::UsdStageRefPtr& stage, !SameBounds(level.bounds, item.bounds)) { return false; } + const auto payloadPath = + payloads.PathFor(TilePayloadName(tile.tile.id, index)); + if (payloadPath.empty()) { + return false; + } std::error_code error; const auto relativePayloadPath = std::filesystem::relative( - TilePayloadPath(payloadDirectory, tile.tile.id, index), - rootLayerPath.parent_path(), error); + payloadPath, rootLayerPath.parent_path(), error); if (error || relativePayloadPath.empty()) { return false; } - manifestEntries.push_back({ + tileEntries.push_back({ tile.tile.id, static_cast(index), item.bounds, @@ -889,7 +988,7 @@ bool AuthorClaimedTilePayloads(const pxr::UsdStageRefPtr& stage, } if (options.tileManifestEntries) { std::vector manifestDiagnostics; - const usdpointcloud::PointTileManifest tileManifest{manifestEntries}; + const usdpointcloud::PointTileManifest tileManifest{tileEntries}; if (!usdpointcloud::ValidatePointTileManifest( tileManifest, manifestDiagnostics)) { return false; @@ -904,11 +1003,11 @@ bool AuthorClaimedTilePayloads(const pxr::UsdStageRefPtr& stage, return false; } - const PayloadTarget payloadTarget{payloadDirectory, rootLayerPath, - payloads}; const auto heuristicPath = pxr::SdfPath(primPath + "/LodHeuristics"); for (const auto& tile : tiles) { + const PayloadTarget payloadTarget{tile.tile.id, rootLayerPath, + payloads}; const auto tilePath = pxr::SdfPath( primPath + "/Tiles/" + TilePrimName(tile.tile.id)); const auto tileHeuristicPath = @@ -925,11 +1024,8 @@ bool AuthorClaimedTilePayloads(const pxr::UsdStageRefPtr& stage, return false; } } - if (options.tileManifestEntries) { - options.tileManifestEntries->insert( - options.tileManifestEntries->end(), manifestEntries.begin(), - manifestEntries.end()); - } + manifestEntries.insert(manifestEntries.end(), tileEntries.begin(), + tileEntries.end()); return true; } @@ -949,24 +1045,29 @@ bool AuthorGeneratedTilePayloads( error = "invalid tiled point-cloud payload request"; return false; } - const std::filesystem::path payloadDirectory(options.directory); - const auto payloadPaths = detail::TilePayloadPaths(payloadDirectory, tiles); - detail::GeneratedPayloadSet payloads(payloadDirectory, options.owner); - if (!payloads.Claim(payloadPaths, error)) { + const auto names = detail::TilePayloadNames(tiles); + detail::PayloadGeneration payloads(options.directory, options.owner); + std::vector manifestEntries; + if (!payloads.Begin(names, error)) { return false; } if (!detail::AuthorClaimedTilePayloads(stage, primPath, tiles, options, - payloads)) { + payloads, manifestEntries)) { error = "unable to author tiled point-cloud payloads"; - payloads.Rollback(); return false; } - if (!payloads.Commit(error)) { - payloads.Rollback(); + if (!detail::CommitTilePayloads(stage->GetRootLayer(), payloads, + manifestEntries, error)) { return false; } - generatedPayloads.insert(generatedPayloads.end(), payloadPaths.begin(), - payloadPaths.end()); + for (const auto& name : names) { + generatedPayloads.push_back(payloads.PublishedPathFor(name)); + } + if (options.tileManifestEntries) { + options.tileManifestEntries->insert( + options.tileManifestEntries->end(), manifestEntries.begin(), + manifestEntries.end()); + } return true; } diff --git a/libs/usd-pointcloud-authoring/src/PointCloudStreaming.cpp b/libs/usd-pointcloud-authoring/src/PointCloudStreaming.cpp index 947b729..74c4b7a 100644 --- a/libs/usd-pointcloud-authoring/src/PointCloudStreaming.cpp +++ b/libs/usd-pointcloud-authoring/src/PointCloudStreaming.cpp @@ -256,7 +256,8 @@ bool AuthorPointCloudTiledAssetFromStream( if (spoolDirectory.empty()) return false; std::map spools; std::size_t bufferedBytes = 0; - detail::GeneratedPayloadSet payloads(options.directory, options.owner); + detail::PayloadGeneration payloads(options.directory, options.owner); + std::vector manifestEntries; const auto cleanup = [&]() { for (auto& entry : spools) entry.second.writer.reset(); std::vector cleanupDiagnostics; @@ -410,16 +411,15 @@ bool AuthorPointCloudTiledAssetFromStream( return false; } - // The tile set is known once the stream is spooled, so every payload - // path is claimed before the first payload is written. - std::vector payloadPaths; - payloadPaths.reserve(spools.size()); + // The tile set is known once the stream is spooled, so every payload is + // declared before the first one is written. + std::vector payloadNames; + payloadNames.reserve(spools.size()); for (const auto& entry : spools) { - payloadPaths.push_back( - detail::TilePayloadPath(options.directory, entry.second.id, 0)); + payloadNames.push_back(detail::TilePayloadName(entry.second.id, 0)); } std::string payloadError; - if (!payloads.Claim(payloadPaths, payloadError)) { + if (!payloads.Begin(payloadNames, payloadError)) { AddError(diagnostics, DiagnosticCode::DecodeFailure, payloadError); cleanup(); return false; @@ -510,7 +510,8 @@ bool AuthorPointCloudTiledAssetFromStream( std::vector singleTile; singleTile.push_back(std::move(tile)); if (!detail::AuthorClaimedTilePayloads( - stage, primPath, singleTile, options, payloads)) { + stage, primPath, singleTile, options, payloads, + manifestEntries)) { AddError(diagnostics, DiagnosticCode::DecodeFailure, "unable to author tiled point-cloud payloads"); return failTile(); @@ -529,11 +530,17 @@ bool AuthorPointCloudTiledAssetFromStream( cleanup(); return false; } - if (!payloads.Commit(payloadError)) { + if (!detail::CommitTilePayloads(stage->GetRootLayer(), payloads, + manifestEntries, payloadError)) { AddError(diagnostics, DiagnosticCode::DecodeFailure, payloadError); cleanup(); return false; } + if (options.tileManifestEntries) { + options.tileManifestEntries->insert( + options.tileManifestEntries->end(), manifestEntries.begin(), + manifestEntries.end()); + } layer->TransferContent(stage->GetRootLayer()); return true; } diff --git a/libs/usd-pointcloud-authoring/src/TiledPayloadAuthoring.h b/libs/usd-pointcloud-authoring/src/TiledPayloadAuthoring.h index 7bcc428..600c2ab 100644 --- a/libs/usd-pointcloud-authoring/src/TiledPayloadAuthoring.h +++ b/libs/usd-pointcloud-authoring/src/TiledPayloadAuthoring.h @@ -1,11 +1,10 @@ #pragma once -#include "GeneratedPayloadSet.h" +#include "PayloadGeneration.h" #include "usdgeo/PointCloudLayer.h" #include -#include #include #include @@ -17,20 +16,36 @@ namespace usdgeo::detail { // identity to anchor one. pxr::UsdStageRefPtr CreateDetachedStage(); -std::filesystem::path TilePayloadPath(const std::filesystem::path& directory, - const usdpointcloud::PointTileId& id, - std::size_t lodIndex); +std::string TilePayloadName(const usdpointcloud::PointTileId& id, + std::size_t lodIndex); -std::vector TilePayloadPaths( - const std::filesystem::path& directory, +std::vector TilePayloadNames( const std::vector& tiles); -// Authors payload-backed tiles whose payload paths `payloads` has already -// claimed, writing each payload file and marking it written. -bool AuthorClaimedTilePayloads(const pxr::UsdStageRefPtr& stage, - const std::string& primPath, - const std::vector& tiles, - const PointCloudPayloadOptions& options, - GeneratedPayloadSet& payloads); +// Authors payload-backed tiles whose payload names `payloads` has declared, +// writing each payload where the generation stages it. Manifest entries are +// appended to `manifestEntries` and name the staged payloads until +// CommitTilePayloads repoints them. +bool AuthorClaimedTilePayloads( + const pxr::UsdStageRefPtr& stage, + const std::string& primPath, + const std::vector& tiles, + const PointCloudPayloadOptions& options, + PayloadGeneration& payloads, + std::vector& manifestEntries); + +// Publishes `payloads`, then points the payload arcs in `layer` and the +// manifest entries at the published generation instead of the staging +// directory. +bool CommitTilePayloads( + const pxr::SdfLayerHandle& layer, + PayloadGeneration& payloads, + std::vector& manifestEntries, + std::string& error); + +// Replaces one exact path component in every payload asset path in `layer`. +void RepointPayloads(const pxr::SdfLayerHandle& layer, + const std::string& fromComponent, + const std::string& toComponent); } // namespace usdgeo::detail diff --git a/libs/usd-pointcloud-authoring/tests/test_pointcloud_cache.cpp b/libs/usd-pointcloud-authoring/tests/test_pointcloud_cache.cpp index b130950..54e014c 100644 --- a/libs/usd-pointcloud-authoring/tests/test_pointcloud_cache.cpp +++ b/libs/usd-pointcloud-authoring/tests/test_pointcloud_cache.cpp @@ -1,7 +1,5 @@ #include "usdgeo/PointCloudCache.h" -#include "usdgeo/CacheKey.h" -#include "usdgeo/PointCloudLayer.h" #include "usdgeo/cache/Cache.h" #include "usdpointcloud/FileFormatArguments.h" @@ -32,6 +30,19 @@ void Check(bool condition) { } } +// Every file named `name` below `directory`. +std::vector FindFiles( + const std::filesystem::path& directory, const std::string& name) { + std::vector matches; + for (const auto& entry : + std::filesystem::recursive_directory_iterator(directory)) { + if (entry.is_regular_file() && entry.path().filename() == name) { + matches.push_back(entry.path()); + } + } + return matches; +} + std::string ReadBytes(const std::filesystem::path& path) { std::ifstream input(path, std::ios::binary); Check(input.good()); @@ -206,8 +217,15 @@ void TestPointCloudCacheMissAndMaterialization() { reference, request, "las-reader-1", hit, errorMessage)); Check(hit); Check(errorMessage.empty()); - const auto requestedPayload = testRoot / "requested_payloads" / "tile.usdc"; - Check(std::filesystem::exists(requestedPayload)); + // The copy lands in the layer's own directory under the requested one, + // in a generation named by the cache entry. + const auto requestedPayloads = testRoot / "requested_payloads"; + const auto materialized = FindFiles(requestedPayloads, "tile.usdc"); + Check(materialized.size() == 1); + const auto requestedPayload = materialized.front(); + const auto generation = requestedPayload.parent_path(); + Check(generation.filename().string().rfind("c-", 0) == 0); + Check(generation.parent_path().parent_path() == requestedPayloads); const auto cachedPrim = requestedLayer->GetPrimAtPath( pxr::SdfPath("/PointCloud/Tile")); @@ -215,7 +233,9 @@ void TestPointCloudCacheMissAndMaterialization() { const auto payloadItems = cachedPrim->GetPayloadList().GetPrependedItems(); Check(payloadItems.size() == 1); Check(payloadItems.front().GetAssetPath() == - "../requested_payloads/tile.usdc"); + "../requested_payloads/" + + generation.parent_path().filename().generic_string() + "/" + + generation.filename().generic_string() + "/tile.usdc"); { const auto requestedStage = pxr::UsdStage::Open(requestedLayer); @@ -229,18 +249,6 @@ void TestPointCloudCacheMissAndMaterialization() { requestedPositions[0] == pxr::GfVec3f(1.0f, 2.0f, 3.0f)); } - // Materialized payloads belong to the layer they were materialized for: - // a later hit replaces them when they are stale, and never overwrites a - // file someone else placed. - const auto requestedOwnerRecord = - requestedPayload.parent_path() / - ("payload-owner-" + - usdgeo::StableCacheKey( - {{"payloadOwner", usdgeo::PointCloudPayloadOwner( - resolverIdentity.identifier, request)}}) + - ".manifest"); - Check(std::filesystem::exists(requestedOwnerRecord)); - const auto cachedPayloadBytes = ReadBytes(payloadPath); const auto loadRequested = [&]() { const auto layer = pxr::SdfLayer::CreateAnonymous("requested-again.usda"); @@ -252,24 +260,29 @@ void TestPointCloudCacheMissAndMaterialization() { Check(loadError.empty()); return loadedHit; }; - std::ofstream(requestedPayload, std::ios::binary | std::ios::trunc) - << "stale payload"; + + // An intact copy from an earlier hit is reused without being written. + const auto materializedWrite = + std::filesystem::last_write_time(requestedPayload); Check(loadRequested()); - Check(ReadBytes(requestedPayload) == cachedPayloadBytes); + Check(FindFiles(requestedPayloads, "tile.usdc") == materialized); + Check(std::filesystem::last_write_time(requestedPayload) == + materializedWrite); - std::filesystem::remove(requestedOwnerRecord); + // A damaged copy is replaced by a fresh one from the cache. std::ofstream(requestedPayload, std::ios::binary | std::ios::trunc) + << "damaged"; + Check(loadRequested()); + Check(FindFiles(requestedPayloads, "tile.usdc") == materialized); + Check(ReadBytes(requestedPayload) == ReadBytes(payloadPath)); + + // A file someone else placed in the requested directory is never + // touched, because the layer's copies live in its own directory. + std::ofstream(requestedPayloads / "tile.usdc", std::ios::binary) << "user payload"; - Check(!loadRequested()); - Check(ReadBytes(requestedPayload) == "user payload"); - - // A file that already holds the cached bytes is taken over, so payloads - // materialized before ownership was recorded keep resolving. - Check(std::filesystem::copy_file( - payloadPath, requestedPayload, - std::filesystem::copy_options::overwrite_existing)); Check(loadRequested()); - Check(std::filesystem::exists(requestedOwnerRecord)); + Check(ReadBytes(requestedPayloads / "tile.usdc") == "user payload"); + Check(ReadBytes(requestedPayload) == ReadBytes(payloadPath)); std::error_code error; std::filesystem::remove(layout.manifest, error); diff --git a/libs/usd-pointcloud-authoring/tests/test_usd_authoring.cpp b/libs/usd-pointcloud-authoring/tests/test_usd_authoring.cpp index 8e34d3d..86df377 100644 --- a/libs/usd-pointcloud-authoring/tests/test_usd_authoring.cpp +++ b/libs/usd-pointcloud-authoring/tests/test_usd_authoring.cpp @@ -58,22 +58,32 @@ std::filesystem::path UniqueTestDirectory(const std::string& name) { return directory; } -std::filesystem::path OwnerRecordPath(const std::filesystem::path& directory, - const std::string& owner) { - return directory / ("payload-owner-" + - usdgeo::StableCacheKey({{"payloadOwner", owner}}) + - ".manifest"); +std::filesystem::path OwnerDirectory(const std::filesystem::path& directory, + const std::string& owner) { + return directory / usdgeo::StableCacheKey({{"payloadOwner", owner}}); } -std::string OwnerRecord(const std::vector& payloads) { - std::string record = "format=usd-pointcloud-payload-owner-v1\n" - "payload.count=" + - std::to_string(payloads.size()) + "\n"; - for (std::size_t index = 0; index < payloads.size(); ++index) { - record += "payload." + std::to_string(index) + "=" + - payloads[index] + "\n"; +std::vector Subdirectories( + const std::filesystem::path& directory, const std::string& prefix) { + std::vector matches; + std::error_code error; + for (const auto& entry : + std::filesystem::directory_iterator(directory, error)) { + if (entry.is_directory() && + entry.path().filename().string().rfind(prefix, 0) == 0) { + matches.push_back(entry.path()); + } } - return record; + std::sort(matches.begin(), matches.end()); + return matches; +} + +// The one published generation an owner has, which a test expects to exist. +std::filesystem::path OnlyGeneration(const std::filesystem::path& ownerDirectory) { + const auto generated = Subdirectories(ownerDirectory, "g-"); + const auto cached = Subdirectories(ownerDirectory, "c-"); + Check(generated.size() + cached.size() == 1); + return generated.empty() ? cached.front() : generated.front(); } class WarningCounter final : public pxr::TfDiagnosticMgr::Delegate { @@ -1258,11 +1268,9 @@ bool AuthorOwnedStream(pxr::SdfLayer* layer, diagnostics); } -void TestOwnedPayloadsAreRegeneratedByTheirOwner() { +void TestOwnedPayloadsLiveInTheirOwnersGeneration() { const auto directory = UniqueTestDirectory("usd_geo_owned_payloads"); - const auto first = directory / "Tile_L0_p0_p0_p0_LOD0.usdc"; - const auto second = directory / "Tile_L0_p1_p0_p0_LOD0.usdc"; - const auto record = OwnerRecordPath(directory, "layer-a"); + const auto ownerDirectory = OwnerDirectory(directory, "layer-a"); std::vector diagnostics; const auto layer = pxr::SdfLayer::CreateAnonymous("owned.usda"); @@ -1270,23 +1278,45 @@ void TestOwnedPayloadsAreRegeneratedByTheirOwner() { {{0.25, 0.0, 0.0}, {1.25, 0.0, 0.0}}, diagnostics)); Check(diagnostics.empty()); - Check(std::filesystem::exists(first) && std::filesystem::exists(second)); - Check(ReadFile(record) == - OwnerRecord({"Tile_L0_p0_p0_p0_LOD0.usdc", - "Tile_L0_p1_p0_p0_LOD0.usdc"})); - - // Reopening the same layer regenerates in place: the payload it still - // produces is replaced, and the one it no longer produces is removed. - const auto reopened = pxr::SdfLayer::CreateAnonymous("reopened.usda"); - Check(AuthorOwnedStream(reopened.operator->(), directory, "layer-a", + const auto generation = OnlyGeneration(ownerDirectory); + const auto first = generation / "Tile_L0_p0_p0_p0_LOD0.usdc"; + Check(std::filesystem::exists(first) && + std::filesystem::exists(generation / "Tile_L0_p1_p0_p0_LOD0.usdc")); + Check(Subdirectories(ownerDirectory, ".tmp-").empty()); + const auto lod = layer->GetPrimAtPath( + pxr::SdfPath("/PointCloud/Tiles/Tile_L0_p0_p0_p0/LOD0")); + Check(lod != nullptr); + const auto arcs = lod->GetPayloadList().GetPrependedItems(); + Check(arcs.size() == 1 && + arcs.front().GetAssetPath() == + ownerDirectory.filename().generic_string() + "/" + + generation.filename().generic_string() + + "/Tile_L0_p0_p0_p0_LOD0.usdc"); + + // Reading the layer again with identical content keeps the published + // generation as it is instead of replacing files that may be open. + const auto firstWrite = std::filesystem::last_write_time(first); + const auto again = pxr::SdfLayer::CreateAnonymous("again.usda"); + Check(AuthorOwnedStream(again.operator->(), directory, "layer-a", + {{0.25, 0.0, 0.0}, {1.25, 0.0, 0.0}}, + diagnostics)); + Check(OnlyGeneration(ownerDirectory) == generation); + Check(std::filesystem::last_write_time(first) == firstWrite); + + // Changed content is published as a new generation, and the superseded + // one is removed. + const auto changed = pxr::SdfLayer::CreateAnonymous("changed.usda"); + Check(AuthorOwnedStream(changed.operator->(), directory, "layer-a", {{0.25, 0.0, 0.0}, {0.75, 0.0, 0.0}}, diagnostics)); - Check(diagnostics.empty()); - Check(std::filesystem::exists(first) && !std::filesystem::exists(second)); - Check(ReadFile(record) == OwnerRecord({"Tile_L0_p0_p0_p0_LOD0.usdc"})); + const auto changedGeneration = OnlyGeneration(ownerDirectory); + Check(changedGeneration != generation); + Check(!std::filesystem::exists(generation)); + Check(!std::filesystem::exists( + changedGeneration / "Tile_L0_p1_p0_p0_LOD0.usdc")); { const auto rootLayerPath = directory / "PointCloud.usda"; - Check(reopened->Export(rootLayerPath.string())); + Check(changed->Export(rootLayerPath.string())); const auto stage = pxr::UsdStage::Open(rootLayerPath.string()); Check(stage); const auto points = pxr::UsdGeomPoints::Get( @@ -1298,117 +1328,119 @@ void TestOwnedPayloadsAreRegeneratedByTheirOwner() { std::filesystem::remove(rootLayerPath); } - // A different layer never replaces them, and neither does a generation - // without an owner. - const auto authoredPayload = ReadFile(first); + // Other layers and user files in the same payload directory never meet + // this owner's payloads: each owner has its own directory. + const auto published = + ReadFile(changedGeneration / "Tile_L0_p0_p0_p0_LOD0.usdc"); + WriteFile(directory / "Tile_L0_p0_p0_p0_LOD0.usdc", "user file"); const auto other = pxr::SdfLayer::CreateAnonymous("other.usda"); - Check(!AuthorOwnedStream(other.operator->(), directory, "layer-b", - {{0.25, 0.0, 0.0}}, diagnostics)); - Check(!diagnostics.empty() && - diagnostics.front().message.find("did not generate") != - std::string::npos); - Check(other->GetPrimAtPath(pxr::SdfPath("/PointCloud")) == nullptr); - Check(!std::filesystem::exists(OwnerRecordPath(directory, "layer-b"))); + Check(AuthorOwnedStream(other.operator->(), directory, "layer-b", + {{0.25, 0.0, 0.0}}, diagnostics)); + Check(OnlyGeneration(OwnerDirectory(directory, "layer-b")) != + changedGeneration); + Check(ReadFile(changedGeneration / "Tile_L0_p0_p0_p0_LOD0.usdc") == + published); + Check(ReadFile(directory / "Tile_L0_p0_p0_p0_LOD0.usdc") == "user file"); + + // Without an owner the directory stays exclusive. Check(!AuthorOwnedStream(other.operator->(), directory, "", {{0.25, 0.0, 0.0}}, diagnostics)); - Check(ReadFile(first) == authoredPayload); - Check(ReadFile(record) == OwnerRecord({"Tile_L0_p0_p0_p0_LOD0.usdc"})); - - // A file the owner never generated is not claimed on the owner's behalf. - WriteFile(second, "user file"); - Check(!AuthorOwnedStream(reopened.operator->(), directory, "layer-a", - {{0.25, 0.0, 0.0}, {1.25, 0.0, 0.0}}, - diagnostics)); - Check(ReadFile(second) == "user file"); - Check(ReadFile(first) == authoredPayload); - Check(ReadFile(record) == OwnerRecord({"Tile_L0_p0_p0_p0_LOD0.usdc"})); + Check(ReadFile(directory / "Tile_L0_p0_p0_p0_LOD0.usdc") == "user file"); std::filesystem::remove_all(directory); } -void TestInterruptedOwnedGenerationIsReplaceable() { +void TestInterruptedOwnedGenerationIsRecovered() { const auto directory = UniqueTestDirectory("usd_geo_interrupted_owned_payloads"); - const auto payload = directory / "Tile_L0_p0_p0_p0_LOD0.usdc"; - const auto record = OwnerRecordPath(directory, "layer-a"); - std::filesystem::create_directories(directory); - // What an interrupted generation leaves behind: its claim, recorded - // before the first write, and a payload no root ever referenced. - WriteFile(payload, "partial payload"); - WriteFile(record, OwnerRecord({"Tile_L0_p0_p0_p0_LOD0.usdc", - "Tile_L0_p3_p0_p0_LOD0.usdc"})); + const auto ownerDirectory = OwnerDirectory(directory, "layer-a"); + // What interrupted generations leave behind: staging directories that + // were never published. A live generation keeps its own fresh. + const auto stale = ownerDirectory / ".tmp-stale"; + const auto live = ownerDirectory / ".tmp-live"; + std::filesystem::create_directories(stale); + std::filesystem::create_directories(live); + WriteFile(stale / "Tile_L0_p0_p0_p0_LOD0.usdc", "partial payload"); + std::filesystem::last_write_time( + stale, std::filesystem::file_time_type::clock::now() - + std::chrono::hours(2)); std::vector diagnostics; const auto layer = pxr::SdfLayer::CreateAnonymous("recovered.usda"); Check(AuthorOwnedStream(layer.operator->(), directory, "layer-a", {{0.25, 0.0, 0.0}}, diagnostics)); Check(diagnostics.empty()); - Check(ReadFile(payload) != "partial payload"); - Check(ReadFile(record) == OwnerRecord({"Tile_L0_p0_p0_p0_LOD0.usdc"})); - - // A record naming a path outside the directory is refused as a whole - // rather than trusted with a removal. - WriteFile(record, OwnerRecord({"../Tile_L0_p0_p0_p0_LOD0.usdc"})); - const auto authoredPayload = ReadFile(payload); - Check(!AuthorOwnedStream(layer.operator->(), directory, "layer-a", - {{0.25, 0.0, 0.0}}, diagnostics)); - Check(!diagnostics.empty() && - diagnostics.front().message.find("is invalid") != std::string::npos); - Check(ReadFile(payload) == authoredPayload); + Check(!std::filesystem::exists(stale)); + Check(std::filesystem::exists(live)); + + // A published generation that lost a file is replaced when the same + // content is published again, rather than trusted. + const auto generation = OnlyGeneration(ownerDirectory); + const auto payload = generation / "Tile_L0_p0_p0_p0_LOD0.usdc"; + std::filesystem::remove(payload); + Check(AuthorOwnedStream(layer.operator->(), directory, "layer-a", + {{0.25, 0.0, 0.0}}, diagnostics)); + Check(OnlyGeneration(ownerDirectory) == generation); + Check(std::filesystem::exists(payload)); std::filesystem::remove_all(directory); } -void TestFailedOwnedGenerationRestoresOwnership() { +void TestFailedOwnedGenerationKeepsPublishedPayloads() { const auto directory = UniqueTestDirectory("usd_geo_failed_owned_payloads"); - const auto first = directory / "Tile_L0_p0_p0_p0_LOD0.usdc"; - const auto second = directory / "Tile_L0_p1_p0_p0_LOD0.usdc"; - const auto record = OwnerRecordPath(directory, "layer-a"); + const auto ownerDirectory = OwnerDirectory(directory, "layer-a"); std::vector diagnostics; const auto layer = pxr::SdfLayer::CreateAnonymous("failed_owned.usda"); Check(AuthorOwnedStream(layer.operator->(), directory, "layer-a", {{0.25, 0.0, 0.0}, {1.25, 0.0, 0.0}}, diagnostics)); - const auto ownership = ReadFile(record); - const auto untouched = ReadFile(second); - const auto previous = ReadFile(first); - - // Cancel as soon as the regeneration has replaced its first payload. - const auto replacedFirst = [&first, &previous]() { - std::ifstream input(first, std::ios::binary); - const std::string bytes{std::istreambuf_iterator(input), - std::istreambuf_iterator()}; - return bytes != previous; + const auto generation = OnlyGeneration(ownerDirectory); + const auto published = ReadDirectoryFiles(generation); + + // Cancel as soon as the regeneration has staged its first payload. + const auto stagedFirst = [&ownerDirectory]() { + for (const auto& staging : Subdirectories(ownerDirectory, ".tmp-")) { + if (std::filesystem::exists(staging / + "Tile_L0_p0_p0_p0_LOD0.usdc")) { + return true; + } + } + return false; }; const auto regenerated = pxr::SdfLayer::CreateAnonymous("cancelled.usda"); Check(!AuthorOwnedStream(regenerated.operator->(), directory, "layer-a", {{0.5, 0.0, 0.0}, {1.25, 0.0, 0.0}}, - diagnostics, replacedFirst)); + diagnostics, stagedFirst)); Check(!diagnostics.empty()); Check(regenerated->GetPrimAtPath(pxr::SdfPath("/PointCloud")) == nullptr); - Check(!std::filesystem::exists(first)); - Check(ReadFile(second) == untouched); - Check(ReadFile(record) == ownership); - Check(AuthorOwnedStream(regenerated.operator->(), directory, "layer-a", - {{0.25, 0.0, 0.0}, {1.25, 0.0, 0.0}}, - diagnostics)); + Check(OnlyGeneration(ownerDirectory) == generation); + Check(ReadDirectoryFiles(generation) == published); + Check(Subdirectories(ownerDirectory, ".tmp-").empty()); std::filesystem::remove_all(directory); - // A cancelled first generation removes the directory it created, - // ownership record included. + // A cancelled first generation removes every directory it created. const auto created = UniqueTestDirectory("usd_geo_cancelled_owned_payloads"); - const auto createdPayload = created / "Tile_L0_p0_p0_p0_LOD0.usdc"; + const auto createdOwner = OwnerDirectory(created, "layer-a"); const auto fresh = pxr::SdfLayer::CreateAnonymous("fresh.usda"); Check(!AuthorOwnedStream( fresh.operator->(), created, "layer-a", {{0.25, 0.0, 0.0}, {1.25, 0.0, 0.0}}, diagnostics, - [&createdPayload]() { return std::filesystem::exists(createdPayload); })); + [&createdOwner]() { + std::error_code error; + for (const auto& staging : + std::filesystem::directory_iterator(createdOwner, error)) { + if (std::filesystem::exists(staging.path() / + "Tile_L0_p0_p0_p0_LOD0.usdc")) { + return true; + } + } + return false; + })); Check(!std::filesystem::exists(created)); } void TestDetachedAuthoringDoesNotResolvePayloads() { const auto directory = UniqueTestDirectory("usd_geo_detached_payloads"); - const auto payload = directory / "Tile_L0_p0_p0_p0_LOD0.usdc"; usdgeo::GeoReference reference; reference.epsgCode = 26910; @@ -1441,23 +1473,27 @@ void TestDetachedAuthoringDoesNotResolvePayloads() { Check(diagnostics.empty()); Check(layer->GetPrimAtPath(pxr::SdfPath( "/PointCloud/Tiles/Tile_L0_p0_p0_p0/LOD0")) != nullptr); + const auto payload = + OnlyGeneration(OwnerDirectory(directory, "detached-layer")) / + "Tile_L0_p0_p0_p0_LOD0.usdc"; Check(std::filesystem::exists(payload)); Check(!pxr::SdfLayer::Find(payload.string())); - // The same layer regenerates over its own payload. + // The same layer is authored again over its own payloads. Check(usdgeo::AuthorPointCloudTiledAssetWithPayloads( layer.operator->(), "/PointCloud", {tile}, options, diagnostics)); + std::filesystem::remove_all(directory); // A failure reports why and leaves the layer as it was. + WriteFile(directory, "not a directory"); const auto untouched = pxr::SdfLayer::CreateAnonymous("untouched.usda"); - options.owner = "another-layer"; Check(!usdgeo::AuthorPointCloudTiledAssetWithPayloads( untouched.operator->(), "/PointCloud", {tile}, options, diagnostics)); Check(!diagnostics.empty() && - diagnostics.front().message.find("did not generate") != + diagnostics.front().message.find("not a usable directory") != std::string::npos); Check(untouched->GetPrimAtPath(pxr::SdfPath("/PointCloud")) == nullptr); - std::filesystem::remove_all(directory); + std::filesystem::remove(directory); const auto streamDirectory = UniqueTestDirectory("usd_geo_detached_stream_payloads"); @@ -1471,7 +1507,9 @@ void TestDetachedAuthoringDoesNotResolvePayloads() { Check(warnings.Count() == 0); } Check(!pxr::SdfLayer::Find( - (streamDirectory / "Tile_L0_p0_p0_p0_LOD0.usdc").string())); + (OnlyGeneration(OwnerDirectory(streamDirectory, "detached-stream")) / + "Tile_L0_p0_p0_p0_LOD0.usdc") + .string())); std::filesystem::remove_all(streamDirectory); } @@ -1521,9 +1559,9 @@ int main() { TestStreamCancellationDuringSpoolReadCleansSpools(); TestStreamFailureDoesNotMutateLayer(); TestStreamPayloadFailureRollsBackGeneratedPayloads(); - TestOwnedPayloadsAreRegeneratedByTheirOwner(); - TestInterruptedOwnedGenerationIsReplaceable(); - TestFailedOwnedGenerationRestoresOwnership(); + TestOwnedPayloadsLiveInTheirOwnersGeneration(); + TestInterruptedOwnedGenerationIsRecovered(); + TestFailedOwnedGenerationKeepsPublishedPayloads(); TestDetachedAuthoringDoesNotResolvePayloads(); return 0; } \ No newline at end of file diff --git a/plugins/pointcloud-copc/README.md b/plugins/pointcloud-copc/README.md index c667a82..a6645e2 100644 --- a/plugins/pointcloud-copc/README.md +++ b/plugins/pointcloud-copc/README.md @@ -27,9 +27,9 @@ while decoded points pass through a bounded buffer. Remote COPC is supported only when the active resolver supplies an asset with efficient random-access reads, such as HTTP byte-range support. The plugin does not implement an HTTP client, transport retries, or a network cache. Remote tiled reads require an -absolute local `payloadDirectory`. Reading a tiled layer again replaces the -payloads it generated there and refuses any other file; the directory records -ownership under a hash, so no resolver identifier is written. See +absolute local `payloadDirectory`. Each tiled layer publishes its payloads in +a directory of its own below it, named by a hash, so no resolver identifier is +written. See [payload ownership](../../docs/architecture/FILE_FORMAT_ARGUMENTS.md#generated-payload-ownership). Generated-USDC cache lookup is enabled for a stable local filesystem identity diff --git a/plugins/pointcloud-copc/src/UsdGeoCopcFileFormat.cpp b/plugins/pointcloud-copc/src/UsdGeoCopcFileFormat.cpp index 5b660f9..887307b 100644 --- a/plugins/pointcloud-copc/src/UsdGeoCopcFileFormat.cpp +++ b/plugins/pointcloud-copc/src/UsdGeoCopcFileFormat.cpp @@ -589,8 +589,8 @@ bool UsdGeoCopcFileFormat::Read(SdfLayer* layer, usdgeo::PointCloudPayloadOptions payloadOptions{ payloadDirectory.string(), resolvedPath, request.tileMemoryLimitBytes}; - payloadOptions.owner = - usdgeo::PointCloudPayloadOwner(resolvedPath, request); + payloadOptions.owner = usdgeo::PointCloudPayloadOwner( + resolvedPath, payloadDirectory, layer->GetFileFormatArguments()); if (!usdgeo::AuthorPointCloudTiledAssetWithPayloads( layer, "/PointCloud", tiles, payloadOptions, diagnostics)) { TF_RUNTIME_ERROR("%s", usdgeocopc::diagnostics::Message( diff --git a/plugins/pointcloud-copc/tests/test_pointcloud_copc.cpp b/plugins/pointcloud-copc/tests/test_pointcloud_copc.cpp index 6192784..e67a7a8 100644 --- a/plugins/pointcloud-copc/tests/test_pointcloud_copc.cpp +++ b/plugins/pointcloud-copc/tests/test_pointcloud_copc.cpp @@ -735,9 +735,13 @@ void TestTiledReadReopens() { } Check(!pxr::SdfLayer::Find(source.string(), arguments), "tiled COPC layer was released"); - Check(std::filesystem::exists(directory / "payloads" / - "Tile_L0_p0_p0_p0_LOD0.usdc"), - "tiled COPC payload exists"); + bool hasPayload = false; + for (const auto& entry : std::filesystem::recursive_directory_iterator( + directory / "payloads")) { + hasPayload = hasPayload || + entry.path().filename() == "Tile_L0_p0_p0_p0_LOD0.usdc"; + } + Check(hasPayload, "tiled COPC payload exists"); std::error_code error; std::filesystem::remove_all(directory, error); } diff --git a/plugins/pointcloud-las/README.md b/plugins/pointcloud-las/README.md index e5167af..51afd43 100644 --- a/plugins/pointcloud-las/README.md +++ b/plugins/pointcloud-las/README.md @@ -63,7 +63,7 @@ canonical map that participates in layer identity. | `tile` | `true` | Routes the pull stream into source-coordinate tile payloads | | `tileSize` | positive source units | Fixed-grid tile width and depth | | `tileMemoryLimit` | positive bytes | Per-tile spool buffer limit | -| `payloadDirectory` | path | Directory for generated USDC payloads; reading the layer again replaces the payloads it generated there and refuses any other file; see [payload ownership](../../docs/architecture/FILE_FORMAT_ARGUMENTS.md#generated-payload-ownership) | +| `payloadDirectory` | path | Directory for generated USDC payloads; each layer publishes its payloads in a directory of its own below it, so reading the layer again never collides with other layers or files; see [payload ownership](../../docs/architecture/FILE_FORMAT_ARGUMENTS.md#generated-payload-ownership) | ```bash usdcat "sample.las:SDF_FORMAT_ARGS:lod=balanced&attributes=xyz,rgb" diff --git a/plugins/pointcloud-las/src/UsdGeoLasFileFormat.cpp b/plugins/pointcloud-las/src/UsdGeoLasFileFormat.cpp index 651c7e9..2b81e01 100644 --- a/plugins/pointcloud-las/src/UsdGeoLasFileFormat.cpp +++ b/plugins/pointcloud-las/src/UsdGeoLasFileFormat.cpp @@ -299,8 +299,8 @@ bool UsdGeoLasFileFormat::Read(SdfLayer* layer, usdgeo::PointCloudPayloadOptions payloadOptions{ payloadDirectory.string(), rootLayerPath, request.tileMemoryLimitBytes}; - payloadOptions.owner = - usdgeo::PointCloudPayloadOwner(resolvedPath, request); + payloadOptions.owner = usdgeo::PointCloudPayloadOwner( + resolvedPath, payloadDirectory, layer->GetFileFormatArguments()); if (!usdgeo::AuthorPointCloudTiledAssetFromStream( layer, "/PointCloud", selected, reference, {request.tileSize, 0}, payloadOptions, diagnostics)) { diff --git a/plugins/pointcloud-las/tests/test_pointcloud_las.cpp b/plugins/pointcloud-las/tests/test_pointcloud_las.cpp index 14a700c..8300f05 100644 --- a/plugins/pointcloud-las/tests/test_pointcloud_las.cpp +++ b/plugins/pointcloud-las/tests/test_pointcloud_las.cpp @@ -91,6 +91,20 @@ std::vector MakeFixture() { return bytes; } +// Tiled reads publish their payloads in a directory of their own below the +// requested one, so tests find a payload by name. +bool HasPayload(const std::filesystem::path& directory, const std::string& name) { + std::error_code error; + for (std::filesystem::recursive_directory_iterator entry(directory, error), + end; + !error && entry != end; entry.increment(error)) { + if (entry->path().filename() == name) { + return true; + } + } + return false; +} + void TestFileFormatIntegration() { const auto plugInfo = std::filesystem::path(USDGEOLAS_SOURCE_DIR) / "plugin" / "resources" / "pointcloud-las" / @@ -217,10 +231,10 @@ void TestFileFormatIntegration() { Check(tiledStage->GetPrimAtPath(pxr::SdfPath( "/PointCloud/Tiles/Tile_L0_p1001_p2001_p0/LOD0")) .IsValid()); - Check(std::filesystem::exists( - tiledPayloadDirectory / "Tile_L0_p1000_p2000_p0_LOD0.usdc")); - Check(std::filesystem::exists( - tiledPayloadDirectory / "Tile_L0_p1001_p2001_p0_LOD0.usdc")); + Check(HasPayload(tiledPayloadDirectory, + "Tile_L0_p1000_p2000_p0_LOD0.usdc")); + Check(HasPayload(tiledPayloadDirectory, + "Tile_L0_p1001_p2001_p0_LOD0.usdc")); } Check(!pxr::SdfLayer::Find(path.string(), tiledArguments)); std::filesystem::remove_all(tiledPayloadDirectory); diff --git a/plugins/pointcloud-laz/README.md b/plugins/pointcloud-laz/README.md index cc44bc4..0a97a2c 100644 --- a/plugins/pointcloud-laz/README.md +++ b/plugins/pointcloud-laz/README.md @@ -66,7 +66,7 @@ layer-identity participation. | `tile` | `true` | Routes the pull stream into source-coordinate tile payloads | | `tileSize` | positive source units | Fixed-grid tile width and depth | | `tileMemoryLimit` | positive bytes | Per-tile spool buffer limit | -| `payloadDirectory` | path | Directory for generated USDC payloads; reading the layer again replaces the payloads it generated there and refuses any other file; see [payload ownership](../../docs/architecture/FILE_FORMAT_ARGUMENTS.md#generated-payload-ownership) | +| `payloadDirectory` | path | Directory for generated USDC payloads; each layer publishes its payloads in a directory of its own below it, so reading the layer again never collides with other layers or files; see [payload ownership](../../docs/architecture/FILE_FORMAT_ARGUMENTS.md#generated-payload-ownership) | ```bash usdcat "sample.laz:SDF_FORMAT_ARGS:lod=preview" diff --git a/plugins/pointcloud-laz/src/UsdGeoLazFileFormat.cpp b/plugins/pointcloud-laz/src/UsdGeoLazFileFormat.cpp index 40e9ef6..0f4b365 100644 --- a/plugins/pointcloud-laz/src/UsdGeoLazFileFormat.cpp +++ b/plugins/pointcloud-laz/src/UsdGeoLazFileFormat.cpp @@ -279,8 +279,8 @@ bool UsdGeoLazFileFormat::Read(SdfLayer* layer, usdgeo::PointCloudPayloadOptions payloadOptions{ payloadDirectory.string(), rootLayerPath, request.tileMemoryLimitBytes}; - payloadOptions.owner = - usdgeo::PointCloudPayloadOwner(resolvedPath, request); + payloadOptions.owner = usdgeo::PointCloudPayloadOwner( + resolvedPath, payloadDirectory, layer->GetFileFormatArguments()); if (!usdgeo::AuthorPointCloudTiledAssetFromStream( layer, "/PointCloud", selected, reference, {request.tileSize, 0}, payloadOptions, diagnostics)) { diff --git a/plugins/pointcloud-ply/README.md b/plugins/pointcloud-ply/README.md index f1a7838..d216776 100644 --- a/plugins/pointcloud-ply/README.md +++ b/plugins/pointcloud-ply/README.md @@ -15,8 +15,8 @@ contracts, and requires an explicit CRS because PLY has no embedded CRS model. The `epsg` file-format argument is required. `linearUnit`, `sourceUpAxis`, and `stageUpAxis` are optional. Tiled reads use `tileSize`, `tileMemoryLimit`, and -`payloadDirectory`; reading the layer again replaces the payloads it generated -there and refuses any other file. See +`payloadDirectory`, below which each layer publishes its payloads in a +directory of its own. See [payload ownership](../../docs/architecture/FILE_FORMAT_ARGUMENTS.md#generated-payload-ownership). Faces, mesh authoring, PLY writing, and metadata-only reads are out of scope. diff --git a/plugins/pointcloud-ply/src/UsdGeoPlyFileFormat.cpp b/plugins/pointcloud-ply/src/UsdGeoPlyFileFormat.cpp index 4f20f91..268b6ef 100644 --- a/plugins/pointcloud-ply/src/UsdGeoPlyFileFormat.cpp +++ b/plugins/pointcloud-ply/src/UsdGeoPlyFileFormat.cpp @@ -207,8 +207,8 @@ bool UsdGeoPlyFileFormat::Read(SdfLayer* layer, usdgeo::PointCloudPayloadOptions payloadOptions{ payloadDirectory.string(), resolvedPath, request.tileMemoryLimitBytes}; - payloadOptions.owner = - usdgeo::PointCloudPayloadOwner(resolvedPath, request); + payloadOptions.owner = usdgeo::PointCloudPayloadOwner( + resolvedPath, payloadDirectory, layer->GetFileFormatArguments()); if (!usdgeo::AuthorPointCloudTiledAssetFromStream( layer, "/PointCloud", selected, reference, {request.tileSize, 0}, payloadOptions, diagnostics)) { diff --git a/plugins/pointcloud-ply/tests/test_pointcloud_ply.cpp b/plugins/pointcloud-ply/tests/test_pointcloud_ply.cpp index 3d732f6..cc4ccf6 100644 --- a/plugins/pointcloud-ply/tests/test_pointcloud_ply.cpp +++ b/plugins/pointcloud-ply/tests/test_pointcloud_ply.cpp @@ -103,8 +103,15 @@ void TestFileFormatIntegration() { Check(tiledStage->GetPrimAtPath(pxr::SdfPath( "/PointCloud/Tiles/Tile_L0_p0_p0_p0/LOD0")) .IsValid()); - Check(std::filesystem::exists( - tiledPayloadDirectory / "Tile_L0_p0_p0_p0_LOD0.usdc")); + // The layer publishes its payloads in a directory of its own below the + // requested one. + bool hasPayload = false; + for (const auto& entry : std::filesystem::recursive_directory_iterator( + tiledPayloadDirectory)) { + hasPayload = hasPayload || + entry.path().filename() == "Tile_L0_p0_p0_p0_LOD0.usdc"; + } + Check(hasPayload); std::filesystem::remove_all(tiledPayloadDirectory); }