jnsmalm / pixi3d

The 3D renderer for PixiJS. Seamless integration with 2D applications.
https://pixi3d.org
MIT License
752 stars 43 forks source link

Memory not cleared when destroying models #178

Closed dougcharnley closed 1 year ago

dougcharnley commented 1 year ago

Hi,

We're currently building an HTML5 web-game for Disney PIXAR using your excellent 3D renderer, but are running into a problem.

As the player progresses through the levels the game will crash on an iOS device (browser page refresh) due to excess memory being used.

We've traced this to the 3D GLTF models not being cleared from memory (specifically the textures) despite following the API and using the following code to remove them:

model.destroy({ children: true, texture: true, baseTexture: true, material: true, geometry: true });

We've created a simple demo to show the issue here: https://codesandbox.io/s/crazy-brook-bk29yq?file=/src/index.js:2129-2269

If you use timelines on the Safari developer panel you can see the memory climbing each time the models are destroyed and reloaded again, simulating what we are seeing in our game.

image

Any help or insight you can provide into what's going on here would be much appreciated, as in its current state we aren't able to ship.

Many thanks,

Doug

jnsmalm commented 1 year ago

Hey! Ok, I'll take a look and see if I can reproduce on my end.

jnsmalm commented 1 year ago

I can reproduce the issue, I have a couple of questions:

  1. Which version of PixiJS are you using?

  2. Are you using embedded textures in your real glTF file like in the example you provided? Not sure how PixiJS caches those. If you are using embedded it might be an idea to try and use separate image files instead and see if it makes a difference. Not sure about this though.

  3. In the example you provided you are creating more models than you are destroying because of the “assetLoader.onComplete.add” you are adding a listener to over and over. Are you sure you are not doing something similar in your real app?

jnsmalm commented 1 year ago

I think I have found the issue and working on a fix, just have to think it through regarding the API.

dougcharnley commented 1 year ago

Hi,

Thanks for looking into this so quickly for us :-)

Looking forward to hearing back about a fix!

Thanks again,

Doug

jnsmalm commented 1 year ago

Released v2.3.2, please let me know if this version fixes the issue.

dougcharnley commented 1 year ago

Excellent thanks for this! It does fix the memory climbing in our small demo so this is a big step forward. We aren't seeing the same improvement in our game but it's far more complicated so it's likely references still hanging about somewhere. Will investigate our code!

Thanks again - much appreciate your speedy efforts!

jnsmalm commented 1 year ago

Hope you can find it and let me know if you need any assistance.

dougcharnley commented 1 year ago

Hi,

Unfortunately, upon further testing, we're finding this fix doesn't actually solve our problem - although it appeared to at first.

When we looked at our demo after updating to 2.3.1 we found that the memory stayed flat when deleting the loaded model and reloading the same model again - which was a big improvement from before where the memory would climb each time we re-loaded the model. However, we also discovered that the memory stayed flat even if we didn't destroy the previously loaded model.

So it would appear the improvement has been in not allocating more memory when loading multiple version of the same model, which is great - but doesn't help our game as it appears the memory is not being released for garbage collection when 'destroy' is called on loaded models.

This demo loads in a 3d model then uses that GLB to create a grid of Models on stage. When you click, it calls destroy on all of them (and doesn't load in anything else). We would expect the memory to go down after this once garbage collection is triggered but it doesn't appear to even after a couple of of minutes. This means when navigating around different sections of our game, the memory builds up until it crashes. Although we are destroying all the models from a given section of the game and loading a new set, the memory isn't released.

https://codesandbox.io/s/jovial-bas-4kbnnx?file=/src/index.js

image

Any help with this would be greatly appreciated!

Many thanks,

Doug

jnsmalm commented 1 year ago

Ok, try to also destroy any resources inside loader:

function destroyResources(name) {
  let gltf = loader["resources"][name].gltf
  for (let texture of [...gltf.textures, ...gltf.images]) {
    texture.destroy(true)
  }
  gltf.textures = []
  gltf.images = []
  gltf.buffers = []
  gltf.descriptor = undefined
  delete loader["resources"]["asset.gltf"]
}

Call it like this in your example:

destroyResources("asset.gltf")

Note that after this the resource needs to be loaded again before creating models with it.

jnsmalm commented 1 year ago

Also, I'm not sure this solution always shows the correct stuff in timeline in Safari, does that timeline also looks inside WebGL memory or only "regular" memory? I would try this kind of function inside your game and see if it improves anything.

One idea could be to create several copies of your glTF file like:

https://peg-s3-bucket.s3.amazonaws.com/pixi3d-test-assets/TextureCubes_1.gltf https://peg-s3-bucket.s3.amazonaws.com/pixi3d-test-assets/TextureCubes_2.gltf https://peg-s3-bucket.s3.amazonaws.com/pixi3d-test-assets/TextureCubes_3.gltf ...

Then load the first one, then destroy it. The load the second one and destroy that ...and so on.

dougcharnley commented 1 year ago

I tried to implement your suggestion but it errors when trying to destroy the images array on the gltf object. If I remove that (but do everything else) it works, and you can console log the gltf and see that its empty. memory still stays as measured by Safari however:

https://codesandbox.io/s/jovial-bas-4kbnnx?file=/src/index.js

Tomorrow I'll see if it increases when destroying and then loading another object like you suggested...

Thanks,

Doug

jnsmalm commented 1 year ago

I made 1000 copies of your file and used the code below. My iPhone 6S (8 year old device) could load, render and destroy all of those assets without crashing.

const { Application, Ticker, Loader, Graphics } = PIXI;
const {
  Model,
  Light,
  LightType,
  LightingEnvironment,
  CameraOrbitControl,
  Color
} = PIXI3D;

let app = new Application({
  backgroundColor: 0xdddddd,
  resizeTo: window,
  antialias: true
});

console.log("testing");

let control = new CameraOrbitControl(app.view);
document.body.appendChild(app.view);

let dirLight = Object.assign(new Light(), {
  type: LightType.directional,
  intensity: 10,
  color: new Color(1, 1, 1)
});
dirLight.rotationQuaternion.setEulerAngles(45, -75, 0);
LightingEnvironment.main.lights.push(dirLight);

const ticker = Ticker.shared;
ticker.autoStart = false;

let square = new Graphics();
square.beginFill(0xff0000, 0.001);
square.drawRect(0, 0, 10000, 10000);

console.log(window.innerWidth, window.innerHeight);

app.stage.addChild(square);

// square.interactive = true;
// square.on(
//   "click",
//   function () {
//     console.log("destroying models");
//     destroyModels(models);
//     // destroyResources("asset.gltf")
//     models = [];

//     // setTimeout(function () {
//     //   console.log("creating models");
//     //   // loadModels();
//     // }, 1000);
//   }.bind(this)
// );

let models = [];

function createModels(_gltf) {
  console.log("creating models");
  for (let i = 0; i < 3; i++) {
    for (let j = 0; j < 3; j++) {
      let model = Model.from(_gltf);
      app.stage.addChild(model);
      model.position.x = 3 * i - 6;
      model.position.y = 3 * j - 6;
      model.position.z = -12;
      models.push(model);
    }
  }
}

function destroyModels() {
  console.log("destroying models");
  for (let i = models.length - 1; i >= 0; i--) {
    models[i].destroy({
      children: true,
      texture: true,
      baseTexture: true,
      material: true,
      geometry: true
    });
  }
  models = []
}

let assetLoader = new Loader();

function loadModels(i) {
  assetLoader.add(
    `/assets/test/TextureCubes_${i}.gltf`
  );
  assetLoader.onComplete.once((loader) => {
    createModels(loader["resources"][`/assets/test/TextureCubes_${i}.gltf`].gltf)
  })
  assetLoader.load();
}

ticker.start();
app.ticker.add(tick);

let i = 0
setInterval(() => {
  loadModels(i);
  console.log(i)
  text.text = i.toString()
  setTimeout(() => {
    destroyModels(models)
    destroyResources(`/assets/test/TextureCubes_${i-1}.gltf`)
  }, 1500)
  i++
}, 3000)

let text = app.stage.addChild(new PIXI.Text(""))

let spin = 180;

function tick() {
  for (let i = 0; i < models.length; i++) {
    if (models[i] == null) continue;
    models[i].rotationQuaternion.setEulerAngles(0, spin, 0);
  }
  spin += 0.1;
}

function destroyResources(name) {
  let gltf = assetLoader["resources"][name].gltf
  for (let texture of [...gltf.textures]) {
    texture.destroy(true)
  }
  gltf.textures = []
  gltf.images = []
  gltf.buffers = []
  gltf.descriptor = undefined
  delete assetLoader["resources"][name]
}
dougcharnley commented 1 year ago

Interesting! so either there's something wrong with our game or theres some mechanism that treats these 1000 files as the same file internally (shares texture).

I'll change the demo to use the actual assets we're using in the game to more closely simulate it and see if still can load that many without it crashing.

Thanks for your continued support on this.

Doug

jnsmalm commented 1 year ago

or theres some mechanism that treats these 1000 files as the same file internally (shares texture).

Probably not sharing textures, If I remove the line texture.destroy(true), the app crashes on iPhone 6S after about 20 iterations.

dougcharnley commented 1 year ago

Yes I think you might be right. Looking at PIXI JS releases it seem as of v7 they have completely re-written their loader class to allow for easier unloading of assets etc but we are using 5.3.0 so it may be we need to go through the loader and call delete on all textures associated with loaded 3d assets, as well as destroying the assets themselves. Seems the loader might holding on to stuff...

We will give this a try and come back to you.

Cheers,

Doug

jnsmalm commented 1 year ago

If you use the destroyResources function I provided it might work.

jnsmalm commented 1 year ago

@dougcharnley Any progress?

dougcharnley commented 1 year ago

Hi,

Some, but we're still having issues. In our case the loader doesn't hold the memory as its created locally and doesn't have any permanent refs (unlike in the demo where it is necessary to clear it out). Instead we are looping through and destroying textures and images in the GLTF object inside our model class. This class is where "PIXI3D.Model.from(gltf)" is called and all data relevant to the actual model is stored for use in the game.

The issue we're having is that when flushing assets between scenes (this causes the destroy to be called on all model assets and triggers the looping though and destroying of textures) we get an error from within PIXI3D. It seems that it only happens on certain model assets and not others, so we are wondering if there is something about the way some 3d assets are structured that causes this bug to trigger when trying to destroy these assets? Anything you can tell us about what exactly is going on in the code with this bug will help point us in the right direction as to its cause. But I do know it doesn't trigger, if I comment out the "this.gltfAssetObject["textures"][i].destroy(true)" line.

Screenshot 2023-05-09 at 09 28 41

Many thanks,

Doug

dougcharnley commented 1 year ago

Apologies, The above error is produced only once I have already hacked around another error! When I restore "core.es.js" back to standard, the error that I see when flushing the 3D assets and looping through and using destroy(true) on the textures is the following:

Screenshot 2023-05-09 at 10 24 52
dougcharnley commented 1 year ago

Ok, we've tracked down the issue to something to do with specific textures used in some of our models. Not sure what the issue was, but by opening the texture and re-exporting the png and then re-exporting the glb, that bug no longer triggers.

However this bug triggers instead, but not when you destroy, it seems to trigger when you go to load the asset again after having used the destroy(true) on an glts object's textures.

image

We can get around this by simply doing this inside core.es.js (which works) but I'd much rather understand whats going here with the binding/unbind stuff and how that relates to us removing and destroying textures.

Screenshot 2023-05-09 at 15 08 26

Thanks,

Doug

jnsmalm commented 1 year ago

I guess it crashes because you are trying to use an already destroyed texture, or you are trying to destroy a texture which has already been destroyed.

When the glTF model file gets loaded, the textures (BaseTexture in PixiJS) are created and stored in the glTFAsset object. This glTFAsset object is stored inside the loader resources.

When a new models is created from this glTFAsset object, it creates local textures (Texture in PixiJS, not BaseTexture) from those base textures inside the glTFAsset.

If you destroy the textures inside the model using destroy(true) you are telling PixiJS to also destroy those base textures inside the glTFAsset. Note that the glTFAsset object itself is not destroyed, only the textures it has.

If you at this point try to create a new model using this same glTFAsset object, it will crash because the textures inside it is gone. Hope this helps.

dougcharnley commented 1 year ago

That makes complete sense, and it does feel like somehow, the loaded object has not been destroyed, and so when going back into a level of the game, instead of reloading the asset afresh it is referencing the previously loaded and deleted one. We are fairly sure we ARE getting rid of everything so that, second time around, we should be dealing with an entirely newly loaded asset but it isn't behaving that way. Below is our destroy function for a model.

this.pixi3DModel is the instance of the model that we are destroying, this.gltfAssetObject is the loaderResource content passed in the the Model class constructor. The loader itself is local and has no references so in theory should be eligible for garbage collection.

Would you say that the below function should be entirely destroying the model and its associated gtlf object or are we missing something?

Thanks

public nuke(deletable:boolean){

    if(deletable == true){

        console.log('nuking model', this.gltfAssetObject)

        this.pixi3DModel.destroy({children:true, texture:true, baseTexture:true, material:true, geometry:true})

        for(let key in this.gltfAssetObject){
            if(key == "images"){
                 for(let i:number = 0; i < this.gltfAssetObject[key].length; i++){
                         PIXI.Texture.removeFromCache(this.gltfAssetObject[key][i]);
                         if(this.gltfAssetObject[key][i].baseTexture){
                             PIXI.BaseTexture.removeFromCache(this.gltfAssetObject[key][i].baseTexture);
                         }

                 }

            }
            if(key == "textures"){
                for(let i:number = 0; i < this.gltfAssetObject[key].length; i++){

                    this.gltfAssetObject[key][i].destroy(true);

                }
             }

        }
        this.gltfAssetObject['textures'] = []
        this.gltfAssetObject['images'] = []
        this.gltfAssetObject['buffers'] = []
        this.gltfAssetObject['descriptor'] = undefined
        delete this.gltfAssetObject
        this.gltfAssetObject = null

        return;
    }
    this.pixi3DModel.destroy({children:true, texture:false, baseTexture:false, material:false, geometry:false})

}
        this.gltfAssetObject['textures'] = []
        this.gltfAssetObject['images'] = []
        this.gltfAssetObject['buffers'] = []
        this.gltfAssetObject['descriptor'] = undefined
        delete this.gltfAssetObject
        this.gltfAssetObject = null

        return;
    }
    this.pixi3DModel.destroy({children:true, texture:false, baseTexture:false, material:false, geometry:false})

}
jnsmalm commented 1 year ago

Yes, it should destroy everything. Please also try to change your loop with this and see if it makes a difference:

for (let image of this.gltfAssetObject.images) {
    image.destroy(true) // True here
}
for (let texture of this.gltfAssetObject.textures) {
    texture.destroy(false) // False here
}
dougcharnley commented 1 year ago

Hi,

can I ask, is there an automatic 'remove duplicates' system going on with regard to textures/images inside of models? It does seem to be the case. We've noticed that if we take 2 different models that both happen to use the same texture for a part of them, load them into the game, then destroy one of them (and remove their texture etc as you describe above) the game will crash and error if we try to add the other model to the stage.

Even though each model has its own textures, it seems that when we load both of them PIXI3D/PIXI is somehow noticing that these 2 models have a texture in common and so is pointing both to that texture, then if we destroy one of the models, and its textures, the other model that remains no longer functions as part of it texture has been removed.

Does that make sense?

Thanks,

Doug

jnsmalm commented 1 year ago

Yes, that will happen. If you are destroying the textures with destroy(true) or destroy({baseTexture: true}) it will destroy the "shared" textures (the shared base textures are inside the glTFAsset object). This is a behaviour inherited from PixiJS and is not Pixi3D specific.

jnsmalm commented 1 year ago

If you want to destroy a model and it's textures (but want to keep the base textures) you should not use destroy(true) or destroy({baseTexture: true}). Then you should simply use destroy().

dougcharnley commented 1 year ago

Yes that makes sense, it seems to be built into PIXI itself. The issues we have is we can see the memory is recovered if we pass true and destroy basetextures as well which is great, but when we re-load the assets again and come back to a scene that uses them, PIXI seems to still think it has the reference already and so we get the error as its trying to use the stuff we destroyed instead of just loading the new stuff and using that. This has to be to do with the PIXITextureCache right? Need to remove all trace to force PIXI to 'start again' and use the freshly loaded assets.

Thanks for all your help by the way. I appreciate that the issues were having is really more of a PIXI thing than a PIXI3D thing at this point.

Cheers,

Doug

jnsmalm commented 1 year ago

but when we re-load the assets again and come back to a scene that uses them, PIXI seems to still think it has the reference already and so we get the error as its trying to use the stuff we destroyed instead of just loading the new stuff and using that.

Does that still happen even when you do the code below?

for (let image of this.gltfAssetObject.images) {
    image.destroy(true) // True here
}
for (let texture of this.gltfAssetObject.textures) {
    texture.destroy(false) // False here
}
jnsmalm commented 1 year ago

Do you also remove the gltf asset from the loader resources?

dougcharnley commented 1 year ago

Hi,

We've found that a big reason for the memory creeping up was in fact the shadows. I guess the system must create internal textures to use for shadows based on the shadowTextureSize property. Can I check with you what correct way to destroy this texture is after leaving a scene and no longer needing it? Will the texture be destroyed if you call destroy on the shadowCastingLight that it's attached to?

Many thanks,

Doug

jnsmalm commented 1 year ago

Ok, let me check the shadows during the day and get back to you.

jnsmalm commented 1 year ago

Just had a quick check in the code. Seems like shadowCastingLight.destroy() should be enough for clearing those internal textures. Let me know if you think there is some memory leak there.

dougcharnley commented 1 year ago

Unfortunately we get this error when using destroy on the shadowCastingLight:

image

We are first disabling shadows on the models that we had applied them too as well:

this.game.app.renderer.plugins.pipeline.disableShadows(this.scenery.pixi3DModel) this.shadowCastingLight.destroy()

Any idea why this would be happening? Is there something else we need to do?

Thanks

jnsmalm commented 1 year ago

You also need to remove the shadow casting light from the shadow renderpass.

pipeline.shadowPass.removeShadowCastingLight(shadowCastingLight)

This is a bit cumbersome I can see, this is because the pipeline.enableShadows function hides some details of how it's actually set up.

dougcharnley commented 1 year ago

Ah yep, that did the trick!

Thannks

jnsmalm commented 1 year ago

Did you manage to resolve the issues?

dougcharnley commented 1 year ago

Yes, thanks for all your help!

jnsmalm commented 1 year ago

Great, please send a link to the final product when ready!