Skip to content

Conversation

@sunag
Copy link
Collaborator

@sunag sunag commented Dec 16, 2019

This PR checks if material uniforms together with material properties in case this logic is broken through NodeMaterial. This is a serious integration part of NodeMaterial in the core, until now seems the best approach for this.

This consists of no case conflicts if the properties of natives materials like MeshStandardMaterial use the property color with a Node value instead of Color.

This PR is entirely linked with this approach ( #18162 ):

var material = new THREE.MeshStandardMaterial();

material.color = new Nodes.CheckerNode();

@sunag
Copy link
Collaborator Author

sunag commented Jan 4, 2020

@mrdoob @WestLangley @Mugen87 Some suggestion? I am not sure if I was able to explain clearly the proposal: #18162

@WestLangley
Copy link
Collaborator

I am not sure of the best solution to the issue described here, however if we are going to be making changes, a related issue that we may want to revisit is the following: Each uniform is a object, and it used to have a .type property. That property is no longer present, so the .value property may no longer be needed, either.

diffuse: { value: new Color( 0xeeeeee ) },
opacity: { value: 1.0 },

@sunag
Copy link
Collaborator Author

sunag commented Jan 5, 2020

That property is no longer present, so the .value property may no longer be needed, either.

Interesting... I think that it contains .needsUpdate property in uniform too.

My main issue is the refresh uniforms maybe we can redesign to something like this:

UniformsLib.common = new Uniforms(

	diffuse: { value: new Color( 0xeeeeee ) },
	opacity: { value: 1.0 },

	map: { value: null },
	uvTransform: { value: new Matrix3() },
	uv2Transform: { value: new Matrix3() },

	alphaMap: { value: null }

).onRefresh( function( uniforms, material ) { 

	uniforms.diffuse.value.copy( material.color );
	uniforms.opacity.value = material.opacity;

	uniforms.map.value = material.map;
...

} );

Thus assigns the responsibility from refresh uniforms to the uniforms class, thus making WebGLRenderer more independent.

@WestLangley
Copy link
Collaborator

Yep, that's what you suggested in #18068 (comment).

This is a serious integration part of NodeMaterial in the core

Understood. You are going to have to get buy-in from @mrdoob. He seems to have strong feelings on the best way to make WebGLRenderer more material-agnostic -- if at all.

@sunag
Copy link
Collaborator Author

sunag commented Mar 29, 2021

I think that we will no longer have this problem since that the new node material work with Node name in final of the property like material.colorNode.

@sunag sunag closed this Mar 29, 2021
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.

2 participants