-
-
Notifications
You must be signed in to change notification settings - Fork 36.2k
Add support for mayas openPBRsurface shader when using the fbx-loader #32584
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
Conversation
| case 'Maya|baseColor': | ||
| parameters.map = scope.getTexture( textureMap, child.ID ); | ||
| if ( parameters.map !== undefined ) { | ||
| parameters.color = ColorManagement.colorSpaceToWorking( new Color("white") ) // MATERIAL COLOR GETS SET TO BLACK BY MAYA WHEN A COLOR MAP IS APPLIED, SO SETTING IT TO WHITE TO RESOLVE THAT PROBLEM |
Check notice
Code scanning / CodeQL
Semicolon insertion Note
the enclosing function
| case 'Maya|emissionColor': | ||
| parameters.emissiveMap = scope.getTexture( textureMap, child.ID ); | ||
| if ( parameters.emissiveMap !== undefined ) { | ||
| parameters.emissive = ColorManagement.colorSpaceToWorking( new Color("white") ) // MATERIAL COLOR GETS SET TO BLACK BY MAYA WHEN A COLOR MAP IS APPLIED, SO SETTING IT TO WHITE TO RESOLVE THAT PROBLEM |
Check notice
Code scanning / CodeQL
Semicolon insertion Note
the enclosing function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your PR breaks the formatting and linter rules. Please make sure your editor properly supports ESLint so the file is correctly formatted when you save it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @Mugen87 for the feedback about the PR breaks and linter rules. Will look into that.
|
Can you please share at least one FBX asset that can be used to test the new code path? It should be added to the collection of |
| // the transparency handling is implemented based on Blender/Unity's approach: https://github.com/sobotka/blender-addons/blob/7d80f2f97161fc8e353a657b179b9aa1f8e5280b/io_scene_fbx/import_fbx.py#L1444-L1459 | ||
|
|
||
| parameters.opacity = 1 - ( materialNode.TransparencyFactor ? parseFloat( materialNode.TransparencyFactor.value ) : 0 ); | ||
| if(materialNode.TransparencyFactor){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parameters.opacity should always be set which is not true anymore with your new code. I suggest you restore the deleted line and just add
if ( materialNode[ 'Maya|transmissionWeight' ] ) {
parameters.opacity = 1 - materialNode[ 'Maya|transmissionWeight' ].value;
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. I will look into that.
Restore comment.
|
|
||
| if ( blobs[ filename ] !== undefined ) images[ id ] = blobs[ filename ]; | ||
| else images[ id ] = images[ id ].split( '\\' ).pop(); | ||
| else images[ id ] = images[ id ].replace(/^.*[\\/]|[?#].*$/g, ''); // Split on / or ?# as well as \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this change required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding this change. I can see that my comment was a bit unclear as to why this is necassary. When I work in maya, forward slashes has to be used for paths. The loader did only account for backslashes, making the path invalid. The regex (shamefully admitting it being AI-generated) takes both and back and forward slash into account. I also noticed it that it would consider ? and #. Which at first seemed uncessary. But then I thought: "if one for some reason would like to read an image from a web-URL, then query parameters may be included... oh now that I think about it again. Maybe that part was unnecssary. Anyway forward slashes is still causing problems.
|
I'm afraid we can't merge these changes without a FBX asset for testing and verification. |
The fbx loader did not recognize maya's openPBRsurface shader, instead it defaulted to the phong material.
Changed so it gets converted to MeshStandardMaterial instead.
Also when parsing the image url's, paths that included "/" instead of "\" would not be parsed correctly. The parsing now use regex to account for that use case as well.
Found issue mentioning this: #15927