Three.js: WebGLBindingState: Add geometry.index cache for VAO

Created on 7 Aug 2020  路  7Comments  路  Source: mrdoob/three.js

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!!
}

````

Bug Regression

All 7 comments

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:

https://jsfiddle.net/e3sur0cx/

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.

Was this page helpful?
0 / 5 - 0 ratings