Proper freeing of textures and cleaning up the loader

[mobileben]

I have an in-game asset that can be swapped out. I want to do the right thing by loading the new asset on-demand and then be a good citizen and release the older textures.

Code looks something like the following:

// Purge old spritesheet
spritesheet = this._game.loader.resources[oldName].spritesheet;
if (spritesheet) {
    spritesheet.destroy();
}

I notice the side effect is the loader does not realize the sprite sheet has been destroyed. So once I try and re-use the original asset, it causes problems since I check for existence in the loader resources to see if I should reload the asset.

Most APIs I've worked with or built have texture systems which are independent of the loader and also clean up properly. So any query for the texture, once destroyed is consistent.

Has there been any thought of, instead of grabbing stuff from Loader's resources, having a TextureManager or SpritesheetManager (you actually could get away with just one which is what typically do)?

Anyways, what is my best way to handle this? Just set the entry to null?

Also, another question I had was related to PIXI.utils.TextureCache. I've found there are times I actually need to use PIXI.utils.TextureCache to find textures. I know it's not ideal and I think I read one shouldn't use it, but there are times I need to get a sub-texture but do not have the atlas name.

Oh, I tested and it does appear the TextureCache is actually cleaned up. 

[mobileben]

Just wanted to add something else. I tried to explicitly purge the Loader resources via something like

delete this._game.loader.resources[name];

I have found that after this is done, on the next load, the spritesheet property is undefined. I load this asset using this code

 

if (!this._game.loader.resources[bname]) {
    const tname = Layout.LayoutManager.shared.decoratedAssetName('assets/' + bname + '.json');
    this._game.loader.add(bname, tname);
    this._game.loader.load((loader: PIXI.Loader, resources: Partial<Record<string, PIXI.LoaderResource>>) => {
        postLoadHandler();
    });
}

 postLoadHandler tries to access the spritesheet via this code (note that resource is split out as a const right now to make it easier to inspect in the debugger).

const resource = this._game.loader.resources[bname];
let spritesheet = resource.spritesheet;

spritesheet is undefined when I try to reload a spritesheet which was forcibly removed using delete as described above.

 

Edited November 9, 2019 by mobileben

[ivan.popelyshev]

you have to remove image resource too

becase we have a line: "if image resource exists, user has to create spritesheet on his own from json + image"

https://github.com/pixijs/pixi.js/blob/dev/packages/spritesheet/src/SpritesheetLoader.js#L31

I would like to chnge that behaviour in future, need to be more clear about it.

Two ways:

1. remove image too

2. add "_image" manually before json, then you can create & parse spritesheet object after loader deals with json and png separately.

Edited November 9, 2019 by ivan.popelyshev

[mobileben]

Thanks for the reply. This seems to work, however I have found that there is far more razing that needs to be done :(.

Here is the snippet I am using. I tried removing items, but it seems all those items are required, which is a lot. 

// Purge old spritesheet
const oldSheet = this._game.loader.resources[oldName].spritesheet;
if (oldSheet) {
    oldSheet.destroy();
}
// PIXI does not clean up the loader or caches
const oldAtlasName = 'assets/' + oldName + Layout.LayoutManager.shared.decoratedSuffix + '.png';
const oldAtlasKey = oldName + '_image';
delete this._game.loader.resources[oldName];
delete this._game.loader.resources[oldAtlasKey];
PIXI.BaseTexture.removeFromCache(oldAtlasKey);
PIXI.BaseTexture.removeFromCache(oldAtlasName);
PIXI.Texture.removeFromCache(oldAtlasKey);
PIXI.Texture.removeFromCache(oldAtlasName);

I would say there is a bit of code smell here. This also makes me wonder whether or not the OpenGL textures are actually truly released, or if I have simply just removed cache entries and references. The reason I cite this is in the destroy code for Texture and BaseTexture, it should in theory clear the cache.

I am assuming that PIXI.utils.TextureCache points to one of these caches.

Does the above look about correct, and is it fair to assume the texture memory has actually been released? Or did I just simply cover the tracks of a texture that still exists?

It would seem better served to decouple loaders with the actual resulting resources. Those resources could have their own managers (or whatever you want to call them). This way if there are multiple loaders, there is still consistency.

A user does something like

app.loader.resources[key].texture

or

app.loader.resources[key].spritesheet

to get a Texture or Spritesheet, respectively. Thus they are already in effect showing intent. If they did something like

TextureManager.getTexture(key)

Or

SpritesheetManager.getSpritesheet(key)

The intent is still covered. Any explicit destroy is then easier to handle. Since Spritesheets know they are dependent on textures, it can use TextureManager to do proper cleanup, for example.

 

Also one last question. How does one access the specific SpritesheetLoader for an Application? I assume there is one created by default.

This is one weakness I think pixi has. Too much "cleverness" being applied in trying to insulate end users from certain things. But the reality with game dev is at some point of time, when you have a lot of assets, control over loaders and assets is actually quite important. Or perhaps that's just the nature of JS coding :D.

 

[ivan.popelyshev]

actually , destroy() on texture automaticlaly removes it from cache.. but you need to do it on all frames to free TextureCache too.. shit... but it doesnt matter, you will just have a number of invalid textures there with no memory backing it

 

Edited November 9, 2019 by ivan.popelyshev

[ivan.popelyshev]

Yes, we dont have good resource system, i'd like to make a pluign like that in future , but i know that EVERY team of 5 or more people writes their own, usually, so its not my priority.

[mobileben]

Okay. I guess I'll just live with what I have now and cross fingers. There is some witchcraft going on, because I found that certain combos required me to clear the cache in BaseTexture versus Texture. These were all spritesheet cases. But right now I really don't want to crawl through that code to figure out why.

Actually I think this could be cleaned up with some small tweaks.

Essentially making Texture and BaseTexture (and perhaps Spritesheet) supporting getting or even destroying a texture by name versus destroying by object. To make this work right, you would probably want to have the texture hold a reference to the loader that loaded it so it could clean it up. The draw back here being it should actually be a weak reference object, and I do not know if Javascript supports that. There are ways around this, for example, requiring creating new loaders through a factory and then having a unique name assigned to them.

You could then something later like:

const texture = Texture.getTexture(key);

Then later do either

texture.destroy();

Or even

Texture.destroyTexture(key);

This could probably be made so that it would not break any existing implementation. Really the part the makes it harder is the dependency of Loader and resource, which really should probably not exist once a resource is loaded.