From 21ecab90b80d13d1c221cdcd5fb1a3fe3264655f Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Wed, 9 Oct 2024 22:18:11 +1100 Subject: [PATCH 01/13] Fix asset lifetime, avoid circular reference counting. --- CesiumGltf/include/CesiumGltf/ImageCesium.h | 2 +- CesiumGltf/include/CesiumGltf/SharedAsset.h | 17 +--- .../include/CesiumGltf/SharedAssetDepot.h | 86 +++++++++++-------- 3 files changed, 55 insertions(+), 50 deletions(-) diff --git a/CesiumGltf/include/CesiumGltf/ImageCesium.h b/CesiumGltf/include/CesiumGltf/ImageCesium.h index 305169110..fbc31d23f 100644 --- a/CesiumGltf/include/CesiumGltf/ImageCesium.h +++ b/CesiumGltf/include/CesiumGltf/ImageCesium.h @@ -112,6 +112,6 @@ struct CESIUMGLTF_API ImageCesium final : public SharedAsset { */ int64_t sizeBytes = -1; - int64_t getSizeBytes() const override { return this->sizeBytes; } + int64_t getSizeBytes() const { return this->sizeBytes; } }; } // namespace CesiumGltf diff --git a/CesiumGltf/include/CesiumGltf/SharedAsset.h b/CesiumGltf/include/CesiumGltf/SharedAsset.h index d80fb2904..095bded53 100644 --- a/CesiumGltf/include/CesiumGltf/SharedAsset.h +++ b/CesiumGltf/include/CesiumGltf/SharedAsset.h @@ -62,8 +62,7 @@ class CESIUMGLTF_API SharedAsset : public CesiumUtility::ExtensibleObject { void addReference() const /*noexcept*/ { const int32_t prevReferences = this->_referenceCount++; if (this->_pDepot && prevReferences <= 0) { - this->_pDepot->unmarkDeletionCandidate( - const_cast(static_cast(this))); + this->_pDepot->unmarkDeletionCandidate(static_cast(this)); } } @@ -77,12 +76,10 @@ class CESIUMGLTF_API SharedAsset : public CesiumUtility::ExtensibleObject { CESIUM_ASSERT(this->_referenceCount > 0); const int32_t references = --this->_referenceCount; if (references == 0) { - CesiumUtility::IntrusivePointer> pDepot = - this->_pDepot; + SharedAssetDepot* pDepot = this->_pDepot; if (pDepot) { // Let the depot manage this object's lifetime. - pDepot->markDeletionCandidate( - const_cast(static_cast(this))); + pDepot->markDeletionCandidate(static_cast(this)); } else { // No depot, so destroy this object directly. delete static_cast(this); @@ -102,17 +99,11 @@ class CESIUMGLTF_API SharedAsset : public CesiumUtility::ExtensibleObject { */ bool isShareable() const { return this->_pDepot != nullptr; } - /** - * The number of bytes of memory usage that this asset takes up. - * This is used for deletion logic by the {@link SharedAssetDepot}. - */ - virtual int64_t getSizeBytes() const = 0; - const std::string& getUniqueAssetId() const { return this->_uniqueAssetId; } private: mutable std::atomic _referenceCount{0}; - CesiumUtility::IntrusivePointer> _pDepot; + SharedAssetDepot* _pDepot; std::string _uniqueAssetId; // To allow the depot to modify _pDepot and _uniqueAssetId. diff --git a/CesiumGltf/include/CesiumGltf/SharedAssetDepot.h b/CesiumGltf/include/CesiumGltf/SharedAssetDepot.h index c12dce504..16259b26d 100644 --- a/CesiumGltf/include/CesiumGltf/SharedAssetDepot.h +++ b/CesiumGltf/include/CesiumGltf/SharedAssetDepot.h @@ -45,15 +45,49 @@ class SharedAssetDepot : public CesiumUtility::ReferenceCountedThreadSafe< SharedAssetDepot() = default; + ~SharedAssetDepot() { + // It's possible the assets will outlive the depot, if they're still in use. + std::lock_guard lock(this->assetsMutex); + + for (auto& pair : this->assets) { + // Transfer ownership to the reference counting system. + CesiumUtility::IntrusivePointer pCounted = + pair.second.release(); + pCounted->_pDepot = nullptr; + pCounted->_uniqueAssetId = "TODO: released"; + } + } + /** * Stores the AssetType in this depot and returns a reference to it, * or returns the existing asset if present. */ CesiumUtility::IntrusivePointer store( - std::string& assetId, + const std::string& assetId, const CesiumUtility::IntrusivePointer& pAsset) { - auto [newIt, added] = this->assets.try_emplace(assetId, pAsset); - return newIt->second; + std::lock_guard lock(this->assetsMutex); + + auto findIt = this->assets.find(assetId); + if (findIt != this->assets.end()) { + // This asset ID already exists in the depot, so we can't add this asset. + return findIt->second.get(); + } + + pAsset->_pDepot = this; + pAsset->_uniqueAssetId = assetId; + + // Now that this asset is owned by the depot, we exclusively + // control its lifetime with a std::unique_ptr. + std::unique_ptr pOwnedAsset(pAsset.get()); + + auto [addIt, added] = this->assets.emplace(assetId, std::move(pOwnedAsset)); + + // We should always add successfully, because we verified it didn't already + // exist. A failed emplace is disastrous because our unique_ptr will end up + // destroying the user's object, which may still be in use. + CESIUM_ASSERT(added); + + return addIt->second.get(); } /** @@ -82,8 +116,8 @@ class SharedAssetDepot : public CesiumUtility::ReferenceCountedThreadSafe< if (existingIt != this->assets.end()) { // We've already loaded an asset with this ID - we can just use that. return asyncSystem - .createResolvedFuture( - CesiumUtility::IntrusivePointer(existingIt->second)) + .createResolvedFuture(CesiumUtility::IntrusivePointer( + existingIt->second.get())) .share(); } @@ -112,30 +146,13 @@ class SharedAssetDepot : public CesiumUtility::ReferenceCountedThreadSafe< // Do this in main thread since we're messing with the collections. .thenInMainThread( [uri, - this, - pAssets = &this->assets, - pPendingAssetsMutex = &this->assetsMutex, - pPendingAssets = &this->pendingAssets]( - CesiumUtility::IntrusivePointer&& pResult) + this](CesiumUtility::IntrusivePointer&& pResult) -> CesiumUtility::IntrusivePointer { - std::lock_guard lock(*pPendingAssetsMutex); - // Get rid of our future. - pPendingAssets->erase(uri); - - if (pResult) { - pResult->_pDepot = this; - pResult->_uniqueAssetId = uri; + this->pendingAssets.erase(uri); - auto [it, ok] = pAssets->emplace(uri, pResult); - if (!ok) { - return nullptr; - } - - return it->second; - } - - return nullptr; + // Store the new asset in the depot. + return this->store(uri, pResult); }); auto [it, ok] = this->pendingAssets.emplace(uri, std::move(future).share()); @@ -182,10 +199,9 @@ class SharedAssetDepot : public CesiumUtility::ReferenceCountedThreadSafe< * Marks the given asset as a candidate for deletion. * Should only be called by {@link SharedAsset}. */ - void markDeletionCandidate( - const CesiumUtility::IntrusivePointer& pAsset) { + void markDeletionCandidate(const AssetType* pAsset) { std::lock_guard lock(this->deletionCandidatesMutex); - this->deletionCandidates.push_back(pAsset); + this->deletionCandidates.push_back(const_cast(pAsset)); this->totalDeletionCandidateMemoryUsage += pAsset->getSizeBytes(); if (this->totalDeletionCandidateMemoryUsage > this->staleAssetSizeLimit) { @@ -193,9 +209,9 @@ class SharedAssetDepot : public CesiumUtility::ReferenceCountedThreadSafe< while (!this->deletionCandidates.empty() && this->totalDeletionCandidateMemoryUsage > this->staleAssetSizeLimit) { - const CesiumUtility::IntrusivePointer& pOldAsset = - this->deletionCandidates.front(); + const AssetType* pOldAsset = this->deletionCandidates.front(); this->deletionCandidates.pop_front(); + CESIUM_ASSERT(pOldAsset->_referenceCount == 0); this->assets.erase(pOldAsset->getUniqueAssetId()); this->totalDeletionCandidateMemoryUsage -= pOldAsset->getSizeBytes(); } @@ -206,8 +222,7 @@ class SharedAssetDepot : public CesiumUtility::ReferenceCountedThreadSafe< * Unmarks the given asset as a candidate for deletion. * Should only be called by {@link SharedAsset}. */ - void unmarkDeletionCandidate( - const CesiumUtility::IntrusivePointer& pAsset) { + void unmarkDeletionCandidate(const AssetType* pAsset) { std::lock_guard lock(this->deletionCandidatesMutex); const std::string& assetId = pAsset->getUniqueAssetId(); for (auto it = this->deletionCandidates.begin(); @@ -222,8 +237,7 @@ class SharedAssetDepot : public CesiumUtility::ReferenceCountedThreadSafe< } // Assets that have a unique ID that can be used to de-duplicate them. - std::unordered_map> - assets; + std::unordered_map> assets; // Futures for assets that still aren't loaded yet. std::unordered_map< std::string, @@ -234,7 +248,7 @@ class SharedAssetDepot : public CesiumUtility::ReferenceCountedThreadSafe< // List of assets that are being considered for deletion, in the order that // they were added. - std::list> deletionCandidates; + std::list deletionCandidates; // The total amount of memory used by all assets in the deletionCandidates // list. std::atomic totalDeletionCandidateMemoryUsage; From ee4b391d1451bd49711128de37b84a0668fdd1f9 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Wed, 9 Oct 2024 22:39:58 +1100 Subject: [PATCH 02/13] Remove TODO. --- CesiumGltf/include/CesiumGltf/SharedAssetDepot.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CesiumGltf/include/CesiumGltf/SharedAssetDepot.h b/CesiumGltf/include/CesiumGltf/SharedAssetDepot.h index 16259b26d..a0c452aaf 100644 --- a/CesiumGltf/include/CesiumGltf/SharedAssetDepot.h +++ b/CesiumGltf/include/CesiumGltf/SharedAssetDepot.h @@ -54,7 +54,7 @@ class SharedAssetDepot : public CesiumUtility::ReferenceCountedThreadSafe< CesiumUtility::IntrusivePointer pCounted = pair.second.release(); pCounted->_pDepot = nullptr; - pCounted->_uniqueAssetId = "TODO: released"; + pCounted->_uniqueAssetId.clear(); } } From 1b0934ec6c91e1c11fe418df1cf3e600138c4e5c Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Wed, 9 Oct 2024 22:48:16 +1100 Subject: [PATCH 03/13] Don't use an iterator after it's erased. --- CesiumGltf/include/CesiumGltf/SharedAssetDepot.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CesiumGltf/include/CesiumGltf/SharedAssetDepot.h b/CesiumGltf/include/CesiumGltf/SharedAssetDepot.h index a0c452aaf..0c4c6dceb 100644 --- a/CesiumGltf/include/CesiumGltf/SharedAssetDepot.h +++ b/CesiumGltf/include/CesiumGltf/SharedAssetDepot.h @@ -229,8 +229,8 @@ class SharedAssetDepot : public CesiumUtility::ReferenceCountedThreadSafe< it != this->deletionCandidates.end(); ++it) { if ((*it)->getUniqueAssetId() == assetId) { - this->deletionCandidates.erase(it); this->totalDeletionCandidateMemoryUsage -= (*it)->getSizeBytes(); + this->deletionCandidates.erase(it); break; } } From 8595163c9201964655736ca6a001fa8a343b8485 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Thu, 10 Oct 2024 20:15:25 +1100 Subject: [PATCH 04/13] getSizeBytes can use sizeInBytes or pixelData.size(). --- CesiumGltf/include/CesiumGltf/ImageCesium.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/CesiumGltf/include/CesiumGltf/ImageCesium.h b/CesiumGltf/include/CesiumGltf/ImageCesium.h index fbc31d23f..55d146dea 100644 --- a/CesiumGltf/include/CesiumGltf/ImageCesium.h +++ b/CesiumGltf/include/CesiumGltf/ImageCesium.h @@ -112,6 +112,14 @@ struct CESIUMGLTF_API ImageCesium final : public SharedAsset { */ int64_t sizeBytes = -1; - int64_t getSizeBytes() const { return this->sizeBytes; } + /** + * @brief Gets the size of this asset, in bytes. + * + * If {@link sizeBytes} is greater than or equal to zero, it is returned. + * Otherwise, the size of the {@link pixelData} array is returned. + */ + int64_t getSizeBytes() const { + return this->sizeBytes >= 0 ? this->sizeBytes : this->pixelData.size(); + } }; } // namespace CesiumGltf From c8d961214c2f0b2d9c2781dcaf8aeb8bbdae24e5 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Thu, 10 Oct 2024 20:16:31 +1100 Subject: [PATCH 05/13] Remove unnecessary SharedAssetSystem::deletionTick. --- CesiumGltf/include/CesiumGltf/SharedAssetSystem.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/CesiumGltf/include/CesiumGltf/SharedAssetSystem.h b/CesiumGltf/include/CesiumGltf/SharedAssetSystem.h index a116b9136..5e38dde0f 100644 --- a/CesiumGltf/include/CesiumGltf/SharedAssetSystem.h +++ b/CesiumGltf/include/CesiumGltf/SharedAssetSystem.h @@ -38,8 +38,6 @@ class SharedAssetSystem const SharedAssetDepot& image(); - void deletionTick(); - private: CesiumUtility::IntrusivePointer> _pImages; From 64597295599032cb808036d7cf1ce0fc0f629a71 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Thu, 10 Oct 2024 20:18:27 +1100 Subject: [PATCH 06/13] Raster overlays store heap-allocated ImageCesiums. --- .../CesiumRasterOverlays/RasterOverlayTile.h | 11 ++++--- .../src/RasterOverlayTile.cpp | 4 +-- .../src/RasterOverlayTileProvider.cpp | 33 ++++++++++--------- .../test/TestAddRasterOverlayToGltf.cpp | 2 +- .../TestQuadtreeRasterOverlayTileProvider.cpp | 8 +++-- .../test/TestTileMapServiceRasterOverlay.cpp | 4 ++- 6 files changed, 37 insertions(+), 25 deletions(-) diff --git a/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTile.h b/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTile.h index f74023e16..f27c3adcc 100644 --- a/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTile.h +++ b/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTile.h @@ -192,8 +192,9 @@ class RasterOverlayTile final * * @return The image data. */ - const CesiumGltf::ImageCesium& getImage() const noexcept { - return this->_image; + CesiumUtility::IntrusivePointer + getImage() const noexcept { + return this->_pImage; } /** @@ -204,7 +205,9 @@ class RasterOverlayTile final * * @return The image data. */ - CesiumGltf::ImageCesium& getImage() noexcept { return this->_image; } + CesiumUtility::IntrusivePointer getImage() noexcept { + return this->_pImage; + } /** * @brief Create the renderer resources for the loaded image. @@ -256,7 +259,7 @@ class RasterOverlayTile final CesiumGeometry::Rectangle _rectangle; std::vector _tileCredits; LoadState _state; - CesiumGltf::ImageCesium _image; + CesiumUtility::IntrusivePointer _pImage; void* _pRendererResources; MoreDetailAvailable _moreDetailAvailable; }; diff --git a/CesiumRasterOverlays/src/RasterOverlayTile.cpp b/CesiumRasterOverlays/src/RasterOverlayTile.cpp index 722ef553b..2b9c19207 100644 --- a/CesiumRasterOverlays/src/RasterOverlayTile.cpp +++ b/CesiumRasterOverlays/src/RasterOverlayTile.cpp @@ -17,7 +17,7 @@ RasterOverlayTile::RasterOverlayTile( _rectangle(CesiumGeometry::Rectangle(0.0, 0.0, 0.0, 0.0)), _tileCredits(), _state(LoadState::Placeholder), - _image(), + _pImage(nullptr), _pRendererResources(nullptr), _moreDetailAvailable(MoreDetailAvailable::Unknown) {} @@ -30,7 +30,7 @@ RasterOverlayTile::RasterOverlayTile( _rectangle(rectangle), _tileCredits(), _state(LoadState::Unloaded), - _image(), + _pImage(nullptr), _pRendererResources(nullptr), _moreDetailAvailable(MoreDetailAvailable::Unknown) {} diff --git a/CesiumRasterOverlays/src/RasterOverlayTileProvider.cpp b/CesiumRasterOverlays/src/RasterOverlayTileProvider.cpp index 3ba53e689..591c2c0c5 100644 --- a/CesiumRasterOverlays/src/RasterOverlayTileProvider.cpp +++ b/CesiumRasterOverlays/src/RasterOverlayTileProvider.cpp @@ -91,8 +91,9 @@ RasterOverlayTileProvider::getTile( void RasterOverlayTileProvider::removeTile(RasterOverlayTile* pTile) noexcept { CESIUM_ASSERT(pTile->getReferenceCount() == 0); - - this->_tileDataBytes -= pTile->getImage().sizeBytes; + if (pTile->getImage()) { + this->_tileDataBytes -= pTile->getImage()->sizeBytes; + } } CesiumAsync::Future @@ -213,7 +214,7 @@ RasterOverlayTileProvider::loadTileImageFromUrl( namespace { struct LoadResult { RasterOverlayTile::LoadState state = RasterOverlayTile::LoadState::Unloaded; - CesiumGltf::ImageCesium image = {}; + CesiumUtility::IntrusivePointer pImage = nullptr; CesiumGeometry::Rectangle rectangle = {}; std::vector credits = {}; void* pRendererResources = nullptr; @@ -288,7 +289,7 @@ static LoadResult createLoadResultFromLoadedImage( LoadResult result; result.state = RasterOverlayTile::LoadState::Loaded; - result.image = std::move(image); + result.pImage = loadedImage.pImage; result.rectangle = loadedImage.rectangle; result.credits = std::move(loadedImage.credits); result.pRendererResources = pRendererResources; @@ -341,7 +342,7 @@ CesiumAsync::Future RasterOverlayTileProvider::doLoad( [thiz, pTile, isThrottledLoad](LoadResult&& result) noexcept { pTile->_rectangle = result.rectangle; pTile->_pRendererResources = result.pRendererResources; - pTile->_image = std::move(result.image); + pTile->_pImage = std::move(result.pImage); pTile->_tileCredits = std::move(result.credits); pTile->_moreDetailAvailable = result.moreDetailAvailable @@ -349,17 +350,19 @@ CesiumAsync::Future RasterOverlayTileProvider::doLoad( : RasterOverlayTile::MoreDetailAvailable::No; pTile->setState(result.state); - ImageCesium& imageCesium = pTile->getImage(); + if (pTile->getImage() != nullptr) { + ImageCesium& imageCesium = *pTile->getImage(); - // If the image size hasn't been overridden, store the pixelData - // size now. We'll add this number to our total memory usage now, - // and remove it when the tile is later unloaded, and we must use - // the same size in each case. - if (imageCesium.sizeBytes < 0) { - imageCesium.sizeBytes = int64_t(imageCesium.pixelData.size()); - } + // If the image size hasn't been overridden, store the pixelData + // size now. We'll add this number to our total memory usage now, + // and remove it when the tile is later unloaded, and we must use + // the same size in each case. + if (imageCesium.sizeBytes < 0) { + imageCesium.sizeBytes = int64_t(imageCesium.pixelData.size()); + } - thiz->_tileDataBytes += imageCesium.sizeBytes; + thiz->_tileDataBytes += imageCesium.sizeBytes; + } thiz->finalizeTileLoad(isThrottledLoad); @@ -368,7 +371,7 @@ CesiumAsync::Future RasterOverlayTileProvider::doLoad( .catchInMainThread( [thiz, pTile, isThrottledLoad](const std::exception& /*e*/) { pTile->_pRendererResources = nullptr; - pTile->_image = {}; + pTile->_pImage = nullptr; pTile->_tileCredits = {}; pTile->_moreDetailAvailable = RasterOverlayTile::MoreDetailAvailable::No; diff --git a/CesiumRasterOverlays/test/TestAddRasterOverlayToGltf.cpp b/CesiumRasterOverlays/test/TestAddRasterOverlayToGltf.cpp index a6427a1d9..21e591018 100644 --- a/CesiumRasterOverlays/test/TestAddRasterOverlayToGltf.cpp +++ b/CesiumRasterOverlays/test/TestAddRasterOverlayToGltf.cpp @@ -186,7 +186,7 @@ TEST_CASE("Add raster overlay to glTF") { // PNG-encode the raster overlay image and store it in the main // buffer. ImageManipulation::savePng( - loadResult.pTile->getImage(), + *loadResult.pTile->getImage(), buffer.cesium.data); BufferView& bufferView = gltf.bufferViews.emplace_back(); diff --git a/CesiumRasterOverlays/test/TestQuadtreeRasterOverlayTileProvider.cpp b/CesiumRasterOverlays/test/TestQuadtreeRasterOverlayTileProvider.cpp index ddfe60440..705f90fe8 100644 --- a/CesiumRasterOverlays/test/TestQuadtreeRasterOverlayTileProvider.cpp +++ b/CesiumRasterOverlays/test/TestQuadtreeRasterOverlayTileProvider.cpp @@ -170,7 +170,9 @@ TEST_CASE("QuadtreeRasterOverlayTileProvider getTile") { CHECK(pTile->getState() == RasterOverlayTile::LoadState::Loaded); - const ImageCesium& image = pTile->getImage(); + REQUIRE(pTile->getImage()); + + const ImageCesium& image = *pTile->getImage(); CHECK(image.width > 0); CHECK(image.height > 0); CHECK(image.pixelData.size() > 0); @@ -224,7 +226,9 @@ TEST_CASE("QuadtreeRasterOverlayTileProvider getTile") { CHECK(pTile->getState() == RasterOverlayTile::LoadState::Loaded); - const ImageCesium& image = pTile->getImage(); + REQUIRE(pTile->getImage()); + + const ImageCesium& image = *pTile->getImage(); CHECK(image.width > 0); CHECK(image.height > 0); CHECK(image.pixelData.size() > 0); diff --git a/CesiumRasterOverlays/test/TestTileMapServiceRasterOverlay.cpp b/CesiumRasterOverlays/test/TestTileMapServiceRasterOverlay.cpp index bf131b6b8..6f3228285 100644 --- a/CesiumRasterOverlays/test/TestTileMapServiceRasterOverlay.cpp +++ b/CesiumRasterOverlays/test/TestTileMapServiceRasterOverlay.cpp @@ -72,7 +72,9 @@ TEST_CASE("TileMapServiceRasterOverlay") { REQUIRE(pTile); waitForFuture(asyncSystem, pTileProvider->loadTile(*pTile)); - ImageCesium& image = pTile->getImage(); + REQUIRE(pTile->getImage()); + + const ImageCesium& image = *pTile->getImage(); CHECK(image.width > 0); CHECK(image.height > 0); } From 36783ad0303c9078077a6caf87abe0a6029c8026 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Thu, 10 Oct 2024 20:18:58 +1100 Subject: [PATCH 07/13] SharedAsset improvements... * Make SharedAsset constructor and destructor protected, so instances of this type can't be created directly. * Maintain `_sizeInDepot` field on `SharedAsset` to defend against the asset size changing between when we add its size and when we remove its size. * Add some doc comments. * Change the staleAssetSizeLimit to 16 MiB. * Add some defensive assertions. * Fix use-after-free when removing deletion candidates. --- CesiumGltf/include/CesiumGltf/SharedAsset.h | 20 +++++-- .../include/CesiumGltf/SharedAssetDepot.h | 59 ++++++++++++------- 2 files changed, 52 insertions(+), 27 deletions(-) diff --git a/CesiumGltf/include/CesiumGltf/SharedAsset.h b/CesiumGltf/include/CesiumGltf/SharedAsset.h index 095bded53..7e8de7284 100644 --- a/CesiumGltf/include/CesiumGltf/SharedAsset.h +++ b/CesiumGltf/include/CesiumGltf/SharedAsset.h @@ -23,9 +23,6 @@ namespace CesiumGltf { template class CESIUMGLTF_API SharedAsset : public CesiumUtility::ExtensibleObject { public: - SharedAsset() = default; - ~SharedAsset() { CESIUM_ASSERT(this->_referenceCount == 0); } - // Assets can be copied, but the fresh instance has no references and is not // in the asset depot. SharedAsset(const SharedAsset& rhs) @@ -99,12 +96,25 @@ class CESIUMGLTF_API SharedAsset : public CesiumUtility::ExtensibleObject { */ bool isShareable() const { return this->_pDepot != nullptr; } + /** + * @brief Gets the unique ID of this asset, if it {@link isShareable}. + * + * If this asset is not shareable, this method will return an empty string. + */ const std::string& getUniqueAssetId() const { return this->_uniqueAssetId; } +protected: + SharedAsset() = default; + ~SharedAsset() { CESIUM_ASSERT(this->_referenceCount == 0); } + private: mutable std::atomic _referenceCount{0}; - SharedAssetDepot* _pDepot; - std::string _uniqueAssetId; + SharedAssetDepot* _pDepot{nullptr}; + std::string _uniqueAssetId{}; + + // The size of this asset when it was counted by the depot. This is stored so + // that the exact same size can be subtracted later. + int64_t _sizeInDepot{0}; // To allow the depot to modify _pDepot and _uniqueAssetId. friend class SharedAssetDepot; diff --git a/CesiumGltf/include/CesiumGltf/SharedAssetDepot.h b/CesiumGltf/include/CesiumGltf/SharedAssetDepot.h index 0c4c6dceb..63d1c4956 100644 --- a/CesiumGltf/include/CesiumGltf/SharedAssetDepot.h +++ b/CesiumGltf/include/CesiumGltf/SharedAssetDepot.h @@ -39,9 +39,9 @@ class SharedAssetDepot : public CesiumUtility::ReferenceCountedThreadSafe< * At that point, assets are cleaned up in the order that they were marked for * deletion until the total dips below this threshold again. * - * Default is 100MB. + * Default is 16MiB. */ - int64_t staleAssetSizeLimit = 1000 * 1000 * 100; + int64_t staleAssetSizeLimit = 16 * 1024 * 1024; SharedAssetDepot() = default; @@ -73,6 +73,11 @@ class SharedAssetDepot : public CesiumUtility::ReferenceCountedThreadSafe< return findIt->second.get(); } + // If this asset ID is pending, but hasn't completed yet, where did this + // asset come from? It shouldn't happen. + CESIUM_ASSERT( + this->pendingAssets.find(assetId) == this->pendingAssets.end()); + pAsset->_pDepot = this; pAsset->_uniqueAssetId = assetId; @@ -148,20 +153,19 @@ class SharedAssetDepot : public CesiumUtility::ReferenceCountedThreadSafe< [uri, this](CesiumUtility::IntrusivePointer&& pResult) -> CesiumUtility::IntrusivePointer { - // Get rid of our future. + // Remove pending asset. this->pendingAssets.erase(uri); // Store the new asset in the depot. return this->store(uri, pResult); }); - auto [it, ok] = this->pendingAssets.emplace(uri, std::move(future).share()); - if (!ok) { - return asyncSystem - .createResolvedFuture>( - nullptr) - .share(); - } + auto [it, added] = + this->pendingAssets.emplace(uri, std::move(future).share()); + + // Should always be added successfully, because we checked above that the + // URI doesn't exist in the map yet. + CESIUM_ASSERT(added); return it->second; } @@ -201,19 +205,28 @@ class SharedAssetDepot : public CesiumUtility::ReferenceCountedThreadSafe< */ void markDeletionCandidate(const AssetType* pAsset) { std::lock_guard lock(this->deletionCandidatesMutex); - this->deletionCandidates.push_back(const_cast(pAsset)); - this->totalDeletionCandidateMemoryUsage += pAsset->getSizeBytes(); + + AssetType* pMutableAsset = const_cast(pAsset); + pMutableAsset->_sizeInDepot = pMutableAsset->getSizeBytes(); + this->totalDeletionCandidateMemoryUsage += pMutableAsset->_sizeInDepot; + + this->deletionCandidates.push_back(pMutableAsset); if (this->totalDeletionCandidateMemoryUsage > this->staleAssetSizeLimit) { std::lock_guard assetsLock(this->assetsMutex); + + // Delete the deletion candidates until we're below the limit. while (!this->deletionCandidates.empty() && this->totalDeletionCandidateMemoryUsage > this->staleAssetSizeLimit) { const AssetType* pOldAsset = this->deletionCandidates.front(); this->deletionCandidates.pop_front(); + + this->totalDeletionCandidateMemoryUsage -= pOldAsset->_sizeInDepot; + + // This will actually delete the asset. CESIUM_ASSERT(pOldAsset->_referenceCount == 0); this->assets.erase(pOldAsset->getUniqueAssetId()); - this->totalDeletionCandidateMemoryUsage -= pOldAsset->getSizeBytes(); } } } @@ -224,15 +237,17 @@ class SharedAssetDepot : public CesiumUtility::ReferenceCountedThreadSafe< */ void unmarkDeletionCandidate(const AssetType* pAsset) { std::lock_guard lock(this->deletionCandidatesMutex); - const std::string& assetId = pAsset->getUniqueAssetId(); - for (auto it = this->deletionCandidates.begin(); - it != this->deletionCandidates.end(); - ++it) { - if ((*it)->getUniqueAssetId() == assetId) { - this->totalDeletionCandidateMemoryUsage -= (*it)->getSizeBytes(); - this->deletionCandidates.erase(it); - break; - } + + auto it = std::find( + this->deletionCandidates.begin(), + this->deletionCandidates.end(), + pAsset); + + CESIUM_ASSERT(it != this->deletionCandidates.end()); + + if (it != this->deletionCandidates.end()) { + this->totalDeletionCandidateMemoryUsage -= (*it)->_sizeInDepot; + this->deletionCandidates.erase(it); } } From e3975911cdea297b23ec0a7cdb5c5b769cb80ad9 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Thu, 10 Oct 2024 20:31:17 +1100 Subject: [PATCH 08/13] Even independent assets can be shared. --- CesiumGltf/include/CesiumGltf/SharedAsset.h | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/CesiumGltf/include/CesiumGltf/SharedAsset.h b/CesiumGltf/include/CesiumGltf/SharedAsset.h index 7e8de7284..b53bcb2b3 100644 --- a/CesiumGltf/include/CesiumGltf/SharedAsset.h +++ b/CesiumGltf/include/CesiumGltf/SharedAsset.h @@ -85,16 +85,16 @@ class CESIUMGLTF_API SharedAsset : public CesiumUtility::ExtensibleObject { } /** - * @brief Determines if this image is shareable because it is managed by an - * asset depot. An image that is not shareable can be understood to be - * exclusively owned by, for example, the glTF that references it. If it is - * shareable, then potentially multiple glTFs reference it. - * - * An example of a non-shareable asset is an image embedded in a Binary glTF - * (GLB) buffer. An example of a shareable asset is an image referenced in a - * glTF by URI. + * @brief Gets the shared asset depot that owns this asset, or nullptr if this + * asset is independent of an asset depot. + */ + const SharedAssetDepot* getDepot() const { return this->_pDepot; } + + /** + * @brief Gets the shared asset depot that owns this asset, or nullptr if this + * asset is independent of an asset depot. */ - bool isShareable() const { return this->_pDepot != nullptr; } + SharedAssetDepot* getDepot() { return this->_pDepot; } /** * @brief Gets the unique ID of this asset, if it {@link isShareable}. @@ -116,7 +116,7 @@ class CESIUMGLTF_API SharedAsset : public CesiumUtility::ExtensibleObject { // that the exact same size can be subtracted later. int64_t _sizeInDepot{0}; - // To allow the depot to modify _pDepot and _uniqueAssetId. + // To allow the depot to modify _pDepot, _uniqueAssetId, and _sizeInDepot. friend class SharedAssetDepot; }; From a9516a0c842c226cf4e7481cc87476f5fb52e575 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Thu, 10 Oct 2024 21:00:16 +1100 Subject: [PATCH 09/13] Move SharedAsset[Depot] to CesiumAsync. --- Cesium3DTilesSelection/src/TilesetContentManager.h | 1 - .../include/CesiumAsync}/SharedAsset.h | 8 ++++---- .../include/CesiumAsync}/SharedAssetDepot.h | 9 +++++---- CesiumGltf/include/CesiumGltf/Image.h | 2 +- CesiumGltf/include/CesiumGltf/ImageCesium.h | 5 +++-- CesiumGltf/include/CesiumGltf/SharedAssetSystem.h | 7 ++++--- CesiumGltf/include/CesiumGltf/TextureView.h | 1 - CesiumGltf/src/SharedAssetSystem.cpp | 5 +++-- CesiumGltfReader/src/GltfReader.cpp | 2 +- 9 files changed, 21 insertions(+), 19 deletions(-) rename {CesiumGltf/include/CesiumGltf => CesiumAsync/include/CesiumAsync}/SharedAsset.h (95%) rename {CesiumGltf/include/CesiumGltf => CesiumAsync/include/CesiumAsync}/SharedAssetDepot.h (98%) diff --git a/Cesium3DTilesSelection/src/TilesetContentManager.h b/Cesium3DTilesSelection/src/TilesetContentManager.h index 5a58e77d6..4a4ec286f 100644 --- a/Cesium3DTilesSelection/src/TilesetContentManager.h +++ b/Cesium3DTilesSelection/src/TilesetContentManager.h @@ -11,7 +11,6 @@ #include #include #include -#include #include #include diff --git a/CesiumGltf/include/CesiumGltf/SharedAsset.h b/CesiumAsync/include/CesiumAsync/SharedAsset.h similarity index 95% rename from CesiumGltf/include/CesiumGltf/SharedAsset.h rename to CesiumAsync/include/CesiumAsync/SharedAsset.h index b53bcb2b3..3a34cfda8 100644 --- a/CesiumGltf/include/CesiumGltf/SharedAsset.h +++ b/CesiumAsync/include/CesiumAsync/SharedAsset.h @@ -1,12 +1,12 @@ #pragma once -#include +#include #include #include #include -namespace CesiumGltf { +namespace CesiumAsync { /** * @brief An asset that is potentially shared between multiple objects, such as @@ -21,7 +21,7 @@ namespace CesiumGltf { * `class MyClass : public SharedAsset { ... };` */ template -class CESIUMGLTF_API SharedAsset : public CesiumUtility::ExtensibleObject { +class CESIUMASYNC_API SharedAsset : public CesiumUtility::ExtensibleObject { public: // Assets can be copied, but the fresh instance has no references and is not // in the asset depot. @@ -120,4 +120,4 @@ class CESIUMGLTF_API SharedAsset : public CesiumUtility::ExtensibleObject { friend class SharedAssetDepot; }; -} // namespace CesiumGltf +} // namespace CesiumAsync diff --git a/CesiumGltf/include/CesiumGltf/SharedAssetDepot.h b/CesiumAsync/include/CesiumAsync/SharedAssetDepot.h similarity index 98% rename from CesiumGltf/include/CesiumGltf/SharedAssetDepot.h rename to CesiumAsync/include/CesiumAsync/SharedAssetDepot.h index 63d1c4956..0c3932122 100644 --- a/CesiumGltf/include/CesiumGltf/SharedAssetDepot.h +++ b/CesiumAsync/include/CesiumAsync/SharedAssetDepot.h @@ -16,7 +16,7 @@ #include #include -namespace CesiumGltf { +namespace CesiumAsync { template class SharedAsset; @@ -26,8 +26,9 @@ template class SharedAsset; * be derived from {@link SharedAsset}. */ template -class SharedAssetDepot : public CesiumUtility::ReferenceCountedThreadSafe< - SharedAssetDepot> { +class CESIUMASYNC_API SharedAssetDepot + : public CesiumUtility::ReferenceCountedThreadSafe< + SharedAssetDepot> { public: /** * @brief The maximum total byte usage of assets that have been loaded but are @@ -273,4 +274,4 @@ class SharedAssetDepot : public CesiumUtility::ReferenceCountedThreadSafe< friend class SharedAsset; }; -} // namespace CesiumGltf +} // namespace CesiumAsync diff --git a/CesiumGltf/include/CesiumGltf/Image.h b/CesiumGltf/include/CesiumGltf/Image.h index 701836eff..b2a45c046 100644 --- a/CesiumGltf/include/CesiumGltf/Image.h +++ b/CesiumGltf/include/CesiumGltf/Image.h @@ -4,7 +4,7 @@ #include "CesiumGltf/ImageSpec.h" #include "CesiumGltf/Library.h" -#include +#include namespace CesiumGltf { /** @copydoc ImageSpec */ diff --git a/CesiumGltf/include/CesiumGltf/ImageCesium.h b/CesiumGltf/include/CesiumGltf/ImageCesium.h index 55d146dea..6feedf656 100644 --- a/CesiumGltf/include/CesiumGltf/ImageCesium.h +++ b/CesiumGltf/include/CesiumGltf/ImageCesium.h @@ -1,8 +1,8 @@ #pragma once +#include "CesiumAsync/SharedAsset.h" #include "CesiumGltf/Ktx2TranscodeTargets.h" #include "CesiumGltf/Library.h" -#include "CesiumGltf/SharedAsset.h" #include #include @@ -30,7 +30,8 @@ struct CESIUMGLTF_API ImageCesiumMipPosition { * @brief Holds {@link Image} properties that are specific to the glTF loader * rather than part of the glTF spec. */ -struct CESIUMGLTF_API ImageCesium final : public SharedAsset { +struct CESIUMGLTF_API ImageCesium final + : public CesiumAsync::SharedAsset { /** * @brief The width of the image in pixels. */ diff --git a/CesiumGltf/include/CesiumGltf/SharedAssetSystem.h b/CesiumGltf/include/CesiumGltf/SharedAssetSystem.h index 5e38dde0f..b8abb23ec 100644 --- a/CesiumGltf/include/CesiumGltf/SharedAssetSystem.h +++ b/CesiumGltf/include/CesiumGltf/SharedAssetSystem.h @@ -1,7 +1,7 @@ #pragma once +#include #include -#include namespace CesiumGltf { @@ -36,10 +36,11 @@ class SharedAssetSystem ->getOrFetch(asyncSystem, pAssetAccessor, factory, uri, headers); } - const SharedAssetDepot& image(); + const CesiumAsync::SharedAssetDepot& image(); private: - CesiumUtility::IntrusivePointer> + CesiumUtility::IntrusivePointer< + CesiumAsync::SharedAssetDepot> _pImages; }; diff --git a/CesiumGltf/include/CesiumGltf/TextureView.h b/CesiumGltf/include/CesiumGltf/TextureView.h index 35faa8938..22f677cef 100644 --- a/CesiumGltf/include/CesiumGltf/TextureView.h +++ b/CesiumGltf/include/CesiumGltf/TextureView.h @@ -3,7 +3,6 @@ #include #include #include -#include #include #include diff --git a/CesiumGltf/src/SharedAssetSystem.cpp b/CesiumGltf/src/SharedAssetSystem.cpp index a07c324b9..aac4114ac 100644 --- a/CesiumGltf/src/SharedAssetSystem.cpp +++ b/CesiumGltf/src/SharedAssetSystem.cpp @@ -4,11 +4,12 @@ namespace CesiumGltf { SharedAssetSystem::SharedAssetSystem() noexcept - : _pImages(new SharedAssetDepot()) {} + : _pImages(new CesiumAsync::SharedAssetDepot()) {} SharedAssetSystem::~SharedAssetSystem() noexcept = default; -const SharedAssetDepot& SharedAssetSystem::image() { +const CesiumAsync::SharedAssetDepot& +SharedAssetSystem::image() { return *this->_pImages; } diff --git a/CesiumGltfReader/src/GltfReader.cpp b/CesiumGltfReader/src/GltfReader.cpp index 0d52739b8..5914cc980 100644 --- a/CesiumGltfReader/src/GltfReader.cpp +++ b/CesiumGltfReader/src/GltfReader.cpp @@ -11,11 +11,11 @@ #include #include #include +#include #include #include #include #include -#include #include #include #include From 7f798379f472bcc6c525019a67dec33f0813cff2 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Thu, 10 Oct 2024 21:56:09 +1100 Subject: [PATCH 10/13] GltfSharedAssetSystem --- .../include/Cesium3DTilesSelection/Tileset.h | 5 +- .../src/TileContentLoadInfo.cpp | 4 +- .../src/TileContentLoadInfo.h | 9 ++-- Cesium3DTilesSelection/src/Tileset.cpp | 5 +- .../src/TilesetContentManager.cpp | 12 ++--- .../src/TilesetContentManager.h | 6 ++- .../src/TilesetJsonLoader.h | 5 +- .../test/TestTilesetContentManager.cpp | 2 +- .../include/CesiumGltf/SharedAssetSystem.h | 47 ------------------- CesiumGltf/src/SharedAssetSystem.cpp | 16 ------- .../include/CesiumGltfReader/GltfReader.h | 11 ++--- .../CesiumGltfReader/GltfSharedAssetSystem.h | 26 ++++++++++ CesiumGltfReader/src/GltfReader.cpp | 5 +- .../src/GltfSharedAssetSystem.cpp | 26 ++++++++++ 14 files changed, 87 insertions(+), 92 deletions(-) delete mode 100644 CesiumGltf/include/CesiumGltf/SharedAssetSystem.h delete mode 100644 CesiumGltf/src/SharedAssetSystem.cpp create mode 100644 CesiumGltfReader/include/CesiumGltfReader/GltfSharedAssetSystem.h create mode 100644 CesiumGltfReader/src/GltfSharedAssetSystem.cpp diff --git a/Cesium3DTilesSelection/include/Cesium3DTilesSelection/Tileset.h b/Cesium3DTilesSelection/include/Cesium3DTilesSelection/Tileset.h index 09dbe01ac..b8f9699bf 100644 --- a/Cesium3DTilesSelection/include/Cesium3DTilesSelection/Tileset.h +++ b/Cesium3DTilesSelection/include/Cesium3DTilesSelection/Tileset.h @@ -184,10 +184,11 @@ class CESIUM3DTILESSELECTION_API Tileset final { /** * @brief Returns the {@link SharedAssetDepot} of this tileset. */ - CesiumGltf::SharedAssetSystem& getSharedAssetSystem() noexcept; + CesiumGltfReader::GltfSharedAssetSystem& getSharedAssetSystem() noexcept; /** @copydoc Tileset::getSharedAssetSystem() */ - const CesiumGltf::SharedAssetSystem& getSharedAssetSystem() const noexcept; + const CesiumGltfReader::GltfSharedAssetSystem& + getSharedAssetSystem() const noexcept; /** * @brief Updates this view but waits for all tiles that meet sse to finish diff --git a/Cesium3DTilesSelection/src/TileContentLoadInfo.cpp b/Cesium3DTilesSelection/src/TileContentLoadInfo.cpp index b09c988cb..f643f71e4 100644 --- a/Cesium3DTilesSelection/src/TileContentLoadInfo.cpp +++ b/Cesium3DTilesSelection/src/TileContentLoadInfo.cpp @@ -9,8 +9,8 @@ TileContentLoadInfo::TileContentLoadInfo( const std::shared_ptr& pPrepareRendererResources_, const std::shared_ptr& pLogger_, - const CesiumUtility::IntrusivePointer - pAssetDepot_, + const CesiumUtility::IntrusivePointer< + CesiumGltfReader::GltfSharedAssetSystem> pAssetDepot_, const TilesetContentOptions& contentOptions_, const Tile& tile) : asyncSystem(asyncSystem_), diff --git a/Cesium3DTilesSelection/src/TileContentLoadInfo.h b/Cesium3DTilesSelection/src/TileContentLoadInfo.h index a887d1c60..4c9d89e3e 100644 --- a/Cesium3DTilesSelection/src/TileContentLoadInfo.h +++ b/Cesium3DTilesSelection/src/TileContentLoadInfo.h @@ -9,7 +9,7 @@ #include #include #include -#include +#include #include #include @@ -25,8 +25,8 @@ struct TileContentLoadInfo { const std::shared_ptr& pPrepareRendererResources, const std::shared_ptr& pLogger, - const CesiumUtility::IntrusivePointer - maybeAssetDepot, + const CesiumUtility::IntrusivePointer< + CesiumGltfReader::GltfSharedAssetSystem> maybeAssetDepot, const TilesetContentOptions& contentOptions, const Tile& tile); @@ -43,7 +43,8 @@ struct TileContentLoadInfo { BoundingVolume tileBoundingVolume; std::optional tileContentBoundingVolume; - CesiumUtility::IntrusivePointer pAssetDepot; + CesiumUtility::IntrusivePointer + pAssetDepot; TileRefine tileRefine; diff --git a/Cesium3DTilesSelection/src/Tileset.cpp b/Cesium3DTilesSelection/src/Tileset.cpp index f5b1b7b5c..a98af5d23 100644 --- a/Cesium3DTilesSelection/src/Tileset.cpp +++ b/Cesium3DTilesSelection/src/Tileset.cpp @@ -149,11 +149,12 @@ const RasterOverlayCollection& Tileset::getOverlays() const noexcept { return this->_pTilesetContentManager->getRasterOverlayCollection(); } -CesiumGltf::SharedAssetSystem& Tileset::getSharedAssetSystem() noexcept { +CesiumGltfReader::GltfSharedAssetSystem& +Tileset::getSharedAssetSystem() noexcept { return *this->_pTilesetContentManager->getSharedAssetSystem(); } -const CesiumGltf::SharedAssetSystem& +const CesiumGltfReader::GltfSharedAssetSystem& Tileset::getSharedAssetSystem() const noexcept { return *this->_pTilesetContentManager->getSharedAssetSystem(); } diff --git a/Cesium3DTilesSelection/src/TilesetContentManager.cpp b/Cesium3DTilesSelection/src/TilesetContentManager.cpp index cf0a8f223..ec4bc45e3 100644 --- a/Cesium3DTilesSelection/src/TilesetContentManager.cpp +++ b/Cesium3DTilesSelection/src/TilesetContentManager.cpp @@ -666,7 +666,7 @@ TilesetContentManager::TilesetContentManager( _tileLoadsInProgress{0}, _loadedTilesCount{0}, _tilesDataUsed{0}, - _pAssetDepot(new CesiumGltf::SharedAssetSystem()), + _pSharedAssets(CesiumGltfReader::GltfSharedAssetSystem::default()), _destructionCompletePromise{externals.asyncSystem.createPromise()}, _destructionCompleteFuture{ this->_destructionCompletePromise.getFuture().share()}, @@ -696,7 +696,7 @@ TilesetContentManager::TilesetContentManager( _tileLoadsInProgress{0}, _loadedTilesCount{0}, _tilesDataUsed{0}, - _pAssetDepot(new CesiumGltf::SharedAssetSystem()), + _pSharedAssets(CesiumGltfReader::GltfSharedAssetSystem::default()), _destructionCompletePromise{externals.asyncSystem.createPromise()}, _destructionCompleteFuture{ this->_destructionCompletePromise.getFuture().share()}, @@ -848,7 +848,7 @@ TilesetContentManager::TilesetContentManager( _tileLoadsInProgress{0}, _loadedTilesCount{0}, _tilesDataUsed{0}, - _pAssetDepot(new CesiumGltf::SharedAssetSystem()), + _pSharedAssets(CesiumGltfReader::GltfSharedAssetSystem::default()), _destructionCompletePromise{externals.asyncSystem.createPromise()}, _destructionCompleteFuture{ this->_destructionCompletePromise.getFuture().share()}, @@ -994,7 +994,7 @@ void TilesetContentManager::loadTileContent( this->_externals.pAssetAccessor, this->_externals.pPrepareRendererResources, this->_externals.pLogger, - this->_pAssetDepot, + this->_pSharedAssets, tilesetOptions.contentOptions, tile}; @@ -1237,9 +1237,9 @@ TilesetContentManager::getTilesetCredits() const noexcept { return this->_tilesetCredits; } -const CesiumUtility::IntrusivePointer& +const CesiumUtility::IntrusivePointer& TilesetContentManager::getSharedAssetSystem() const noexcept { - return this->_pAssetDepot; + return this->_pSharedAssets; } int32_t TilesetContentManager::getNumberOfTilesLoading() const noexcept { diff --git a/Cesium3DTilesSelection/src/TilesetContentManager.h b/Cesium3DTilesSelection/src/TilesetContentManager.h index 4a4ec286f..12bd65484 100644 --- a/Cesium3DTilesSelection/src/TilesetContentManager.h +++ b/Cesium3DTilesSelection/src/TilesetContentManager.h @@ -115,7 +115,8 @@ class TilesetContentManager const std::vector& getTilesetCredits() const noexcept; - const CesiumUtility::IntrusivePointer& + const CesiumUtility::IntrusivePointer< + CesiumGltfReader::GltfSharedAssetSystem>& getSharedAssetSystem() const noexcept; int32_t getNumberOfTilesLoading() const noexcept; @@ -171,7 +172,8 @@ class TilesetContentManager int64_t _tilesDataUsed; // Stores assets that might be shared between tiles. - CesiumUtility::IntrusivePointer _pAssetDepot; + CesiumUtility::IntrusivePointer + _pSharedAssets; CesiumAsync::Promise _destructionCompletePromise; CesiumAsync::SharedFuture _destructionCompleteFuture; diff --git a/Cesium3DTilesSelection/src/TilesetJsonLoader.h b/Cesium3DTilesSelection/src/TilesetJsonLoader.h index 0039edc6e..fa590bdd6 100644 --- a/Cesium3DTilesSelection/src/TilesetJsonLoader.h +++ b/Cesium3DTilesSelection/src/TilesetJsonLoader.h @@ -6,7 +6,7 @@ #include #include #include -#include +#include #include @@ -56,7 +56,8 @@ class TilesetJsonLoader : public TilesetContentLoader { private: std::string _baseUrl; CesiumGeospatial::Ellipsoid _ellipsoid; - CesiumUtility::IntrusivePointer _pAssetDepot; + CesiumUtility::IntrusivePointer + _pSharedAssets; /** * @brief The axis that was declared as the "up-axis" for glTF content. diff --git a/Cesium3DTilesSelection/test/TestTilesetContentManager.cpp b/Cesium3DTilesSelection/test/TestTilesetContentManager.cpp index f297c54d4..af048b2ea 100644 --- a/Cesium3DTilesSelection/test/TestTilesetContentManager.cpp +++ b/Cesium3DTilesSelection/test/TestTilesetContentManager.cpp @@ -1682,7 +1682,7 @@ TEST_CASE("Test the tileset content manager's post processing for gltf") { CHECK(images.size() == 1); } - CHECK(pManager->getSharedAssetSystem()->image().getDistinctCount() == 2); + CHECK(pManager->getSharedAssetSystem()->pImage->getDistinctCount() == 2); // unload the tile content for (auto& child : containerTile.getChildren()) { diff --git a/CesiumGltf/include/CesiumGltf/SharedAssetSystem.h b/CesiumGltf/include/CesiumGltf/SharedAssetSystem.h deleted file mode 100644 index b8abb23ec..000000000 --- a/CesiumGltf/include/CesiumGltf/SharedAssetSystem.h +++ /dev/null @@ -1,47 +0,0 @@ -#pragma once - -#include -#include - -namespace CesiumGltf { - -struct ImageCesium; - -/** - * @brief Contains assets that are potentially shared across multiple glTF - * models. - */ -class SharedAssetSystem - : public CesiumUtility::ReferenceCountedThreadSafe { -public: - SharedAssetSystem() noexcept; - ~SharedAssetSystem() noexcept; - - SharedAssetSystem(const SharedAssetSystem&) = delete; - void operator=(const SharedAssetSystem& other) = delete; - - /** - * Obtains an existing {@link ImageCesium} or constructs a new one using the provided factory. - */ - template - CesiumAsync::SharedFuture< - CesiumUtility::IntrusivePointer> - getOrFetch( - const CesiumAsync::AsyncSystem& asyncSystem, - const std::shared_ptr& pAssetAccessor, - const Factory& factory, - const std::string& uri, - const std::vector& headers) { - return this->_pImages - ->getOrFetch(asyncSystem, pAssetAccessor, factory, uri, headers); - } - - const CesiumAsync::SharedAssetDepot& image(); - -private: - CesiumUtility::IntrusivePointer< - CesiumAsync::SharedAssetDepot> - _pImages; -}; - -} // namespace CesiumGltf diff --git a/CesiumGltf/src/SharedAssetSystem.cpp b/CesiumGltf/src/SharedAssetSystem.cpp deleted file mode 100644 index aac4114ac..000000000 --- a/CesiumGltf/src/SharedAssetSystem.cpp +++ /dev/null @@ -1,16 +0,0 @@ -#include -#include - -namespace CesiumGltf { - -SharedAssetSystem::SharedAssetSystem() noexcept - : _pImages(new CesiumAsync::SharedAssetDepot()) {} - -SharedAssetSystem::~SharedAssetSystem() noexcept = default; - -const CesiumAsync::SharedAssetDepot& -SharedAssetSystem::image() { - return *this->_pImages; -} - -} // namespace CesiumGltf diff --git a/CesiumGltfReader/include/CesiumGltfReader/GltfReader.h b/CesiumGltfReader/include/CesiumGltfReader/GltfReader.h index a1e941bd7..d8f086429 100644 --- a/CesiumGltfReader/include/CesiumGltfReader/GltfReader.h +++ b/CesiumGltfReader/include/CesiumGltfReader/GltfReader.h @@ -10,7 +10,7 @@ #include #include #include -#include +#include #include #include @@ -111,12 +111,11 @@ struct CESIUMGLTFREADER_API GltfReaderOptions { CesiumGltf::Ktx2TranscodeTargets ktx2TranscodeTargets; /** - * The depot that will be used to store all of the shared assets that might - * appear in this glTF. If not present, assets will not be shared between - * glTFs, even if they're loaded from the same URL. + * The shared asset system that will be used to store all of the shared assets + * that might appear in this glTF. */ - CesiumUtility::IntrusivePointer pSharedAssets = - nullptr; + CesiumUtility::IntrusivePointer pSharedAssets = + GltfSharedAssetSystem::default(); }; /** diff --git a/CesiumGltfReader/include/CesiumGltfReader/GltfSharedAssetSystem.h b/CesiumGltfReader/include/CesiumGltfReader/GltfSharedAssetSystem.h new file mode 100644 index 000000000..71d6c2536 --- /dev/null +++ b/CesiumGltfReader/include/CesiumGltfReader/GltfSharedAssetSystem.h @@ -0,0 +1,26 @@ +#pragma once + +#include +#include + +namespace CesiumGltf { +struct ImageCesium; +} + +namespace CesiumGltfReader { + +/** + * @brief Contains assets that are potentially shared across multiple glTF + * models. + */ +class GltfSharedAssetSystem + : public CesiumUtility::ReferenceCountedThreadSafe { +public: + static CesiumUtility::IntrusivePointer default(); + + CesiumUtility::IntrusivePointer< + CesiumAsync::SharedAssetDepot> + pImage; +}; + +} // namespace CesiumGltfReader diff --git a/CesiumGltfReader/src/GltfReader.cpp b/CesiumGltfReader/src/GltfReader.cpp index 5914cc980..495ea5a37 100644 --- a/CesiumGltfReader/src/GltfReader.cpp +++ b/CesiumGltfReader/src/GltfReader.cpp @@ -555,7 +555,8 @@ void CesiumGltfReader::GltfReader::postprocessGltf( const std::shared_ptr& pAssetAccessor, const std::string& uri, const std::vector& headers) { - if (options.pSharedAssets == nullptr) { + if (options.pSharedAssets == nullptr || + options.pSharedAssets->pImage == nullptr) { // We don't have a depot, we have to fetch this the old way. return pAssetAccessor->get(asyncSystem, uri, headers) .thenInWorkerThread( @@ -580,7 +581,7 @@ void CesiumGltfReader::GltfReader::postprocessGltf( .share(); } else { // We have a depot, this is easy! - return options.pSharedAssets->getOrFetch( + return options.pSharedAssets->pImage->getOrFetch( asyncSystem, pAssetAccessor, ImageAssetFactory(options.ktx2TranscodeTargets), diff --git a/CesiumGltfReader/src/GltfSharedAssetSystem.cpp b/CesiumGltfReader/src/GltfSharedAssetSystem.cpp new file mode 100644 index 000000000..0f98f38df --- /dev/null +++ b/CesiumGltfReader/src/GltfSharedAssetSystem.cpp @@ -0,0 +1,26 @@ +#include +#include + +namespace CesiumGltfReader { + +namespace { + +CesiumUtility::IntrusivePointer createDefault() { + CesiumUtility::IntrusivePointer p = + new GltfSharedAssetSystem(); + + p->pImage.emplace(); + + return p; +} + +} // namespace + +/*static*/ CesiumUtility::IntrusivePointer + GltfSharedAssetSystem::default() { + static CesiumUtility::IntrusivePointer pDefault = + createDefault(); + return pDefault; +} + +} // namespace CesiumGltfReader From 526e660687d64d98b3f1aea3b8338dafbac0b1db Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Thu, 10 Oct 2024 22:19:52 +1100 Subject: [PATCH 11/13] default -> getDefault --- Cesium3DTilesSelection/src/TilesetContentManager.cpp | 6 +++--- CesiumGltfReader/include/CesiumGltfReader/GltfReader.h | 2 +- .../include/CesiumGltfReader/GltfSharedAssetSystem.h | 2 +- CesiumGltfReader/src/GltfSharedAssetSystem.cpp | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Cesium3DTilesSelection/src/TilesetContentManager.cpp b/Cesium3DTilesSelection/src/TilesetContentManager.cpp index ec4bc45e3..7595bae46 100644 --- a/Cesium3DTilesSelection/src/TilesetContentManager.cpp +++ b/Cesium3DTilesSelection/src/TilesetContentManager.cpp @@ -666,7 +666,7 @@ TilesetContentManager::TilesetContentManager( _tileLoadsInProgress{0}, _loadedTilesCount{0}, _tilesDataUsed{0}, - _pSharedAssets(CesiumGltfReader::GltfSharedAssetSystem::default()), + _pSharedAssets(CesiumGltfReader::GltfSharedAssetSystem::getDefault()), _destructionCompletePromise{externals.asyncSystem.createPromise()}, _destructionCompleteFuture{ this->_destructionCompletePromise.getFuture().share()}, @@ -696,7 +696,7 @@ TilesetContentManager::TilesetContentManager( _tileLoadsInProgress{0}, _loadedTilesCount{0}, _tilesDataUsed{0}, - _pSharedAssets(CesiumGltfReader::GltfSharedAssetSystem::default()), + _pSharedAssets(CesiumGltfReader::GltfSharedAssetSystem::getDefault()), _destructionCompletePromise{externals.asyncSystem.createPromise()}, _destructionCompleteFuture{ this->_destructionCompletePromise.getFuture().share()}, @@ -848,7 +848,7 @@ TilesetContentManager::TilesetContentManager( _tileLoadsInProgress{0}, _loadedTilesCount{0}, _tilesDataUsed{0}, - _pSharedAssets(CesiumGltfReader::GltfSharedAssetSystem::default()), + _pSharedAssets(CesiumGltfReader::GltfSharedAssetSystem::getDefault()), _destructionCompletePromise{externals.asyncSystem.createPromise()}, _destructionCompleteFuture{ this->_destructionCompletePromise.getFuture().share()}, diff --git a/CesiumGltfReader/include/CesiumGltfReader/GltfReader.h b/CesiumGltfReader/include/CesiumGltfReader/GltfReader.h index d8f086429..221d39627 100644 --- a/CesiumGltfReader/include/CesiumGltfReader/GltfReader.h +++ b/CesiumGltfReader/include/CesiumGltfReader/GltfReader.h @@ -115,7 +115,7 @@ struct CESIUMGLTFREADER_API GltfReaderOptions { * that might appear in this glTF. */ CesiumUtility::IntrusivePointer pSharedAssets = - GltfSharedAssetSystem::default(); + GltfSharedAssetSystem::getDefault(); }; /** diff --git a/CesiumGltfReader/include/CesiumGltfReader/GltfSharedAssetSystem.h b/CesiumGltfReader/include/CesiumGltfReader/GltfSharedAssetSystem.h index 71d6c2536..188b6fb5c 100644 --- a/CesiumGltfReader/include/CesiumGltfReader/GltfSharedAssetSystem.h +++ b/CesiumGltfReader/include/CesiumGltfReader/GltfSharedAssetSystem.h @@ -16,7 +16,7 @@ namespace CesiumGltfReader { class GltfSharedAssetSystem : public CesiumUtility::ReferenceCountedThreadSafe { public: - static CesiumUtility::IntrusivePointer default(); + static CesiumUtility::IntrusivePointer getDefault(); CesiumUtility::IntrusivePointer< CesiumAsync::SharedAssetDepot> diff --git a/CesiumGltfReader/src/GltfSharedAssetSystem.cpp b/CesiumGltfReader/src/GltfSharedAssetSystem.cpp index 0f98f38df..584a842f9 100644 --- a/CesiumGltfReader/src/GltfSharedAssetSystem.cpp +++ b/CesiumGltfReader/src/GltfSharedAssetSystem.cpp @@ -17,7 +17,7 @@ CesiumUtility::IntrusivePointer createDefault() { } // namespace /*static*/ CesiumUtility::IntrusivePointer - GltfSharedAssetSystem::default() { +GltfSharedAssetSystem::getDefault() { static CesiumUtility::IntrusivePointer pDefault = createDefault(); return pDefault; From df1f887e6d2d2cd599f5fb3168cde67b496ea3d3 Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Thu, 10 Oct 2024 11:26:58 -0400 Subject: [PATCH 12/13] Doc comments, rename ImageCesium --- .../IPrepareRendererResources.h | 2 +- .../test/SimplePrepareRendererResource.h | 2 +- .../test/TestTilesetContentManager.cpp | 4 +- CesiumAsync/include/CesiumAsync/SharedAsset.h | 41 ++++++++++++++++--- .../include/CesiumGltf/FeatureIdTextureView.h | 2 +- CesiumGltf/include/CesiumGltf/Image.h | 4 +- .../{ImageCesium.h => ImageAsset.h} | 4 +- .../CesiumGltf/PropertyTexturePropertyView.h | 10 ++--- .../include/CesiumGltf/PropertyTextureView.h | 4 +- CesiumGltf/include/CesiumGltf/TextureView.h | 14 +++---- CesiumGltf/src/PropertyTextureView.cpp | 4 +- CesiumGltf/src/TextureView.cpp | 10 ++--- CesiumGltf/test/TestFeatureIdTextureView.cpp | 2 +- .../test/TestPropertyTexturePropertyView.cpp | 36 ++++++++-------- .../CesiumGltfContent/ImageManipulation.h | 10 ++--- CesiumGltfContent/src/ImageManipulation.cpp | 10 ++--- .../test/TestImageManipulation.cpp | 6 +-- .../include/CesiumGltfReader/GltfReader.h | 4 +- .../CesiumGltfReader/GltfSharedAssetSystem.h | 4 +- .../include/CesiumGltfReader/ImageDecoder.h | 8 ++-- CesiumGltfReader/src/GltfReader.cpp | 12 +++--- .../src/GltfSharedAssetSystem.cpp | 2 +- CesiumGltfReader/src/ImageDecoder.cpp | 4 +- CesiumGltfReader/test/TestGltfReader.cpp | 8 ++-- .../include/CesiumGltfWriter/GltfWriter.h | 4 +- .../IPrepareRasterOverlayRendererResources.h | 4 +- .../CesiumRasterOverlays/RasterOverlayTile.h | 6 +-- .../RasterOverlayTileProvider.h | 2 +- .../src/DebugColorizeTilesRasterOverlay.cpp | 2 +- .../src/QuadtreeRasterOverlayTileProvider.cpp | 10 ++--- .../src/RasterOverlayTileProvider.cpp | 8 ++-- .../src/RasterizedPolygonsOverlay.cpp | 2 +- .../TestQuadtreeRasterOverlayTileProvider.cpp | 4 +- .../test/TestTileMapServiceRasterOverlay.cpp | 2 +- 34 files changed, 140 insertions(+), 111 deletions(-) rename CesiumGltf/include/CesiumGltf/{ImageCesium.h => ImageAsset.h} (97%) diff --git a/Cesium3DTilesSelection/include/Cesium3DTilesSelection/IPrepareRendererResources.h b/Cesium3DTilesSelection/include/Cesium3DTilesSelection/IPrepareRendererResources.h index 19efe0537..b7977fd62 100644 --- a/Cesium3DTilesSelection/include/Cesium3DTilesSelection/IPrepareRendererResources.h +++ b/Cesium3DTilesSelection/include/Cesium3DTilesSelection/IPrepareRendererResources.h @@ -20,7 +20,7 @@ struct Rectangle; } namespace CesiumGltf { -struct ImageCesium; +struct ImageAsset; struct Model; } // namespace CesiumGltf diff --git a/Cesium3DTilesSelection/test/SimplePrepareRendererResource.h b/Cesium3DTilesSelection/test/SimplePrepareRendererResource.h index 19126357c..e980a0057 100644 --- a/Cesium3DTilesSelection/test/SimplePrepareRendererResource.h +++ b/Cesium3DTilesSelection/test/SimplePrepareRendererResource.h @@ -68,7 +68,7 @@ class SimplePrepareRendererResource } virtual void* prepareRasterInLoadThread( - CesiumGltf::ImageCesium& /*image*/, + CesiumGltf::ImageAsset& /*image*/, const std::any& /*rendererOptions*/) override { return new AllocationResult{totalAllocation}; } diff --git a/Cesium3DTilesSelection/test/TestTilesetContentManager.cpp b/Cesium3DTilesSelection/test/TestTilesetContentManager.cpp index af048b2ea..72f809743 100644 --- a/Cesium3DTilesSelection/test/TestTilesetContentManager.cpp +++ b/Cesium3DTilesSelection/test/TestTilesetContentManager.cpp @@ -1298,8 +1298,8 @@ TEST_CASE("Test the tileset content manager's post processing for gltf") { CesiumAsync::Future loadTileImage(RasterOverlayTile& overlayTile) override { - CesiumUtility::IntrusivePointer pImage; - CesiumGltf::ImageCesium& image = pImage.emplace(); + CesiumUtility::IntrusivePointer pImage; + CesiumGltf::ImageAsset& image = pImage.emplace(); image.width = 1; image.height = 1; image.channels = 1; diff --git a/CesiumAsync/include/CesiumAsync/SharedAsset.h b/CesiumAsync/include/CesiumAsync/SharedAsset.h index 3a34cfda8..39b03c3e0 100644 --- a/CesiumAsync/include/CesiumAsync/SharedAsset.h +++ b/CesiumAsync/include/CesiumAsync/SharedAsset.h @@ -19,28 +19,57 @@ namespace CesiumAsync { * @tparam T The type that is _deriving_ from this class. For example, you * should declare your class as * `class MyClass : public SharedAsset { ... };` + * + * @remarks @parblock + * A `SharedAsset` can be in one of three possible states: + * + * **Independent Asset** + * An independent asset isn't affiliated with an asset depot at all. + * Its lifetime is controlled exclusively by IntrusivePointer / reference + * counting. When the asset's reference count goes to zero, it deletes itself. + * An independent asset's `_pDepot` is nullptr. + * + * **Active Depot Asset** + * This is an asset that is owned by an asset depot and that is in use, meaning + * it has a reference count greater than zero. The asset depot owns the asset + * via an `std::unique_ptr`, not via adding to the reference count. So when the + * reference count goes to zero, only the asset depot itself still has a + * reference to it, so it becomes an inactive depot asset. + * + * **Inactive Depot Asset** + * This is also an asset that is owned by the asset depot, but there are no + * other references to it (it has a reference count of zero). It is found in the + * asset depot's `deletionCandidates` list. When a reference to it is added, it + * is removed from `deletionCandidates` and it becomes an active depot asset. + * @endparblock */ template class CESIUMASYNC_API SharedAsset : public CesiumUtility::ExtensibleObject { public: - // Assets can be copied, but the fresh instance has no references and is not - // in the asset depot. + /** + * Assets can be copied, but the fresh instance has no references and is not + * in the asset depot. + */ SharedAsset(const SharedAsset& rhs) : ExtensibleObject(rhs), _referenceCount(0), _pDepot(nullptr), _uniqueAssetId() {} - // After a move construction, the content of the asset is moved to the new - // instance, but the asset depot still references the old instance. + /** + * After a move construction, the content of the asset is moved to the new + * instance, but the asset depot still references the old instance. + */ SharedAsset(SharedAsset&& rhs) : ExtensibleObject(std::move(rhs)), _referenceCount(0), _pDepot(nullptr), _uniqueAssetId() {} - // Assignment does not affect the asset's relationship with the depot, but is - // useful to assign the data in derived classes. + /** + * Assignment does not affect the asset's relationship with the depot, but is + * useful to assign the data in derived classes. + */ SharedAsset& operator=(const SharedAsset& rhs) { CesiumUtility::ExtensibleObject::operator=(rhs); return *this; diff --git a/CesiumGltf/include/CesiumGltf/FeatureIdTextureView.h b/CesiumGltf/include/CesiumGltf/FeatureIdTextureView.h index 807838618..1cfce2c7d 100644 --- a/CesiumGltf/include/CesiumGltf/FeatureIdTextureView.h +++ b/CesiumGltf/include/CesiumGltf/FeatureIdTextureView.h @@ -2,7 +2,7 @@ #include "CesiumGltf/FeatureIdTexture.h" #include "CesiumGltf/Image.h" -#include "CesiumGltf/ImageCesium.h" +#include "CesiumGltf/ImageAsset.h" #include "CesiumGltf/KhrTextureTransform.h" #include "CesiumGltf/Texture.h" #include "CesiumGltf/TextureView.h" diff --git a/CesiumGltf/include/CesiumGltf/Image.h b/CesiumGltf/include/CesiumGltf/Image.h index b2a45c046..9b4fb0d43 100644 --- a/CesiumGltf/include/CesiumGltf/Image.h +++ b/CesiumGltf/include/CesiumGltf/Image.h @@ -1,6 +1,6 @@ #pragma once -#include "CesiumGltf/ImageCesium.h" +#include "CesiumGltf/ImageAsset.h" #include "CesiumGltf/ImageSpec.h" #include "CesiumGltf/Library.h" @@ -14,6 +14,6 @@ struct CESIUMGLTF_API Image final : public ImageSpec { * part of the glTF spec. When an image is loaded from a URL, multiple `Image` * instances may all point to the same `ImageCesium` instance. */ - CesiumUtility::IntrusivePointer pCesium; + CesiumUtility::IntrusivePointer pCesium; }; } // namespace CesiumGltf diff --git a/CesiumGltf/include/CesiumGltf/ImageCesium.h b/CesiumGltf/include/CesiumGltf/ImageAsset.h similarity index 97% rename from CesiumGltf/include/CesiumGltf/ImageCesium.h rename to CesiumGltf/include/CesiumGltf/ImageAsset.h index 6feedf656..3772500a6 100644 --- a/CesiumGltf/include/CesiumGltf/ImageCesium.h +++ b/CesiumGltf/include/CesiumGltf/ImageAsset.h @@ -30,8 +30,8 @@ struct CESIUMGLTF_API ImageCesiumMipPosition { * @brief Holds {@link Image} properties that are specific to the glTF loader * rather than part of the glTF spec. */ -struct CESIUMGLTF_API ImageCesium final - : public CesiumAsync::SharedAsset { +struct CESIUMGLTF_API ImageAsset final + : public CesiumAsync::SharedAsset { /** * @brief The width of the image in pixels. */ diff --git a/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h b/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h index f3dcd168e..d35b46e7a 100644 --- a/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h +++ b/CesiumGltf/include/CesiumGltf/PropertyTexturePropertyView.h @@ -1,6 +1,6 @@ #pragma once -#include "CesiumGltf/ImageCesium.h" +#include "CesiumGltf/ImageAsset.h" #include "CesiumGltf/KhrTextureTransform.h" #include "CesiumGltf/PropertyTextureProperty.h" #include "CesiumGltf/PropertyTransformations.h" @@ -290,7 +290,7 @@ class PropertyTexturePropertyView * @param property The {@link PropertyTextureProperty} * @param classProperty The {@link ClassProperty} this property conforms to. * @param sampler The {@link Sampler} used by the property. - * @param image The {@link ImageCesium} used by the property. + * @param image The {@link ImageAsset} used by the property. * @param channels The value of {@link PropertyTextureProperty::channels}. * @param options The options for constructing the view. */ @@ -298,7 +298,7 @@ class PropertyTexturePropertyView const PropertyTextureProperty& property, const ClassProperty& classProperty, const Sampler& sampler, - const ImageCesium& image, + const ImageAsset& image, const TextureViewOptions& options = TextureViewOptions()) noexcept : PropertyView(classProperty, property), TextureView( @@ -523,7 +523,7 @@ class PropertyTexturePropertyView * @param property The {@link PropertyTextureProperty} * @param classProperty The {@link ClassProperty} this property conforms to. * @param sampler The {@link Sampler} used by the property. - * @param image The {@link ImageCesium} used by the property. + * @param image The {@link ImageAsset} used by the property. * @param channels The value of {@link PropertyTextureProperty::channels}. * @param options The options for constructing the view. */ @@ -531,7 +531,7 @@ class PropertyTexturePropertyView const PropertyTextureProperty& property, const ClassProperty& classProperty, const Sampler& sampler, - const ImageCesium& image, + const ImageAsset& image, const TextureViewOptions& options = TextureViewOptions()) noexcept : PropertyView(classProperty, property), TextureView( diff --git a/CesiumGltf/include/CesiumGltf/PropertyTextureView.h b/CesiumGltf/include/CesiumGltf/PropertyTextureView.h index 6fc8ec158..42a18619e 100644 --- a/CesiumGltf/include/CesiumGltf/PropertyTextureView.h +++ b/CesiumGltf/include/CesiumGltf/PropertyTextureView.h @@ -748,7 +748,7 @@ class PropertyTextureView { return PropertyTexturePropertyView(status); } - const CesiumUtility::IntrusivePointer& pImage = + const CesiumUtility::IntrusivePointer& pImage = _pModel->images[imageIndex].pCesium; const std::vector& channels = propertyTextureProperty.channels; @@ -781,7 +781,7 @@ class PropertyTextureView { PropertyViewStatusType checkChannels( const std::vector& channels, - const ImageCesium& image) const noexcept; + const ImageAsset& image) const noexcept; const Model* _pModel; const PropertyTexture* _pPropertyTexture; diff --git a/CesiumGltf/include/CesiumGltf/TextureView.h b/CesiumGltf/include/CesiumGltf/TextureView.h index 22f677cef..ef681dd74 100644 --- a/CesiumGltf/include/CesiumGltf/TextureView.h +++ b/CesiumGltf/include/CesiumGltf/TextureView.h @@ -1,6 +1,6 @@ #pragma once -#include +#include #include #include #include @@ -113,10 +113,10 @@ class TextureView { /** * @brief Constructs a view of the texture specified by the given {@link Sampler} - * and {@link ImageCesium}. + * and {@link ImageAsset}. * * @param sampler The {@link Sampler} used by the texture. - * @param image The {@link ImageCesium} used by the texture. + * @param image The {@link ImageAsset} used by the texture. * @param textureCoordinateSetIndex The set index for the `TEXCOORD_n` * attribute used to sample this texture. * @param pKhrTextureTransformExtension A pointer to the KHR_texture_transform @@ -125,7 +125,7 @@ class TextureView { */ TextureView( const Sampler& sampler, - const ImageCesium& image, + const ImageAsset& image, int64_t textureCoordinateSetIndex, const ExtensionKhrTextureTransform* pKhrTextureTransformExtension = nullptr, @@ -173,7 +173,7 @@ class TextureView { * This will be nullptr if the texture view runs into * problems during construction. */ - const ImageCesium* getImage() const noexcept { + const ImageAsset* getImage() const noexcept { if (this->_pImageCopy) { return this->_pImageCopy.get(); } @@ -211,12 +211,12 @@ class TextureView { TextureViewStatus _textureViewStatus; const Sampler* _pSampler; - CesiumUtility::IntrusivePointer _pImage; + CesiumUtility::IntrusivePointer _pImage; int64_t _texCoordSetIndex; bool _applyTextureTransform; std::optional _textureTransform; - CesiumUtility::IntrusivePointer _pImageCopy; + CesiumUtility::IntrusivePointer _pImageCopy; }; } // namespace CesiumGltf diff --git a/CesiumGltf/src/PropertyTextureView.cpp b/CesiumGltf/src/PropertyTextureView.cpp index 1b0da8aa2..16ad1fae4 100644 --- a/CesiumGltf/src/PropertyTextureView.cpp +++ b/CesiumGltf/src/PropertyTextureView.cpp @@ -80,7 +80,7 @@ PropertyTextureView::checkImage(const int32_t imageIndex) const noexcept { return PropertyTexturePropertyViewStatus::ErrorInvalidImage; } - const CesiumUtility::IntrusivePointer& pImage = + const CesiumUtility::IntrusivePointer& pImage = _pModel->images[static_cast(imageIndex)].pCesium; if (pImage->width < 1 || pImage->height < 1) { @@ -96,7 +96,7 @@ PropertyTextureView::checkImage(const int32_t imageIndex) const noexcept { PropertyViewStatusType PropertyTextureView::checkChannels( const std::vector& channels, - const ImageCesium& image) const noexcept { + const ImageAsset& image) const noexcept { if (channels.size() <= 0 || channels.size() > 4) { return PropertyTexturePropertyViewStatus::ErrorInvalidChannels; } diff --git a/CesiumGltf/src/TextureView.cpp b/CesiumGltf/src/TextureView.cpp index c41dfae0f..84d1507cc 100644 --- a/CesiumGltf/src/TextureView.cpp +++ b/CesiumGltf/src/TextureView.cpp @@ -72,7 +72,7 @@ TextureView::TextureView( if (options.makeImageCopy) { this->_pImageCopy = - this->_pImage ? new ImageCesium(*this->_pImage) : nullptr; + this->_pImage ? new ImageAsset(*this->_pImage) : nullptr; } this->_textureViewStatus = TextureViewStatus::Valid; @@ -80,13 +80,13 @@ TextureView::TextureView( TextureView::TextureView( const Sampler& sampler, - const ImageCesium& image, + const ImageAsset& image, int64_t textureCoordinateSetIndex, const ExtensionKhrTextureTransform* pKhrTextureTransformExtension, const TextureViewOptions& options) noexcept : _textureViewStatus(TextureViewStatus::ErrorUninitialized), _pSampler(&sampler), - _pImage(new ImageCesium(image)), + _pImage(new ImageAsset(image)), _texCoordSetIndex(textureCoordinateSetIndex), _applyTextureTransform(options.applyKhrTextureTransformExtension), _textureTransform(std::nullopt), @@ -106,7 +106,7 @@ TextureView::TextureView( if (options.makeImageCopy) { this->_pImageCopy = - this->_pImage ? new ImageCesium(*this->_pImage) : nullptr; + this->_pImage ? new ImageAsset(*this->_pImage) : nullptr; } this->_textureViewStatus = TextureViewStatus::Valid; @@ -132,7 +132,7 @@ std::vector TextureView::sampleNearestPixel( u = applySamplerWrapS(u, this->_pSampler->wrapS); v = applySamplerWrapT(v, this->_pSampler->wrapT); - const ImageCesium& image = + const ImageAsset& image = this->_pImageCopy != nullptr ? *this->_pImageCopy : *this->_pImage; // For nearest filtering, std::floor is used instead of std::round. diff --git a/CesiumGltf/test/TestFeatureIdTextureView.cpp b/CesiumGltf/test/TestFeatureIdTextureView.cpp index 12bee9135..3acd813ac 100644 --- a/CesiumGltf/test/TestFeatureIdTextureView.cpp +++ b/CesiumGltf/test/TestFeatureIdTextureView.cpp @@ -414,7 +414,7 @@ TEST_CASE("Test FeatureIdTextureView with makeImageCopy = true") { std::vector emptyData; image.pCesium->pixelData.swap(emptyData); - const ImageCesium* pImage = view.getImage(); + const ImageAsset* pImage = view.getImage(); REQUIRE(pImage); REQUIRE(pImage->width == image.pCesium->width); REQUIRE(pImage->height == image.pCesium->height); diff --git a/CesiumGltf/test/TestPropertyTexturePropertyView.cpp b/CesiumGltf/test/TestPropertyTexturePropertyView.cpp index ae3a815fb..eb71c2ac3 100644 --- a/CesiumGltf/test/TestPropertyTexturePropertyView.cpp +++ b/CesiumGltf/test/TestPropertyTexturePropertyView.cpp @@ -27,7 +27,7 @@ void checkTextureValues( convertPropertyComponentTypeToString(componentType); Sampler sampler; - ImageCesium image; + ImageAsset image; image.width = 2; image.height = 2; image.channels = static_cast(sizeof(T)); @@ -99,7 +99,7 @@ void checkTextureValues( classProperty.defaultProperty = defaultValue; Sampler sampler; - ImageCesium image; + ImageAsset image; image.width = 2; image.height = 2; image.channels = static_cast(sizeof(T)); @@ -173,7 +173,7 @@ void checkNormalizedTextureValues( classProperty.defaultProperty = defaultValue; Sampler sampler; - ImageCesium image; + ImageAsset image; image.width = 2; image.height = 2; image.channels = static_cast(sizeof(T)); @@ -243,7 +243,7 @@ void checkTextureArrayValues( classProperty.count = count; Sampler sampler; - ImageCesium image; + ImageAsset image; image.width = 2; image.height = 2; image.channels = @@ -329,7 +329,7 @@ void checkTextureArrayValues( classProperty.defaultProperty = defaultValue; Sampler sampler; - ImageCesium image; + ImageAsset image; image.width = 2; image.height = 2; image.channels = @@ -429,7 +429,7 @@ void checkNormalizedTextureArrayValues( classProperty.defaultProperty = defaultValue; Sampler sampler; - ImageCesium image; + ImageAsset image; image.width = 2; image.height = 2; image.channels = @@ -1397,7 +1397,7 @@ TEST_CASE("Check that PropertyTextureProperty values override class property " classProperty.max = 10.0f; Sampler sampler; - ImageCesium image; + ImageAsset image; image.width = 2; image.height = 2; image.channels = 4; @@ -1474,7 +1474,7 @@ TEST_CASE("Check that non-adjacent channels resolve to expected output") { classProperty.componentType = ClassProperty::ComponentType::UINT8; Sampler sampler; - ImageCesium image; + ImageAsset image; image.width = 2; image.height = 2; image.channels = 4; @@ -1516,7 +1516,7 @@ TEST_CASE("Check that non-adjacent channels resolve to expected output") { classProperty.componentType = ClassProperty::ComponentType::UINT16; Sampler sampler; - ImageCesium image; + ImageAsset image; image.width = 2; image.height = 2; image.channels = 4; @@ -1557,7 +1557,7 @@ TEST_CASE("Check that non-adjacent channels resolve to expected output") { classProperty.componentType = ClassProperty::ComponentType::UINT8; Sampler sampler; - ImageCesium image; + ImageAsset image; image.width = 2; image.height = 2; image.channels = 4; @@ -1603,7 +1603,7 @@ TEST_CASE("Check that non-adjacent channels resolve to expected output") { classProperty.array = true; Sampler sampler; - ImageCesium image; + ImageAsset image; image.width = 2; image.height = 2; image.channels = 4; @@ -1660,7 +1660,7 @@ TEST_CASE("Check sampling with different sampler values") { classProperty.type = ClassProperty::Type::SCALAR; classProperty.componentType = ClassProperty::ComponentType::UINT8; - ImageCesium image; + ImageAsset image; image.width = 2; image.height = 2; image.channels = 1; @@ -1793,7 +1793,7 @@ TEST_CASE("Test PropertyTextureProperty constructs with " sampler.wrapS = Sampler::WrapS::REPEAT; sampler.wrapT = Sampler::WrapT::REPEAT; - ImageCesium image; + ImageAsset image; image.width = 2; image.height = 2; image.channels = 1; @@ -1866,7 +1866,7 @@ TEST_CASE("Test normalized PropertyTextureProperty constructs with " sampler.wrapS = Sampler::WrapS::REPEAT; sampler.wrapT = Sampler::WrapT::REPEAT; - ImageCesium image; + ImageAsset image; image.width = 2; image.height = 2; image.channels = 1; @@ -1940,7 +1940,7 @@ TEST_CASE("Test PropertyTextureProperty constructs with " sampler.wrapS = Sampler::WrapS::REPEAT; sampler.wrapT = Sampler::WrapT::REPEAT; - ImageCesium image; + ImageAsset image; image.width = 2; image.height = 2; image.channels = 1; @@ -1963,7 +1963,7 @@ TEST_CASE("Test PropertyTextureProperty constructs with " std::vector emptyData; image.pixelData.swap(emptyData); - const ImageCesium* pImage = view.getImage(); + const ImageAsset* pImage = view.getImage(); REQUIRE(pImage); REQUIRE(pImage->width == image.width); REQUIRE(pImage->height == image.height); @@ -2007,7 +2007,7 @@ TEST_CASE("Test normalized PropertyTextureProperty constructs with " sampler.wrapS = Sampler::WrapS::REPEAT; sampler.wrapT = Sampler::WrapT::REPEAT; - ImageCesium image; + ImageAsset image; image.width = 2; image.height = 2; image.channels = 1; @@ -2030,7 +2030,7 @@ TEST_CASE("Test normalized PropertyTextureProperty constructs with " std::vector emptyData; image.pixelData.swap(emptyData); - const ImageCesium* pImage = view.getImage(); + const ImageAsset* pImage = view.getImage(); REQUIRE(pImage); REQUIRE(pImage->width == image.width); REQUIRE(pImage->height == image.height); diff --git a/CesiumGltfContent/include/CesiumGltfContent/ImageManipulation.h b/CesiumGltfContent/include/CesiumGltfContent/ImageManipulation.h index 82b3e2466..2f350629b 100644 --- a/CesiumGltfContent/include/CesiumGltfContent/ImageManipulation.h +++ b/CesiumGltfContent/include/CesiumGltfContent/ImageManipulation.h @@ -7,7 +7,7 @@ // Forward declarations namespace CesiumGltf { -struct ImageCesium; +struct ImageAsset; } namespace CesiumGltfContent { @@ -91,9 +91,9 @@ class CESIUMGLTFCONTENT_API ImageManipulation { * incompatible formats. */ static bool blitImage( - CesiumGltf::ImageCesium& target, + CesiumGltf::ImageAsset& target, const PixelRectangle& targetPixels, - const CesiumGltf::ImageCesium& source, + const CesiumGltf::ImageAsset& source, const PixelRectangle& sourcePixels); /** @@ -103,7 +103,7 @@ class CESIUMGLTFCONTENT_API ImageManipulation { * @return The byte buffer containing the image. If the buffer is empty, the * image could not be written. */ - static std::vector savePng(const CesiumGltf::ImageCesium& image); + static std::vector savePng(const CesiumGltf::ImageAsset& image); /** * @brief Saves an image to an existing byte buffer in PNG format. @@ -114,7 +114,7 @@ class CESIUMGLTFCONTENT_API ImageManipulation { * could not be written. */ static void - savePng(const CesiumGltf::ImageCesium& image, std::vector& output); + savePng(const CesiumGltf::ImageAsset& image, std::vector& output); }; } // namespace CesiumGltfContent diff --git a/CesiumGltfContent/src/ImageManipulation.cpp b/CesiumGltfContent/src/ImageManipulation.cpp index c8de11e06..9bd129a3a 100644 --- a/CesiumGltfContent/src/ImageManipulation.cpp +++ b/CesiumGltfContent/src/ImageManipulation.cpp @@ -1,4 +1,4 @@ -#include +#include #include #include @@ -43,9 +43,9 @@ void ImageManipulation::unsafeBlitImage( } bool ImageManipulation::blitImage( - CesiumGltf::ImageCesium& target, + CesiumGltf::ImageAsset& target, const PixelRectangle& targetPixels, - const CesiumGltf::ImageCesium& source, + const CesiumGltf::ImageAsset& source, const PixelRectangle& sourcePixels) { if (sourcePixels.x < 0 || sourcePixels.y < 0 || sourcePixels.width < 0 || @@ -134,7 +134,7 @@ void writePngToVector(void* context, void* data, int size) { } // namespace /*static*/ void ImageManipulation::savePng( - const CesiumGltf::ImageCesium& image, + const CesiumGltf::ImageAsset& image, std::vector& output) { if (image.bytesPerChannel != 1) { // Only 8-bit images can be written. @@ -152,7 +152,7 @@ void writePngToVector(void* context, void* data, int size) { } /*static*/ std::vector -ImageManipulation::savePng(const CesiumGltf::ImageCesium& image) { +ImageManipulation::savePng(const CesiumGltf::ImageAsset& image) { std::vector result; savePng(image, result); return result; diff --git a/CesiumGltfContent/test/TestImageManipulation.cpp b/CesiumGltfContent/test/TestImageManipulation.cpp index 38833411b..227e9a8c6 100644 --- a/CesiumGltfContent/test/TestImageManipulation.cpp +++ b/CesiumGltfContent/test/TestImageManipulation.cpp @@ -1,4 +1,4 @@ -#include +#include #include #include @@ -128,7 +128,7 @@ TEST_CASE("ImageManipulation::unsafeBlitImage subset of source") { } TEST_CASE("ImageManipulation::blitImage") { - ImageCesium target; + ImageAsset target; target.bytesPerChannel = 2; target.channels = 4; target.width = 15; @@ -139,7 +139,7 @@ TEST_CASE("ImageManipulation::blitImage") { target.bytesPerChannel), std::byte(1)); - ImageCesium source; + ImageAsset source; source.bytesPerChannel = 2; source.channels = 4; source.width = 10; diff --git a/CesiumGltfReader/include/CesiumGltfReader/GltfReader.h b/CesiumGltfReader/include/CesiumGltfReader/GltfReader.h index 221d39627..f534ed6b9 100644 --- a/CesiumGltfReader/include/CesiumGltfReader/GltfReader.h +++ b/CesiumGltfReader/include/CesiumGltfReader/GltfReader.h @@ -7,7 +7,7 @@ #include #include #include -#include +#include #include #include #include @@ -217,7 +217,7 @@ class CESIUMGLTFREADER_API GltfReader { * @return A string describing the error, if unable to generate mipmaps. */ static std::optional - generateMipMaps(CesiumGltf::ImageCesium& image) { + generateMipMaps(CesiumGltf::ImageAsset& image) { return ImageDecoder::generateMipMaps(image); } diff --git a/CesiumGltfReader/include/CesiumGltfReader/GltfSharedAssetSystem.h b/CesiumGltfReader/include/CesiumGltfReader/GltfSharedAssetSystem.h index 188b6fb5c..46c4fb4f0 100644 --- a/CesiumGltfReader/include/CesiumGltfReader/GltfSharedAssetSystem.h +++ b/CesiumGltfReader/include/CesiumGltfReader/GltfSharedAssetSystem.h @@ -4,7 +4,7 @@ #include namespace CesiumGltf { -struct ImageCesium; +struct ImageAsset; } namespace CesiumGltfReader { @@ -19,7 +19,7 @@ class GltfSharedAssetSystem static CesiumUtility::IntrusivePointer getDefault(); CesiumUtility::IntrusivePointer< - CesiumAsync::SharedAssetDepot> + CesiumAsync::SharedAssetDepot> pImage; }; diff --git a/CesiumGltfReader/include/CesiumGltfReader/ImageDecoder.h b/CesiumGltfReader/include/CesiumGltfReader/ImageDecoder.h index 76b783657..b2d60d2df 100644 --- a/CesiumGltfReader/include/CesiumGltfReader/ImageDecoder.h +++ b/CesiumGltfReader/include/CesiumGltfReader/ImageDecoder.h @@ -1,4 +1,4 @@ -#include "CesiumGltf/ImageCesium.h" +#include "CesiumGltf/ImageAsset.h" #include "CesiumGltfReader/Library.h" #include @@ -18,11 +18,11 @@ namespace CesiumGltfReader { struct CESIUMGLTFREADER_API ImageReaderResult { /** - * @brief The {@link ImageCesium} that was read. + * @brief The {@link ImageAsset} that was read. * * This will be `std::nullopt` if the image could not be read. */ - CesiumUtility::IntrusivePointer pImage; + CesiumUtility::IntrusivePointer pImage; /** * @brief Error messages that occurred while trying to read the image. @@ -63,7 +63,7 @@ class ImageDecoder { * @return A string describing the error, if unable to generate mipmaps. */ static std::optional - generateMipMaps(CesiumGltf::ImageCesium& image); + generateMipMaps(CesiumGltf::ImageAsset& image); }; } // namespace CesiumGltfReader diff --git a/CesiumGltfReader/src/GltfReader.cpp b/CesiumGltfReader/src/GltfReader.cpp index 495ea5a37..03e2b5017 100644 --- a/CesiumGltfReader/src/GltfReader.cpp +++ b/CesiumGltfReader/src/GltfReader.cpp @@ -14,7 +14,7 @@ #include #include #include -#include +#include #include #include #include @@ -43,13 +43,13 @@ using namespace CesiumUtility; namespace { /** - * Used to construct an ImageCesium. + * Used to construct an ImageAsset. */ struct ImageAssetFactory { ImageAssetFactory(const Ktx2TranscodeTargets& ktx2TranscodeTargets_) : ktx2TranscodeTargets(ktx2TranscodeTargets_) {} - CesiumUtility::IntrusivePointer + CesiumUtility::IntrusivePointer createFrom(const gsl::span& data) const { ImageReaderResult imageResult = ImageDecoder::readImage(data, this->ktx2TranscodeTargets); @@ -565,7 +565,7 @@ void CesiumGltfReader::GltfReader::postprocessGltf( const IAssetResponse* pResponse = pRequest->response(); - CesiumUtility::IntrusivePointer pAsset = + CesiumUtility::IntrusivePointer pAsset = nullptr; if (pResponse) { @@ -590,12 +590,12 @@ void CesiumGltfReader::GltfReader::postprocessGltf( } }; - SharedFuture> future = + SharedFuture> future = getAsset(asyncSystem, pAssetAccessor, uri, tHeaders); resolvedBuffers.push_back(future.thenInWorkerThread( [pImage = - &image](const IntrusivePointer& pLoadedImage) { + &image](const IntrusivePointer& pLoadedImage) { std::string imageUri = *pImage->uri; pImage->uri = std::nullopt; diff --git a/CesiumGltfReader/src/GltfSharedAssetSystem.cpp b/CesiumGltfReader/src/GltfSharedAssetSystem.cpp index 584a842f9..a5c803535 100644 --- a/CesiumGltfReader/src/GltfSharedAssetSystem.cpp +++ b/CesiumGltfReader/src/GltfSharedAssetSystem.cpp @@ -1,4 +1,4 @@ -#include +#include #include namespace CesiumGltfReader { diff --git a/CesiumGltfReader/src/ImageDecoder.cpp b/CesiumGltfReader/src/ImageDecoder.cpp index a55549473..664f7910c 100644 --- a/CesiumGltfReader/src/ImageDecoder.cpp +++ b/CesiumGltfReader/src/ImageDecoder.cpp @@ -77,7 +77,7 @@ ImageReaderResult ImageDecoder::readImage( ImageReaderResult result; - CesiumGltf::ImageCesium& image = result.pImage.emplace(); + CesiumGltf::ImageAsset& image = result.pImage.emplace(); if (isKtx(data)) { ktxTexture2* pTexture = nullptr; @@ -364,7 +364,7 @@ ImageReaderResult ImageDecoder::readImage( } /*static*/ -std::optional ImageDecoder::generateMipMaps(ImageCesium& image) { +std::optional ImageDecoder::generateMipMaps(ImageAsset& image) { if (!image.mipPositions.empty() || image.compressedPixelFormat != GpuCompressedPixelFormat::NONE) { // No error message needed, since this is not technically a failure. diff --git a/CesiumGltfReader/test/TestGltfReader.cpp b/CesiumGltfReader/test/TestGltfReader.cpp index 01fa6b0dd..18561464d 100644 --- a/CesiumGltfReader/test/TestGltfReader.cpp +++ b/CesiumGltfReader/test/TestGltfReader.cpp @@ -574,7 +574,7 @@ TEST_CASE("Can correctly interpret mipmaps in KTX2 files") { GltfReader::readImage(data, Ktx2TranscodeTargets{}); REQUIRE(imageResult.pImage); - const ImageCesium& image = *imageResult.pImage; + const ImageAsset& image = *imageResult.pImage; REQUIRE(image.mipPositions.size() == 1); CHECK(image.mipPositions[0].byteOffset == 0); CHECK(image.mipPositions[0].byteSize > 0); @@ -594,7 +594,7 @@ TEST_CASE("Can correctly interpret mipmaps in KTX2 files") { GltfReader::readImage(data, Ktx2TranscodeTargets{}); REQUIRE(imageResult.pImage); - const ImageCesium& image = *imageResult.pImage; + const ImageAsset& image = *imageResult.pImage; REQUIRE(image.mipPositions.size() == 0); CHECK(image.pixelData.size() > 0); } @@ -608,7 +608,7 @@ TEST_CASE("Can correctly interpret mipmaps in KTX2 files") { GltfReader::readImage(data, Ktx2TranscodeTargets{}); REQUIRE(imageResult.pImage); - const ImageCesium& image = *imageResult.pImage; + const ImageAsset& image = *imageResult.pImage; REQUIRE(image.mipPositions.size() == 9); CHECK(image.mipPositions[0].byteSize > 0); CHECK( @@ -689,7 +689,7 @@ TEST_CASE("Decodes images with data uris") { REQUIRE(model.images.size() == 1); - const ImageCesium& image = *model.images.front().pCesium; + const ImageAsset& image = *model.images.front().pCesium; CHECK(image.width == 256); CHECK(image.height == 256); diff --git a/CesiumGltfWriter/include/CesiumGltfWriter/GltfWriter.h b/CesiumGltfWriter/include/CesiumGltfWriter/GltfWriter.h index da7efde9a..97a2e6df5 100644 --- a/CesiumGltfWriter/include/CesiumGltfWriter/GltfWriter.h +++ b/CesiumGltfWriter/include/CesiumGltfWriter/GltfWriter.h @@ -75,7 +75,7 @@ class CESIUMGLTFWRITER_API GltfWriter { * @brief Serializes the provided model into a glTF JSON byte vector. * * @details Ignores internal data such as {@link CesiumGltf::BufferCesium} - * and {@link CesiumGltf::ImageCesium} when serializing the glTF. Internal + * and {@link CesiumGltf::ImageAsset} when serializing the glTF. Internal * data must either be converted to data uris or saved as external files. The * buffer.uri and image.uri fields must be set accordingly prior to calling * this function. @@ -93,7 +93,7 @@ class CESIUMGLTFWRITER_API GltfWriter { * * @details The first buffer object implicitly refers to the GLB binary chunk * and should not have a uri. Ignores internal data such as - * {@link CesiumGltf::BufferCesium} and {@link CesiumGltf::ImageCesium}. + * {@link CesiumGltf::BufferCesium} and {@link CesiumGltf::ImageAsset}. * * @param model The model. * @param bufferData The buffer data to store in the GLB binary chunk. diff --git a/CesiumRasterOverlays/include/CesiumRasterOverlays/IPrepareRasterOverlayRendererResources.h b/CesiumRasterOverlays/include/CesiumRasterOverlays/IPrepareRasterOverlayRendererResources.h index d9d88878f..7272fb16d 100644 --- a/CesiumRasterOverlays/include/CesiumRasterOverlays/IPrepareRasterOverlayRendererResources.h +++ b/CesiumRasterOverlays/include/CesiumRasterOverlays/IPrepareRasterOverlayRendererResources.h @@ -5,7 +5,7 @@ #include namespace CesiumGltf { -struct ImageCesium; +struct ImageAsset; } namespace CesiumRasterOverlays { @@ -28,7 +28,7 @@ class CESIUMRASTEROVERLAYS_API IPrepareRasterOverlayRendererResources { * `pLoadThreadResult` parameter. */ virtual void* prepareRasterInLoadThread( - CesiumGltf::ImageCesium& image, + CesiumGltf::ImageAsset& image, const std::any& rendererOptions) = 0; /** diff --git a/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTile.h b/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTile.h index f27c3adcc..48c2c13a5 100644 --- a/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTile.h +++ b/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTile.h @@ -192,7 +192,7 @@ class RasterOverlayTile final * * @return The image data. */ - CesiumUtility::IntrusivePointer + CesiumUtility::IntrusivePointer getImage() const noexcept { return this->_pImage; } @@ -205,7 +205,7 @@ class RasterOverlayTile final * * @return The image data. */ - CesiumUtility::IntrusivePointer getImage() noexcept { + CesiumUtility::IntrusivePointer getImage() noexcept { return this->_pImage; } @@ -259,7 +259,7 @@ class RasterOverlayTile final CesiumGeometry::Rectangle _rectangle; std::vector _tileCredits; LoadState _state; - CesiumUtility::IntrusivePointer _pImage; + CesiumUtility::IntrusivePointer _pImage; void* _pRendererResources; MoreDetailAvailable _moreDetailAvailable; }; diff --git a/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTileProvider.h b/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTileProvider.h index daf143391..db1113b5d 100644 --- a/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTileProvider.h +++ b/CesiumRasterOverlays/include/CesiumRasterOverlays/RasterOverlayTileProvider.h @@ -32,7 +32,7 @@ struct CESIUMRASTEROVERLAYS_API LoadedRasterOverlayImage { * This will be an empty optional if the loading failed. In this case, * the `errors` vector will contain the corresponding error messages. */ - CesiumUtility::IntrusivePointer pImage{nullptr}; + CesiumUtility::IntrusivePointer pImage{nullptr}; /** * @brief The projected rectangle defining the bounds of this image. diff --git a/CesiumRasterOverlays/src/DebugColorizeTilesRasterOverlay.cpp b/CesiumRasterOverlays/src/DebugColorizeTilesRasterOverlay.cpp index fb728490c..d3a9e96ef 100644 --- a/CesiumRasterOverlays/src/DebugColorizeTilesRasterOverlay.cpp +++ b/CesiumRasterOverlays/src/DebugColorizeTilesRasterOverlay.cpp @@ -42,7 +42,7 @@ class DebugTileProvider : public RasterOverlayTileProvider { result.rectangle = overlayTile.getRectangle(); - ImageCesium& image = result.pImage.emplace(); + ImageAsset& image = result.pImage.emplace(); image.width = 1; image.height = 1; image.channels = 4; diff --git a/CesiumRasterOverlays/src/QuadtreeRasterOverlayTileProvider.cpp b/CesiumRasterOverlays/src/QuadtreeRasterOverlayTileProvider.cpp index 9e4c95333..1c4499757 100644 --- a/CesiumRasterOverlays/src/QuadtreeRasterOverlayTileProvider.cpp +++ b/CesiumRasterOverlays/src/QuadtreeRasterOverlayTileProvider.cpp @@ -381,7 +381,7 @@ QuadtreeRasterOverlayTileProvider::getQuadtreeTile( namespace { PixelRectangle computePixelRectangle( - const ImageCesium& image, + const ImageAsset& image, const Rectangle& totalRectangle, const Rectangle& partRectangle) { // Pixel coordinates are measured from the top left. @@ -417,9 +417,9 @@ PixelRectangle computePixelRectangle( // source image where the source subset rectangle overlaps the target // rectangle is copied to the target image. void blitImage( - ImageCesium& target, + ImageAsset& target, const Rectangle& targetRectangle, - const ImageCesium& source, + const ImageAsset& source, const Rectangle& sourceRectangle, const std::optional& sourceSubset) { const Rectangle sourceToCopy = sourceSubset.value_or(sourceRectangle); @@ -478,7 +478,7 @@ QuadtreeRasterOverlayTileProvider::loadTileImage( } } return LoadedRasterOverlayImage{ - new ImageCesium(), + new ImageAsset(), Rectangle(), {}, std::move(errors), @@ -681,7 +681,7 @@ QuadtreeRasterOverlayTileProvider::combineImages( result.moreDetailAvailable = false; result.errorList = std::move(errors); - ImageCesium& target = result.pImage.emplace(); + ImageAsset& target = result.pImage.emplace(); target.bytesPerChannel = measurements.bytesPerChannel; target.channels = measurements.channels; target.width = measurements.widthPixels; diff --git a/CesiumRasterOverlays/src/RasterOverlayTileProvider.cpp b/CesiumRasterOverlays/src/RasterOverlayTileProvider.cpp index 591c2c0c5..381497083 100644 --- a/CesiumRasterOverlays/src/RasterOverlayTileProvider.cpp +++ b/CesiumRasterOverlays/src/RasterOverlayTileProvider.cpp @@ -167,7 +167,7 @@ RasterOverlayTileProvider::loadTileImageFromUrl( if (pResponse->data().empty()) { if (options.allowEmptyImages) { return LoadedRasterOverlayImage{ - new CesiumGltf::ImageCesium(), + new CesiumGltf::ImageAsset(), options.rectangle, std::move(options.credits), {}, @@ -214,7 +214,7 @@ RasterOverlayTileProvider::loadTileImageFromUrl( namespace { struct LoadResult { RasterOverlayTile::LoadState state = RasterOverlayTile::LoadState::Unloaded; - CesiumUtility::IntrusivePointer pImage = nullptr; + CesiumUtility::IntrusivePointer pImage = nullptr; CesiumGeometry::Rectangle rectangle = {}; std::vector credits = {}; void* pRendererResources = nullptr; @@ -268,7 +268,7 @@ static LoadResult createLoadResultFromLoadedImage( "Warnings while loading image for tile"); } - CesiumGltf::ImageCesium& image = *loadedImage.pImage; + CesiumGltf::ImageAsset& image = *loadedImage.pImage; const int32_t bytesPerPixel = image.channels * image.bytesPerChannel; const int64_t requiredBytes = @@ -351,7 +351,7 @@ CesiumAsync::Future RasterOverlayTileProvider::doLoad( pTile->setState(result.state); if (pTile->getImage() != nullptr) { - ImageCesium& imageCesium = *pTile->getImage(); + ImageAsset& imageCesium = *pTile->getImage(); // If the image size hasn't been overridden, store the pixelData // size now. We'll add this number to our total memory usage now, diff --git a/CesiumRasterOverlays/src/RasterizedPolygonsOverlay.cpp b/CesiumRasterOverlays/src/RasterizedPolygonsOverlay.cpp index d8da4114c..8b8868ba9 100644 --- a/CesiumRasterOverlays/src/RasterizedPolygonsOverlay.cpp +++ b/CesiumRasterOverlays/src/RasterizedPolygonsOverlay.cpp @@ -24,7 +24,7 @@ void rasterizePolygons( const std::vector& cartographicPolygons, bool invertSelection) { - CesiumGltf::ImageCesium& image = loaded.pImage.emplace(); + CesiumGltf::ImageAsset& image = loaded.pImage.emplace(); std::byte insideColor; std::byte outsideColor; diff --git a/CesiumRasterOverlays/test/TestQuadtreeRasterOverlayTileProvider.cpp b/CesiumRasterOverlays/test/TestQuadtreeRasterOverlayTileProvider.cpp index 705f90fe8..1abd0189d 100644 --- a/CesiumRasterOverlays/test/TestQuadtreeRasterOverlayTileProvider.cpp +++ b/CesiumRasterOverlays/test/TestQuadtreeRasterOverlayTileProvider.cpp @@ -172,7 +172,7 @@ TEST_CASE("QuadtreeRasterOverlayTileProvider getTile") { REQUIRE(pTile->getImage()); - const ImageCesium& image = *pTile->getImage(); + const ImageAsset& image = *pTile->getImage(); CHECK(image.width > 0); CHECK(image.height > 0); CHECK(image.pixelData.size() > 0); @@ -228,7 +228,7 @@ TEST_CASE("QuadtreeRasterOverlayTileProvider getTile") { REQUIRE(pTile->getImage()); - const ImageCesium& image = *pTile->getImage(); + const ImageAsset& image = *pTile->getImage(); CHECK(image.width > 0); CHECK(image.height > 0); CHECK(image.pixelData.size() > 0); diff --git a/CesiumRasterOverlays/test/TestTileMapServiceRasterOverlay.cpp b/CesiumRasterOverlays/test/TestTileMapServiceRasterOverlay.cpp index 6f3228285..0c9a802cc 100644 --- a/CesiumRasterOverlays/test/TestTileMapServiceRasterOverlay.cpp +++ b/CesiumRasterOverlays/test/TestTileMapServiceRasterOverlay.cpp @@ -74,7 +74,7 @@ TEST_CASE("TileMapServiceRasterOverlay") { REQUIRE(pTile->getImage()); - const ImageCesium& image = *pTile->getImage(); + const ImageAsset& image = *pTile->getImage(); CHECK(image.width > 0); CHECK(image.height > 0); } From 883e36f100c6a1487bf1103bafb937a66578705b Mon Sep 17 00:00:00 2001 From: Ashley Rogers Date: Thu, 10 Oct 2024 15:44:42 -0400 Subject: [PATCH 13/13] Rename ImageAssetMipPosition --- CesiumGltf/include/CesiumGltf/Image.h | 2 +- CesiumGltf/include/CesiumGltf/ImageAsset.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CesiumGltf/include/CesiumGltf/Image.h b/CesiumGltf/include/CesiumGltf/Image.h index 9b4fb0d43..31ff9bc9d 100644 --- a/CesiumGltf/include/CesiumGltf/Image.h +++ b/CesiumGltf/include/CesiumGltf/Image.h @@ -12,7 +12,7 @@ struct CESIUMGLTF_API Image final : public ImageSpec { /** * @brief Holds properties that are specific to the glTF loader rather than * part of the glTF spec. When an image is loaded from a URL, multiple `Image` - * instances may all point to the same `ImageCesium` instance. + * instances may all point to the same `ImageAsset` instance. */ CesiumUtility::IntrusivePointer pCesium; }; diff --git a/CesiumGltf/include/CesiumGltf/ImageAsset.h b/CesiumGltf/include/CesiumGltf/ImageAsset.h index 3772500a6..f7614c437 100644 --- a/CesiumGltf/include/CesiumGltf/ImageAsset.h +++ b/CesiumGltf/include/CesiumGltf/ImageAsset.h @@ -14,7 +14,7 @@ namespace CesiumGltf { /** * @brief The byte range within a buffer where this mip exists. */ -struct CESIUMGLTF_API ImageCesiumMipPosition { +struct CESIUMGLTF_API ImageAssetMipPosition { /** * @brief The byte index where this mip begins. */ @@ -67,7 +67,7 @@ struct CESIUMGLTF_API ImageAsset final * biggest and etc. If this is empty, assume the entire buffer is a single * image, the mip map will need to be generated on the client in this case. */ - std::vector mipPositions; + std::vector mipPositions; /** * @brief The pixel data.