From b48f1381af990562fbe1bbb8bb078d02febddf8f Mon Sep 17 00:00:00 2001 From: Tim Moore Date: Tue, 11 Jun 2024 16:36:47 +0200 Subject: [PATCH 01/11] Pass up axis to GltfConverters The i3dm converter needs to know the coordinate axis convention of the tileset in order to correctly construct instance transformations. Resolves #904. --- .../Cesium3DTilesContent/BinaryToGltfConverter.h | 3 ++- .../include/Cesium3DTilesContent/GltfConverters.h | 8 ++++++-- Cesium3DTilesContent/src/BinaryToGltfConverter.cpp | 10 ++++++++-- Cesium3DTilesSelection/src/ImplicitOctreeLoader.cpp | 3 ++- Cesium3DTilesSelection/src/ImplicitQuadtreeLoader.cpp | 3 ++- Cesium3DTilesSelection/src/TilesetJsonLoader.cpp | 3 ++- 6 files changed, 22 insertions(+), 8 deletions(-) diff --git a/Cesium3DTilesContent/include/Cesium3DTilesContent/BinaryToGltfConverter.h b/Cesium3DTilesContent/include/Cesium3DTilesContent/BinaryToGltfConverter.h index ed4459de4..6e95e98e2 100644 --- a/Cesium3DTilesContent/include/Cesium3DTilesContent/BinaryToGltfConverter.h +++ b/Cesium3DTilesContent/include/Cesium3DTilesContent/BinaryToGltfConverter.h @@ -21,7 +21,8 @@ struct BinaryToGltfConverter { private: static GltfConverterResult convertImmediate( const gsl::span& gltfBinary, - const CesiumGltfReader::GltfReaderOptions& options); + const CesiumGltfReader::GltfReaderOptions& options, + const AssetFetcher& assetFetcher); static CesiumGltfReader::GltfReader _gltfReader; }; } // namespace Cesium3DTilesContent diff --git a/Cesium3DTilesContent/include/Cesium3DTilesContent/GltfConverters.h b/Cesium3DTilesContent/include/Cesium3DTilesContent/GltfConverters.h index f988b8535..c4837b17b 100644 --- a/Cesium3DTilesContent/include/Cesium3DTilesContent/GltfConverters.h +++ b/Cesium3DTilesContent/include/Cesium3DTilesContent/GltfConverters.h @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -30,12 +31,14 @@ struct CESIUM3DTILESCONTENT_API AssetFetcher { const std::shared_ptr& pAssetAccessor_, const std::string& baseUrl_, const glm::dmat4 tileTransform_, - const std::vector& requestHeaders_) + const std::vector& requestHeaders_, + CesiumGeometry::Axis upAxis_) : asyncSystem(asyncSystem_), pAssetAccessor(pAssetAccessor_), baseUrl(baseUrl_), tileTransform(tileTransform_), - requestHeaders(requestHeaders_) {} + requestHeaders(requestHeaders_), + upAxis(upAxis_) {} CesiumAsync::Future get(const std::string& relativeUrl) const; @@ -45,6 +48,7 @@ struct CESIUM3DTILESCONTENT_API AssetFetcher { const std::string baseUrl; glm::dmat4 tileTransform; // For ENU transforms in i3dm const std::vector& requestHeaders; + const CesiumGeometry::Axis upAxis; }; /** diff --git a/Cesium3DTilesContent/src/BinaryToGltfConverter.cpp b/Cesium3DTilesContent/src/BinaryToGltfConverter.cpp index 9a9bcf401..396f96df9 100644 --- a/Cesium3DTilesContent/src/BinaryToGltfConverter.cpp +++ b/Cesium3DTilesContent/src/BinaryToGltfConverter.cpp @@ -6,10 +6,16 @@ CesiumGltfReader::GltfReader BinaryToGltfConverter::_gltfReader; GltfConverterResult BinaryToGltfConverter::convertImmediate( const gsl::span& gltfBinary, - const CesiumGltfReader::GltfReaderOptions& options) { + const CesiumGltfReader::GltfReaderOptions& options, + const AssetFetcher& assetFetcher) { CesiumGltfReader::GltfReaderResult loadedGltf = _gltfReader.readGltf(gltfBinary, options); + if (loadedGltf.model) { + loadedGltf.model->extras["gltfUpAxis"] = + static_cast>( + assetFetcher.upAxis); + } GltfConverterResult result; result.model = std::move(loadedGltf.model); result.errors.errors = std::move(loadedGltf.errors); @@ -22,6 +28,6 @@ CesiumAsync::Future BinaryToGltfConverter::convert( const CesiumGltfReader::GltfReaderOptions& options, const AssetFetcher& assetFetcher) { return assetFetcher.asyncSystem.createResolvedFuture( - convertImmediate(gltfBinary, options)); + convertImmediate(gltfBinary, options, assetFetcher)); } } // namespace Cesium3DTilesContent diff --git a/Cesium3DTilesSelection/src/ImplicitOctreeLoader.cpp b/Cesium3DTilesSelection/src/ImplicitOctreeLoader.cpp index a67d41aad..1c5bbddaf 100644 --- a/Cesium3DTilesSelection/src/ImplicitOctreeLoader.cpp +++ b/Cesium3DTilesSelection/src/ImplicitOctreeLoader.cpp @@ -159,7 +159,8 @@ CesiumAsync::Future requestTileContent( pAssetAccessor, tileUrl, tileTransform, - requestHeaders}; + requestHeaders, + CesiumGeometry::Axis::Y}; return converter(responseData, gltfOptions, assetFetcher) .thenImmediately([pLogger, tileUrl, pCompletedRequest]( GltfConverterResult&& result) { diff --git a/Cesium3DTilesSelection/src/ImplicitQuadtreeLoader.cpp b/Cesium3DTilesSelection/src/ImplicitQuadtreeLoader.cpp index d2833425a..373ab718b 100644 --- a/Cesium3DTilesSelection/src/ImplicitQuadtreeLoader.cpp +++ b/Cesium3DTilesSelection/src/ImplicitQuadtreeLoader.cpp @@ -169,7 +169,8 @@ CesiumAsync::Future requestTileContent( pAssetAccessor, tileUrl, tileTransform, - requestHeaders}; + requestHeaders, + CesiumGeometry::Axis::Y}; return converter(responseData, gltfOptions, assetFetcher) .thenImmediately([pLogger, tileUrl, pCompletedRequest]( GltfConverterResult&& result) { diff --git a/Cesium3DTilesSelection/src/TilesetJsonLoader.cpp b/Cesium3DTilesSelection/src/TilesetJsonLoader.cpp index f9b021e8f..21151cf9f 100644 --- a/Cesium3DTilesSelection/src/TilesetJsonLoader.cpp +++ b/Cesium3DTilesSelection/src/TilesetJsonLoader.cpp @@ -908,7 +908,8 @@ TilesetJsonLoader::loadTileContent(const TileLoadInput& loadInput) { pAssetAccessor, tileUrl, tileTransform, - requestHeaders}; + requestHeaders, + upAxis}; CesiumGltfReader::GltfReaderOptions gltfOptions; gltfOptions.ktx2TranscodeTargets = contentOptions.ktx2TranscodeTargets; From 9c9a5d9dc273f102e5be5e43d55023d44652c68f Mon Sep 17 00:00:00 2001 From: Tim Moore Date: Mon, 24 Jun 2024 14:39:05 +0200 Subject: [PATCH 02/11] Resolve external data for external i3dm glb content Calling GltfReader::resolveExternalData() during the i3dm conversion correctly handles relative path references in the .glb payload or in an external .glb file. Leaving it to Cesium3DTilesSelection::TilesetContentManager::loadTileContent(), where external data is resolved for normal 3D Tiles, gets relative paths wrong. Resolves #905. --- .../src/I3dmToGltfConverter.cpp | 104 ++++++++++++------ 1 file changed, 68 insertions(+), 36 deletions(-) diff --git a/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp b/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp index 68039efbe..894b2660e 100644 --- a/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp +++ b/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include @@ -420,8 +421,8 @@ CesiumAsync::Future convertI3dmContent( CesiumGeospatial::LocalHorizontalCoordinateSystem enu( (glm::dvec3(worldPos))); const glm::dmat4& ecef = enu.getLocalToEcefTransformation(); - // Express the rotation in the tile's coordinate system, like explicit - // I3dm instance rotations. + // Express the rotation in the tile's coordinate system, just like + // explicit i3dm instance rotations. glm::dmat4 tileFrame = worldTransformInv * ecef; return rotationFromUpRight( glm::vec3(tileFrame[1]), @@ -454,44 +455,75 @@ CesiumAsync::Future convertI3dmContent( }); } repositionInstances(decodedInstances); - AssetFetcherResult assetFetcherResult; + std::string baseUri; if (header.gltfFormat == 0) { - // Recursively fetch and read the glTF content. - auto gltfUri = std::string( + std::string gltfUri( reinterpret_cast(gltfData.data()), gltfData.size()); - return assetFetcher.get(gltfUri) - .thenImmediately( - [options, assetFetcher](AssetFetcherResult&& assetFetcherResult) - -> CesiumAsync::Future { - if (assetFetcherResult.errorList.hasErrors()) { - GltfConverterResult errorResult; - errorResult.errors.merge(assetFetcherResult.errorList); - return assetFetcher.asyncSystem.createResolvedFuture( - std::move(errorResult)); - } - return BinaryToGltfConverter::convert( - assetFetcherResult.bytes, - options, - assetFetcher); - }) - .thenImmediately([convertedI3dm = std::move(convertedI3dm)]( - GltfConverterResult&& converterResult) mutable { - if (converterResult.model) - convertedI3dm.gltfResult.model = std::move(converterResult.model); - convertedI3dm.gltfResult.errors.merge(converterResult.errors); - return convertedI3dm; - }); + baseUri = CesiumUtility::Uri::resolve(assetFetcher.baseUrl, gltfUri); } else { - return BinaryToGltfConverter::convert(gltfData, options, assetFetcher) - .thenImmediately([convertedI3dm = std::move(convertedI3dm)]( - GltfConverterResult&& converterResult) mutable { - if (converterResult.model) - convertedI3dm.gltfResult.model = std::move(converterResult.model); - convertedI3dm.gltfResult.errors.merge(converterResult.errors); - return convertedI3dm; - }); - } + baseUri = assetFetcher.baseUrl; + } + // This use of a lambda is the only way timoore knows to conditionally pass + // different futures to a then...() member function. + return + [&]() -> CesiumAsync::Future { + if (header.gltfFormat == 0) { + // Recursively fetch and read the glTF content. + return assetFetcher.get(baseUri).thenImmediately( + [options, assetFetcher](AssetFetcherResult&& assetFetcherResult) + -> CesiumAsync::Future { + if (assetFetcherResult.errorList.hasErrors()) { + GltfConverterResult errorResult; + errorResult.errors.merge(assetFetcherResult.errorList); + return assetFetcher.asyncSystem.createResolvedFuture( + std::move(errorResult)); + } + return BinaryToGltfConverter::convert( + assetFetcherResult.bytes, + options, + assetFetcher); + }); + } else { + return BinaryToGltfConverter::convert( + gltfData, + options, + assetFetcher); + } + }() + .thenImmediately([options, assetFetcher, baseUri]( + GltfConverterResult&& converterResult) { + if (converterResult.model.has_value()) { + CesiumGltfReader::GltfReaderResult readerResult{ + std::move(*converterResult.model)}; + CesiumAsync::HttpHeaders externalRequestHeaders( + assetFetcher.requestHeaders.begin(), + assetFetcher.requestHeaders.end()); + return CesiumGltfReader::GltfReader::resolveExternalData( + assetFetcher.asyncSystem, + baseUri, + externalRequestHeaders, + assetFetcher.pAssetAccessor, + options, + std::move(readerResult)); + } + return assetFetcher.asyncSystem.createResolvedFuture( + CesiumGltfReader::GltfReaderResult{ + std::nullopt, + std::move(converterResult.errors.errors)}); + }) + .thenImmediately([convertedI3dm = std::move(convertedI3dm)]( + CesiumGltfReader::GltfReaderResult&& + readerResult) mutable { + if (readerResult.model) + convertedI3dm.gltfResult.model = + std::move(readerResult.model); + CesiumUtility::ErrorList resolvedExternalErrors{ + std::move(readerResult.errors)}; + convertedI3dm.gltfResult.errors.merge( + resolvedExternalErrors); + return convertedI3dm; + }); } glm::dmat4 From 2283f5c380e8f3fc79cba9e6b12d40068ac12deb Mon Sep 17 00:00:00 2001 From: Tim Moore Date: Mon, 24 Jun 2024 15:56:21 +0200 Subject: [PATCH 03/11] Add axis argument to Cesium3DTilesContent test code --- Cesium3DTilesContent/test/ConvertTileToGltf.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Cesium3DTilesContent/test/ConvertTileToGltf.cpp b/Cesium3DTilesContent/test/ConvertTileToGltf.cpp index 792bbced8..c529ca3d0 100644 --- a/Cesium3DTilesContent/test/ConvertTileToGltf.cpp +++ b/Cesium3DTilesContent/test/ConvertTileToGltf.cpp @@ -20,7 +20,8 @@ AssetFetcher ConvertTileToGltf::makeAssetFetcher(const std::string& baseUrl) { fileAccessor, baseUrl, glm::dmat4(1.0), - requestHeaders); + requestHeaders, + CesiumGeometry::Axis::Y); } GltfConverterResult ConvertTileToGltf::fromB3dm( From 8fc74b6aac9d4c832ed7da4489c42111f71437f3 Mon Sep 17 00:00:00 2001 From: Tim Moore Date: Wed, 26 Jun 2024 12:09:50 +0200 Subject: [PATCH 04/11] Add all members for ErrorList initializer lists and others gcc / clang is less tolerant than Visual Studio, which lets one leave off the trailing members of a braced initializer list. --- Cesium3DTilesContent/src/I3dmToGltfConverter.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp b/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp index 11592d3e7..8744a14e2 100644 --- a/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp +++ b/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp @@ -495,7 +495,7 @@ CesiumAsync::Future convertI3dmContent( GltfConverterResult&& converterResult) { if (converterResult.model.has_value()) { CesiumGltfReader::GltfReaderResult readerResult{ - std::move(*converterResult.model)}; + std::move(*converterResult.model), {}, {}}; CesiumAsync::HttpHeaders externalRequestHeaders( assetFetcher.requestHeaders.begin(), assetFetcher.requestHeaders.end()); @@ -510,7 +510,8 @@ CesiumAsync::Future convertI3dmContent( return assetFetcher.asyncSystem.createResolvedFuture( CesiumGltfReader::GltfReaderResult{ std::nullopt, - std::move(converterResult.errors.errors)}); + std::move(converterResult.errors.errors), + {}}); }) .thenImmediately([convertedI3dm = std::move(convertedI3dm)]( CesiumGltfReader::GltfReaderResult&& @@ -519,7 +520,8 @@ CesiumAsync::Future convertI3dmContent( convertedI3dm.gltfResult.model = std::move(readerResult.model); CesiumUtility::ErrorList resolvedExternalErrors{ - std::move(readerResult.errors)}; + std::move(readerResult.errors), + {}}; convertedI3dm.gltfResult.errors.merge( resolvedExternalErrors); return convertedI3dm; From 9aaeb639e740333c5b77b043223ad30ebd9edd12 Mon Sep 17 00:00:00 2001 From: Tim Moore Date: Wed, 26 Jun 2024 10:35:58 +0200 Subject: [PATCH 05/11] AssetFetcher: keep a copy of request headers instead of a reference This was a bug waiting to happen and should always have been a copy. Fortunately, the new use of the AssetFetcher to resolve external data for i3dm files provoked a crash in the tests. --- .../include/Cesium3DTilesContent/GltfConverters.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cesium3DTilesContent/include/Cesium3DTilesContent/GltfConverters.h b/Cesium3DTilesContent/include/Cesium3DTilesContent/GltfConverters.h index c4837b17b..d830dfbc6 100644 --- a/Cesium3DTilesContent/include/Cesium3DTilesContent/GltfConverters.h +++ b/Cesium3DTilesContent/include/Cesium3DTilesContent/GltfConverters.h @@ -47,7 +47,7 @@ struct CESIUM3DTILESCONTENT_API AssetFetcher { const std::shared_ptr pAssetAccessor; const std::string baseUrl; glm::dmat4 tileTransform; // For ENU transforms in i3dm - const std::vector& requestHeaders; + const std::vector requestHeaders; const CesiumGeometry::Axis upAxis; }; From e6ec85d078918c8077e02cbf1017b7103f9064a3 Mon Sep 17 00:00:00 2001 From: Tim Moore Date: Wed, 26 Jun 2024 10:47:10 +0200 Subject: [PATCH 06/11] run format --- Cesium3DTilesContent/src/I3dmToGltfConverter.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp b/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp index 8744a14e2..4346656fc 100644 --- a/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp +++ b/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp @@ -495,7 +495,9 @@ CesiumAsync::Future convertI3dmContent( GltfConverterResult&& converterResult) { if (converterResult.model.has_value()) { CesiumGltfReader::GltfReaderResult readerResult{ - std::move(*converterResult.model), {}, {}}; + std::move(*converterResult.model), + {}, + {}}; CesiumAsync::HttpHeaders externalRequestHeaders( assetFetcher.requestHeaders.begin(), assetFetcher.requestHeaders.end()); From ef1438fa25088edcfee8267d41fe5184146e22f7 Mon Sep 17 00:00:00 2001 From: Tim Moore Date: Thu, 27 Jun 2024 19:14:59 +0200 Subject: [PATCH 07/11] Modify comment --- Cesium3DTilesContent/src/I3dmToGltfConverter.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp b/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp index 4346656fc..4ac0a40c4 100644 --- a/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp +++ b/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp @@ -464,8 +464,11 @@ CesiumAsync::Future convertI3dmContent( } else { baseUri = assetFetcher.baseUrl; } - // This use of a lambda is the only way timoore knows to conditionally pass - // different futures to a then...() member function. + // The future object argument to the .then...() member functions must be an + // rvalue. This use of a lambda is the only way timoore could think of to + // conditionally pass different futures to thenImmediately(). It can also be + // done by using a ternary operator assignment to a future variable and then + // std::move(). return [&]() -> CesiumAsync::Future { if (header.gltfFormat == 0) { From c6de588e1453e92ef32235c0d3921d475269e981 Mon Sep 17 00:00:00 2001 From: Tim Moore Date: Thu, 27 Jun 2024 19:16:49 +0200 Subject: [PATCH 08/11] Strip trailing whitespace from i3dm external content URL This whitespace may be added to keep the size of an i3dm file 8 byte aligned. --- Cesium3DTilesContent/src/I3dmToGltfConverter.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp b/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp index 4ac0a40c4..60cebfdfe 100644 --- a/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp +++ b/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp @@ -457,9 +457,16 @@ CesiumAsync::Future convertI3dmContent( repositionInstances(decodedInstances); std::string baseUri; if (header.gltfFormat == 0) { + // The spec says that the URL can be padded with ' ' (0x20) characters in + // order to make the size of the whole i3dm file 8-byte aligned. + auto rLastNotSpace = + std::find_if_not(gltfData.rbegin(), gltfData.rend(), [](auto&& b) { + return static_cast(b) == ' '; + }); + auto spaceDistance = gltfData.rend() - rLastNotSpace; std::string gltfUri( reinterpret_cast(gltfData.data()), - gltfData.size()); + spaceDistance); baseUri = CesiumUtility::Uri::resolve(assetFetcher.baseUrl, gltfUri); } else { baseUri = assetFetcher.baseUrl; From 38b727a674c471c1e8b108d4c0b9918dfc91f669 Mon Sep 17 00:00:00 2001 From: Tim Moore Date: Fri, 28 Jun 2024 11:10:09 +0200 Subject: [PATCH 09/11] Cast iterator distance to size_t --- Cesium3DTilesContent/src/I3dmToGltfConverter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp b/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp index 60cebfdfe..132cca50c 100644 --- a/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp +++ b/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp @@ -463,7 +463,7 @@ CesiumAsync::Future convertI3dmContent( std::find_if_not(gltfData.rbegin(), gltfData.rend(), [](auto&& b) { return static_cast(b) == ' '; }); - auto spaceDistance = gltfData.rend() - rLastNotSpace; + auto spaceDistance = static_cast(gltfData.rend() - rLastNotSpace); std::string gltfUri( reinterpret_cast(gltfData.data()), spaceDistance); From 0d68e3a672ba0e04b0b8610ce63cdacaf33b5817 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Sat, 29 Jun 2024 10:54:47 +1000 Subject: [PATCH 10/11] Small tweaks from review. --- .../Cesium3DTilesContent/GltfConverters.h | 10 +- .../src/I3dmToGltfConverter.cpp | 124 ++++++++---------- 2 files changed, 63 insertions(+), 71 deletions(-) diff --git a/Cesium3DTilesContent/include/Cesium3DTilesContent/GltfConverters.h b/Cesium3DTilesContent/include/Cesium3DTilesContent/GltfConverters.h index d830dfbc6..bbfaef488 100644 --- a/Cesium3DTilesContent/include/Cesium3DTilesContent/GltfConverters.h +++ b/Cesium3DTilesContent/include/Cesium3DTilesContent/GltfConverters.h @@ -43,12 +43,12 @@ struct CESIUM3DTILESCONTENT_API AssetFetcher { CesiumAsync::Future get(const std::string& relativeUrl) const; - const CesiumAsync::AsyncSystem& asyncSystem; - const std::shared_ptr pAssetAccessor; - const std::string baseUrl; + CesiumAsync::AsyncSystem asyncSystem; + std::shared_ptr pAssetAccessor; + std::string baseUrl; glm::dmat4 tileTransform; // For ENU transforms in i3dm - const std::vector requestHeaders; - const CesiumGeometry::Axis upAxis; + std::vector requestHeaders; + CesiumGeometry::Axis upAxis; }; /** diff --git a/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp b/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp index 132cca50c..a01a32a9a 100644 --- a/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp +++ b/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp @@ -461,7 +461,7 @@ CesiumAsync::Future convertI3dmContent( // order to make the size of the whole i3dm file 8-byte aligned. auto rLastNotSpace = std::find_if_not(gltfData.rbegin(), gltfData.rend(), [](auto&& b) { - return static_cast(b) == ' '; + return b == std::byte(' '); }); auto spaceDistance = static_cast(gltfData.rend() - rLastNotSpace); std::string gltfUri( @@ -471,73 +471,65 @@ CesiumAsync::Future convertI3dmContent( } else { baseUri = assetFetcher.baseUrl; } - // The future object argument to the .then...() member functions must be an - // rvalue. This use of a lambda is the only way timoore could think of to - // conditionally pass different futures to thenImmediately(). It can also be - // done by using a ternary operator assignment to a future variable and then - // std::move(). - return - [&]() -> CesiumAsync::Future { - if (header.gltfFormat == 0) { - // Recursively fetch and read the glTF content. - return assetFetcher.get(baseUri).thenImmediately( - [options, assetFetcher](AssetFetcherResult&& assetFetcherResult) - -> CesiumAsync::Future { - if (assetFetcherResult.errorList.hasErrors()) { - GltfConverterResult errorResult; - errorResult.errors.merge(assetFetcherResult.errorList); - return assetFetcher.asyncSystem.createResolvedFuture( - std::move(errorResult)); - } - return BinaryToGltfConverter::convert( - assetFetcherResult.bytes, - options, - assetFetcher); - }); - } else { - return BinaryToGltfConverter::convert( - gltfData, + + auto getGltf = [&]() -> CesiumAsync::Future { + if (header.gltfFormat == 0) { + // Recursively fetch and read the glTF content. + return assetFetcher.get(baseUri).thenImmediately( + [options, assetFetcher](AssetFetcherResult&& assetFetcherResult) + -> CesiumAsync::Future { + if (assetFetcherResult.errorList.hasErrors()) { + GltfConverterResult errorResult; + errorResult.errors.merge(assetFetcherResult.errorList); + return assetFetcher.asyncSystem.createResolvedFuture( + std::move(errorResult)); + } + return BinaryToGltfConverter::convert( + assetFetcherResult.bytes, + options, + assetFetcher); + }); + } else { + return BinaryToGltfConverter::convert(gltfData, options, assetFetcher); + } + }; + + return getGltf() + .thenImmediately([options, assetFetcher, baseUri]( + GltfConverterResult&& converterResult) { + if (converterResult.model.has_value()) { + CesiumGltfReader::GltfReaderResult readerResult{ + std::move(*converterResult.model), + {}, + {}}; + CesiumAsync::HttpHeaders externalRequestHeaders( + assetFetcher.requestHeaders.begin(), + assetFetcher.requestHeaders.end()); + return CesiumGltfReader::GltfReader::resolveExternalData( + assetFetcher.asyncSystem, + baseUri, + externalRequestHeaders, + assetFetcher.pAssetAccessor, options, - assetFetcher); + std::move(readerResult)); } - }() - .thenImmediately([options, assetFetcher, baseUri]( - GltfConverterResult&& converterResult) { - if (converterResult.model.has_value()) { - CesiumGltfReader::GltfReaderResult readerResult{ - std::move(*converterResult.model), - {}, - {}}; - CesiumAsync::HttpHeaders externalRequestHeaders( - assetFetcher.requestHeaders.begin(), - assetFetcher.requestHeaders.end()); - return CesiumGltfReader::GltfReader::resolveExternalData( - assetFetcher.asyncSystem, - baseUri, - externalRequestHeaders, - assetFetcher.pAssetAccessor, - options, - std::move(readerResult)); - } - return assetFetcher.asyncSystem.createResolvedFuture( - CesiumGltfReader::GltfReaderResult{ - std::nullopt, - std::move(converterResult.errors.errors), - {}}); - }) - .thenImmediately([convertedI3dm = std::move(convertedI3dm)]( - CesiumGltfReader::GltfReaderResult&& - readerResult) mutable { - if (readerResult.model) - convertedI3dm.gltfResult.model = - std::move(readerResult.model); - CesiumUtility::ErrorList resolvedExternalErrors{ - std::move(readerResult.errors), - {}}; - convertedI3dm.gltfResult.errors.merge( - resolvedExternalErrors); - return convertedI3dm; - }); + return assetFetcher.asyncSystem.createResolvedFuture( + CesiumGltfReader::GltfReaderResult{ + std::nullopt, + std::move(converterResult.errors.errors), + {}}); + }) + .thenImmediately( + [convertedI3dm = std::move(convertedI3dm)]( + CesiumGltfReader::GltfReaderResult&& readerResult) mutable { + if (readerResult.model) + convertedI3dm.gltfResult.model = std::move(readerResult.model); + CesiumUtility::ErrorList resolvedExternalErrors{ + std::move(readerResult.errors), + {}}; + convertedI3dm.gltfResult.errors.merge(resolvedExternalErrors); + return convertedI3dm; + }); } glm::dmat4 From 83eccb0dc7ae9737cc9c278e60149973a4a036d0 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Sat, 29 Jun 2024 11:07:25 +1000 Subject: [PATCH 11/11] Update CHANGES.md. --- CHANGES.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index e81e50f13..1834baa92 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -14,7 +14,11 @@ ##### Fixes :wrench: -- When a 3D Tiles Instanced 3D Mesh (i3dm) file contains an instance transform that cannot be decomposed into position, rotation, and scale, a warning will now be logged and an identity transformation will be used. Previously, an undefined transformation would be used. +- Fixed several problems with the loader for the 3D Tiles Instanced 3D Mesh (i3dm) format: + - When an instance transform cannot be decomposed into position, rotation, and scale, a warning will now be logged and an identity transformation will be used. Previously, an undefined transformation would be used. + - The `gltfUpAxis` property is now accounted for, if present. + - Paths to images in i3dm content are now resolved correctly. + - Extraneous spaces at the end of an external glTF URI are now ignored. These are sometimes added as padding in order to meet alignment requirements. - Removed an overly-eager degenerate triangle test in the 2D version of `IntersectionTests::pointInTriangle` that could discard intersections in small - but valid - triangles. - Fixed a bug while upsampling tiles for raster overlays that could cause them to have an incorrect bounding box, which in some cases would lead to the raster overlay being missing entirely from the upsampled tile.