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

Remove workaround for normals not matching winding order #12460

Merged
merged 2 commits into from
Jul 17, 2022

Conversation

x2048
Copy link
Contributor

@x2048 x2048 commented Jun 19, 2022

Fixes #12430.

After this, correct shading of entities requires correct or zero normals in the models. There is a related fix for the B3D exporter for Blender: GreenXenith/io_scene_b3d#7.

To do

This PR is Ready for Review.

How to test

Use the test from #12430.

@appgurueu

@SmallJoker SmallJoker added this to the 5.6.0 milestone Jun 20, 2022
@x2048 x2048 requested a review from lhofhansl June 24, 2022 20:32
@SmallJoker
Copy link
Member

Is the cube supposed to look like this? The shading looks correct now, hence I assume that's the bugfix?

PR:
grafik

master:
grafik

@appgurueu
Copy link
Contributor

Is the cube supposed to look like this? The shading looks correct now, hence I assume that's the bugfix?

PR: grafik

That's definitely an improvement. The cube is actually supposed to look like a full cube. Looks like backface culling still determines normals based on winding order... Either way this is low-priority for me as I've just made the winding order of my faces match the normals.

Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

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

Works better than before 👍

@rubenwardy rubenwardy merged commit 7b6c4bf into minetest:master Jul 17, 2022
@x2048 x2048 deleted the normals branch July 17, 2022 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Minetest recalculates normals if they don't match the winding order
5 participants