three.js/src/renderers/webgl/WebGLLights.js
Line 319
in
8c6d24b
Related PR: #14568
This line is executed 60 times per second thus produces garbage as seen on Chrome’s Allocation Profiler:
The hash value of WebGLLights should be an object instead of concatanated string:
hash: {
stateID: -1,
directionalLength: -1,
pointLength: -1,
spotLength: -1,
rectAreaLength: -1,
hemiLength: -1,
shadowsLength: -1
}
In the WebGLRenderer.js the values should be compared as:
} else if ( materialProperties.lightsHash.stateID !== lights.state.hash.stateID ||
materialProperties.lightsHash.directionalLength !== lights.state.hash.directionalLength ||
materialProperties.lightsHash.pointLength !== lights.state.hash.pointLength ||
materialProperties.lightsHash.spotLength !== lights.state.hash.spotLength ||
materialProperties.lightsHash.rectAreaLength !== lights.state.hash.rectAreaLength ||
materialProperties.lightsHash.hemiLength !== lights.state.hash.hemiLength ||
materialProperties.lightsHash.shadowsLength !== lights.state.hash.shadowsLength ) {
Since JS assigns objects by reference materialProperties.lightsHash should be updated like this to avoid potenial bugs caused by string to object switch:
materialProperties.lightsHash.stateID = lights.state.hash.stateID;
materialProperties.lightsHash.directionalLength = lights.state.hash.directionalLength;
materialProperties.lightsHash.pointLength = lights.state.hash.pointLength;
materialProperties.lightsHash.spotLength = lights.state.hash.spotLength;
materialProperties.lightsHash.rectAreaLength = lights.state.hash.rectAreaLength;
materialProperties.lightsHash.hemiLength = lights.state.hash.hemiLength;
materialProperties.lightsHash.shadowsLength = lights.state.hash.shadowsLength;
instead of:
materialProperties.lightsHash = lights.state.hash
Any ideas/opinions?
@Usnul
You’re absolutely right abot the hash concept here but this issue is about reducing the GC activity by avoiding creation of new objects in the render loop. In the solution proposed by me the objects are created only once but their properties (which are all integer values, no strings here) are changed inside the loop so it should be quite cheap considering the memory usage. If I understood your proposal correctly
a new String object would be created inside the render loop anyway when we hash couple of integers together (AFAIK you cannot reuse Strings since they are immutable objects). So your proposal is exactly what I suggest avoiding in this issue.
I’m not saying it’s not a good idea or anything, it is just against what this issue proposes that’s all 🙂