Now, the needsUpdate method only compares geometry.attributes, if i change geometry.index when using VAO, it may cause bugs.
Add geometry.index cache for VAO will help:
````
function needsUpdate( geometry ) {
const cachedAttributes = currentState.attributes;
const geometryAttributes = geometry.attributes;
if ( Object.keys( cachedAttributes ).length !== Object.keys( geometryAttributes ).length ) return true;
for ( const key in geometryAttributes ) {
const cachedAttribute = cachedAttributes[ key ];
const geometryAttribute = geometryAttributes[ key ];
if ( cachedAttribute.attribute !== geometryAttribute ) return true;
if ( cachedAttribute.data !== geometryAttribute.data ) return true;
}
if ( currentState.index !== geometry.index ) { // Fix!!
return true;
}
return false;
}
function saveCache( geometry ) {
const cache = {};
const attributes = geometry.attributes;
for ( const key in attributes ) {
const attribute = attributes[ key ];
const data = {};
data.attribute = attribute;
if ( attribute.data ) {
data.data = attribute.data;
}
cache[ key ] = data;
}
currentState.attributes = cache;
currentState.index = geometry.index; // Fix!!
}
````
Sorry, I'm not able to reproduce. Below there are two fiddles, one modifying an existing index and the other one creating a new one. Both index modifications flip the second face.
Modify existing index: https://jsfiddle.net/g5yotb98/
Add new index: https://jsfiddle.net/g5yotb98/1/
Please update one of the fiddles to demonstrate the problem.
https://jsfiddle.net/shawn0326/y9zskg4m/2/
Your example works well only because the new index happens to be bound after the VAO is bound. I modified the example so that the second VAO skips the expected index binding.
I'd like to better understand why this happens. If I reuse the same material, everything works fine:
Um, it probably works since in my modified example only a single VAO exists (because there is only a single program).
Okay, it seems you are right but a second review of @takahirox would be great!
Thanks for the report! Let me look into...
Yes, this is actually a bug. Good catch and sorry for the bug!
Probably, as you suggested, we need to cache index and check it.
@shawn0326
Would you like to make PR to fix? Or do you want me to make?
@Mugen87
If I reuse the same material, everything works fine
it probably works since in my modified example only a single VAO exists (because there is only a single program).
Correct, if you reuse the same material in the example only a single VAO exists so you don't see the problem.
Ok, I'd like to make PR.