From 1554a37d688b1f67716e7e7ea070623b8a96a7a2 Mon Sep 17 00:00:00 2001 From: brunojezek Date: Wed, 30 Aug 2023 12:09:49 +0200 Subject: [PATCH] Small corrections based on reviews --- filament/include/filament/VertexBuffer.h | 4 +- filament/src/components/RenderableManager.cpp | 122 +++++++++--------- filament/src/details/SkinningBuffer.cpp | 4 +- filament/src/details/VertexBuffer.cpp | 4 +- samples/helloskinningbuffer_morebones.cpp | 4 +- samples/skinningtest.cpp | 10 +- shaders/src/getters.vs | 2 +- 7 files changed, 77 insertions(+), 73 deletions(-) diff --git a/filament/include/filament/VertexBuffer.h b/filament/include/filament/VertexBuffer.h index 9c76e075da7c..dd844c375a0e 100644 --- a/filament/include/filament/VertexBuffer.h +++ b/filament/include/filament/VertexBuffer.h @@ -147,11 +147,13 @@ class UTILS_PUBLIC VertexBuffer : public FilamentAPI { * set in RenderableManager:Builder:boneIndicesAndWeights methods. * Works with or without buffer objects. * + * @param enabled If true, enables advanced skinning mode. False by default. + * * @return A reference to this Builder for chaining calls. * * @see RenderableManager:Builder:boneIndicesAndWeights */ - Builder& advancedSkinning() noexcept; + Builder& advancedSkinning(bool enabled) noexcept; /** * Creates the VertexBuffer object and returns a pointer to it. diff --git a/filament/src/components/RenderableManager.cpp b/filament/src/components/RenderableManager.cpp index b3e068c8f5c6..7d7cdcbe75d8 100644 --- a/filament/src/components/RenderableManager.cpp +++ b/filament/src/components/RenderableManager.cpp @@ -292,7 +292,7 @@ void RenderableManager::BuilderDetails::processBoneIndicesAndWights(Engine& engi entity.getId(), primitiveIndex); for (size_t iVertex = 0; iVertex < vertexCount; iVertex++) { auto bonesPerVertex = bonePairsForPrimitive[iVertex].size(); - maxPairsCount += bonesPerVertex; + maxPairsCount += bonesPerVertex; maxPairsCountPerVertex = max(bonesPerVertex, (uint) maxPairsCountPerVertex); } } @@ -302,71 +302,73 @@ void RenderableManager::BuilderDetails::processBoneIndicesAndWights(Engine& engi // final texture data, indices and weights mBoneIndicesAndWeights = utils::FixedCapacityVector(maxPairsCount); // temporary indices and weights for one vertex - std::unique_ptr tempPairs(new float2[maxPairsCountPerVertex]()); + std::unique_ptr tempPairs = std::make_unique + (maxPairsCountPerVertex); for (auto iBonePair = mBonePairs.begin(); iBonePair != mBonePairs.end(); ++iBonePair) { auto primitiveIndex = iBonePair->first; auto bonePairsForPrimitive = iBonePair->second; - if (bonePairsForPrimitive.size()) { - size_t vertexCount = mEntries[primitiveIndex].vertices->getVertexCount(); - std::unique_ptr skinJoints( - new uint16_t[4 * vertexCount]()); // temporary indices for one vertex - std::unique_ptr skinWeights( - new float[4 * vertexCount]()); // temporary weights for one vertex - for (size_t iVertex = 0; iVertex < vertexCount; iVertex++) { - size_t tempPairCount = 0; - float boneWeightsSum = 0; - for (size_t k = 0; k < bonePairsForPrimitive[iVertex].size(); k++) { - auto boneWeight = bonePairsForPrimitive[iVertex][k][1]; - auto boneIndex= bonePairsForPrimitive[iVertex][k][0]; - ASSERT_PRECONDITION(boneWeight >= 0, - "[entity=%u, primitive @ %u] bone weight (%f) of vertex=%u is negative ", - entity.getId(), primitiveIndex, boneWeight, iVertex); - if (boneWeight) { - ASSERT_PRECONDITION(boneIndex >= 0, - "[entity=%u, primitive @ %u] bone index (%i) of vertex=%u is negative ", - entity.getId(), primitiveIndex, (int) boneIndex, iVertex); - ASSERT_PRECONDITION(boneIndex < mSkinningBoneCount, - "[entity=%u, primitive @ %u] bone index (%i) of vertex=%u is bigger then bone count (%u) ", - entity.getId(), primitiveIndex, (int) boneIndex, iVertex, mSkinningBoneCount); - boneWeightsSum += boneWeight; - tempPairs[tempPairCount][0] = boneIndex; - tempPairs[tempPairCount][1] = boneWeight; - tempPairCount++; - } + if (!bonePairsForPrimitive.size()) { + continue; + } + size_t vertexCount = mEntries[primitiveIndex].vertices->getVertexCount(); + std::unique_ptr skinJoints = std::make_unique + (4 * vertexCount); // temporary indices for one vertex + std::unique_ptr skinWeights = std::make_unique + (4 * vertexCount); // temporary weights for one vertex + for (size_t iVertex = 0; iVertex < vertexCount; iVertex++) { + size_t tempPairCount = 0; + float boneWeightsSum = 0; + for (size_t k = 0; k < bonePairsForPrimitive[iVertex].size(); k++) { + auto boneWeight = bonePairsForPrimitive[iVertex][k][1]; + auto boneIndex = bonePairsForPrimitive[iVertex][k][0]; + ASSERT_PRECONDITION(boneWeight >= 0, + "[entity=%u, primitive @ %u] bone weight (%f) of vertex=%u is negative ", + entity.getId(), primitiveIndex, boneWeight, iVertex); + if (boneWeight) { + ASSERT_PRECONDITION(boneIndex >= 0, + "[entity=%u, primitive @ %u] bone index (%i) of vertex=%u is negative ", + entity.getId(), primitiveIndex, (int) boneIndex, iVertex); + ASSERT_PRECONDITION(boneIndex < mSkinningBoneCount, + "[entity=%u, primitive @ %u] bone index (%i) of vertex=%u is bigger then bone count (%u) ", + entity.getId(), primitiveIndex, (int) boneIndex, iVertex, mSkinningBoneCount); + boneWeightsSum += boneWeight; + tempPairs[tempPairCount][0] = boneIndex; + tempPairs[tempPairCount][1] = boneWeight; + tempPairCount++; } + } - ASSERT_PRECONDITION(boneWeightsSum > 0, - "[entity=%u, primitive @ %u] sum of bone weights of vertex=%u is %f, it should be positive.", - entity.getId(), primitiveIndex, iVertex, boneWeightsSum); - if (abs(boneWeightsSum - 1.f) > std::numeric_limits::epsilon()) { - utils::slog.w << "Warning of skinning: [entity=%" << entity.getId() - << ", primitive @ %" << primitiveIndex - << "] sum of bone weights of vertex=" << iVertex << " is " << boneWeightsSum - << ", it should be one. Weights will be normalized." << utils::io::endl; - } - // prepare data for vertex attributes - auto offset = iVertex * 4; - // set attributes, indices and weights, for <= 4 pairs - for (size_t j = 0, c = min(tempPairCount, 4ul); j < c; j++) { - skinJoints[j + offset] = tempPairs[j][0]; - skinWeights[j + offset] = tempPairs[j][1] / boneWeightsSum; - } - // prepare data for texture - if (tempPairCount > 4) { // set attributes, indices and weights, for > 4 pairs - skinWeights[3 + offset] = -(float) (pairsCount + 1); // negative offset to texture 0..-1, 1..-2 - skinJoints[3 + offset] = (uint16_t) tempPairCount; // number pairs per vertex in texture - for (size_t j = 3; j < tempPairCount; j++) { - mBoneIndicesAndWeights[pairsCount][0] = tempPairs[j][0]; - mBoneIndicesAndWeights[pairsCount][1] = tempPairs[j][1] / boneWeightsSum; - pairsCount++; - } + ASSERT_PRECONDITION(boneWeightsSum > 0, + "[entity=%u, primitive @ %u] sum of bone weights of vertex=%u is %f, it should be positive.", + entity.getId(), primitiveIndex, iVertex, boneWeightsSum); + if (abs(boneWeightsSum - 1.f) > std::numeric_limits::epsilon()) { + utils::slog.w << "Warning of skinning: [entity=%" << entity.getId() + << ", primitive @ %" << primitiveIndex + << "] sum of bone weights of vertex=" << iVertex << " is " << boneWeightsSum + << ", it should be one. Weights will be normalized." << utils::io::endl; + } + // prepare data for vertex attributes + auto offset = iVertex * 4; + // set attributes, indices and weights, for <= 4 pairs + for (size_t j = 0, c = min(tempPairCount, 4ul); j < c; j++) { + skinJoints[j + offset] = tempPairs[j][0]; + skinWeights[j + offset] = tempPairs[j][1] / boneWeightsSum; + } + // prepare data for texture + if (tempPairCount > 4) { // set attributes, indices and weights, for > 4 pairs + skinWeights[3 + offset] = -(float) (pairsCount + 1); // negative offset to texture 0..-1, 1..-2 + skinJoints[3 + offset] = (uint16_t) tempPairCount; // number pairs per vertex in texture + for (size_t j = 3; j < tempPairCount; j++) { + mBoneIndicesAndWeights[pairsCount][0] = tempPairs[j][0]; + mBoneIndicesAndWeights[pairsCount][1] = tempPairs[j][1] / boneWeightsSum; + pairsCount++; } - } // for all vertices per primitive - downcast(mEntries[primitiveIndex].vertices) - ->updateBoneIndicesAndWeights(downcast(engine), - std::move(skinJoints), - std::move(skinWeights)); - } + } + } // for all vertices per primitive + downcast(mEntries[primitiveIndex].vertices) + ->updateBoneIndicesAndWeights(downcast(engine), + std::move(skinJoints), + std::move(skinWeights)); } // for all primitives } mBoneIndicesAndWeightsCount = pairsCount; // only part of mBoneIndicesAndWeights is used for real data diff --git a/filament/src/details/SkinningBuffer.cpp b/filament/src/details/SkinningBuffer.cpp index 2600cf4351cf..323a66ee97e2 100644 --- a/filament/src/details/SkinningBuffer.cpp +++ b/filament/src/details/SkinningBuffer.cpp @@ -211,7 +211,7 @@ void updateDataAt(backend::DriverApi& driver, textureWidth, lineCount, 1, PixelBufferDescriptor::make( out, textureWidth * lineCount * elementSize, - format, type,[allocation](void const*, size_t) {} + format, type, [allocation](void const*, size_t) {} )); out += lineCount * textureWidth; } @@ -222,7 +222,7 @@ void updateDataAt(backend::DriverApi& driver, lastLineCount, 1, 1, PixelBufferDescriptor::make( out, lastLineCount * elementSize, - format, type,[allocation](void const*, size_t) {} + format, type, [allocation](void const*, size_t) {} )); } } diff --git a/filament/src/details/VertexBuffer.cpp b/filament/src/details/VertexBuffer.cpp index bd4ff95a0ada..f557e25d233a 100644 --- a/filament/src/details/VertexBuffer.cpp +++ b/filament/src/details/VertexBuffer.cpp @@ -114,8 +114,8 @@ VertexBuffer::Builder& VertexBuffer::Builder::normalized(VertexAttribute attribu return *this; } -VertexBuffer::Builder& VertexBuffer::Builder::advancedSkinning() noexcept { - mImpl->mAdvancedSkinningEnabled = true; +VertexBuffer::Builder& VertexBuffer::Builder::advancedSkinning(bool enabled) noexcept { + mImpl->mAdvancedSkinningEnabled = enabled; return *this; } diff --git a/samples/helloskinningbuffer_morebones.cpp b/samples/helloskinningbuffer_morebones.cpp index a8e30fae37d0..63bea07b1658 100644 --- a/samples/helloskinningbuffer_morebones.cpp +++ b/samples/helloskinningbuffer_morebones.cpp @@ -104,7 +104,7 @@ int main(int argc, char** argv) { .attribute(VertexAttribute::POSITION, 0, VertexBuffer::AttributeType::FLOAT2, 0, 12) .attribute(VertexAttribute::COLOR, 0, VertexBuffer::AttributeType::UBYTE4, 8, 12) .normalized(VertexAttribute::COLOR) - .advancedSkinning() + .advancedSkinning(true) .build(*engine); app.vb1->setBufferAt(*engine, 0, VertexBuffer::BufferDescriptor(TRIANGLE_VERTICES, 36, nullptr)); @@ -114,7 +114,7 @@ int main(int argc, char** argv) { .attribute(VertexAttribute::POSITION, 0, VertexBuffer::AttributeType::FLOAT2, 0, 12) .attribute(VertexAttribute::COLOR, 0, VertexBuffer::AttributeType::UBYTE4, 8, 12) .normalized(VertexAttribute::COLOR) - .advancedSkinning() + .advancedSkinning(true) .build(*engine); app.vb2->setBufferAt(*engine, 0, VertexBuffer::BufferDescriptor(TRIANGLE_VERTICES, 36, nullptr)); diff --git a/samples/skinningtest.cpp b/samples/skinningtest.cpp index f0fbd0c6fe3e..70d70e8326c9 100644 --- a/samples/skinningtest.cpp +++ b/samples/skinningtest.cpp @@ -292,7 +292,7 @@ int main(int argc, char** argv) { VertexBuffer::AttributeType::UBYTE4, 8, 12) .normalized(VertexAttribute::COLOR) .enableBufferObjects() - .advancedSkinning() + .advancedSkinning(true) .build(*engine); app.bos[app.boCount] = BufferObject::Builder() .size(3 * sizeof(Vertex)) @@ -314,7 +314,7 @@ int main(int argc, char** argv) { VertexBuffer::AttributeType::UBYTE4, 8, 12) .normalized(VertexAttribute::COLOR) .enableBufferObjects() - .advancedSkinning() + .advancedSkinning(true) .build(*engine); app.bos[app.boCount] = BufferObject::Builder() .size(3 * sizeof(Vertex)) @@ -337,7 +337,7 @@ int main(int argc, char** argv) { VertexBuffer::AttributeType::UBYTE4, 8, 12) .normalized(VertexAttribute::COLOR) .enableBufferObjects() - .advancedSkinning() + .advancedSkinning(true) .build(*engine); app.bos[app.boCount] = BufferObject::Builder() .size(3 * sizeof(Vertex)) @@ -361,7 +361,7 @@ int main(int argc, char** argv) { VertexBuffer::AttributeType::UBYTE4, 8, 12) .normalized(VertexAttribute::COLOR) .enableBufferObjects() - .advancedSkinning() + .advancedSkinning(true) .build(*engine); app.bos[app.boCount] = BufferObject::Builder() .size(6 * sizeof(Vertex)) @@ -383,7 +383,7 @@ int main(int argc, char** argv) { VertexBuffer::AttributeType::UBYTE4, 8, 12) .normalized(VertexAttribute::COLOR) .enableBufferObjects() - .advancedSkinning() + .advancedSkinning(true) .build(*engine); app.bos[app.boCount] = BufferObject::Builder() .size(3 * sizeof(Vertex)) diff --git a/shaders/src/getters.vs b/shaders/src/getters.vs index b77dc053849c..b5186c40213e 100644 --- a/shaders/src/getters.vs +++ b/shaders/src/getters.vs @@ -78,7 +78,7 @@ void skinPosition(inout vec3 p, const uvec4 ids, const vec4 weights) { posSum += weights.z * mulBoneVertex(p, uint(ids.z)); uint pairIndex = uint(-weights.w - 1.); uint pairStop = pairIndex + uint(ids.w - 3u); - for (uint i = pairIndex; i < pairStop; i = i + 1u) { + for (uint i = pairIndex; i < pairStop; ++i) { ivec2 texcoord = ivec2(i % MAX_SKINNING_BUFFER_WIDTH, i / MAX_SKINNING_BUFFER_WIDTH); vec2 indexWeight = texelFetch(bonesBuffer_indicesAndWeights, texcoord, 0).rg; posSum += mulBoneVertex(p, uint(indexWeight.r)) * indexWeight.g;