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

Shaders: Clean up #22053

Merged
merged 1 commit into from
Jul 7, 2021
Merged

Shaders: Clean up #22053

merged 1 commit into from
Jul 7, 2021

Conversation

WestLangley
Copy link
Collaborator

@WestLangley WestLangley commented Jun 28, 2021

The shader chunks are now in the same order as in other materials.

@WestLangley WestLangley added this to the r130 milestone Jun 28, 2021
@WestLangley WestLangley changed the title Shaders: Properly support skinning in MeshBasicMaterial vertex shader Shaders: Clean up Jun 28, 2021
@WestLangley
Copy link
Collaborator Author

WestLangley commented Jun 28, 2021

@Mugen87 This is a bit complicated... The primary goal was clean up -- to set a consistent order. I did that.

Afterwards, I had to change the #if statement to get the shader to work properly. This is a red flag that there is a design flaw.

The #define in the normal chunks are missing USE_NORMAL. That macro does not exist yet.

@WestLangley
Copy link
Collaborator Author

This can be merged.

@mrdoob mrdoob modified the milestones: r130, r131 Jun 30, 2021
@WestLangley
Copy link
Collaborator Author

My earlier comment was referring to the fact that the shaders assume normals exist in the geometry. Of course, they don't always.

That is a larger issue, and I do not think it should block this PR.

@mrdoob mrdoob merged commit 6023196 into mrdoob:dev Jul 7, 2021
@mrdoob
Copy link
Owner

mrdoob commented Jul 7, 2021

Thanks!

@WestLangley WestLangley deleted the dev_basic_vert branch July 7, 2021 16:49
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.

3 participants