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

Shared identical external textures from GLB files are not accounted for in resource statistics #11897

Open
javagl opened this issue Mar 25, 2024 · 6 comments

Comments

@javagl
Copy link
Contributor

javagl commented Mar 25, 2024

EDIT: This issue originally claimed that external textures that are referred to from multiple GLB files are not shared internally. A pragmatic logging statement in the GltfImageLoader class suggests that the textures are shared, and actually only loaded once. But loading such an asset still causes undesired behavior, with the details updated below accordingly.


The following archive contains test data for this, namely a tileset and a simple Sandcastle for loading it:

ExternalTextureSharing-2024-03-25.zip

The tileset consists of a simple tileset.json that refers to 100 GLB files. These GLB files each contain trivial geometry (a unit square, two triangles). All these 100 GLB files refer to only two external textures (that are stored outside of the GLB, and referred to, from the GLBs, via their URI). These are PNG files with 2048x2048 pixels.

So the whole tileset cotains two 2k-textures and a few bytes of geometry data, but when more tiles are loaded, it bails out with the message

"The tiles needed to meet maximumScreenSpaceError would use more memory than allocated for this tileset."

and starts omitting some tiles:

Cesium External Texture Sharing

After a short investigation, it turns out that the external textures indeed are loaded only once. But the mechanism for computing the memory usage does not take this into account. The ModelStatistics are accumulating the data for each model, and reporting the total size as the totalMemoryUsageBytes, which eventually triggers the message about the exceeded memory.

@kring
Copy link
Member

kring commented Mar 26, 2024

CC CesiumGS/cesium-native#497
That issue actually makes it sounds like CesiumJS handled this at one time?

@javagl
Copy link
Contributor Author

javagl commented Mar 26, 2024

@kring Others may have to confirm this, but I also thought that this was supposed to be implemented. I tested it, inspired by the forum post at https://community.cesium.com/t/how-does-cesiumjs-manage-textures-shared-across-tiles/30404/2 , and it looks like it is not working as expected.

But note the NOTE that I edited in: It might be that CesiumJS is actually doing the texture sharing, and it is only the "memory tracking" functionality that does not properly take this into account, and assumes that the tiles need more memory than they actually need. (I think there's no really easy way to confirm that - e.g. something like the "Windows Task Manager - GPU memory" overview might not really be accurate enough here...)

@timoore
Copy link

timoore commented Mar 26, 2024

But note the NOTE that I edited in: It might be that CesiumJS is actually doing the texture sharing, and it is only the "memory tracking" functionality that does not properly take this into account, and assumes that the tiles need more memory than they actually need. (I think there's no really easy way to confirm that - e.g. something like the "Windows Task Manager - GPU memory" overview might not really be accurate enough here...)

You can run renderdoc with chrome and see the allocated textures; see e.g. https://edw.is/renderdoc-webgl/ . I don't know if there are more integrated tools than that.

@javagl javagl changed the title Identical external textures from GLB files are not shared Shared identical external textures from GLB files are not accounted for in resource statistics Apr 1, 2024
@javagl
Copy link
Contributor Author

javagl commented Apr 1, 2024

An update: It now looks like the textures are shared, but only the mechanism for computing the "total memory usage" is flawed by not taking this into account, and causes tiles (unnecessarily) to be omitted. The issue text has been updated accordingly.

@javagl
Copy link
Contributor Author

javagl commented Apr 12, 2024

I had a short look at how this could be addressed.


Some context - people might want to skip this if they already have a mental model of all that...:

  • The computations are based on the Cesium3DTileContent.texturesByteLength value
  • The specific one relevant here is the Model3DTileContent
  • This, in turn, obtains the value from the ModelStatistics, which keeps track of the texture size in its addTexture function
    • Note: This one already does check for duplicates, based on the texture._id!
  • The ModelStatistics are updated as part of the PrimitiveStatisticsPipelineStage, with the loaded textures.
  • Based on the resulting values, the Cesium3DTilesetStatistics.texturesByteLength will be updated when tiles are loaded/unloaded

Now, there are two parts to this problem.

The (maybe) simpler one is identifying identical textures.

Each Texture object already has an _id field. This is just initialized with a GUID when the texture is created. But there already is a cacheKey in GltfTextureLoader that could be used for identifying identical textures. This will be a string like
texture:http://localhost:8003/texture1.png-sampler-10497-10497-9729-9729-context-a4d9a28d-3340-4e39-8cbf-76f8423e1643 that could be forwarded to the Texture, to be used as its ID (as part of the CreateTextureJob.


The more difficult question is: How to handle these IDs and the size, globally for the whole tileset?

The information about the textures and their sizes has to be stored and maintained on the level of the tileset. This would probably reside in the Cesium3DTilesetStatistics. And it would probably boil down to some mapping from the texture._id of the loaded textures to their size.

This mapping has to be updated when tiles are loaded/unloaded. And right now, the update of the statistics is solely happening based on the information that is provided by a Cesium3DTileContent. So one could naively say that this interface also has to maintain such a texture._id -> size mapping. But this would be a considerable change in that interface (that is implemented by several classes). (EDIT: In the tileset statistics, this mapping would have to be maintained with some sort of reference counter, of course...). One could probably think about other options, but I don't have a really great idea how one could "nicely" transport this information from the Model3DTileContent to the tileset level. (And with 'nicely', I mean without breaking the interface and/or doing some instanceof Model3DTileContent checks....)

I'll think about some options, and maybe try out a few things, but maybe someone already has a strong preference for one approach here.

@javagl
Copy link
Contributor Author

javagl commented Aug 22, 2024

This came up in the forum, as the underlying reason for https://community.cesium.com/t/implicit-tiling-inconsistency-with-vs-without-textures/34640

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants