Memory leaks and texture cache

[ivanpopelyshev]

In this topic users discuss memory leaks and texture cache problems.

I didnt find single "destroy()" in js files that we edit, something is definitely wrong :)

[holywyvern]

This may patch that:

(function () {

function _generalDestroy =  function() {
    for (var x in this) {
      // we should not free bitmaps automatically because the cache loader may mess with us, 
      //we would have to destroy them manually
      if (this[x] && this[x].destroy && (!(this[x] instanceof Bitmap)) ) {
         this[x].destroy();
      }
    }
    if (this.children) {
      this.children.forEach(function (i) {  
          if (i && i.destroy) {
            i.destroy();
          } 
      });
    }
}

Bitmap.prototype.destroy=_generalDestroy;
Sprite.prototype.destroy= _generalDestroy;
TilingSprite.prototype.destroy = _generalDestroy;
Window.prototype.destroy= _generalDestroy;
WindowLayer.prototype.destroy = _generalDestroy;
Tilemap.prototype.destroy =_generalDestroy;
Spriteset_Map.prototype.destroy = _generalDestroy;
Spriteset_Battle.prototype.destroy = _generalDestroy;
Scene_Base.prototype.terminate = _generalDestroy; 

})();

[ivanpopelyshev]

1. All these classes have Pixi.Container as parent, right?
2. The only thing we can patch right now is Scene_Base.destroy -> Spriteset_Map.destroy -> _lowerBitmap._baseTexture.destroy(), _upperBitmap._baseTexture.destroy(). Others are cached , and we dont want to destroy things that next scene will use :)
3. we can re-use tilemaps if screen have the same width/height, which i believe is always true.

[holywyvern]

Anything the next scene uses, are bitmaps and audio files, they never reuse sprites ever. That's the logic of rpg maker. When you make something here, you should always think about plugin and mantaining plugin compatibility.

So we have to actually destroy as much as we could.

[ivanpopelyshev]

They re-use textures from cache:

ImageManager.loadNormalBitmap = function(path, hue) {
    var key = path + ':' + hue;
    if (!this._cache[key]) {
        var bitmap = Bitmap.load(path);
        bitmap.addLoadListener(function() {
            bitmap.rotateHue(hue);
        });
        this._cache[key] = bitmap;
    }
    return this._cache[key];
};

if we destroy one of those, next scene wont show it.

[holywyvern]

yes, but only bitmap textures, That is why

&& (!(this[x] instanceof Bitmap))

Exists.

It's not a big performance lost, after all you don't destroy scenes at 60 fps

[ivanpopelyshev]

Well, that's the main problem: we have to call destroy on bitmaps or re-use them. Everything else is handled by GC.

[holywyvern]

Yes, but they are not memory leaks because they are in the cache.

We can add a Cache.clear method than gets rid of all of them if for some reeason that's a concern for you.

That was in older rm versions so people is familiar with it.

I think this should be fixed, but not now, this is not the scope of the project right now, this is only a pixi v2 > v3 migration and nothing else.

Bugfixes should be made after the migration is done, because degica/kadokawa may fix bugs by themselves anytime, but they won't plan on moving on version3...

[ivanpopelyshev]

I'll add backport branch, so this kind of things (tilemap, memory optimization) will appear in both v3 and v2 versions.

[ivanpopelyshev]

Ok, no more memory leaks with shaderTilemap.

But we still have memory leaks in 1) tinter 2) tilingsprite

[holywyvern]

And Sprite itself right? If we solve the Sprite leak, we should fix the TillingSprite leak too Because Sprite extends TillingSprite.

[ivanpopelyshev]

Most sprites use bitmaps that are cached in ImageManager, that's no leak, that's caching issue

[holywyvern]

Well no, the baseTexture is only cached (that woudn't be a problem) But Pixi sprites implements another texture over the base texture.

[holywyvern]

reading PIXI v3, tiling sprites doesn't make a new texture anymore and are created via shaders (wich actually seems like a better idea than the v2 approach)

[ivanpopelyshev]

Extra texture over baseTexture doesnt matter, GC can handle that, its just extra js object. Yes, I'm reading this thing too. The main problem with stage - I still dont understand when can we destroy it. Single call of destroy() will save it.

My TileRenderer scans all old vertex buffers so if they were used long time ago, it removes them. BTW, you had huge leak/problem in snap(), I fixed that. Please dont create extra WebGLRenderers :)

[ivanpopelyshev]

I've got an idea of new library for pixi.js which can handle all rpgmaker's memory leaks without modification of rpgmaker. May be it'll work for pixiv2 too. Point is, we just unload all textures that were created with new BaseTexture() and werent used last 10 seconds.

[holywyvern]

that means you have to alter the constructor to check if it's used or not ? I don't really know if that may be easy to do... you may want to check if they are not used on the cache too... Or remove them from the cache when that happens

[ivanpopelyshev]

Good boys use BaseTexture.fromCanvas, BaseTexture.fromImage, or pixi loader.

Your approach uses new PIXI.BaseTexture(). That way it doesnt appear in PIXI.utils.BaseTextureCache.

I need to modify WebGLRenderer.updateTexture and destroyTexture, that way if texture has no imageUrl or pixiId, then I assign it temporaryTextureId and push it to temporary cache. Every 10 seconds I scan that cache and remove "old" textures from videomemory.