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

Clarify why the planemesh looks odd #10501

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

The-oldone
Copy link
Contributor

It confused me for a decent amount of time why it said the lighting looked flat, I believe this will clarify it as the mesh isn't a flat plane as currently written. It isn't mentioned anywhere either that this randomness is to make the material seem more grainy.

It confused me for a decent amount of time why it said the lighting looked flat.
@AThousandShips AThousandShips added enhancement area:manual Issues and PRs related to the Manual/Tutorials section of the documentation topic:shaders labels Jan 13, 2025
@AThousandShips AThousandShips requested a review from a team January 13, 2025 14:26
@tetrapod00
Copy link
Contributor

I think the unchanged version is more correct. The problem really is that the Mesh resource being used is a flat plane, and the vertex normal values are those of a flat plane. So "as if it were a flat plane" is correct. When the vertices are offset in the vertex() function, that doesn't change the vertices of the Mesh resource, and it doesn't change the normals of those vertices. The next paragraph explains this already.

Changing it to "perfectly flat material without irregularities" is less correct, and potentially misleading. "Flat" in the context of materials or shaders usually refers to "flat shading" or "flat interpolation" of vertex values.

There may still be some wording or clarity improvements that can be made here. But as is, I don't think this change is good to merge.

Copy link
Contributor

@skyace65 skyace65 left a comment

Choose a reason for hiding this comment

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

I agree with Tetrapod. I don't see these changes as an improvement, and I don't the existing phrasing as incorrect.

@The-oldone
Copy link
Contributor Author

The problem really is that the Mesh resource being used is a flat plane, and the vertex normal values are those of a flat plane.

Does this mean that the lighting will land on the mesh according to the modified vertices, but will get reflected as if the normals were of a flat plane? Sorry I'm new to this

And if that is the case, why does adding a normal map only change the texture of the material? As far as I can see, the texture of the material obtains some bumps all across the material in the documentation. Am I wrong and the lighting actually changes from being that of a flat plane to that of a bumpy plane? And final question, if we have to change the lighting to reflect the light from the changed mesh properly, why are we using two different randomly generated textures? Unless they are of the same seed, they will have a different set of bumps, and so will not produce the same set of normals as that of the first texture.

P.S. I'm on mobile, so I can't select the comment to reply to

@tetrapod00
Copy link
Contributor

Does this mean that the lighting will land on the mesh according to the modified vertices, but will get reflected as if the normals were of a flat plane?

It will have the position of the modified (moved) vertices, but the normals of the flat plane. I think that's what you mean?

if we have to change the lighting to reflect the light from the changed mesh properly, why are we using two different randomly generated textures?

I believe the intention is that the two textures should be using the same settings and seed, with the only difference one is a normal map. If that is the case, then we can use the normal map version of the noise texture as the normals of the mesh, since the two match. If that isn't the case, something is wrong with the tutorial. It might be addressed in #8553? I remember these tutorials do need an update, but it's been a little while since I looked and I don't remember if this is one of the current problems with them.

@The-oldone
Copy link
Contributor Author

The-oldone commented Jan 16, 2025

It will have the position of the modified (moved) vertices, but the normals of the flat plane. I think that's what you mean?

Indeed, that's what I wanted to confirm

I believe the intention is that the two textures should be using the same settings and seed, with the only difference one is a normal map. If that is the case, then we can use the normal map version of the noise texture as the normals of the mesh, since the two match. If that isn't the case, something is wrong with the tutorial.
Very well, if using FastNoiseLite without changing anything produces the exact same result i.e. operates with the exact same seed, the tutorial is good.

I'm not entirely sure the normal map is used to adjust the lighting normals, since using a normal map to adjust the vertices in the shader gives such a drastically different result, but anyways, if you guys feel this is redundant, feel free to close this.

@AThousandShips AThousandShips changed the title Clarified why the planemesh looks odd Clarify why the planemesh looks odd Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:manual Issues and PRs related to the Manual/Tutorials section of the documentation enhancement topic:shaders
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants