Add support for flat shading with indiced meshes #3008

sweepline · 2021-10-22T20:20:20Z

I made this as an alternative for #1808 when for example
making procedually generated terrain.

Objective

Flat shading for meshes with indices, specifically terrain from heightmaps in my use case.

Solution

Extend StandardMaterial to use the partial derivative (dFdx, dFdy) for computing facet normals
in the fragment shader.

Use the partial derivative (dFdx, dFdy) for computing facet normals in the fragment shader. I made this as an alternative for bevyengine#1808 when for example making procedually generated terrain.

alice-i-cecile · 2021-10-22T20:33:12Z

Awesome, thanks for the contribution! Rendering is currently being reworked; can you retarget this to the new renderer and the pipelined-rendering branch?

I'll take a look now though, and vet what I can :)

alice-i-cecile · 2021-10-22T20:34:48Z

crates/bevy_pbr/src/material.rs

@@ -42,6 +42,10 @@ pub struct StandardMaterial {
    #[render_resources(ignore)]
    #[shader_def]
    pub unlit: bool,
+    /// This allows for flat shading even with indiced meshes.


This comment could be a bit clearer: rendering can be intimidating for beginners so it's helpful to be explicit in the docs.

Rhetorically: what is flat shading, and why does it interact with indiced meshes?

alice-i-cecile · 2021-10-22T20:35:59Z

examples/README.md

@@ -107,6 +107,7 @@ Example | File | Description
 `update_gltf_scene` | [`3d/update_gltf_scene.rs`](./3d/update_gltf_scene.rs) | Update a scene from a gltf file, either by spawning the scene as a child of another entity, or by accessing the entities of the scene
 `wireframe` | [`3d/wireframe.rs`](./3d/wireframe.rs) | Showcases wireframe rendering
 `z_sort_debug` | [`3d/z_sort_debug.rs`](./3d/z_sort_debug.rs) | Visualizes camera Z-ordering
+`flat_shading` | [`3d/flat_shading.rs`](./3d/flat_shading.rs) | Simple 3D scene showing flat and normal (smooth) shading


Suggested change

`flat_shading` | [`3d/flat_shading.rs`](./3d/flat_shading.rs) | Simple 3D scene showing flat and normal (smooth) shading

`flat_shading` | [`3d/flat_shading.rs`](./3d/flat_shading.rs) | Simple 3D scene showing flat and smooth (default) shading

alice-i-cecile · 2021-10-22T20:37:32Z

examples/3d/flat_shading.rs

+    mut meshes: ResMut<Assets<Mesh>>,
+    mut materials: ResMut<Assets<StandardMaterial>>,
+) {
+    // flat


I think it would be clearer if you gave the meshes different colors, and then referenced those colors in the code comments.

The Color consts listed here https://docs.rs/bevy/0.5.0/bevy/render/color/enum.Color.html#impl are very helpful for clarity there.

alice-i-cecile · 2021-10-22T20:38:06Z

examples/3d/flat_shading.rs

@@ -0,0 +1,57 @@
+use bevy::prelude::*;


At the top of this example, I'd love to see a quick write-up of what flat vs smooth shading means, and why you might prefer one over the other.

Thanks, I've improved the commenting.

OptimisticPeach

Left a few comments about the method. The derivative method can be a bit iffy and the specific method implemented should probably be left up to the user in my opinion, and this be somewhat of a debug feature like unlit shading.

OptimisticPeach · 2021-10-22T20:41:09Z

crates/bevy_pbr/src/render_graph/pbr_pipeline/pbr.frag

+#    ifdef STANDARDMATERIAL_FLAT_SHADING
+    vec3 N = normalize(cross(dFdy(v_WorldPosition), dFdx(v_WorldPosition)));
+#    endif
+#    ifndef STANDARDMATERIAL_FLAT_SHADING


Couldn't this become an #else?

OptimisticPeach · 2021-10-22T20:42:02Z

crates/bevy_pbr/src/render_graph/pbr_pipeline/pbr.frag

+    vec3 N = normalize(cross(dFdy(v_WorldPosition), dFdx(v_WorldPosition)));
+#    endif
+#    ifndef STANDARDMATERIAL_FLAT_SHADING
+    vec3 N = v_WorldNormal;


This used to be normalize-ed because the interpolations occurs throughout the triangle and produces non-normalized results.

Thanks, that was definitely not intentional to remove.

OptimisticPeach · 2021-10-22T20:51:19Z

crates/bevy_pbr/src/render_graph/pbr_pipeline/pbr.frag

@@ -368,7 +371,12 @@ void main() {

    float roughness = perceptualRoughnessToRoughness(perceptual_roughness);

-    vec3 N = normalize(v_WorldNormal);
+#    ifdef STANDARDMATERIAL_FLAT_SHADING
+    vec3 N = normalize(cross(dFdy(v_WorldPosition), dFdx(v_WorldPosition)));


Because the derivative solution to making flat shading can cause artifacts should the edge of the rasterized mesh not contain the entire group (IIRC, dfdx and dfdy are computed using the neighbouring pixels' values, given that fragments are calculated in 2x2 groups), I'd probably make it clear that this is not foolproof and might not work in all situations.

Some other solutions which are more complex:

Use flat interpolators, however that's only possible in certain special cases of geometry (Only possible when you can assign one vertex to each triangle).

Index using primitive_id or equivalent into a buffer containing precalculated normal values. This is a bit of a pain because it takes a while to set up, however it lets you save on space because then you don't need to store values per-vertex. Although, another thing to note is that it's not incredibly portable since it uses the same hardware as geometry shaders (although it itself is not a geometry shader based approach).

Use geometry shaders (Not a good idea).

Flatten the indices by copying vertices, and then calculating the new normals. This approach isn't great because of memory usage, but is definitely the simplest to explain.

I added a better docstring to the field saying it can cause artifacts, I also added a docstring to the field saying its possible to do the last option by calling Mesh::duplicate_vertices() and Mesh::compute_flat_normals() while not setting this field true. Would that be acceptable?

I believe that the new docstring is good; the other methods I described should really be up to the user.

🤔 I really should write up a blog post somewhere about the various methods to achieve flat shading...

OptimisticPeach · 2021-10-22T20:52:57Z

crates/bevy_pbr/src/render_graph/pbr_pipeline/pbr.vert

    v_WorldNormal = mat3(Model) * Vertex_Normal;
+#endif
    v_Uv = Vertex_Uv;
 #ifdef STANDARDMATERIAL_NORMAL_MAP
    v_WorldTangent = vec4(mat3(Model) * Vertex_Tangent.xyz, Vertex_Tangent.w);


Have you given any thought as to how this interacts with tangents? (Just curious as though I don't know how I'd approach this to be honest).

I did not, I assumed since the tangent uses N and not v_WorldNormal in the fragment shader it would kind of just work

Ok, thinking this through a little harder, the tangent is used in tandem with the normal to create a bitangent. After that, all three are used to create a matrix to transform the sampled normal from the map.

As a user I'd expect one of two things to happen:

The tangent to not vary and instead be calculated using the dFdx method as well (just running dFdx on the position will give you the tangent, and the bitangent is calculated as normal. Don't forget about normalizing).

Flat shading and normal map sampling are mutually exclusive, since you're not really flat shading anymore.

While I would probably feel like the first option would be what I go with, it seems that the second one is more logical.

OptimisticPeach · 2021-10-22T20:55:51Z

Other than the comments I've added above, I'm glad to see someone try to bring flat shading into bevy!

Thanks for your contributions!

mockersf · 2021-10-22T21:02:24Z

there has been quite a few changes on pbr on the rendering rework... for example, this is the new shader replacing the one you changed in this pr: https://github.com/bevyengine/bevy/blob/pipelined-rendering/pipelined/bevy_pbr2/src/render/pbr.wgsl

sweepline · 2021-10-23T10:30:10Z

there has been quite a few changes on pbr on the rendering rework... for example, this is the new shader replacing the one you changed in this pr: https://github.com/bevyengine/bevy/blob/pipelined-rendering/pipelined/bevy_pbr2/src/render/pbr.wgsl

I will port this to the pipelined-rendering branch, should I close this PR then? I assume that branch is close to release?

Fixed not normalizing normal in fragment shader Many docstring and comment improvements

alice-i-cecile · 2021-10-23T15:02:21Z

I will port this to the pipelined-rendering branch, should I close this PR then?

You should be able to just retarget this pr, which would be preferred if it's not too much of a mess so we can preserve the thread.

Very rough guess is that the branch is a week or two off.

sweepline · 2021-10-25T06:56:05Z

I talked to @superdump on discord and I think it was decided that it was not currently feasible to implement this in a nice way on the StandardMaterial on the new renderer branch. So I'm closing this for now, ill probably make a seperate thing for this and post it later.

Add support for flat shading with indiced meshes

69af890

Use the partial derivative (dFdx, dFdy) for computing facet normals in the fragment shader. I made this as an alternative for bevyengine#1808 when for example making procedually generated terrain.

github-actions bot added the S-Needs-Triage This issue needs to be labelled label Oct 22, 2021

alice-i-cecile reviewed Oct 22, 2021

View reviewed changes

alice-i-cecile added the S-Needs-Review label Oct 22, 2021

OptimisticPeach reviewed Oct 22, 2021

View reviewed changes

Fixes and improvments from codereview

01d2f01

Fixed not normalizing normal in fragment shader Many docstring and comment improvements

sweepline force-pushed the indice-based-flat-shading branch from 7c6a80a to 01d2f01 Compare October 23, 2021 10:32

sweepline closed this Oct 25, 2021

sweepline mentioned this pull request Oct 28, 2021

Manually created meshes are oriented wrong on pipelined-rendering #3030

Closed

alice-i-cecile added the hacktoberfest-accepted label Nov 3, 2021

sweepline deleted the indice-based-flat-shading branch February 24, 2022 18:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for flat shading with indiced meshes #3008

Add support for flat shading with indiced meshes #3008

sweepline commented Oct 22, 2021

alice-i-cecile commented Oct 22, 2021

alice-i-cecile Oct 22, 2021

alice-i-cecile Oct 22, 2021

alice-i-cecile Oct 22, 2021

alice-i-cecile Oct 22, 2021

sweepline Oct 23, 2021

OptimisticPeach left a comment

OptimisticPeach Oct 22, 2021

OptimisticPeach Oct 22, 2021

sweepline Oct 23, 2021

OptimisticPeach Oct 22, 2021

sweepline Oct 23, 2021

OptimisticPeach Oct 23, 2021

OptimisticPeach Oct 22, 2021

sweepline Oct 23, 2021

OptimisticPeach Oct 23, 2021

OptimisticPeach commented Oct 22, 2021

mockersf commented Oct 22, 2021

sweepline commented Oct 23, 2021

alice-i-cecile commented Oct 23, 2021 •

edited

Loading

sweepline commented Oct 25, 2021

	`flat_shading` \| [`3d/flat_shading.rs`](./3d/flat_shading.rs) \| Simple 3D scene showing flat and normal (smooth) shading
	`flat_shading` \| [`3d/flat_shading.rs`](./3d/flat_shading.rs) \| Simple 3D scene showing flat and smooth (default) shading

Add support for flat shading with indiced meshes #3008

Add support for flat shading with indiced meshes #3008

Conversation

sweepline commented Oct 22, 2021

Objective

Solution

alice-i-cecile commented Oct 22, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OptimisticPeach left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OptimisticPeach commented Oct 22, 2021

mockersf commented Oct 22, 2021

sweepline commented Oct 23, 2021

alice-i-cecile commented Oct 23, 2021 • edited Loading

sweepline commented Oct 25, 2021

alice-i-cecile commented Oct 23, 2021 •

edited

Loading