Update Lensflare.js to be able to dispose textures

[gadlol]

Update Lensflare.js so to be able to properly dispose the textures otherwise we have memory issues...

In order to properly dispose the textures added into the Lensflare ( new THREE.Lensflare() ) by the ( new THREE.LensflareElement() ) we must place them into the object. So then we can do for example:

for( var i = scene.children.length - 1; i >= 0; i-- ) {
  for( var j = 0, n = scene.children[i].elements.length; j <n; j++ ){
    scene.children[i].elements[j].texture.dispose();
  }
}

Note: we could probably use the "children" array to store the THREE.LensflareElement instead of "elements".

Note: the Lensflare dispose method does not dispose textures.

[Mugen87]

I would keep elements private and just set elements = []; in Lensflare.dispose() . It's the task of the user to keep track of the lens flare textures and dispose them, if necessary.

[gadlol]

By setting elements = []; the textures do not get removed from WebGLRenderer memory. That is a 100% sure, you can check it in the webGLRenderer.info.memory.textures .

So in the Lensflare.dispose() we could make a loop and use elements[i].texture.dispose()

BUT Actually its not only about disposing textures.

What if you want to change on the fly for example the size of the LensflareElement ?

By having elements private you cannot access LensflareElement properties.

[Mugen87]

By setting elements = []; the textures do not get removed from WebGLRenderer memory.

Yeah, that's clear. I refer to something different. If the user creates textures, then it's also his responsibility to dispose them. Why? Because, the user can apply these textures also to other materials. Lensflare won't know about this.

Lensflare.dispose() should focus on entities, that are actually allocated by Lensflare. Setting elements = []; should just free references.

[mrdoob]

I think this is modification to Lensflare's own dispose function that @Mugen87 is proposing:

this.dispose = function () {

	material1a.dispose();
	material1b.dispose();
	material2.dispose();

	tempMap.dispose();
	occlusionMap.dispose();

	for ( var i = 0; i < elements.length; i ++ ) {

		elements[ i ].texture.dispose();

	}

};

And I agree 👍

[gadlol]

@mrdoob better use : for ( var i = 0, l = elements.length; i < l; i ++ ) { just like in the line 223

[mrdoob]

Indeed!

Would you like to amend the PR?

[gadlol]

Would you like to amend the PR?

I am sorry but I don't understand what you mean by that...

[mrdoob]

Would you like to change this PR with the proposed implementation?

[gadlol]

yes

[mrdoob]

Better to keep the var here.

[gadlol]

yes i did not notice that...

[mrdoob]

Okay, I'll clean it up after merging.

[mrdoob]

Thanks!