Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MaterialX: Add normal, tangent, texcoord, geomcolor, position space support #27456

Merged
merged 1 commit into from
Dec 28, 2023

Conversation

hybridherbst
Copy link
Contributor

Description

Adds support for geometry attributes and (partial) parsing of geometry attribute space as described in the title.

There are two // TODO comments in the code:

  • not sure how to get the texcoord index from the child node
  • note sure if/how nodes should support multiple color attributes; it's not supported in MeshPhysicalMaterial right now.

cc @sunag

This contribution is funded by Needle

@sunag
Copy link
Collaborator

sunag commented Dec 27, 2023

note sure if/how nodes should support multiple color attributes; it's not supported in MeshPhysicalMaterial right now.

It would be compatible in any case, if we proceed it would be interesting to support index as we did in UVNode.

@hybridherbst
Copy link
Contributor Author

Thanks! Could you point me in the right direction for getting the UV index so I can remove the comment?
I think color index can be deferred for later (I can keep the comment).

@sunag
Copy link
Collaborator

sunag commented Dec 27, 2023

Could you point me in the right direction for getting the UV index so I can remove the comment?

I think that getChildByName() can solve this issue.

if ( element === 'texcoord' ) {

	const index = this.getChildByName( 'index' );
	const indexValue = parseInt( index.value );

	node = uv( indexValue );

} 

@sunag
Copy link
Collaborator

sunag commented Dec 27, 2023

I think color index can be deferred for later (I can keep the comment).

It seems positive to me to do this implementation. I added something like.

class VertexColorNode extends AttributeNode {

	constructor( index = 0 ) {

		super( null, 'vec4' );

		this.isVertexColorNode = true;

		this.index = index;

	}

	getAttributeName( /*builder*/ ) {

		const index = this.index;

		return 'color' + ( index > 0 ? index : '' );

	}

	...

}

@hybridherbst
Copy link
Contributor Author

OK, added both. I unfortunately don't have a file at hand to test multi-UV and also none for testing multi-color (seems Blender only exports the first color channel).

…upport

rename to VertexColorNode

move vertex color fallback to VertexColorNode

remove unused Vector4 import

add index support to UV and color nodes

linting

another rename
@hybridherbst hybridherbst force-pushed the feature/materialx-geom-attributes branch from 605a78c to b6b703f Compare December 28, 2023 09:09
@sunag sunag merged commit c337d03 into mrdoob:dev Dec 28, 2023
11 checks passed
@hybridherbst hybridherbst deleted the feature/materialx-geom-attributes branch December 28, 2023 19:06
AdaRoseCannon pushed a commit to AdaRoseCannon/three.js that referenced this pull request Jan 15, 2024
…upport (mrdoob#27456)

rename to VertexColorNode

move vertex color fallback to VertexColorNode

remove unused Vector4 import

add index support to UV and color nodes

linting

another rename
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