Skip to content

Conversation

david-lively
Copy link
Contributor

@david-lively david-lively commented Aug 14, 2025

The glTF 2.0 specification requires that flat normals are generated when they are missing from the source data.

This PR modifies the CesiumDefaultTilesetShader to optionally generate face normals in the pixel shader. This is more space-efficient than the previous approach of duplicating vertices. The new boolean shader uniform, computeFlatNormals controls this behavior.

This feature will be enabled when all of these conditions are true:

  1. The model does not have vertex normals
  2. Generate Smooth Normals is not enabled
  3. The material is lit, or the material is unlit and Cesium3DTileset.ignoreKhrMaterialsUnlit is true.
int32_t computeFlatNormalsPropertyID =   materialProperties.getComputeFlatNormalsID();
bool computeFlatNormals = !primitiveInfo.hasNormals &&
    !primitiveInfo.isUnlit &&
     primitive.mode != MeshPrimitive::Mode::POINTS;
material.SetFloat(computeFlatNormalsPropertyID, computeFlatNormals);

Note that any custom material that previously relied on CPU-generated flat normals should be updated to use the new feature. In CesiumDefaultTilesetShader, the necessary Shader Graph block is imlemented like so:

image

@david-lively david-lively marked this pull request as ready for review August 14, 2025 20:35
@david-lively david-lively requested a review from kring August 14, 2025 20:35
@david-lively david-lively marked this pull request as draft August 15, 2025 21:10
@david-lively david-lively marked this pull request as ready for review August 20, 2025 19:17
@david-lively david-lively marked this pull request as draft August 20, 2025 19:17
@david-lively david-lively marked this pull request as ready for review August 20, 2025 19:58
@david-lively david-lively requested a review from j9liu August 20, 2025 21:29
Removed an extraneous semicolon.
Copy link
Contributor

@j9liu j9liu left a comment

Choose a reason for hiding this comment

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

Thanks @david-lively! I still have to test this on my own end, but I took a look through the code so I could get feedback to you sooner than later. No major changes, just some cleanup + word tweak suggestions ^^

@j9liu j9liu added this to the September 2025 milestone Aug 22, 2025
@kring
Copy link
Member

kring commented Sep 1, 2025

@david-lively I'm going to move this out of the September release because I think you're still working on it, and it's not critical to the release.

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.

3 participants