Skip to content

Commit d13c45d

Browse files
committed
Addressing PR comments
1 parent f0757ca commit d13c45d

7 files changed

Lines changed: 36 additions & 50 deletions

CHANGES.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@
55
##### Additions :tada:
66

77
- Added option to ignore the `KHR_material_unlit` extension to force default lighting on tilesets.
8-
- Moved flat normal generation into `CesiumDefaultTilesetShader` pixel shader. Custom lit materials should expose a boolean property `computeFlatNormals` and generate normals accordingly.
8+
9+
##### Breaking Changes :mega:
10+
11+
- Flat normals are now computed in `CesiumDefaultTilesetShader` to avoid duplicating vertices as tiles are loaded. Existing copies of this shader should incorporate this logic using the boolean `computeFlatNormals` property to maintain a correct appearance.
12+
913

1014
## v1.17.0 - 2025-08-01
1115

Editor/Cesium3DTilesetEditor.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -422,9 +422,9 @@ private void DrawRenderProperties()
422422
"Generate Smooth Normals",
423423
"Whether to generate smooth normals when normals are missing in the glTF." +
424424
"\n\n" +
425-
"This option allows the glTFs to be rendered with smooth normals when the " +
426-
" original glTF is missing normals. If normals are missing and this option " +
427-
"is not enabled, flat normals should be generated in the shader. "
425+
"This option allows glTFs to be rendered with smooth normals when the original glTF " +
426+
"is missing normals. If this option is not enabled and normals are missing, " +
427+
"flat normals will be generated in the shader. "
428428
);
429429
EditorGUILayout.PropertyField(this._generateSmoothNormals, generateSmoothNormalsContent);
430430

Runtime/Cesium3DTileset.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -575,8 +575,7 @@ public Material opaqueMaterial
575575
/// </summary>
576576
/// <remarks>
577577
/// This option allows the glTFs to be rendered with smooth normals instead when the original glTF is missing normals.
578-
/// If normals are missing and this option is not enabled, flat normals should be generated
579-
/// in the shader.
578+
/// If normals are missing and this option is not enabled, flat normals will be generated in the shader.
580579
/// </remarks>
581580
public bool generateSmoothNormals
582581
{

Runtime/Resources/CesiumDefaultTilesetMaterial.mat

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ Material:
100100
- _AlphaCutoffEnable: 1
101101
- _AlphaDstBlend: 0
102102
- _AlphaSrcBlend: 1
103-
- _AlphaToMask: 1
103+
- _AlphaToMask: 0
104104
- _AlphaToMaskInspectorValue: 0
105105
- _BUILTIN_AlphaClip: 1
106106
- _BUILTIN_Blend: 0

Runtime/Resources/CesiumDefaultTilesetShader.shadergraph

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3226,7 +3226,7 @@
32263226
"else"
32273227
],
32283228
"m_Precision": 0,
3229-
"m_PreviewExpanded": false,
3229+
"m_PreviewExpanded": true,
32303230
"m_DismissedVersion": 0,
32313231
"m_PreviewMode": 0,
32323232
"m_CustomColors": {
@@ -3262,7 +3262,7 @@
32623262
"m_Position": {
32633263
"serializedVersion": "2",
32643264
"x": 2038.0,
3265-
"y": -219.0,
3265+
"y": -220.0,
32663266
"width": 132.0,
32673267
"height": 94.0
32683268
}
@@ -5998,10 +5998,10 @@
59985998
"m_Expanded": true,
59995999
"m_Position": {
60006000
"serializedVersion": "2",
6001-
"x": 2202.0,
6002-
"y": -221.0,
6001+
"x": 2199.0,
6002+
"y": -220.0,
60036003
"width": 211.0,
6004-
"height": 157.00001525878907
6004+
"height": 157.0
60056005
}
60066006
},
60076007
"m_Slots": [
@@ -7640,7 +7640,7 @@
76407640
"m_Title": "Compute Flat Normals",
76417641
"m_Position": {
76427642
"x": 1465.0,
7643-
"y": -280.0
7643+
"y": -279.0
76447644
}
76457645
}
76467646

@@ -8227,7 +8227,7 @@
82278227
"m_Title": "glTF PBR Metallic-Roughness",
82288228
"m_Position": {
82298229
"x": -1865.0,
8230-
"y": 1692.0001220703125
8230+
"y": 1692.0
82318231
}
82328232
}
82338233

@@ -9591,8 +9591,8 @@
95919591
"m_Expanded": true,
95929592
"m_Position": {
95939593
"serializedVersion": "2",
9594-
"x": 1885.0,
9595-
"y": -219.0,
9594+
"x": 1882.0,
9595+
"y": -220.0,
95969596
"width": 130.0,
95979597
"height": 118.0
95989598
}
@@ -10121,10 +10121,10 @@
1012110121
"m_Expanded": true,
1012210122
"m_Position": {
1012310123
"serializedVersion": "2",
10124-
"x": -1539.60009765625,
10125-
"y": 1179.2000732421875,
10124+
"x": -1540.0,
10125+
"y": 1179.0,
1012610126
"width": 356.0,
10127-
"height": 95.2000732421875
10127+
"height": 95.0
1012810128
}
1012910129
},
1013010130
"m_Slots": [
@@ -10612,7 +10612,7 @@
1061210612
"m_Title": "glTF Emissive Texture",
1061310613
"m_Position": {
1061410614
"x": -1865.0,
10615-
"y": 2351.999755859375
10615+
"y": 2352.0
1061610616
}
1061710617
}
1061810618

@@ -10729,9 +10729,9 @@
1072910729
"m_Position": {
1073010730
"serializedVersion": "2",
1073110731
"x": 2079.0,
10732-
"y": -13.99997615814209,
10732+
"y": -13.999979019165039,
1073310733
"width": 186.0,
10734-
"height": 33.999969482421878
10734+
"height": 33.99996566772461
1073510735
}
1073610736
},
1073710737
"m_Slots": [
@@ -14371,7 +14371,7 @@
1437114371
"m_Title": "glTF Normal Texture",
1437214372
"m_Position": {
1437314373
"x": -1868.0,
14374-
"y": 947.0001220703125
14374+
"y": 947.0
1437514375
}
1437614376
}
1437714377

native~/Runtime/src/UnityPrepareRendererResources.cpp

Lines changed: 8 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,6 @@ template <typename TIndex> struct CopyVertexColors {
112112
uint8_t* pWritePos;
113113
size_t stride;
114114
size_t vertexCount;
115-
bool duplicateVertices;
116115
TIndex* indices;
117116

118117
struct Color32 {
@@ -130,28 +129,14 @@ template <typename TIndex> struct CopyVertexColors {
130129
}
131130

132131
bool success = true;
133-
if (duplicateVertices) {
134-
for (size_t i = 0; success && i < vertexCount; ++i) {
135-
TIndex vertexIndex = indices[i];
136-
if (vertexIndex < 0 || vertexIndex >= colorView.size()) {
137-
success = false;
138-
} else {
139-
Color32& packedColor = *reinterpret_cast<Color32*>(pWritePos);
140-
success = CopyVertexColors::convertColor(
141-
colorView[vertexIndex],
142-
packedColor);
143-
pWritePos += stride;
144-
}
145-
}
146-
} else {
147-
for (size_t i = 0; success && i < vertexCount; ++i) {
148-
if (i >= colorView.size()) {
149-
success = false;
150-
} else {
151-
Color32& packedColor = *reinterpret_cast<Color32*>(pWritePos);
152-
success = CopyVertexColors::convertColor(colorView[i], packedColor);
153-
pWritePos += stride;
154-
}
132+
133+
for (size_t i = 0; success && i < vertexCount; ++i) {
134+
if (i >= colorView.size()) {
135+
success = false;
136+
} else {
137+
Color32& packedColor = *reinterpret_cast<Color32*>(pWritePos);
138+
success = CopyVertexColors::convertColor(colorView[i], packedColor);
139+
pWritePos += stride;
155140
}
156141
}
157142

@@ -563,8 +548,6 @@ void loadPrimitive(
563548
pBufferStart + colorByteOffset,
564549
stride,
565550
static_cast<size_t>(vertexCount),
566-
// computeFlatNormals,
567-
false,
568551
indices});
569552
}
570553

native~/Runtime/src/UnityPrepareRendererResources.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ struct CesiumPrimitiveInfo {
5050

5151
/**
5252
* @brief Whether the primitive contains normals
53-
* @remarks If normals are not present amd using a lit material, they should
54-
* be generated on the CPU or in the shader.
53+
* @remarks If normals are not present and using a lit material,
54+
* they should be generated on the CPU or in the material's shader.
5555
*/
5656
bool hasNormals = false;
5757

0 commit comments

Comments
 (0)