Skip to content

Commit

Permalink
More small tweaks.
Browse files Browse the repository at this point in the history
  • Loading branch information
kring committed Oct 30, 2024
1 parent 8b60ebd commit e216b54
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 27 deletions.
2 changes: 1 addition & 1 deletion CesiumAsync/include/CesiumAsync/SharedAsset.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ template <typename TAssetType, typename TAssetKey> class SharedAssetDepot;
* 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.
* An independent asset's {@link getDepot} returns nullptr.
*
* **Active Depot Asset**
* This is an asset that is owned by an asset depot and that is in use, meaning
Expand Down
25 changes: 11 additions & 14 deletions CesiumAsync/include/CesiumAsync/SharedAssetDepot.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,18 +103,6 @@ class CESIUMASYNC_API SharedAssetDepot
const TAssetKey& assetKey) {
// We need to take care here to avoid two assets starting to load before the
// first asset has added an entry and set its maybePendingAsset field.

// Calling the factory function while holding the mutex unnecessarily
// limits parallelism. It can even lead to a bug in the scenario where the
// `thenInWorkerThread` continuation is invoked immediately in the current
// thread, before `thenInWorkerThread` itself returns. That would result
// in an attempt to lock the mutex recursively, which is not allowed.

// So we jump through some hoops here to publish "this thread is working
// on it", then unlock the mutex, and _then_ actually call the factory
// function.
Promise<void> promise = asyncSystem.createPromise<void>();

std::unique_lock lock(this->_mutex);

auto existingIt = this->_assets.find(assetKey);
Expand All @@ -131,6 +119,17 @@ class CESIUMASYNC_API SharedAssetDepot
}
}

// Calling the factory function while holding the mutex unnecessarily
// limits parallelism. It can even lead to a bug in the scenario where the
// `thenInWorkerThread` continuation is invoked immediately in the current
// thread, before `thenInWorkerThread` itself returns. That would result
// in an attempt to lock the mutex recursively, which is not allowed.

// So we jump through some hoops here to publish "this thread is working
// on it", then unlock the mutex, and _then_ actually call the factory
// function.
Promise<void> promise = asyncSystem.createPromise<void>();

// We haven't loaded or started to load this asset yet.
// Let's do that now.
CesiumUtility::IntrusivePointer<SharedAssetDepot<TAssetType, TAssetKey>>
Expand Down Expand Up @@ -440,8 +439,6 @@ class CESIUMASYNC_API SharedAssetDepot
// it are dropped.
CesiumUtility::IntrusivePointer<SharedAssetDepot<TAssetType, TAssetKey>>
_pKeepAlive;

friend class SharedAsset<TAssetType>;
};

} // namespace CesiumAsync
8 changes: 0 additions & 8 deletions CesiumGltf/include/CesiumGltf/FeatureIdTextureView.h
Original file line number Diff line number Diff line change
@@ -1,17 +1,9 @@
#pragma once

#include "CesiumGltf/FeatureIdTexture.h"
#include "CesiumGltf/Image.h"
#include "CesiumGltf/ImageAsset.h"
#include "CesiumGltf/KhrTextureTransform.h"
#include "CesiumGltf/Texture.h"
#include "CesiumGltf/TextureView.h"

#include <algorithm>
#include <cmath>
#include <cstddef>
#include <cstdint>
#include <optional>

namespace CesiumGltf {

Expand Down
4 changes: 2 additions & 2 deletions CesiumGltf/include/CesiumGltf/ImageAsset.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ struct CESIUMGLTF_API ImageAssetMipPosition {
};

/**
* @brief Holds {@link Image} properties that are specific to the glTF loader
* rather than part of the glTF spec.
* @brief A 2D image asset, including its pixel data. The image may have
* mipmaps, and it may be encoded in a GPU compression format.
*/
struct CESIUMGLTF_API ImageAsset final
: public CesiumAsync::SharedAsset<ImageAsset> {
Expand Down
2 changes: 1 addition & 1 deletion CesiumGltf/src/PropertyTextureView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ PropertyTextureView::checkImage(const int32_t imageIndex) const noexcept {
const CesiumUtility::IntrusivePointer<ImageAsset>& pImage =
_pModel->images[static_cast<size_t>(imageIndex)].pAsset;

if (pImage->width < 1 || pImage->height < 1) {
if (!pImage || pImage->width < 1 || pImage->height < 1) {
return PropertyTexturePropertyViewStatus::ErrorEmptyImage;
}

Expand Down
2 changes: 1 addition & 1 deletion CesiumGltf/src/TextureView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ TextureView::TextureView(
}

this->_pImage = model.images[static_cast<size_t>(texture.source)].pAsset;
if (this->_pImage->width < 1 || this->_pImage->height < 1) {
if (!this->_pImage || this->_pImage->width < 1 || this->_pImage->height < 1) {
this->_textureViewStatus = TextureViewStatus::ErrorEmptyImage;
return;
}
Expand Down

0 comments on commit e216b54

Please sign in to comment.