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

FBXLoader: Add TransparencyFactor #17279

Merged
merged 2 commits into from
Apr 16, 2020
Merged

FBXLoader: Add TransparencyFactor #17279

merged 2 commits into from
Apr 16, 2020

Conversation

safu9
Copy link
Contributor

@safu9 safu9 commented Aug 19, 2019

I had warning message when loading fbx file,
THREE.FBXLoader: TransparancyFactor map is not supported in three.js, skipping texture.

Since TransparancyFactor seems to be another name of TransparentColor, I added it in FBXLoader.

Related: #11598

@looeee
Copy link
Collaborator

looeee commented Aug 19, 2019

Since TransparancyFactor seems to be another name of TransparentColor

TransparencyFactor maps to opacity, while TransparentColor maps to alphaMap so I don't think this is correct.

There's also an FBX property Opacity which is what the loader currently uses, and TransparencyFactor is ignored. It's possible that's not correct though.

Are you testing this change with any models? If so, can you share them?

@safu9
Copy link
Contributor Author

safu9 commented Aug 26, 2019

Sorry for late reply.
I compared with the same model exported by Blender.

Before:
Screenshot from 2019-08-26 22-04-07
After this PR:
Screenshot from 2019-08-26 22-07-44

But I found some parts are not working correctly ... (z-buffer?)
Screenshot from 2019-08-26 22-08-40

@safu9
Copy link
Contributor Author

safu9 commented Aug 26, 2019

Model file is here. Tree.zip

@mrdoob mrdoob requested a review from looeee August 26, 2019 21:28
@mrdoob mrdoob added this to the rXX milestone Aug 26, 2019
@mrdoob
Copy link
Owner

mrdoob commented Feb 14, 2020

@looeee what do you want to do with this one?

@looeee
Copy link
Collaborator

looeee commented Apr 14, 2020

Sorry it took me so long to check this.

It seems like the easy solution here is to manually set material.alphaTest = 0.5:

2020-04-14 12_44_19-three js webgl - FBX loader

Treating TransparencyFactor as an alphaMap as in this PR is not correct since that makes the leaves transparent. At least in this case, it's presumably meant to be an alpha mask map, which could be handled in the loader like this:

case 'TransparencyFactor':
        parameters.alphaMap = scope.getTexture( textureMap, child.ID );
	parameters.alphaTest = 0.5;
	break;

However, when I do this, the mask is inverted:

2020-04-14 12_57_59-three js webgl - FBX loader

I'm not sure why this is happening, seem to be that

material.alphaMap = texture;
material.alphaTest = 0.5;

... and

material.map = texture;
material.alphaTest = 0.5;

...give inverse results, for the same texture.

@looeee
Copy link
Collaborator

looeee commented Apr 14, 2020

I've found one more model that uses TransparencyFactor in this thread:

facebookincubator/FBX2glTF#158

However, in that case it looks like a standard transparency map. They also note there that they've never encountered a model with this property before. Both of these files are created by a recent Blender version (2.79 and 2.80) so maybe they have started to use it.

I've changed my mind on this - I think treating this the same way as TransparentColor is OK. Users will have to manually set alphaTest when required (as usual).

@looeee
Copy link
Collaborator

looeee commented Apr 15, 2020

@mrdoob this can be merged now.

@mrdoob mrdoob changed the title Add TransparancyFactor in FBXLoader Add TransparencyFactor in FBXLoader Apr 16, 2020
@mrdoob mrdoob changed the title Add TransparencyFactor in FBXLoader FBXLoader: Add TransparencyFactor Apr 16, 2020
@mrdoob mrdoob modified the milestones: rXXX, r116 Apr 16, 2020
@mrdoob mrdoob merged commit 72751af into mrdoob:dev Apr 16, 2020
@mrdoob
Copy link
Owner

mrdoob commented Apr 16, 2020

Thanks!

@safu9
Copy link
Contributor Author

safu9 commented Apr 16, 2020

Thank you!

@safu9 safu9 deleted the dev-fbxloader branch April 16, 2020 11:46
@looeee
Copy link
Collaborator

looeee commented Apr 16, 2020

@safu9 thanks for your patience ^_^

And note that you probably still need to manually set material.alphaTest = 0.5; here.

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.

None yet

3 participants