From bea02427edee198f81eb10a2fc33d992f3804866 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Mon, 3 Jun 2024 16:50:04 -0700 Subject: [PATCH] keep the normal transforms in float during skinning FIXES=[342459864] --- NEW_RELEASE_NOTES.md | 2 ++ filament/src/details/SkinningBuffer.cpp | 10 ++---- .../include/filament/MaterialEnums.h | 2 +- .../include/private/filament/UibStructs.h | 5 +-- shaders/src/common_types.glsl | 3 +- shaders/src/getters.vs | 35 +++++++++++-------- 6 files changed, 32 insertions(+), 25 deletions(-) diff --git a/NEW_RELEASE_NOTES.md b/NEW_RELEASE_NOTES.md index 4a1a9c7fa7..d6831f9f35 100644 --- a/NEW_RELEASE_NOTES.md +++ b/NEW_RELEASE_NOTES.md @@ -7,3 +7,5 @@ for next branch cut* header. appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md). ## Release notes for next branch cut + +- engine: fix skinning normals with large transforms (b/342459864) [⚠️ **New Material Version**] diff --git a/filament/src/details/SkinningBuffer.cpp b/filament/src/details/SkinningBuffer.cpp index d35fe1c70d..b709263867 100644 --- a/filament/src/details/SkinningBuffer.cpp +++ b/filament/src/details/SkinningBuffer.cpp @@ -114,6 +114,7 @@ void FSkinningBuffer::setBones(FEngine& engine, setBones(engine, mHandle, transforms, count, offset); } +UTILS_UNUSED static uint32_t packHalf2x16(half2 v) noexcept { uint32_t lo = getBits(v[0]); uint32_t hi = getBits(v[1]); @@ -144,13 +145,8 @@ PerRenderableBoneUib::BoneData FSkinningBuffer::makeBone(mat4f transform) noexce transform[1], transform[2] }, - .cof = { - packHalf2x16({ cofactors[0].x, cofactors[0].y }), - packHalf2x16({ cofactors[0].z, cofactors[1].x }), - packHalf2x16({ cofactors[1].y, cofactors[1].z }), - packHalf2x16({ cofactors[2].x, cofactors[2].y }) - // cofactor[2][2] is not stored because we don't have space for it - } + .cof0 = cofactors[0], + .cof1x = cofactors[1].x }; } diff --git a/libs/filabridge/include/filament/MaterialEnums.h b/libs/filabridge/include/filament/MaterialEnums.h index 25eb74d63b..c15c6c457c 100644 --- a/libs/filabridge/include/filament/MaterialEnums.h +++ b/libs/filabridge/include/filament/MaterialEnums.h @@ -28,7 +28,7 @@ namespace filament { // update this when a new version of filament wouldn't work with older materials -static constexpr size_t MATERIAL_VERSION = 52; +static constexpr size_t MATERIAL_VERSION = 53; /** * Supported shading models diff --git a/libs/filabridge/include/private/filament/UibStructs.h b/libs/filabridge/include/private/filament/UibStructs.h index 6a6f14ece1..4f06f43b40 100644 --- a/libs/filabridge/include/private/filament/UibStructs.h +++ b/libs/filabridge/include/private/filament/UibStructs.h @@ -327,8 +327,9 @@ struct PerRenderableBoneUib { // NOLINT(cppcoreguidelines-pro-type-member-init) struct alignas(16) BoneData { // bone transform, last row assumed [0,0,0,1] math::float4 transform[3]; - // 8 first cofactor matrix of transform's upper left - math::uint4 cof; + // 4 first cofactor matrix of transform's upper left + math::float3 cof0; + float cof1x; }; BoneData bones[CONFIG_MAX_BONE_COUNT]; }; diff --git a/shaders/src/common_types.glsl b/shaders/src/common_types.glsl index cf24b40929..3208c1ba95 100644 --- a/shaders/src/common_types.glsl +++ b/shaders/src/common_types.glsl @@ -19,7 +19,8 @@ struct ShadowData { #if defined(VARIANT_HAS_SKINNING_OR_MORPHING) struct BoneData { highp mat3x4 transform; // bone transform is mat4x3 stored in row-major (last row [0,0,0,1]) - highp uvec4 cof; // 8 first cofactor matrix of transform's upper left + highp float3 cof0; // 3 first cofactor matrix of transform's upper left + highp float cof1x; // 4th cofactor }; #endif diff --git a/shaders/src/getters.vs b/shaders/src/getters.vs index cccc08319c..024ec9ff41 100644 --- a/shaders/src/getters.vs +++ b/shaders/src/getters.vs @@ -34,25 +34,32 @@ int getVertexIndex() { #if defined(VARIANT_HAS_SKINNING_OR_MORPHING) #define MAX_SKINNING_BUFFER_WIDTH 2048u -vec3 mulBoneNormal(vec3 n, uint i) { +vec3 mulBoneNormal(vec3 n, uint j) { highp mat3 cof; - // the first 8 elements of the cofactor matrix are stored as fp16 - highp vec2 x0y0 = unpackHalf2x16(bonesUniforms.bones[i].cof[0]); - highp vec2 z0x1 = unpackHalf2x16(bonesUniforms.bones[i].cof[1]); - highp vec2 y1z1 = unpackHalf2x16(bonesUniforms.bones[i].cof[2]); - highp vec2 x2y2 = unpackHalf2x16(bonesUniforms.bones[i].cof[3]); - // the last element must be computed by hand - highp float a = bonesUniforms.bones[i].transform[0][0]; - highp float b = bonesUniforms.bones[i].transform[0][1]; - highp float d = bonesUniforms.bones[i].transform[1][0]; - highp float e = bonesUniforms.bones[i].transform[1][1]; + highp float a = bonesUniforms.bones[j].transform[0][0]; + highp float b = bonesUniforms.bones[j].transform[0][1]; + highp float c = bonesUniforms.bones[j].transform[0][2]; + highp float d = bonesUniforms.bones[j].transform[1][0]; + highp float e = bonesUniforms.bones[j].transform[1][1]; + highp float f = bonesUniforms.bones[j].transform[1][2]; + highp float g = bonesUniforms.bones[j].transform[2][0]; + highp float h = bonesUniforms.bones[j].transform[2][1]; + highp float i = bonesUniforms.bones[j].transform[2][2]; - cof[0].xyz = vec3(x0y0, z0x1.x); - cof[1].xyz = vec3(z0x1.y, y1z1); - cof[2].xyz = vec3(x2y2, a * e - b * d); + cof[0] = bonesUniforms.bones[j].cof0; + + cof[1].xyz = vec3( + bonesUniforms.bones[j].cof1x, + a * i - c * g, + c * d - a * f); + + cof[2].xyz = vec3( + d * h - e * g, + b * g - a * h, + a * e - b * d); return normalize(cof * n); }