Cache Source instead of Texture instances in material system#5455
Cache Source instead of Texture instances in material system#5455dmarcos merged 1 commit intoaframevr:masterfrom
Conversation
|
|
||
| // Dispose old texture if present | ||
| if (material[materialName]) { | ||
| material[materialName].dispose(); |
There was a problem hiding this comment.
Before this PR, the texture wouldn't be disposed when switching, effectively 'leaking' them. This did have the benefit of keeping it on the GPU in case the texture would be used later on. For example when a user repeatedly switches between two textures.
With this change, upon disposal the texture will be freed from the GPU (unless it's still being used elsewhere). So when reintroducing the texture in the scene, an upload takes place. The repeatedly switching would then incur a texture upload every time it switches.
The new behaviour is definitely more correct since A-Frame has no other way to track the (intended) lifecycle of a texture. But we might need to provide users with ways to keep textures 'alive'. Do note, however, that it's about the specific "texture source + texture properties". One such way might be to keep materials alive, see #5457
|
This need rebase |
|
@dmarcos Rebased |
|
Thanks. So much good work. |
Description:
Final part of #5449, resolving #5441, #5120, #1372. Instead of caching textures and trying to do reference counting, the underlying Sources are cached instead. Actual reference counting is done by Three.js internally, which allows the same texture source to be shared with multiple textures, even if they differ in
offsetorrepeat(which before would result in uploads).This PR also improves the disposing behaviour, which previously only disposed when the material unregistered itself from the material system, missing any textures that were no longer assigned to that material. This also means that since Texture instances aren't shared, they can be changed by users without affecting other textures, while still using the same underlying image.
Changes proposed:
systems/materialcacheSourceinstances instead of Texturesutils/materialrequest theseSourceinstances and create/update the material'sTextureinstance