fix TransformManager high precision mode

The residual would be corrupted when using transactions.
This commit is contained in:
Mathias Agopian
2022-12-06 11:05:40 -08:00
parent a4797a631e
commit 4c55fc3c48
4 changed files with 48 additions and 35 deletions

View File

@@ -12,9 +12,11 @@ A new header is inserted each time a *tag* is created.
- engine: add missing getters on `MaterialInstance`
- WebGL: add missing `ColorGrading` JS bindings
- engine: improvements/cleanup of Shadow mapping code [⚠️ **Recompile Materials**]
- engine: fix TransformManager high precision mode when using transactions.
## v1.28.3
- backend: add support for GGP platform
- engine: primitives with `CullingMode::FRONT_AND_BACK` are now skipped.

View File

@@ -56,7 +56,7 @@ void FTransformManager::create(Entity entity, Instance parent, const mat4f& loca
if (UTILS_UNLIKELY(manager.hasComponent(entity))) {
destroy(entity);
}
Instance i = manager.addComponent(entity);
Instance const i = manager.addComponent(entity);
assert_invariant(i);
assert_invariant(i != parent);
@@ -78,7 +78,7 @@ void FTransformManager::create(Entity entity, Instance parent, const mat4& local
if (UTILS_UNLIKELY(manager.hasComponent(entity))) {
destroy(entity);
}
Instance i = manager.addComponent(entity);
Instance const i = manager.addComponent(entity);
assert_invariant(i);
assert_invariant(i != parent);
@@ -96,7 +96,7 @@ void FTransformManager::setParent(Instance i, Instance parent) noexcept {
validateNode(i);
if (i) {
auto& manager = mManager;
Instance oldParent = manager[i].parent;
Instance const oldParent = manager[i].parent;
if (oldParent != parent) {
// TODO: on debug builds, ensure that the new parent isn't one of our descendant
removeNode(i);
@@ -144,7 +144,7 @@ TransformManager::children_iterator FTransformManager::getChildrenEnd(Instance)
void FTransformManager::destroy(Entity e) noexcept {
// update the reference of the element we're removing
auto& manager = mManager;
Instance i = manager.getInstance(e);
Instance const i = manager.getInstance(e);
validateNode(i);
if (i) {
// 1) remove the entry from the linked lists
@@ -158,7 +158,7 @@ void FTransformManager::destroy(Entity e) noexcept {
}
// 2) remove the component
Instance moved = manager.removeComponent(e);
Instance const moved = manager.removeComponent(e);
// 3) update the references to the entry now with Instance i
if (moved != i) {
@@ -200,14 +200,15 @@ void FTransformManager::updateNodeTransform(Instance i) noexcept {
// find our parent's world transform, if any
// note: by using the raw_array() we don't need to check that parent is valid.
Instance parent = manager[i].parent;
computeWorldTransform(manager[i].world, manager[i].worldTranslationLo,
Instance const parent = manager[i].parent;
FTransformManager::computeWorldTransform(
manager[i].world, manager[i].worldTranslationLo,
manager[parent].world, manager[i].local,
manager[parent].worldTranslationLo, manager[i].localTranslationLo,
mAccurateTranslations);
// update our children's world transforms
Instance child = manager[i].firstChild;
Instance const child = manager[i].firstChild;
if (UTILS_UNLIKELY(child)) { // assume we don't have a hierarchy in the common case
transformChildren(manager, child);
}
@@ -237,10 +238,11 @@ void FTransformManager::computeAllWorldTransforms() noexcept {
while (UTILS_UNLIKELY(Instance(manager[i].parent) > i)) {
swapNode(i, manager[i].parent);
}
Instance parent = manager[i].parent;
Instance const parent = manager[i].parent;
assert_invariant(parent < i);
computeWorldTransform(manager[i].world, manager[i].worldTranslationLo,
FTransformManager::computeWorldTransform(
manager[i].world, manager[i].worldTranslationLo,
manager[parent].world, manager[i].local,
manager[parent].worldTranslationLo, manager[i].localTranslationLo,
accurate);
@@ -258,7 +260,7 @@ void FTransformManager::insertNode(Instance i, Instance parent) noexcept {
manager[i].next = 0;
if (parent) {
// we insert ourselves first in the parent's list
Instance next = manager[parent].firstChild;
Instance const next = manager[parent].firstChild;
manager[i].next = next;
// we're our parent's first child now
manager[parent].firstChild = i;
@@ -279,8 +281,10 @@ void FTransformManager::swapNode(Instance i, Instance j) noexcept {
auto& manager = mManager;
// swap the content of the nodes directly
std::swap(manager.elementAt<LOCAL>(i), manager.elementAt<LOCAL>(j));
std::swap(manager.elementAt<WORLD>(i), manager.elementAt<WORLD>(j));
std::swap(manager.elementAt<LOCAL>(i), manager.elementAt<LOCAL>(j));
std::swap(manager.elementAt<LOCAL_LO>(i), manager.elementAt<LOCAL_LO>(j));
std::swap(manager.elementAt<WORLD>(i), manager.elementAt<WORLD>(j));
std::swap(manager.elementAt<WORLD_LO>(i), manager.elementAt<WORLD_LO>(j));
manager.swap(i, j); // this swaps the data relative to SingleInstanceComponentManager
// now swap the linked-list references, to do that correctly we must use a temporary
@@ -310,13 +314,13 @@ void FTransformManager::swapNode(Instance i, Instance j) noexcept {
updateNode(j);
}
// removes an node from the graph, but doesn't removes it or its children from the array
// removes a node from the graph, but doesn't remove it or its children from the array
// (making everybody orphaned).
void FTransformManager::removeNode(Instance i) noexcept {
auto& manager = mManager;
Instance parent = manager[i].parent;
Instance prev = manager[i].prev;
Instance next = manager[i].next;
Instance const parent = manager[i].parent;
Instance const prev = manager[i].prev;
Instance const next = manager[i].next;
if (prev) {
manager[prev].next = next;
} else if (parent) {
@@ -341,9 +345,9 @@ void FTransformManager::removeNode(Instance i) noexcept {
void FTransformManager::updateNode(Instance i) noexcept {
auto& manager = mManager;
// update our preview sibling's next reference (to ourselves)
Instance parent = manager[i].parent;
Instance prev = manager[i].prev;
Instance next = manager[i].next;
Instance const parent = manager[i].parent;
Instance const prev = manager[i].prev;
Instance const next = manager[i].next;
if (prev) {
manager[prev].next = i;
} else if (parent) {
@@ -371,14 +375,15 @@ void FTransformManager::transformChildren(Sim& manager, Instance i) noexcept {
const bool accurate = mAccurateTranslations;
while (i) {
// update child's world transform
Instance parent = manager[i].parent;
computeWorldTransform(manager[i].world, manager[i].worldTranslationLo,
Instance const parent = manager[i].parent;
FTransformManager::computeWorldTransform(
manager[i].world, manager[i].worldTranslationLo,
manager[parent].world, manager[i].local,
manager[parent].worldTranslationLo, manager[i].localTranslationLo,
accurate);
// assume we don't have a deep hierarchy
Instance child = manager[i].firstChild;
Instance const child = manager[i].firstChild;
if (UTILS_UNLIKELY(child)) {
transformChildren(manager, child);
}
@@ -423,14 +428,14 @@ void FTransformManager::computeWorldTransform(
}
void FTransformManager::validateNode(Instance i) noexcept {
void FTransformManager::validateNode(UTILS_UNUSED_IN_RELEASE Instance i) noexcept {
#ifndef NDEBUG
auto& manager = mManager;
if (i) {
Instance parent = manager[i].parent;
Instance firstChild = manager[i].firstChild;
Instance prev = manager[i].prev;
Instance next = manager[i].next;
Instance const parent = manager[i].parent;
Instance const firstChild = manager[i].firstChild;
Instance const prev = manager[i].prev;
Instance const next = manager[i].next;
assert_invariant(parent != i);
assert_invariant(prev != i);
assert_invariant(next != i);

View File

@@ -130,7 +130,7 @@ private:
void computeAllWorldTransforms() noexcept;
void computeWorldTransform(math::mat4f& outWorld, math::float3& inoutWorldTranslationLo,
static void computeWorldTransform(math::mat4f& outWorld, math::float3& inoutWorldTranslationLo,
math::mat4f const& pt, math::mat4f const& local,
math::float3 const& ptTranslationLo, math::float3 const& localTranslationLo,
bool accurate);
@@ -166,7 +166,7 @@ private:
typename Base::SoA& getSoA() { return mData; }
struct Proxy {
// all of this gets inlined
// all of these gets inlined
UTILS_ALWAYS_INLINE
Proxy(Base& sim, utils::EntityInstanceBase::Type i) noexcept
: local{ sim, i } { }

View File

@@ -181,6 +181,7 @@ TEST(FilamentTest, SkinningMath) {
TEST(FilamentTest, TransformManager) {
filament::FTransformManager tcm;
tcm.setAccurateTranslationsEnabled(true);
EntityManager& em = EntityManager::get();
std::array<Entity, 3> entities;
em.create(entities.size(), entities.data());
@@ -245,8 +246,9 @@ TEST(FilamentTest, TransformManager) {
EXPECT_LT(tcm.getInstance(entities[1]), tcm.getInstance(entities[2]));
// local transaction reorders parent/child
auto const t = mat4::translation(double3(1.0 / 3.0));
tcm.openLocalTransformTransaction();
tcm.setTransform(newParent, mat4f{ float4{ 8 }});
tcm.setTransform(newParent, t);
tcm.commitLocalTransformTransaction();
// local transaction invalidates Instances
@@ -258,10 +260,14 @@ TEST(FilamentTest, TransformManager) {
EXPECT_GT(child, newParent);
// check transform propagation
EXPECT_EQ(tcm.getTransform(newParent), mat4f{ float4{ 8 }});
EXPECT_EQ(tcm.getWorldTransform(newParent), mat4f{ float4{ 8 }});
EXPECT_EQ(tcm.getTransform(child), mat4f{ float4{ 1 }});
EXPECT_EQ(tcm.getWorldTransform(child), mat4f{ float4{ 8 }});
// our "high precision" mode only preserves 48 bits of the double mantissa (out of 53).
// This constant loses 5 bits out of 1/3
const mat4 PRECISION_KILLER_5BITS = mat4::translation(double3(16.0));
EXPECT_EQ(tcm.getTransformAccurate(newParent) + PRECISION_KILLER_5BITS, t + PRECISION_KILLER_5BITS);
EXPECT_EQ(tcm.getWorldTransformAccurate(newParent) + PRECISION_KILLER_5BITS, t + PRECISION_KILLER_5BITS);
EXPECT_EQ(tcm.getWorldTransformAccurate(child) + PRECISION_KILLER_5BITS, t + PRECISION_KILLER_5BITS);
EXPECT_EQ(tcm.getTransformAccurate(child), mat4{ 1.0 });
// check children iterators
size_t c = 0;