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

Impossible to render smooth cone mesh properly after #5666 #5891

Closed
HackerFoo opened this issue Sep 6, 2022 · 7 comments · Fixed by #10905
Closed

Impossible to render smooth cone mesh properly after #5666 #5891

HackerFoo opened this issue Sep 6, 2022 · 7 comments · Fixed by #10905
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior

Comments

@HackerFoo
Copy link
Contributor

When rendering a smooth cone mesh, there is no way to assign a normal to the tip vertex of each triangle on the side without creases. Fortunately, you can assign the normal to zero, and the math works out if normals are normalized in the fragment shader.

See here for details: https://stackoverflow.com/questions/15283508/low-polygon-cone-smooth-shading-at-the-tip/37187730#37187730

#5666 normalizes in the vertex shader instead, which breaks this.

With #5666:

Screen Shot 2022-09-05 at 16 53 51

After reverting #5666:

Screen Shot 2022-09-05 at 16 52 47

@HackerFoo HackerFoo added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled A-Rendering Drawing game state to the screen and removed S-Needs-Triage This issue needs to be labelled labels Sep 6, 2022
@HackerFoo
Copy link
Contributor Author

This happens because normalizing zero produces an invalid result.

Normalizing in the vertex shader invalidates all of the triangles but the bottom, but when normalizing in the fragment shader, only the pixel at the tip is invalid; otherwise the normals on the side are properly interpolated before normalization.

@HackerFoo
Copy link
Contributor Author

Also, note that if normals may not be normalized in the fragment shader despite being normalized in the vertex shader, because interpolating normalized vectors does not necessarily result in a normalized vector. Larger angles between interpolated normals will result in smaller magnitude normals, so this won't be apparent with dense or flat meshes.

@alice-i-cecile
Copy link
Member

CC @superdump.

@alice-i-cecile alice-i-cecile added this to the Bevy 0.9 milestone Sep 6, 2022
@superdump
Copy link
Contributor

At first glance, this seems like a hack. I appreciate that it works, but I wonder if there is another way.

I would rather step back and try to tackle the original problem than this solution with a vertex normal of 0,0,0. What is your mesh? A triangle list of triangles arranged to form a cone? If using a normal that points out of the tip, what do you see?

@superdump
Copy link
Contributor

Also, note that if normals may not be normalized in the fragment shader despite being normalized in the vertex shader, because interpolating normalized vectors does not necessarily result in a normalized vector. Larger angles between interpolated normals will result in smaller magnitude normals, so this won't be apparent with dense or flat meshes.

If there is no other way then it could be that when using normal mapping we normalise in the vertex shader as mikktspace specified that this is how it must be done to be an exact inverse of the baking process as it was designed to be cheaper to calculate if possible and so the baked normal maps normalise the vertex normal at the fragment stage, and then when not normal mapping, we normalise in the fragment shader.

@HackerFoo
Copy link
Contributor Author

HackerFoo commented Sep 6, 2022

At first glance, this seems like a hack. I appreciate that it works, but I wonder if there is another way.

It seems like a hack, but it's actually a reasonable solution, explained below.

I would rather step back and try to tackle the original problem than this solution with a vertex normal of 0,0,0. What is your mesh? A triangle list of triangles arranged to form a cone? If using a normal that points out of the tip, what do you see?

A cone is a (tessellated) circular base, surrounded by triangles that all meet at the tip, represented as a triangle list.

The normals for all points but the tip are easy to calculate. There are two obvious candidates for the tip normal:

  1. The average of the base normals - this produces ugly creases, because each triangle flattens as the average normal takes over towards the tip.

Example:
Screen Shot 2022-09-05 at 23 52 37
Notice the base is smooth, with increasingly sharp creases to the tip.

Zero normal tip (correct):
Screen Shot 2022-09-05 at 23 53 06
2. In the direction of the tip - this is completely wrong, shading the cone as if the tip was round. Imagine an infinitely long cone, and the surrounding interpolated normals will be almost perpendicular to their true normal.

Example (looking down the tip):
Screen Shot 2022-09-05 at 21 51 07
Notice that, except for shadows, it looks like a ball.

Zero normal tip (correct):
Screen Shot 2022-09-05 at 21 51 37

What is needed is to blend the normals from the base vertices, but all the way to the tip. This is exactly what a zero tip normal does, because if you look at the three vertices normals as weighted normals, then the tip contributes nothing, as if each point on the triangle surface was projected to the base of the cone before calculating the normal. This the same smooth shading throughout the triangle.

@HackerFoo
Copy link
Contributor Author

Summarized from discussion:

Using a loop somewhere between the tip and the base won't work, because it will break the smooth surface.
Using a loop at the tip (a cylinder with one end shrunk to a point) won't work. This would replace each triangle with a quad, but when the quads are split into triangles, you end up with two triangles for each quad, where one is degenerate (a line, because two of its vertices are at the tip.) The remaining triangles suffer from the same problem as (1) above.

@alice-i-cecile alice-i-cecile removed this from the Bevy 0.9 milestone Oct 23, 2022
github-merge-queue bot pushed a commit that referenced this issue Dec 10, 2023
# Objective

Fixes #5891.

For mikktspace normal maps, normals must be renormalized in vertex
shaders to match the way mikktspace bakes vertex tangents and normal
maps so that the exact inverse process is applied when shading.

However, for invalid normals like `vec3<f32>(0.0, 0.0, 0.0)`, this
normalization causes NaN values, and because it's in the vertex shader,
it affects the entire triangle and causes it to be shaded as black:

![incorrectly shaded
cone](https://github.com/bevyengine/bevy/assets/57632562/3334b3a9-f72a-4a08-853e-8077a346f5c9)

*A cone with a tip that has a vertex normal of [0, 0, 0], causing the
mesh to be shaded as black.*

In some cases, normals of zero are actually *useful*. For example, a
smoothly shaded cone without creases requires the apex vertex normal to
be zero, because there is no singular normal that works correctly, so
the apex shouldn't contribute to the overall shading. Duplicate vertices
for the apex fix some shading issues, but it causes visible creases and
is more expensive. See #5891 and #10298 for more details.

For correctly shaded cones and other similar low-density shapes with
sharp tips, vertex normals of zero can not be normalized in the vertex
shader.

## Solution

Only normalize the vertex normals and tangents in the vertex shader if
the normal isn't [0, 0, 0]. This way, mikktspace normal maps should
still work for everything except the zero normals, and the zero normals
will only be normalized in the fragment shader.

This allows us to render cones correctly:

![smooth cone with some
banding](https://github.com/bevyengine/bevy/assets/57632562/6b36e264-22c6-453b-a6de-c404b314ca1a)

Notice how there is still a weird shadow banding effect in one area. I
noticed that it can be fixed by normalizing
[here](https://github.com/bevyengine/bevy/blob/d2614f2d802d0fb8000821a81553b600cc85f734/crates/bevy_pbr/src/render/pbr_functions.wgsl#L51),
which produces a perfectly smooth cone without duplicate vertices:

![smooth
cone](https://github.com/bevyengine/bevy/assets/57632562/64f9ad5d-b249-4eae-880b-a1e61e07ae73)

I didn't add this change yet, because it seems a bit arbitrary. I can
add it here if that'd be useful or make another PR though.
github-merge-queue bot pushed a commit that referenced this issue May 13, 2024
# Objective

The `Cone` primitive should support meshing.

## Solution

Implement meshing for the `Cone` primitive. The default cone has a
height of 1 and a base radius of 0.5, and is centered at the origin.

An issue with cone meshes is that the tip does not really have a normal
that works, even with duplicated vertices. This PR uses only a single
vertex for the tip, with a normal of zero; this results in an "invalid"
normal that gets ignored by the fragment shader. This seems to be the
only approach we have for perfectly smooth cones. For discussion on the
topic, see #10298 and #5891.

Another thing to note is that the cone uses polar coordinates for the
UVs:

<img
src="https://github.com/bevyengine/bevy/assets/57632562/e101ded9-110a-4ac4-a98d-f1e4d740a24a"
alt="cone" width="400" />

This way, textures are applied as if looking at the cone from above:

<img
src="https://github.com/bevyengine/bevy/assets/57632562/8dea00f1-a283-4bc4-9676-91e8d4adb07a"
alt="texture" width="200" />

<img
src="https://github.com/bevyengine/bevy/assets/57632562/d9d1b5e6-a8ba-4690-b599-904dd85777a1"
alt="cone" width="200" />
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants