From f6da7fd1ae0b3e2ddc5462746f43787e69bc28c9 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Fri, 31 Jan 2020 14:51:51 -0800 Subject: [PATCH] gltfio: add nominal support for flat shading. This "fixes" the new animated Fox model in the glTF conformance suite. As per the glTF spec, we now generate per-face normals for the case where normals are not specified in the model. Note that true flat shading (i.e. flat interpolation) is not a requirement in the glTF spec, since some Khronos members advocate for WebGL 1.0 compatibility. Fixes #2088. --- RELEASE_NOTES.md | 1 + .../include/geometry/SurfaceOrientation.h | 30 +++++++++----- libs/geometry/src/SurfaceOrientation.cpp | 41 +++++++++++++++---- libs/gltfio/src/AssetLoader.cpp | 26 ++++++++++-- libs/gltfio/src/ResourceLoader.cpp | 14 ++++--- 5 files changed, 83 insertions(+), 29 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 6abd774aac5..993d5abc390 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -9,6 +9,7 @@ A new header is inserted each time a *tag* is created. - Screen-space refraction is now supported. - Removed depth-prepass related APIs. - gltfio: add asynchronous API to ResourceLoader. +- gltfio: generate normals for flat-shaded models that do not have normals. ## v1.4.5 diff --git a/libs/geometry/include/geometry/SurfaceOrientation.h b/libs/geometry/include/geometry/SurfaceOrientation.h index 268bf7130e7..d4fa861eff2 100644 --- a/libs/geometry/include/geometry/SurfaceOrientation.h +++ b/libs/geometry/include/geometry/SurfaceOrientation.h @@ -41,12 +41,25 @@ class SurfaceOrientation { * The Builder is used to construct an immutable surface orientation helper. * * Clients provide pointers into their own data, which is synchronously consumed during build(). - * At a minimum, clients must supply a vertex count and normals buffer. They can supply data in - * any of the following three combinations: + * At a minimum, clients must supply a vertex count. They can supply data in any of the + * following combinations: * - * 1. vec3 normals only (not recommended) - * 2. vec3 normals + vec4 tangents (sign of W determines bitangent orientation) - * 3. vec3 normals + vec2 uvs + vec3 positions + uint3 indices + * 1. normals only ........................... not recommended, selects arbitrary orientation + * 2. normals + tangents ..................... sign of W determines bitangent orientation + * 3. normals + uvs + positions + indices .... selects Lengyel’s Method + * 4. positions + indices .................... generates normals for flat shading only + * + * Additionally, the client-side data has the following type constraints: + * + * - Normals must be float3 + * - Tangents must be float4 + * - UVs must be float2 + * - Positions must be float3 + * - Triangles must be uint3 or ushort3 + * + * Currently, mikktspace is not supported because it requires re-indexing the mesh. Instead + * we use the method described by Eric Lengyel in "Foundations of Game Engine Development" + * (Volume 2, Chapter 7). */ class Builder { public: @@ -56,14 +69,11 @@ class SurfaceOrientation { Builder& operator=(Builder&& that) noexcept; /** - * These two attributes are required. They are not passed into the constructor to force - * calling code to be self-documenting. - * @{ + * This attribute is required. */ Builder& vertexCount(size_t vertexCount) noexcept; - Builder& normals(const filament::math::float3*, size_t stride = 0) noexcept; - /** @} */ + Builder& normals(const filament::math::float3*, size_t stride = 0) noexcept; Builder& tangents(const filament::math::float4*, size_t stride = 0) noexcept; Builder& uvs(const filament::math::float2*, size_t stride = 0) noexcept; Builder& positions(const filament::math::float3*, size_t stride = 0) noexcept; diff --git a/libs/geometry/src/SurfaceOrientation.cpp b/libs/geometry/src/SurfaceOrientation.cpp index cd552dccd4f..ed26f6c7ced 100644 --- a/libs/geometry/src/SurfaceOrientation.cpp +++ b/libs/geometry/src/SurfaceOrientation.cpp @@ -46,6 +46,7 @@ struct OrientationBuilderImpl { SurfaceOrientation buildWithNormalsOnly(); SurfaceOrientation buildWithSuppliedTangents(); SurfaceOrientation buildWithUvs(); + SurfaceOrientation buildWithFlatNormals(); }; struct OrientationImpl { @@ -110,21 +111,23 @@ Builder& Builder::triangles(const ushort3* triangles) noexcept { } SurfaceOrientation Builder::build() { - ASSERT_PRECONDITION(mImpl->normals != nullptr, "Normals are required."); ASSERT_PRECONDITION(mImpl->vertexCount > 0, "Vertex count must be non-zero."); + if (mImpl->triangles16 || mImpl->triangles32) { + ASSERT_PRECONDITION(mImpl->positions, "Positions are required."); + ASSERT_PRECONDITION(!mImpl->triangles16 || !mImpl->triangles32, + "Choose 16 or 32-bit indices, not both."); + ASSERT_PRECONDITION(mImpl->triangleCount > 0, "Triangle count is required."); + if (mImpl->normals == nullptr) { + return mImpl->buildWithFlatNormals(); + } + } + ASSERT_PRECONDITION(mImpl->normals != nullptr, "Normals are required."); if (mImpl->tangents != nullptr) { return mImpl->buildWithSuppliedTangents(); } if (mImpl->uvs == nullptr) { return mImpl->buildWithNormalsOnly(); } - bool hasTriangles = mImpl->triangles16 || mImpl->triangles32; - bool bothTypes = mImpl->triangles16 && mImpl->triangles32; - ASSERT_PRECONDITION(hasTriangles && mImpl->positions, - "When using UVs, positions and triangles are required."); - ASSERT_PRECONDITION(!bothTypes, "Choose 16 or 32-bit indices, not both."); - ASSERT_PRECONDITION(mImpl->triangleCount > 0, - "When using UVs, triangle count is required."); return mImpl->buildWithUvs(); } @@ -191,7 +194,8 @@ SurfaceOrientation OrientationBuilderImpl::buildWithSuppliedTangents() { // http://www.terathon.com/code/tangent.html // // We considered mikktspace (which thankfully has a zlib-style license) but it would require -// re-indexing via meshoptimizer and is therefore a bit heavyweight. +// re-indexing (i.e. welding) and is therefore a bit heavyweight. Note that the welding could be +// done via meshoptimizer. // SurfaceOrientation OrientationBuilderImpl::buildWithUvs() { ASSERT_PRECONDITION(this->normalStride == 0, "Non-zero normal stride not yet supported."); @@ -308,5 +312,24 @@ void SurfaceOrientation::getQuats(quath* out, size_t quatCount, size_t stride) c } } +SurfaceOrientation OrientationBuilderImpl::buildWithFlatNormals() { + float3* normals = new float3[vertexCount]; + for (size_t a = 0; a < triangleCount; ++a) { + const uint3 tri = triangles16 ? uint3(triangles16[a]) : triangles32[a]; + const float3 v1 = positions[tri.x]; + const float3 v2 = positions[tri.y]; + const float3 v3 = positions[tri.z]; + const float3 normal = normalize(cross(v2 - v1, v3 - v1)); + normals[tri.x] = normal; + normals[tri.y] = normal; + normals[tri.z] = normal; + } + this->normals = normals; + SurfaceOrientation result = buildWithNormalsOnly(); + this->normals = nullptr; + delete[] normals; + return result; +} + } // namespace geometry } // namespace filament diff --git a/libs/gltfio/src/AssetLoader.cpp b/libs/gltfio/src/AssetLoader.cpp index 80bfef58c24..16d426ad628 100644 --- a/libs/gltfio/src/AssetLoader.cpp +++ b/libs/gltfio/src/AssetLoader.cpp @@ -550,6 +550,12 @@ bool FAssetLoader::createPrimitive(const cgltf_primitive* inPrim, Primitive* out vbb.normalized(semantic, inputAccessor->normalized); } + // If the model is lit but does not have normals, we'll need to generate flat normals. + if (inPrim->material && !inPrim->material->unlit && !hasNormals) { + vbb.attribute(VertexAttribute::TANGENTS, slot++, VertexBuffer::AttributeType::SHORT4); + vbb.normalized(VertexAttribute::TANGENTS); + } + cgltf_size targetsCount = inPrim->targets_count; if (targetsCount > MAX_MORPH_TARGETS) { slog.w << "Too many morph targets in " << name << io::endl; @@ -633,10 +639,6 @@ bool FAssetLoader::createPrimitive(const cgltf_primitive* inPrim, Primitive* out } } - if (inPrim->material && !inPrim->material->unlit && !hasNormals) { - slog.w << "Missing normals in " << name << io::endl; - } - if (needsDummyData) { slot++; } @@ -695,6 +697,22 @@ bool FAssetLoader::createPrimitive(const cgltf_primitive* inPrim, Primitive* out }); } + // If the model is lit but does not have normals, we'll need to generate flat normals. + if (inPrim->material && !inPrim->material->unlit && !hasNormals) { + mResult->mBufferBindings.push_back({ + .uri = "", + .totalSize = 0, + .bufferIndex = uint8_t(slot++), + .vertexBuffer = vertices, + .indexBuffer = nullptr, + .convertBytesToShorts = false, + .generateTrivialIndices = false, + .generateDummyData = false, + .generateTangents = true, + .sparseAccessor = false, + }); + } + for (cgltf_size targetIndex = 0; targetIndex < targetsCount; targetIndex++) { const cgltf_morph_target& morphTarget = inPrim->targets[targetIndex]; for (cgltf_size aindex = 0; aindex < morphTarget.attributes_count; aindex++) { diff --git a/libs/gltfio/src/ResourceLoader.cpp b/libs/gltfio/src/ResourceLoader.cpp index 4eab7c92d62..24f5ec596b2 100644 --- a/libs/gltfio/src/ResourceLoader.cpp +++ b/libs/gltfio/src/ResourceLoader.cpp @@ -690,7 +690,7 @@ void ResourceLoader::computeTangents(FFilamentAsset* asset) const { // At a minimum we need normals to generate tangents. auto normalsInfo = accessors[cgltf_attribute_type_normal]; - if (normalsInfo == nullptr || vertexCount == 0) { + if (vertexCount == 0) { return; } @@ -700,11 +700,13 @@ void ResourceLoader::computeTangents(FFilamentAsset* asset) const { sob.vertexCount(vertexCount); // Convert normals into packed floats. - assert(normalsInfo->count == vertexCount); - assert(normalsInfo->type == cgltf_type_vec3); - fp32Normals.resize(vertexCount); - cgltf_accessor_unpack_floats(normalsInfo, &fp32Normals[0].x, vertexCount * 3); - sob.normals(fp32Normals.data()); + if (normalsInfo) { + assert(normalsInfo->count == vertexCount); + assert(normalsInfo->type == cgltf_type_vec3); + fp32Normals.resize(vertexCount); + cgltf_accessor_unpack_floats(normalsInfo, &fp32Normals[0].x, vertexCount * 3); + sob.normals(fp32Normals.data()); + } // Convert tangents into packed floats. auto tangentsInfo = accessors[cgltf_attribute_type_tangent];