Skip to content

Add flat shading support - #9191

Merged
mvaligursky merged 2 commits into
mainfrom
mv-flat-shading
Aug 18, 2026
Merged

Add flat shading support#9191
mvaligursky merged 2 commits into
mainfrom
mv-flat-shading

Conversation

@mvaligursky

Copy link
Copy Markdown
Contributor

Adds flat shading, resolving #239. When enabled, each fragment is shaded using the geometric normal of its triangle instead of the normal interpolated from the vertex normals, giving the surface a faceted look.

The normal is taken from the screen space derivatives of the world position. This is exact rather than an approximation - both derivatives lie in the plane of the triangle, so their cross product is the normal of that plane - and it works on skinned and morphed geometry at no additional cost, as it operates on the final world space positions. It also does not need the mesh to supply vertex normals at all.

The returned normal is oriented to match the triangle winding, so it agrees with correctly authored vertex normals on both front and back faces. Flat shading therefore only changes the faceting: cull, frontFace and twoSidedLighting all behave as they do for smooth shading.

Changes:

  • New flatNormalPS chunk (GLSL and WGSL) providing getFlatNormal(worldPos)
  • Material#flatShading, backed by a FLAT_SHADING define. As all material types resolve their defines through ShaderUtils.getCoreDefines, this needs no lit option plumbing, and it reaches StandardMaterial, LitMaterial and ShaderMaterial alike
  • The tbnBasis uniform declaration in TBN.js is now behind an #ifndef guard so it can be shared with the new chunk
  • Two independent sign corrections that do not double count: tbnBasis compensates for the flipY projection flip and WebGPU's Y-down framebuffer space, then gl_FrontFacing / pcFrontFacing restores the winding orientation

API Changes:

  • Added Material#flatShading (boolean, defaults to false). As with other material properties, call Material#update() after changing it
  • Added flatShading to the standard material asset parameters
  • Added the flatNormalPS shader chunk, usable from ShaderMaterial shaders via #include "flatNormalPS" together with #ifdef FLAT_SHADING

Examples:

  • graphics/asset-viewer - flat shading toggle, exercising it across transmission, volume, IOR, iridescence, sheen and clearcoat materials
  • graphics/lights - flat shading toggle on the statue and ground, showing that shadows and light cookies remain correct
  • shaders/shader-hatch - flat shading toggle demonstrating the ShaderMaterial route. Its lighting moved from the vertex to the fragment shader, which derivatives require, so it is now evaluated per pixel

Adds Material#flatShading, which shades each fragment using the geometric
normal of its triangle instead of the normal interpolated from the vertex
normals, giving the surface a faceted look.

It is implemented from the screen space derivatives of the world position in
a new flatNormalPS chunk, and so it is exact, works on skinned and morphed
geometry at no additional cost, and does not need the mesh to supply vertex
normals. The normal is oriented to match the triangle winding, keeping flat
shading orthogonal to cull, frontFace and two sided lighting.

The property is backed by a FLAT_SHADING define, which ShaderMaterial shaders
can handle themselves using the same chunk.
@github-actions

Copy link
Copy Markdown

Public API report

This PR changes the public API surface (+6 / −0), per the docs' rules (@ignore / @Private / undocumented are excluded).

Show API diff
+Material.get flatShading(): boolean
+Material.set flatShading(value: boolean)
+ShaderMaterial.get flatShading(): boolean
+ShaderMaterial.set flatShading(value: boolean)
+StandardMaterial.get flatShading(): boolean
+StandardMaterial.set flatShading(value: boolean)

Informational only — this never fails the build.

@github-actions

Copy link
Copy Markdown

Build size report

This PR changes the size of the minified bundles.

Bundle Minified Gzip Brotli
playcanvas.min.js 2369.3 KB (+1.2 KB, +0.05%) 608.6 KB (+0.4 KB, +0.06%) 472.4 KB (+0.0 KB, +0.00%)
playcanvas.min.mjs 2366.7 KB (+1.2 KB, +0.05%) 607.5 KB (+0.4 KB, +0.06%) 471.9 KB (+0.3 KB, +0.06%)

@mvaligursky mvaligursky left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Automated PR review by Codex (GPT-5).

I found one blocking WebGPU compatibility issue in the advertised normal-free mesh path. Beyond that finding, I reviewed the public API and serialization plumbing, material define propagation/copying, GLSL/WGSL chunk integration, front-face and render-target sign handling, ShaderMaterial usage, pass behavior, and hot-path/variant impact.

Validation performed: all current CI checks are green; the focused StandardMaterial suite passes (22 tests); the StandardMaterial and ShaderMaterial examples compile and render on WebGL2 and WebGPU; and I exercised a positions/indices-only mesh on both backends. The latter exposes the inline issue below.

The lit shader still declares the vertex_normal attribute for flat shaded
variants, as the tangent, GGX and area light paths depend on it, so the
documented guarantee was not delivered. Drop the claim from the property
docs and both chunks, and note the limitation instead.

@mvaligursky mvaligursky left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Automated re-review by Codex (GPT-5) of changes through dfcb1234d1ea8f05bc5cd30fc40411daaf81ab1a.

I reviewed the response and the three-file follow-up diff. The clarification is correct: a lit shader requiring vertex_normal for a normal-free mesh is pre-existing behavior on main, not a regression introduced by flatShading. The narrower fix is clean: the public JSDoc no longer promises support for meshes without normals, and both shader chunks explicitly document the current limitation. The broader vertex-normal dependency refactor is reasonably left to a dedicated, cross-backend change. The previous P1 thread is therefore resolved.

One non-code item remains: please update the PR description before merging. It still says that flat shading “does not need the mesh to supply vertex normals at all,” which now contradicts both the implementation and the updated chunk comments.

No additional code findings. The follow-up diff passes git diff --check, and all current CI checks are green.

@mvaligursky
mvaligursky merged commit b6572e5 into main Aug 18, 2026
10 checks passed
@mvaligursky
mvaligursky deleted the mv-flat-shading branch August 18, 2026 10:24
@willeastcott willeastcott mentioned this pull request Aug 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant