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

Update MToon shader to r126 phong shader equivalent #703

Merged
merged 5 commits into from
May 26, 2021
Merged

Conversation

0b5vr
Copy link
Contributor

@0b5vr 0b5vr commented May 10, 2021

will resolve #693

  • Update MToon shader to r126 phong shader equivalent
    • This will resolve the problem that MToon cannot be used with shadows
  • I have tried to make a custom depth material for MToon but I reverted
    • It does not give us benefits for most of cases (it might make models render properly for cases where the model uses cutoff with irregular alpha thresholds)
    • It won't be required in VRM1.0 version anyway

@0b5vr 0b5vr added the bug Something isn't working label May 10, 2021
@0b5vr 0b5vr added this to the v0.6.4 milestone May 10, 2021
@0b5vr 0b5vr requested a review from nyamadan May 10, 2021 09:46
@0b5vr 0b5vr self-assigned this May 10, 2021
@@ -540,6 +549,8 @@ export class MToonMaterial extends THREE.ShaderMaterial {
BLENDMODE_TRANSPARENT:
this._blendMode === MToonMaterialRenderMode.Transparent ||
this._blendMode === MToonMaterialRenderMode.TransparentWithZWrite,
MTOON_USE_UV: useUvInVert || useUvInFrag, // we can't use `USE_UV` , it will be redefined in WebGLProgram.js
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MTOON_USE_UV 使ってる間は USE_UV と混線してほしくないから #undef USE_UV してしまってもいいかなとちょっと考えたけど、three.js含め #undef 使ってないから下手に #undef 使うほうが混乱するかな…。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

そうですね、たぶんこのシェーダ内で USE_UV を使いたくなることはまずないと思うし USE_UV が使われることはあってはならないはずなのですが、別にundefしてまで名前を変えずに運用することもないかなと判断しました。コメントありがとうございます。

Copy link
Contributor

@nyamadan nyamadan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@0b5vr 0b5vr merged commit 7101f75 into dev May 26, 2021
@0b5vr 0b5vr deleted the shader-update branch May 26, 2021 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

THREE.WebGLProgram: shader error.when set renderer.shadowMap.enabled to true
2 participants