Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
##### Fixes :wrench:

- Textures and UV coordinates from glTFs are now flipped to properly comply with Unity's U-right, V-up convention.
- Vertex tangents are now properly imported when present in glTFs.

## v1.18.1 - 2025-10-01

Expand Down
7 changes: 7 additions & 0 deletions CONTRIBUTING.md.meta

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

114 changes: 67 additions & 47 deletions native~/Runtime/src/UnityPrepareRendererResources.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,15 @@ void loadPrimitive(
computeFlatNormals = hasNormals = true;
}

bool hasTangents = false;
auto tangentAccessorIt = primitive.attributes.find("TANGENT");
AccessorView<UnityEngine::Vector4> tangentView;
if (tangentAcccessorIt != primitive.attributes.end()) {
tangentView =
AccessorView<UnityEngine::Vector4>(gltf, tangentAcccessorIt->second);
hasTangents = tangentView.status() == AccessorViewStatus::Valid;
}

// Check if we need to upgrade to a large index type to accommodate the
// larger number of vertices we need for flat normals.
if (computeFlatNormals && indexFormat == IndexFormat::UInt16 &&
Expand Down Expand Up @@ -450,6 +459,15 @@ void loadPrimitive(
++numberOfAttributes;
}

if (hasTangents) {
assert(numberOfAttributes < MAX_ATTRIBUTES);
descriptor[numberOfAttributes].attribute = VertexAttribute::Tangent;
descriptor[numberOfAttributes].format = VertexAttributeFormat::Float32;
descriptor[numberOfAttributes].dimension = 4;
descriptor[numberOfAttributes].stream = streamIndex;
++numberOfAttributes;
}

// Add the COLOR_0 attribute, if it exists.
auto colorAccessorIt = primitive.attributes.find("COLOR_0");
bool hasVertexColors =
Expand Down Expand Up @@ -554,9 +572,9 @@ void loadPrimitive(
attributes.Item(i, descriptor[i]);
}

int32_t vertexCount = computeFlatNormals
? indexCount
: static_cast<int32_t>(positionView.size());
const int32_t vertexCount = computeFlatNormals
? indexCount
: static_cast<int32_t>(positionView.size());
meshData.SetVertexBufferParams(vertexCount, attributes);

NativeArray1<uint8_t> nativeVertexBuffer =
Expand All @@ -571,74 +589,76 @@ void loadPrimitive(
// The vertex layout will be as follows:
// 1. position
// 2. normals (skip if N/A)
// 3. vertex colors (skip if N/A)
// 4. texcoords (first all TEXCOORD_i, then all _CESIUMOVERLAY_i)
// 3. tangents (skip if N/A)
// 4. vertex colors (skip if N/A)
// 5. texcoords (first all TEXCOORD_i, then all _CESIUMOVERLAY_i)

size_t stride = sizeof(Vector3);
size_t normalByteOffset, colorByteOffset;
size_t normalByteOffset;
if (hasNormals) {
normalByteOffset = stride;
stride += sizeof(Vector3);
}

if (hasTangents) {
stride += sizeof(Vector4);
}

size_t colorByteOffset;
if (hasVertexColors) {
colorByteOffset = stride;
stride += sizeof(uint32_t);
}

stride += numTexCoords * sizeof(Vector2);

const bool duplicateVertices = computeFlatNormals;
if (computeFlatNormals) {
::computeFlatNormals(
pWritePos + normalByteOffset,
stride,
indices,
indexCount,
positionView);
for (int64_t i = 0; i < vertexCount; ++i) {
TIndex vertexIndex = indices[i];
*reinterpret_cast<Vector3*>(pWritePos) = positionView[vertexIndex];
// skip position and normal
pWritePos += 2 * sizeof(Vector3);
// Skip the slot allocated for vertex colors, we will fill them in
// bulk later.
if (hasVertexColors) {
pWritePos += sizeof(uint32_t);
}
for (uint32_t texCoordIndex = 0; texCoordIndex < numTexCoords;
++texCoordIndex) {
Vector2 texCoord = texCoordViews[texCoordIndex][vertexIndex];
// Flip Y to comply with Unity's V-up coordinate convention
texCoord.y = 1 - texCoord.y;
*reinterpret_cast<Vector2*>(pWritePos) = texCoord;
pWritePos += sizeof(Vector2);
}
}
} else {
for (int64_t i = 0; i < vertexCount; ++i) {
*reinterpret_cast<Vector3*>(pWritePos) = positionView[i];
}

for (int64_t i = 0; i < vertexCount; ++i) {
const TIndex vertexIndex = duplicateVertices ? indices[i] : i;
*reinterpret_cast<Vector3*>(pWritePos) = positionView[vertexIndex];
pWritePos += sizeof(Vector3);

if (computeFlatNormals) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm I'm not sure how much of an impact it has here, but we tend to avoid putting if statements in a loop body if possible. I know in Unreal we prefer to use near-identical for loops if it can avoid a few if statements mid-loop. Maybe others have an opinion?

Copy link
Contributor Author

@david-lively david-lively Oct 10, 2025

Choose a reason for hiding this comment

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

@j9liu Hmm. Not sure I see the benefit here. The existing implementation already had if for vertex colors and normals. To completely get rid of the conditionals, we'd need 8 nearly-identical permutations of this loop.

// skip computed normal
pWritePos += sizeof(Vector3);
} else if (hasNormals) {
*reinterpret_cast<Vector3*>(pWritePos) = normalView[vertexIndex];
pWritePos += sizeof(Vector3);
if (hasNormals) {
*reinterpret_cast<Vector3*>(pWritePos) = normalView[i];
pWritePos += sizeof(Vector3);
}
// Skip the slot allocated for vertex colors, we will fill them in
// bulk later.
if (hasVertexColors) {
pWritePos += sizeof(uint32_t);
}
for (uint32_t texCoordIndex = 0; texCoordIndex < numTexCoords;
++texCoordIndex) {
Vector2 texCoord = texCoordViews[texCoordIndex][i];
// Flip Y to comply with Unity's V-up coordinate convention
texCoord.y = 1 - texCoord.y;
*reinterpret_cast<Vector2*>(pWritePos) = texCoord;
pWritePos += sizeof(Vector2);
}
}

if (hasTangents) {
*reinterpret_cast<Vector4*>(pWritePos) = tangentView[vertexIndex];
pWritePos += sizeof(Vector4);
}

// Skip the slot allocated for vertex colors, we will fill them in
// bulk later.
if (hasVertexColors) {
pWritePos += sizeof(uint32_t);
}

for (uint32_t texCoordIndex = 0; texCoordIndex < numTexCoords;
++texCoordIndex) {
Vector2 texCoord = texCoordViews[texCoordIndex][vertexIndex];
// Flip Y to comply with Unity's V-up coordinate convention
texCoord.y = 1 - texCoord.y;
*reinterpret_cast<Vector2*>(pWritePos) = texCoord;
pWritePos += sizeof(Vector2);
}
}

// Fill in vertex colors separately, if they exist.
if (hasVertexColors) {
// Color comes after position and normal.
// Color comes after position, normal and tangent
createAccessorView(
gltf,
colorAccessorIt->second,
Expand All @@ -650,7 +670,7 @@ void loadPrimitive(
indices});
}

if (computeFlatNormals) {
if (duplicateVertices) {
// rewrite indices
for (TIndex i = 0; i < indexCount; i++) {
indices[i] = i;
Expand Down
Loading