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

Fixing incorrect normal map when using triplanar world mapping and mesh rotation #83658

Merged
merged 1 commit into from
Oct 24, 2023

Conversation

RPicster
Copy link
Contributor

Fixes #82311

On the original PRs (#82317) branch I unfortunately did a dumb move and it was faster moving the commit to a new branch. Sorry for the extra work @clayjohn @AThousandShips

This PR includes the feedback that @clayjohn gave on the original PR.

@akien-mga akien-mga added this to the 4.2 milestone Oct 20, 2023
@akien-mga akien-mga requested a review from a team October 20, 2023 07:54
@clayjohn
Copy link
Member

It looks like you missed my comment in the original PR.

For ease of reference, the correct code (for world space) is:

vec3 normal = MODEL_NORMAL_MATRIX * NORMAL;
TANGENT = vec3(0.0,0.0,-1.0) * abs(normal.x);
TANGENT+= vec3(1.0,0.0,0.0) * abs(normal.y);
TANGENT+= vec3(1.0,0.0,0.0) * abs(normal.z);
TANGENT = inverse(MODEL_NORMAL_MATRIX)* normalize(TANGENT);
BINORMAL = vec3(0.0,1.0,0.0) * abs(normal.x);
BINORMAL+= vec3(0.0,0.0,-1.0) * abs(normal.y);
BINORMAL+= vec3(0.0,1.0,0.0) * abs(normal.z);
BINORMAL = inverse(MODEL_NORMAL_MATRIX)* normalize(BINORMAL);

You can't rely on the post-multiplication trick like in this PR as it only works for planes.

@RPicster RPicster force-pushed the triplanar-world-normal-maps branch from 29fdcce to 54464f0 Compare October 23, 2023 10:22
scene/resources/material.cpp Outdated Show resolved Hide resolved
scene/resources/material.cpp Outdated Show resolved Hide resolved
@akien-mga akien-mga changed the title Fixing incorrect normal map when using triplanar world mapping and me… Fixing incorrect normal map when using triplanar world mapping and mesh rotation Oct 23, 2023
@RPicster RPicster force-pushed the triplanar-world-normal-maps branch from 54464f0 to e9fb7e3 Compare October 23, 2023 11:07
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Looks great to me!

scene/resources/material.cpp Outdated Show resolved Hide resolved
@RPicster RPicster force-pushed the triplanar-world-normal-maps branch from e9fb7e3 to 73918b0 Compare October 23, 2023 12:54
Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

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

Can't speak for the functionality but the style looks good!

@RPicster
Copy link
Contributor Author

Can't speak for the functionality but the style looks good!

This is a good point. I didn't even test the latest version.
Don't merge yet, I'll do some testing first 😅

@akien-mga akien-mga merged commit b629049 into godotengine:master Oct 24, 2023
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Don't merge yet, I'll do some testing first 😅

Welp I didn't see this, just two approvals :)
You can mark PRs as drafts to prevent from merging them (and then turn them back to ready to review/merge once done).

@RPicster
Copy link
Contributor Author

Don't merge yet, I'll do some testing first 😅

Welp I didn't see this, just two approvals :) You can mark PRs as drafts to prevent from merging them (and then turn them back to ready to review/merge once done).

Argh 🙈

But today is our lucky day! I just tested everything and it works perfectly ⭐

@RPicster RPicster deleted the triplanar-world-normal-maps branch October 27, 2023 12:48
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.

Triplanar World mapping - incorrect normal direction for normal map
4 participants