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.

Sign up for GitHub

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

Jump to bottom

MMDToonMaterial: Removed unnecessary envMap shader chunks. #25128

Merged
merged 1 commit into from
Jan 4, 2023
Merged

MMDToonMaterial: Removed unnecessary envMap shader chunks. #25128

merged 1 commit into from
Jan 4, 2023

Conversation

WestLangley
Copy link
Collaborator

... as proposed in #25117 (comment).

/ping @takahirox

@WestLangley WestLangley added this to the r148 milestone Dec 13, 2022
@WestLangley
Copy link
Collaborator Author

@takahirox I gave it a shot. :-) This is compatible with the three.js examples, but based on #25117 (comment), I understand MMDLoader needs to be corrected...

@Mugen87 Mugen87 requested a review from takahirox December 14, 2022 12:26
@mrdoob mrdoob modified the milestones: r148, r149 Dec 22, 2022
@WestLangley
Copy link
Collaborator Author

@Mugen87 At this point, I think merging this is a reasonable, low-risk option.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 3, 2023 •
edited
Loading

When reading #25117 (comment), it seems this PR requires further changes.

TBH, since this isn't an urgent topic, I would prefer to let @takahirox review and approve the PR.

@WestLangley
Copy link
Collaborator Author

it seems this PR requires further changes.

Actually, MMDLoader requires changes, which I am not going to try to fix.

This PR just removes shader chunks from MMDToonShader that should not be used.

@takahirox
Copy link
Collaborator

takahirox commented Jan 4, 2023 •
edited
Loading

Sorry, I have been a bit too busy. I will make anothre PR to fix the problem and then review this PR　soon. (But the change looks good already.)

@takahirox takahirox mentioned this pull request Jan 4, 2023
Merged
@takahirox
Copy link
Collaborator

Made a PR #25226

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 4, 2023

Since #25226 is in, this one can be merged, too 🎉 .

@Mugen87 Mugen87 merged commit 1da2870 into mrdoob:dev Jan 4, 2023
@Mugen87 Mugen87 changed the title MMDToonMaterial: removed unnecessary envmap shader chunks MMDToonMaterial: Removed unnecessary envMap shader chunks. Jan 4, 2023
@WestLangley WestLangley deleted the dev-mmd_toon_envmap branch January 4, 2023 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@takahirox takahirox Awaiting requested review from takahirox

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
r149
Development

Successfully merging this pull request may close these issues.
4 participants
@WestLangley @Mugen87 @takahirox @mrdoob