Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix matrix -> FTransform conversion for matrices with small scales #1599

Merged
merged 7 commits into from
Jan 31, 2025
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
##### Fixes :wrench:

- Fixed another bug in `CesiumSubLevelSwitcherComponent` that could prevent all sub-levels from loading if a single sub-level failed to load.
- Worked around a limitation in Unreal's `FMatrix` -> `FTransform` conversion that caused models using a small scale factor (such as where vertex positions are expressed in millimeters) to fail to render because their scale was treated as 0.0.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some slight rewording:

Suggested change
- Worked around a limitation in Unreal's `FMatrix` -> `FTransform` conversion that caused models using a small scale factor (such as where vertex positions are expressed in millimeters) to fail to render because their scale was treated as 0.0.
- Worked around a limitation in Unreal's `FMatrix` -> `FTransform` conversion that prevented models with a small scale factor (e.g., where vertex positions are expressed in millimeters) from rendering because their scale was treated as 0.0.

- Fixed crash when calling `SampleHeightMostDetailed` blueprint function without a valid tileset.

### v2.12.0 - 2025-01-02
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ void UCesiumBoundingVolumeComponent::UpdateOcclusion(
}

void UCesiumBoundingVolumeComponent::_updateTransform() {
const FTransform transform = FTransform(
VecMath::createMatrix(this->_cesiumToUnreal * this->_tileTransform));
const FTransform transform =
VecMath::createTransform(this->_cesiumToUnreal * this->_tileTransform);

this->SetRelativeTransform_Direct(transform);
this->SetComponentToWorld(transform);
Expand Down
3 changes: 1 addition & 2 deletions Source/CesiumRuntime/Private/CesiumGlobeAnchorComponent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -732,8 +732,7 @@ void UCesiumGlobeAnchorComponent::_updateFromNativeGlobeAnchor(
glm::dmat4 anchorToLocal = nativeAnchor.getAnchorToLocalTransform(
pGeoreference->GetCoordinateSystem());

this->_setCurrentRelativeTransform(
FTransform(VecMath::createMatrix(anchorToLocal)));
this->_setCurrentRelativeTransform(VecMath::createTransform(anchorToLocal));
} else {
this->_lastRelativeTransformIsValid = false;
}
Expand Down
3 changes: 1 addition & 2 deletions Source/CesiumRuntime/Private/CesiumGltfComponent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2040,8 +2040,7 @@ static void loadInstancingData(
for (int64_t i = 0; i < count; ++i) {
glm::dmat4 unrealMat =
yInvertMatrix * instanceTransforms[i] * yInvertMatrix;
auto unrealFMatrix = VecMath::createMatrix(unrealMat);
result.InstanceTransforms[i].SetFromMatrix(unrealFMatrix);
result.InstanceTransforms[i] = VecMath::createTransform(unrealMat);
}
if (pInstanceFeatures) {
result.pInstanceFeatures =
Expand Down
4 changes: 2 additions & 2 deletions Source/CesiumRuntime/Private/CesiumGltfPrimitiveComponent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ bool UpdateTransformFromCesiumAux(
const glm::dmat4& CesiumToUnrealTransform,
CesiumComponent* cesiumComponent) {
const CesiumPrimitiveData& primData = cesiumComponent->getPrimitiveData();
const FTransform transform = FTransform(VecMath::createMatrix(
CesiumToUnrealTransform * primData.HighPrecisionNodeTransform));
const FTransform transform = VecMath::createTransform(
CesiumToUnrealTransform * primData.HighPrecisionNodeTransform);

if (cesiumComponent->Mobility == EComponentMobility::Movable) {
// For movable objects, move the component in the normal way, but don't
Expand Down
5 changes: 2 additions & 3 deletions Source/CesiumRuntime/Private/CesiumSubLevelComponent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,7 @@ void UCesiumSubLevelComponent::PlaceOriginAtEcef(const FVector& NewOriginEcef) {
}

pOwner->Modify();
pOwner->SetActorTransform(
FTransform(VecMath::createMatrix(NewLevelTransform)));
pOwner->SetActorTransform(VecMath::createTransform(NewLevelTransform));

// Set the new sub-level georeference origin.
this->Modify();
Expand Down Expand Up @@ -355,7 +354,7 @@ void UCesiumSubLevelComponent::PlaceOriginAtEcef(const FVector& NewOriginEcef) {
Tileset->Modify();
Root->Modify();
Root->SetRelativeTransform(
FTransform(VecMath::createMatrix(RelativeTransformInNew)),
VecMath::createTransform(RelativeTransformInNew),
false,
nullptr,
ETeleportType::TeleportPhysics);
Expand Down
77 changes: 77 additions & 0 deletions Source/CesiumRuntime/Private/Tests/VecMath.spec.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// Copyright 2020-2025 CesiumGS, Inc. and Contributors

#include "VecMath.h"
#include "Misc/AutomationTest.h"

BEGIN_DEFINE_SPEC(
FVecMathSpec,
"Cesium.Unit.VecMath",
EAutomationTestFlags::EditorContext | EAutomationTestFlags::ClientContext |
EAutomationTestFlags::ServerContext |
EAutomationTestFlags::CommandletContext |
EAutomationTestFlags::ProductFilter)
END_DEFINE_SPEC(FVecMathSpec)

void FVecMathSpec::Define() {
Describe("createTransform", [this]() {
It("matches FMatrix -> FTransform for larger scales", [this]() {
FTransform original = FTransform(
FQuat::MakeFromRotator(FRotator(10.0, 20.0, 30.0)),
FVector(3000.0, 2000.0, 1000.0),
FVector(1.0, 2.0, 3.0));

FMatrix originalUnrealMatrix = original.ToMatrixWithScale();
glm::dmat4 originalGlmMatrix =
VecMath::createMatrix4D(originalUnrealMatrix);

FTransform viaUnrealMatrix =
FTransform(VecMath::createMatrix(originalGlmMatrix));
FTransform viaVecMath = VecMath::createTransform(originalGlmMatrix);

TestNearlyEqual(
TEXT("Translation"),
viaVecMath.GetTranslation(),
viaUnrealMatrix.GetTranslation(),
1e-8);
TestNearlyEqual(
TEXT("Rotation"),
viaVecMath.GetRotation().Rotator(),
viaUnrealMatrix.GetRotation().Rotator(),
1e-10);
TestNearlyEqual(
TEXT("Scale"),
viaVecMath.GetScale3D(),
viaUnrealMatrix.GetScale3D(),
1e-11);
});

It("returns correct values when scale is small", [this]() {
FTransform original = FTransform(
FQuat::MakeFromRotator(FRotator(10.0, 20.0, 30.0)),
FVector(3000.0, 2000.0, 1000.0),
FVector(1e-7, 2e-7, 3e-7));

FMatrix originalUnrealMatrix = original.ToMatrixWithScale();
glm::dmat4 originalGlmMatrix =
VecMath::createMatrix4D(originalUnrealMatrix);

FTransform viaVecMath = VecMath::createTransform(originalGlmMatrix);

TestNearlyEqual(
TEXT("Translation"),
viaVecMath.GetTranslation(),
original.GetTranslation(),
1e-8);
TestNearlyEqual(
TEXT("Rotation"),
viaVecMath.GetRotation().Rotator(),
original.GetRotation().Rotator(),
1e-10);
TestNearlyEqual(
TEXT("Scale"),
viaVecMath.GetScale3D(),
original.GetScale3D(),
1e-18);
});
});
}
19 changes: 18 additions & 1 deletion Source/CesiumRuntime/Private/VecMath.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

#include "VecMath.h"

#include "CesiumUtility/Math.h"
#include "Math/Quat.h"
#include "Math/RotationMatrix.h"
#include <CesiumGeometry/Transforms.h>
#include <CesiumUtility/Math.h>
#include <glm/gtc/quaternion.hpp>

glm::dmat4 VecMath::createMatrix4D(const FMatrix& m) noexcept {
Expand Down Expand Up @@ -123,6 +124,22 @@ FMatrix VecMath::createMatrix(const glm::dmat4& m) noexcept {
FVector(m[3].x, m[3].y, m[3].z));
}

FTransform VecMath::createTransform(const glm::dmat4& m) noexcept {
glm::dvec3 translation;
glm::dquat rotation;
glm::dvec3 scale;
CesiumGeometry::Transforms::computeTranslationRotationScaleFromMatrix(
m,
&translation,
&rotation,
&scale);

return FTransform(
VecMath::createQuaternion(rotation),
VecMath::createVector(translation),
VecMath::createVector(scale));
}

FMatrix VecMath::createMatrix(const glm::dmat3& m) noexcept {
return FMatrix(
FVector(m[0].x, m[0].y, m[0].z),
Expand Down
11 changes: 11 additions & 0 deletions Source/CesiumRuntime/Private/VecMath.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,22 @@ class VecMath {
/**
* @brief Create a `FMatrix` from the given `glm` matrix.
*
* If the ultimate goal is to create an `FTransform`, use
* {@link createTransform} instead.
*
* @param m The `glm` matrix.
* @return The `FMatrix`.
*/
static FMatrix createMatrix(const glm::dmat4& m) noexcept;

/**
* @brief Create a `FTransform` from the given `glm` matrix.
*
* @param m The `glm` matrix.
* @return The `FTransform`.
*/
static FTransform createTransform(const glm::dmat4& m) noexcept;

/**
* @brief Create a `FMatrix` from the given `glm` columns
*
Expand Down