Draft for handling shared textures in statistics #12331
Conversation
|
Thank you for the pull request, @javagl! ✅ We can confirm we have a CLA on file for you. |
|
@javagl the approach here seems reasonable. I briefly looked at the code but will wait until it's ready for a deeper review. I agree that |
|
I did a cleanup pass. And wanted to add some specs. For that, I created a smaller/simpler version of the example that was posted in the issue. And I wanted to insert a test that resembles an existing one. This test should do some basic santity checks for the underlying reference counting...
I'm setting the camera to a fixed, hard-wired position that (in a Sandcastle) definitely shows the tiles....
But in the specs, it simply doesn't load anything (i.e. |
|
However, I pushed the stub of the spec. Maybe someone knows why no tiles are rendered at 32dd865#diff-d583bed57d63fe55e0948b41083f208cf988cca081e41845659480a3ccbfefc8R997 . Otherwise, there will be a lot of trial-and-error here... |
|
The cleanup had a pending commit with a fix for a wrong increment/decrement and a log output that was supposed to be removed. This commit was added now. About the specs... well, that was painful 🤕 It took quite a while until I figured out why the specs ~"just didn't work". Eventually, this was resolved by adding a That said, the tests are passing now. Screenshots from the spec "debug canvas" are here for illustration: It starts with a view where nothing is visible, resulting in 0 bytes for textures and geometry. Then it moves into this view:
This causes one geometry and one texture to be loaded. Then it moves to this view:
This causes four geometries but only two(!) textures to be loaded. Then it moves back to the previous (single-tile) view, trims all loaded tiles, and verifies that it now again has only one geometry and one texture. |
I see this as well, but I also have a KTX2 version of the model where memory usage stays within more reasonable limits and doesn't print the message. So seems like things are working as expected. I also confirmed that statistics and caching are working correctly for tilesets that doesn't have shared texture references. Nothing else to add, the approach looks good to me. |
Addresses #11897
Description
Issue summary
An attempt of a tl;dr of the issue #11897 :
Cesium keeps track of the total size of resources that are currently kept in memory when loading a tileset, including the size of textures that are loaded. When the total memory consumption exceeds a certain threshold, then the "maximum screen space error" is increased, causing some tiles to be unloaded, to free up memory. This is indicated by a console message
These statistics do not properly handle the case where a tileset contains GLB files that refer to the same textures (PNG or JPG) via URIs. In this case, the memory requirements for the textures have been tracked for each GLB that referred to the texture, even though the texture was only loaded once.
Approach
Some options for resolving this have already been mentioned in a comment to the issue. And this PR is a DRAFT that shows how this could be resolved.
("DRAFT means: Don't look at the code yet... it's littered with debug logs...)
The main changes are the following:
Textureobject had a (random) UUID. Now it can receive a custom ID. When the texture is loaded with aGltfTextureLoader, then it receives its "cacheKey" as the ID. This is as unique as the texture loader, and may be a string liketexture:http://localhost:8003/texture1.png-sampler-10497-10497-9729-9729-context-a4d9a28d-3340-4e39-8cbf-76f8423e1643ModelStatisticsoriginally contained a set of all texture IDs of the model. Now, this set is actually a mapping from the texture ID to the byte length of that textureModel3DTileContentoffers two functions: One for obtaining the IDs of all textures that are contained in the model (via its statistics), and one for looking up the byte length of that textureCesium3DTilesetStatisticsexamines theModel3DTileContent, to properly update the global statistics...The last point is the most important one:
The
Cesium3DTilesetStatisticshas a functionupdatePointAndFeatureCounts. When a tile is loaded or unloaded, this function receives aCesium3DTileContent, and updates the global statistics based on the size information from this tile content.Now, this function dedicatedly checks whether the given content is a
Model3DTileContent. If it is, then it examines the texture IDs that are stored in this content. It internally maintains a "reference counter" for each texture ID, and updates its totaltexturesByteLengthaccordingly:texturesByteLengthtexturesByteLengthResults
I inserted some debug logs, and excerpts of the relevant ones are shown here:
For the "ExternalTextureSharing-2024-03-25.zip" example that was attached in the issue:
The old output was this:
It loaded the textures, and at some point, exceeded the memory, printing the warning message, and falling into a ping-pong of loading and unloading tiles.
The new output is this:
It loads the two textures, and ... that's it.
For another tileset, the old output was this:
The same pattern: It loads the textures, prints the message, and then helplessly loads and unloads data...
The new output:
It still does print the message (because the textures just are too large...). But it prints it much later, and after unloading some data, it remains stable.
An example of dumping out the reference counters for the textures in this tileset after an update (shortened IDs here) is
Showing that there are, for example, 16 GLB files that use the
wood_birch_color.jpg, but it is only counted once in the statistics.TODO:
The way of how this information is currently handled in the
Cesium3DTilesetStatisticsis not so nice. What bugs me most is thatif (content instanceof Model3DTileContent)there. But there are basically two options:Cesium3DTileContentinstanceofBoth are not nice. The latter is "less 'not-nice'".
More generally, about the
updatePointAndFeatureCounts: I did insert some JSDoc there. But when it's necessary to write such a long comment, then it's clear that the method should be refactored. I'd do this when wrapping up this PR (together with all the cleanups and comments that have to be done at other places that are changed here)Issue number and link
#11897
Testing plan
Load the example tileset that was posted in the issue, and notice that all tiles are loaded, it does not print the "The tiles ... would use more memory" message.
Do the same with any other tileset that has the structure of many GLBs that refer to the same textures.
Author checklist
CONTRIBUTORS.mdCHANGES.mdwith a short summary of my change