ggetz commented 1 day ago

Description

As demonstrated in this Sandcastle example, it's possible for the environment map generation to puts a large load on browser resources when many models are added in the same frame, and that can cause the browser to hang and potentially crash.

This fix ensure that the environment map computations are distributed across several frames once they hit an upper limit, preventing the large simultaneous load on browser resources.

Issue number and link

N/A

Testing plan

Try the above Sandcastle at each various option. Even at 10 x 10 grids, frame rate should still be interactive.
Make sure that otherwise, environment maps are being populated, such as in the "glTF PBR Extensions" example

Author checklist

[x] I have submitted a Contributor License Agreement
[x] I have added my name to CONTRIBUTORS.md
[x] I have updated CHANGES.md with a short summary of my change
[ ] I have added or updated unit tests to ensure consistent code coverage
[ ] ~I have updated the inline documentation, and included code examples where relevant~
[x] I have performed a self-review of my code

github-actions[bot] commented 1 day ago

Thank you for the pull request, @ggetz!

:white_check_mark: We can confirm we have a CLA on file for you.

ggetz commented 1 day ago

@jjhembd Are you available to review?

javagl commented 1 day ago

I tried this out, and for the "10 cubed" case, it seems to work at the first glance. prints the usual CesiumWidget.js:50 [Violation] 'requestAnimationFrame' handler took 56ms which is not a big deal. But after printing that ~50 times, it says

...
CesiumWidget.js:50 [Violation] 'requestAnimationFrame' handler took 57ms
CesiumWidget.js:50 [Violation] 'requestAnimationFrame' handler took 51ms
CesiumWidget.js:50 [Violation] 'requestAnimationFrame' handler took 62ms
WebGL: CONTEXT_LOST_WEBGL: loseContext: context lost
An error occurred while rendering.  Rendering has stopped.
DeveloperError: Expected width to be greater than 0, actual value was 0
Error
    at new DeveloperError (http://localhost:8080/Build/CesiumUnminified/index.js:8786:11)
    at Check.typeOf.number.greaterThan (http://localhost:8080/Build/CesiumUnminified/index.js:8865:11)
    at Texture (http://localhost:8080/Build/CesiumUnminified/index.js:36496:31)
    at FramebufferManager.update (http://localhost:8080/Build/CesiumUnminified/index.js:41320:34)
    at GlobeDepth.update (http://localhost:8080/Build/CesiumUnminified/index.js:201636:29)
    at updateAndClearFramebuffers (http://localhost:8080/Build/CesiumUnminified/index.js:221608:21)
    at Scene4.updateAndExecuteCommands (http://localhost:8080/Build/CesiumUnminified/index.js:221242:3)
    at render (http://localhost:8080/Build/CesiumUnminified/index.js:221993:9)
    at tryAndCatchError (http://localhost:8080/Build/CesiumUnminified/index.js:222007:5)
    at Scene4.render (http://localhost:8080/Build/CesiumUnminified/index.js:222062:5)

Maybe someone can confirm or report something else.

ggetz commented 1 day ago

Thanks for checking @javagl! Perhaps we need a different way of determining the maximum number of commands per frame. If you have the time, you could try setting DynamicEnvironmentMapManager._maximumComputeCommandCount on this line to a lower value.

javagl commented 1 day ago

I have to emphasize that I don't even have a vague idea what ~"distributing 'browser resources' over multiple frames" means here (which 'resources' actually have to be 'distributed' in that manner?). But from the symptoms, it looks like this could be a true memory leak that either 1. kicks in in a single frame or 2. (after this PR) kicks in after multiple frames. Specifically, I'll probably have a look at that in the context of https://community.cesium.com/t/procedural-ibl-gpu-memory-leak/36760 and try to see if anything is "leaked" here (regardless of the number of frames that it is distributed over). Given that I'm not familiar with that part of the code, it would only be a short debugstepping-pass, but I'll give it a short try.

ggetz commented 1 day ago

it looks like this could be a true memory leak that either 1. kicks in in a single frame or 2. (after this PR) kicks in after multiple frames. Specifically, I'll probably have a look at that in the context of https://community.cesium.com/t/procedural-ibl-gpu-memory-leak/36760 and try to see if anything is "leaked" here (regardless of the number of frames that it is distributed over)

I agree and I'm looking into that thread as well. I still think the change in this PR is good overall as it avoid running tons and tons of commands all in one frame.

javagl commented 22 hours ago

I added some debug logs, and pragmatically plugged https://github.com/greggman/webgl-memory into the code, to get an idea where the "leak" might be. (That's a pretty useful library, by the way). I used a slightly adjusted sandcastle for these tests (see below), to select more meaningful numbers of objects for that test.

I'll post condensed versions of the console logs here. ^{(Why is it impossible to copy-and-paste this log output? *sigh* - we're evolving, but backwards...)}

The message that starts with "Creating textures..." is a log output that I inserted in updateSpecularMaps, after noticing that this is likely the culprit here (some details below)

One cube

Creating textures, mipmapLevels=10 for 6 faces, total: 60, base size 256x256
CubeMap.TOTAL_TEXTURES 2
info  
  memory: 
    buffer: 724
    drawingbuffer: 10469736
    renderbuffer: 13959648
    texture: 33784562
    total: 58214670
  resources: 
    buffer: 4
    framebuffer: 6
    program: 6
    query: 0
    renderbuffer: 2
    sampler: 0
    shader: 0
    sync: 0
    texture: 18
    transformFeedback: 0
    vertexArray: 2

10 cubes

Creating textures, mipmapLevels=10 for 6 faces, total: 60, base size 256x256
(10 times...)
Creating textures, mipmapLevels=10 for 6 faces, total: 60, base size 256x256
CubeMap.TOTAL_TEXTURES 11
info  
  memory: 
    buffer: 2572
    drawingbuffer: 10469736
    renderbuffer: 13959648
    texture: 188975090
    total: 213407046
  resources: 
    buffer: 26
    framebuffer: 7
    program: 8
    query: 0
    renderbuffer: 2
    sampler: 0
    shader: 0
    sync: 0
    texture: 631
    transformFeedback: 0
    vertexArray: 22

100 cubes

Creating textures, mipmapLevels=10 for 6 faces, total: 60, base size 256x256
(100 times)
Creating textures, mipmapLevels=10 for 6 faces, total: 60, base size 256x256
CubeMap.TOTAL_TEXTURES 101
info
  memory: 
    buffer: 19372
    drawingbuffer: 10469736
    renderbuffer: 13959648
    texture: 1698936050
    total: 1723384806
  resources: 
    buffer: 226
    framebuffer: 7
    program: 8
    query: 0
    renderbuffer: 2
    sampler: 0
    shader: 0
    sync: 0
    texture: 6211
    transformFeedback: 0
    vertexArray: 212

Summary:

The number of 'texture' objects that are allocated is basically numberOfObjects * 60 (!). This is due to the line linked above: There are 10 mipmap levels for each of the 6 sides of the cubemaps. Each of these maps has a top-level size of 256x256.

The total size of the allocated texture memory is

For 1 object: 33784562 (33 MB)
For 10 objects: 188975090 (188 MB)
For 100 objects: 1698936050: (~1.7 Giga(!)bytes)

Conclusion

It just needs that much memory 🤷‍♂️

But from what I've grasped from looking over the code quickly, it looks like the _specularMapTextures are no longer required after that ComputeCommand is executed. I'm not 100% sure about that. But one related thought was that it might be possible to get rid of some of the textures sooner than later. A quick attempt was to insert

      // XXX
      console.log("Destroy specular textures");
      const length = manager._specularMapTextures.length;
      for (let i = 0; i < length; ++i) {
        manager._specularMapTextures[i] =
        manager._specularMapTextures[i] && manager._specularMapTextures[i].destroy();
      }
      // XXX

at https://github.com/CesiumGS/cesium/blob/b17154b1b2388be98d3bd3bc00cb53c91b15a063/packages/engine/Source/Scene/DynamicEnvironmentMapManager.js#L572 (because it looked like this could be a place after which these textures are actually no longer required). This did seem to have some effect, but not solve it completely. Maybe someone can more easily (and confidently) identify (further) points where some textures could be detroyed eagerly, to free up memory.

The sandcastle

const viewer = new Cesium.Viewer("cesiumContainer", {
  globe: false,
  infoBox: false,
  selectionIndicator: false,
  shouldAnimate: true,
  skyBox: false
});

viewer.scene.debugShowFramesPerSecond = true;

function createModel(url, height, amount = 1) {
  viewer.entities.removeAll();
  for (let i = 0; i < amount; i++){ 
      const position = Cesium.Cartesian3.fromDegrees(
        -123.0744619 + i * 0.0001,
        44.0503706,
        height,
      );
      const heading = Cesium.Math.toRadians(135);
      const pitch = 0;
      const roll = 0;
      const hpr = new Cesium.HeadingPitchRoll(heading, pitch, roll);
      const orientation = Cesium.Transforms.headingPitchRollQuaternion(position, hpr);

      const entity = viewer.entities.add({
        name: url,
        position: position,
        orientation: orientation,
        model: {
          uri: url,
         // minimumPixelSize: 128,
          maximumScale: 20000,
        },
      });
      viewer.trackedEntity = entity;
    }
}

const options = [
 {
    text: "Unlit Box - 1",
    onselect: function () {
      createModel("../../SampleData/models/BoxUnlit/BoxUnlit.gltf", 10.0, 1);
    },
  },
  {
    text: "Unlit Box - 10",
    onselect: function () {
      createModel("../../SampleData/models/BoxUnlit/BoxUnlit.gltf", 10.0, 10);
    },
  },
  {
    text: "Unlit Box - 100",
    onselect: function () {
      createModel("../../SampleData/models/BoxUnlit/BoxUnlit.gltf", 10.0, 100);
    },
  },
  {
    text: "Unlit Box - 1000",
    onselect: function () {
      createModel("../../SampleData/models/BoxUnlit/BoxUnlit.gltf", 10.0, 1000);
    },
  },
];

Sandcastle.addToolbarMenu(options);

CesiumGS / cesium

Throttle total resources used for environment maps #12320

Description

Issue number and link

Testing plan

Author checklist

One cube

10 cubes

100 cubes

Summary:

Conclusion