Skip to content

Commit

Permalink
Fix possible change of scale sign when decomposing matrix (#7138)
Browse files Browse the repository at this point in the history
Co-authored-by: Mathias Agopian <[email protected]>
  • Loading branch information
mackong and pixelflinger authored Sep 12, 2023
1 parent 1ff0a2d commit d9c2893
Show file tree
Hide file tree
Showing 10 changed files with 415 additions and 15 deletions.
1 change: 1 addition & 0 deletions NEW_RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md).

## Release notes for next branch cut

- gltfio: Fix possible change of scale sign when decomposing transform matrix for animation
- engine: Fixes "stable" shadows (see b/299310624)
3 changes: 3 additions & 0 deletions android/gltfio-android/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ set(GLTFIO_SRCS
${GLTFIO_DIR}/include/gltfio/FilamentInstance.h
${GLTFIO_DIR}/include/gltfio/MaterialProvider.h
${GLTFIO_DIR}/include/gltfio/NodeManager.h
${GLTFIO_DIR}/include/gltfio/TrsTransformManager.h
${GLTFIO_DIR}/include/gltfio/ResourceLoader.h
${GLTFIO_DIR}/include/gltfio/TextureProvider.h
${GLTFIO_DIR}/include/gltfio/math.h
Expand All @@ -69,10 +70,12 @@ set(GLTFIO_SRCS
${GLTFIO_DIR}/src/FilamentAsset.cpp
${GLTFIO_DIR}/src/FilamentInstance.cpp
${GLTFIO_DIR}/src/FNodeManager.h
${GLTFIO_DIR}/src/FTrsTransformManager.h
${GLTFIO_DIR}/src/GltfEnums.h
${GLTFIO_DIR}/src/Ktx2Provider.cpp
${GLTFIO_DIR}/src/MaterialProvider.cpp
${GLTFIO_DIR}/src/NodeManager.cpp
${GLTFIO_DIR}/src/TrsTransformManager.cpp
${GLTFIO_DIR}/src/ResourceLoader.cpp
${GLTFIO_DIR}/src/StbProvider.cpp
${GLTFIO_DIR}/src/TangentsJob.cpp
Expand Down
3 changes: 3 additions & 0 deletions libs/gltfio/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ set(PUBLIC_HDRS
include/gltfio/FilamentInstance.h
include/gltfio/MaterialProvider.h
include/gltfio/NodeManager.h
include/gltfio/TrsTransformManager.h
include/gltfio/ResourceLoader.h
include/gltfio/TextureProvider.h
include/gltfio/math.h
Expand All @@ -35,10 +36,12 @@ set(SRCS
src/FilamentAsset.cpp
src/FilamentInstance.cpp
src/FNodeManager.h
src/FTrsTransformManager.h
src/GltfEnums.h
src/Ktx2Provider.cpp
src/MaterialProvider.cpp
src/NodeManager.cpp
src/TrsTransformManager.cpp
src/ResourceLoader.cpp
src/StbProvider.cpp
src/TangentsJob.cpp
Expand Down
114 changes: 114 additions & 0 deletions libs/gltfio/include/gltfio/TrsTransformManager.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/*
* Copyright (C) 2023 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#ifndef GLTFIO_TRSTRANSFORMMANAGER_H
#define GLTFIO_TRSTRANSFORMMANAGER_H

#include <filament/FilamentAPI.h>

#include <utils/compiler.h>
#include <utils/EntityInstance.h>
#include <math/quat.h>
#include <math/vec3.h>
#include <math/mat4.h>

using namespace filament::math;

namespace utils {
class Entity;
} // namespace utils

namespace filament::gltfio {

class FTrsTransformManager;

/**
* TrsTransformManager is used to add entities with glTF-specific trs information.
*
* Trs information here just used for Animation, DON'T use for transform.
*/
class UTILS_PUBLIC TrsTransformManager {
public:
using Instance = utils::EntityInstance<TrsTransformManager>;
using Entity = utils::Entity;

/**
* Returns whether a particular Entity is associated with a component of this TrsTransformManager
* @param e An Entity.
* @return true if this Entity has a component associated with this manager.
*/
bool hasComponent(Entity e) const noexcept;

/**
* Gets an Instance representing the trs transform component associated with the given Entity.
* @param e An Entity.
* @return An Instance object, which represents the trs transform component associated with the Entity e.
* @note Use Instance::isValid() to make sure the component exists.
* @see hasComponent()
*/
Instance getInstance(Entity e) const noexcept;

/**
* Creates a trs transform component and associates it with the given entity.
* @param entity An Entity to associate a trs transform component with.
* @param translation The translation to initialize the trs transform component with.
* @param rotation The rotation to initialize the trs transform component with.
* @param scale The scale to initialize the trs transform component with.
*
* If this component already exists on the given entity, it is first destroyed as if
* destroy(Entity e) was called.
*
* @see destroy()
*/
void create(Entity entity);
void create(Entity entity, const float3& translation, const quatf& rotation,
const float3& scale); //!< \overload

/**
* Destroys this component from the given entity.
* @param e An entity.
*
* @see create()
*/
void destroy(Entity e) noexcept;

void setTranslation(Instance ci, const float3& translation) noexcept;
const float3& getTranslation(Instance ci) const noexcept;

void setRotation(Instance ci, const quatf& rotation) noexcept;
const quatf& getRotation(Instance ci) const noexcept;

void setScale(Instance ci, const float3& scale) noexcept;
const float3& getScale(Instance ci) const noexcept;

void setTrs(Instance ci, const float3& translation, const quatf& rotation,
const float3& scale) noexcept;
const mat4f getTransform(Instance ci) const noexcept;

protected:
TrsTransformManager() noexcept = default;
~TrsTransformManager() = default;

public:
TrsTransformManager(TrsTransformManager const&) = delete;
TrsTransformManager(TrsTransformManager&&) = delete;
TrsTransformManager& operator=(TrsTransformManager const&) = delete;
TrsTransformManager& operator=(TrsTransformManager&&) = delete;
};

} // namespace filament::gltfio

#endif // GLTFIO_TRSTRANSFORMMANAGER_H
22 changes: 11 additions & 11 deletions libs/gltfio/src/Animator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include "FFilamentAsset.h"
#include "FFilamentInstance.h"
#include "FTrsTransformManager.h"
#include "downcast.h"

#include <filament/VertexBuffer.h>
Expand Down Expand Up @@ -74,6 +75,7 @@ struct AnimatorImpl {
FFilamentInstance* instance = nullptr;
RenderableManager* renderableManager;
TransformManager* transformManager;
TrsTransformManager* trsTransformManager;
vector<float> weights;
FixedCapacityVector<mat4f> crossFade;
void addChannels(const FixedCapacityVector<Entity>& nodeMap, const cgltf_animation& srcAnim,
Expand Down Expand Up @@ -190,6 +192,7 @@ Animator::Animator(FFilamentAsset const* asset, FFilamentInstance* instance) {
mImpl->instance = instance;
mImpl->renderableManager = &asset->mEngine->getRenderableManager();
mImpl->transformManager = &asset->mEngine->getTransformManager();
mImpl->trsTransformManager = asset->getTrsTransformManager();

const cgltf_data* srcAsset = asset->mSourceAsset->hierarchy;
const cgltf_animation* srcAnims = srcAsset->animations;
Expand Down Expand Up @@ -429,20 +432,13 @@ void AnimatorImpl::applyAnimation(const Channel& channel, float t, size_t prevIn
size_t nextIndex) {
const Sampler* sampler = channel.sourceData;
const TimeValues& times = sampler->times;
TrsTransformManager::Instance trsNode = trsTransformManager->getInstance(channel.targetEntity);
TransformManager::Instance node = transformManager->getInstance(channel.targetEntity);

// Perform the interpolation. This is a simple but inefficient implementation; Filament
// stores transforms as mat4's but glTF animation is based on TRS (translation rotation
// scale).
mat4f xform = transformManager->getTransform(node);
float3 scale;
quatf rotation;
float3 translation;
decomposeMatrix(xform, &translation, &rotation, &scale);

switch (channel.transformType) {

case Channel::SCALE: {
float3 scale;
const float3* srcVec3 = (const float3*) sampler->values.data();
if (sampler->interpolation == Sampler::CUBIC) {
float3 vert0 = srcVec3[prevIndex * 3 + 1];
Expand All @@ -453,10 +449,12 @@ void AnimatorImpl::applyAnimation(const Channel& channel, float t, size_t prevIn
} else {
scale = ((1 - t) * srcVec3[prevIndex]) + (t * srcVec3[nextIndex]);
}
trsTransformManager->setScale(trsNode, scale);
break;
}

case Channel::TRANSLATION: {
float3 translation;
const float3* srcVec3 = (const float3*) sampler->values.data();
if (sampler->interpolation == Sampler::CUBIC) {
float3 vert0 = srcVec3[prevIndex * 3 + 1];
Expand All @@ -467,10 +465,12 @@ void AnimatorImpl::applyAnimation(const Channel& channel, float t, size_t prevIn
} else {
translation = ((1 - t) * srcVec3[prevIndex]) + (t * srcVec3[nextIndex]);
}
trsTransformManager->setTranslation(trsNode, translation);
break;
}

case Channel::ROTATION: {
quatf rotation;
const quatf* srcQuat = (const quatf*) sampler->values.data();
if (sampler->interpolation == Sampler::CUBIC) {
quatf vert0 = srcQuat[prevIndex * 3 + 1];
Expand All @@ -481,6 +481,7 @@ void AnimatorImpl::applyAnimation(const Channel& channel, float t, size_t prevIn
} else {
rotation = slerp(srcQuat[prevIndex], srcQuat[nextIndex], t);
}
trsTransformManager->setRotation(trsNode, rotation);
break;
}

Expand Down Expand Up @@ -519,8 +520,7 @@ void AnimatorImpl::applyAnimation(const Channel& channel, float t, size_t prevIn
}
}

xform = composeMatrix(translation, rotation, scale);
transformManager->setTransform(node, xform);
transformManager->setTransform(node, trsTransformManager->getTransform(trsNode));
}

void AnimatorImpl::resetBoneMatrices(FFilamentInstance* instance) {
Expand Down
9 changes: 7 additions & 2 deletions libs/gltfio/src/AssetLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

#include "FFilamentAsset.h"
#include "FNodeManager.h"
#include "FTrsTransformManager.h"
#include "GltfEnums.h"

#include <filament/Box.h>
Expand Down Expand Up @@ -295,6 +296,7 @@ struct FAssetLoader : public AssetLoader {
MaterialProvider& mMaterials;
Engine& mEngine;
FNodeManager mNodeManager;
FTrsTransformManager mTrsTransformManager;

// Transient state used only for the asset currently being loaded:
FFilamentAsset* mAsset;
Expand Down Expand Up @@ -400,7 +402,8 @@ void FAssetLoader::createRootAsset(const cgltf_data* srcAsset) {
#endif

mDummyBufferObject = nullptr;
mAsset = new FFilamentAsset(&mEngine, mNameManager, &mEntityManager, &mNodeManager, srcAsset);
mAsset = new FFilamentAsset(&mEngine, mNameManager, &mEntityManager, &mNodeManager,
&mTrsTransformManager, srcAsset);

// It is not an error for a glTF file to have zero scenes.
mAsset->mScenes.clear();
Expand Down Expand Up @@ -563,7 +566,9 @@ void FAssetLoader::recurseEntities(const cgltf_data* srcAsset, const cgltf_node*
quatf* rotation = (quatf*) &node->rotation[0];
float3* scale = (float3*) &node->scale[0];
float3* translation = (float3*) &node->translation[0];
localTransform = composeMatrix(*translation, *rotation, *scale);
mTrsTransformManager.create(entity, *translation, *rotation, *scale);
localTransform = mTrsTransformManager.getTransform(
mTrsTransformManager.getInstance(entity));
}

auto parentTransform = mTransformManager.getInstance(parent);
Expand Down
10 changes: 8 additions & 2 deletions libs/gltfio/src/FFilamentAsset.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include <gltfio/FilamentAsset.h>
#include <gltfio/NodeManager.h>
#include <gltfio/TrsTransformManager.h>

#include <filament/Engine.h>
#include <filament/IndexBuffer.h>
Expand Down Expand Up @@ -110,9 +111,9 @@ using MeshCache = utils::FixedCapacityVector<utils::FixedCapacityVector<Primitiv
struct FFilamentAsset : public FilamentAsset {
FFilamentAsset(Engine* engine, utils::NameComponentManager* names,
utils::EntityManager* entityManager, NodeManager* nodeManager,
const cgltf_data* srcAsset) :
TrsTransformManager* trsTransformManager, const cgltf_data* srcAsset) :
mEngine(engine), mNameManager(names), mEntityManager(entityManager),
mNodeManager(nodeManager),
mNodeManager(nodeManager), mTrsTransformManager(trsTransformManager),
mSourceAsset(new SourceAsset {(cgltf_data*)srcAsset}),
mTextures(srcAsset->textures_count),
mMeshCache(srcAsset->meshes_count) {}
Expand Down Expand Up @@ -195,6 +196,10 @@ struct FFilamentAsset : public FilamentAsset {
return mEngine;
}

TrsTransformManager* getTrsTransformManager() const noexcept {
return mTrsTransformManager;
}

void releaseSourceData() noexcept;

const void* getSourceAsset() const noexcept {
Expand Down Expand Up @@ -242,6 +247,7 @@ struct FFilamentAsset : public FilamentAsset {
utils::NameComponentManager* const mNameManager;
utils::EntityManager* const mEntityManager;
NodeManager* const mNodeManager;
TrsTransformManager* const mTrsTransformManager;
std::vector<utils::Entity> mEntities; // sorted such that renderables come first
std::vector<utils::Entity> mLightEntities;
std::vector<utils::Entity> mCameraEntities;
Expand Down
Loading

0 comments on commit d9c2893

Please sign in to comment.