Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More tweaks to the shared asset system #965

Merged
merged 10 commits into from
Oct 10, 2024

Conversation

kring
Copy link
Member

@kring kring commented Oct 10, 2024

This is a PR into #964.

  • Moved SharedAsset and SharedAssetDepot into CesiumAsync.
  • Renamed SharedAssetSystem to GltfSharedAssetSystem and moved it to CesiumGltfReader.
  • Guard against the size of an asset changing in between when it's added to the deletion list and when it's removed, which would disrupt the depot size tracking.
  • Remove SharedAsset::isShareable. Even non-depot assets can be shared, so this method was misleading.
  • Lots of small tweaks to the shared asset system.
  • Make SharedAsset constructor and destructor protected, so instances of
    this type can't be created directly.
  • Change the staleAssetSizeLimit to 16 MiB.
  • Add some defensive assertions.
  • Fix use-after-free when removing deletion candidates.

Still to do:

  • Construct SharedAssetDepot with a Factory, instead of passing one in on getOrFetch call.
  • Rename ImageCesium to ImageAsset.
  • Put asset state documentation from Fix asset lifetime, avoid circular reference counting. #964 somewhere in the code.
  • Convert non-Doxygen comments at the top of SharedAsset to Doxygen.
  • Kind of vague, but I'm not happy with GltfSharedAssetSystem just yet.

* 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.
@azrogers azrogers merged commit a230812 into shared-assets-lifetime Oct 10, 2024
24 checks passed
@azrogers azrogers deleted the shared-assets-tweaks branch October 10, 2024 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants