-
-
Notifications
You must be signed in to change notification settings - Fork 36.1k
TDSLoader: parse opacity of material #17456
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
some model is transparent, so it should parse MAT_TRANSPARENCY, and convert MAT_TRANSPARENCY to material.oapcity
| material.shininess = shininess; | ||
| this.debugMessage( ' Shininess : ' + shininess ); | ||
|
|
||
| } else if ( next === MAT_TRANSPARENCY ) { |
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.
Did you test this PR?
Where is MAT_TRANSPARENCY defined?
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.
yes, i had tested, first i read code of lib3ds
case CHK_MAT_SHININESS: {
lib3ds_chunk_read_reset(&c, io);
int_percentage_read(&material->shininess, io);
break;
}
case CHK_MAT_SHIN2PCT: {
lib3ds_chunk_read_reset(&c, io);
int_percentage_read(&material->shin_strength, io);
break;
}
case CHK_MAT_TRANSPARENCY: {
lib3ds_chunk_read_reset(&c, io);
int_percentage_read(&material->transparency, io);
break;
}then, There also has MAT_TRANSPARENCY parameter in TDSLoader
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.
MAT_TRANSPARENCY is undefined, so your code will throw an error.
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.
var MAT_TRANSPARENCY = 0xA050;
it is defined in TDSLoader.js at line 965
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.
The line is commented out:
three.js/examples/js/loaders/TDSLoader.js
Line 921 in c52fef3
| // var MAT_TRANSPARENCY = 0xA050; |
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.
oh, the version of my local computer THREE.js is v101, that line didn't commented
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.
Better use the dev branch as base for your changes when submitting PRs.
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.
yes, it is
dev
branch in my github account
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.
You just said that you used r101 as base for your PR. dev is on r109dev...
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.
sorry, maybe i didn't say very clearly, In my github account, i fork your dev version, and at my local computer, i used 101 version.
| var opacity = this.readWord( data ); | ||
| material.opacity = opacity*0.01; | ||
| this.debugMessage( ' Opacity : ' + opacity ); | ||
| material.transparent = opacity<100 ? true : false; |
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.
Formatting
var opacity = this.readWord( data );
material.opacity = opacity * 0.01;
this.debugMessage( ' Opacity : ' + opacity );
material.transparent = opacity < 100 ? true : false;Please remember to use tabs, not spaces.
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.
@wemamawe Still interested in fixing these formatting issues? Besides, the loader is still missing the declaration of MAT_TRANSPARENCY and thus produces a runtime error.
Live demo: https://jsfiddle.net/7ju6Lw28/
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.
@Mugen87 I delete that coment befor var MAT_TRANSPARENCY = 0xA050;
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.
I delete that coment befor var MAT_TRANSPARENCY = 0xA050;
Have you forgotten to commit this change? I can't see the respective line in the diff (besides, the live demo is still broken).
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.
|
@wemamawe here you can see the changes in this PR: https://github.com/mrdoob/three.js/pull/17456/files As you can see, it's still missing the |
|
Resolved style issues and updated module here: #17534 |
|
Thanks! |

some model is transparent, so it should parse MAT_TRANSPARENCY, and convert MAT_TRANSPARENCY to material.oapcity