Skip to content

Conversation

@fernandojsg
Copy link
Collaborator

Address the issue exposed on #16355
Currently I'm removing the cache on the array of matrices, textures and vectors.
Babylon.js is using a similar approach, they just cache single elements: scalar, vector2, vector3, vector4 and matrix (actually just 4x4).

The only arrays left without caching are the setValue2iv, setValue3iv and setValue4iv as they are shared between single and pure array setters.

I don't have an strong opinion about these, but we could leave the current setValue*iv with cache for single setters and introduce setValue*iva for arrays without cache. Thoughts?

@Mugen87
Copy link
Collaborator

Mugen87 commented May 1, 2019

I don't have an strong opinion about these, but we could leave the current setValueiv with cache for single setters and introduce setValueiva for arrays without cache.

Sounds good. In this way the caching separation is more clear.

@fernandojsg
Copy link
Collaborator Author

fernandojsg commented May 1, 2019

@Mugen87 done! I have also renamed the functions for consistency across singular and pure array, eg:

BEFORE SINGLE BEFORE ARRAY AFTER SINGLE AFTER ARRAY
setValue1f setValue1fv setValueV1f setValueV1fa
setValue2fv setValueV2a setValueV2f setValueV2fa
setValue3fv setValueV3a setValueV3f setValueV3fa
setValue4fv setValueV4a setValueV4f setValueV4fa
------------- ------------- ------------- -------------
setValue2fm setValueM2a setValueM2 setValueM2a
setValue3fm setValueM3a setValueM3 setValueM3a
setValue4fm setValueM4a setValueM4 setValueM4a
------------- ------------- ------------- -------------
setValue1i setValue1iv setValueV1i setValueV1ia
setValue2iv setValue2iv setValueV2i setValueV2ia
setValue3iv setValue4iv setValueV3i setValueV3ia
setValue4iv setValue3iv setValueV4i setValueV4ia

@fernandojsg
Copy link
Collaborator Author

Maybe we could even be more explicit eg: setValueV2fa => setArrayValueV2f or setValueV2fArray

@mrdoob
Copy link
Owner

mrdoob commented May 2, 2019

Maybe we could even be more explicit eg: setValueV2fa => setArrayValueV2f or setValueV2fArray

I vote for setValueV2fArray.

@mrdoob mrdoob added this to the r105 milestone May 2, 2019
@fernandojsg
Copy link
Collaborator Author

Maybe we could even be more explicit eg: setValueV2fa => setArrayValueV2f or setValueV2fArray

I vote for setValueV2fArray.

done

@mrdoob mrdoob merged commit 6381be8 into mrdoob:dev May 22, 2019
@mrdoob
Copy link
Owner

mrdoob commented May 22, 2019

Thanks!

@mrdoob mrdoob changed the title Remove cache on uniform arrays WebGLUniforms: Remove cache on uniform arrays May 22, 2019
@fernandojsg fernandojsg deleted the removecache branch May 22, 2019 06:28
@Mugen87
Copy link
Collaborator

Mugen87 commented May 22, 2019

Um, it seems I'm getting the following error now in webgl_postprocessing.

three.js:16397 Uncaught TypeError: Failed to execute 'uniform1iv' on 'WebGLRenderingContext': No function was found that matched the signature provided.

Some other examples (e.g. webgl_animation_keyframes) are broken with the same error.

@Mugen87
Copy link
Collaborator

Mugen87 commented May 22, 2019

The problem is that uniform1iv needs an array as a parameter. But since the function is used for single values, the mentioned error occurs. So for the above case, using uniform1i solves the issue. Hence setValueV1i should look like so similar to setValueV1f.

function setValueV1i( gl, v ) {

	var cache = this.cache;

	if ( cache[ 0 ] === v ) return;

	gl.uniform1i( this.addr, v );

	cache[ 0 ] = v;

}

@Mugen87
Copy link
Collaborator

Mugen87 commented May 22, 2019

I've merged a quick fix so dev is not broken anymore.

@fernandojsg
Copy link
Collaborator Author

@Mugen87 good catch! sorry about that :)

@Mugen87
Copy link
Collaborator

Mugen87 commented May 22, 2019

NP^^. I've also missed it during testing. Not all examples were affected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants