Skip to content

Commit

Permalink
Fix: Move TRS struct out of Node
Browse files Browse the repository at this point in the history
This caused compile issues on GCC, as TRS was not recognized as a default initialisable type.
  • Loading branch information
spnda committed Jan 5, 2024
1 parent eee2b0e commit 159f6fe
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 12 deletions.
5 changes: 5 additions & 0 deletions cmake/compiler_flags.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ macro(fastgltf_compiler_flags TARGET)

# https://github.com/simdjson/simdjson/blob/master/doc/basics.md#performance-tips
target_compile_options(${TARGET} PRIVATE $<$<CONFIG:RELEASE>:-DNDEBUG>)

if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
# For the conversion of ARM Neon vectors (say int16x8_t to int8x16_t)
target_compile_options(${TARGET} PRIVATE -flax-vector-conversions)
endif()
endif()
endif()
endmacro()
Expand Down
11 changes: 6 additions & 5 deletions include/fastgltf/types.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1336,6 +1336,12 @@ namespace fastgltf {
FASTGLTF_STD_PMR_NS::string name;
};

struct TRS {
std::array<num, 3> translation = {{ 0.f, 0.f, 0.f }};
std::array<num, 4> rotation = {{ 0.f, 0.f, 0.f, 1.f }};
std::array<num, 3> scale = {{ 1.f, 1.f, 1.f }};
};

struct Node {
Optional<std::size_t> meshIndex;
Optional<std::size_t> skinIndex;
Expand All @@ -1349,11 +1355,6 @@ namespace fastgltf {
FASTGLTF_FG_PMR_NS::MaybeSmallVector<std::size_t> children;
FASTGLTF_FG_PMR_NS::MaybeSmallVector<num> weights;

struct TRS {
std::array<num, 3> translation = {{ 0.f, 0.f, 0.f }};
std::array<num, 4> rotation = {{ 0.f, 0.f, 0.f, 1.f }};
std::array<num, 3> scale = {{ 1.f, 1.f, 1.f }};
};
using TransformMatrix = std::array<num, 16>;

/**
Expand Down
6 changes: 3 additions & 3 deletions src/fastgltf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -998,7 +998,7 @@ fg::Error fg::validate(const fastgltf::Asset& asset) {
if (node.meshIndex.has_value() && asset.meshes.size() <= node.meshIndex.value())
return Error::InvalidGltf;

if (const auto* pTRS = std::get_if<Node::TRS>(&node.transform)) {
if (const auto* pTRS = std::get_if<TRS>(&node.transform)) {
for (const auto& x : pTRS->rotation)
if (x > 1.0 || x < -1.0)
return Error::InvalidGltf;
Expand Down Expand Up @@ -2897,14 +2897,14 @@ fg::Error fg::Parser::parseNodes(simdjson::dom::array& nodes, Asset& asset) {
}

if (hasBit(options, Options::DecomposeNodeMatrices)) {
Node::TRS trs = {};
TRS trs = {};
decomposeTransformMatrix(transformMatrix, trs.scale, trs.rotation, trs.translation);
node.transform = trs;
} else {
node.transform = transformMatrix;
}
} else if (error == NO_SUCH_FIELD) {
Node::TRS trs = {};
TRS trs = {};

// There's no matrix, let's see if there's scale, rotation, or rotation fields.
if (auto error = nodeObject["scale"].get_array().get(array); error == SUCCESS) {
Expand Down
8 changes: 4 additions & 4 deletions tests/basic_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ TEST_CASE("Loading some basic glTF", "[gltf-loader]") {

REQUIRE(cube->nodes.size() == 1);
REQUIRE(cube->nodes.front().name == "Cube");
REQUIRE(std::holds_alternative<fastgltf::Node::TRS>(cube->nodes.front().transform));
REQUIRE(std::holds_alternative<fastgltf::TRS>(cube->nodes.front().transform));

REQUIRE(cube->accessors.size() == 5);
REQUIRE(cube->accessors[0].type == fastgltf::AccessorType::Scalar);
Expand Down Expand Up @@ -384,10 +384,10 @@ TEST_CASE("Test TRS parsing and optional decomposition", "[gltf-loader]") {
REQUIRE(assetWithMatrix->nodes.size() == 2);
REQUIRE(assetDecomposed->nodes.size() == 2);
REQUIRE(std::holds_alternative<fastgltf::Node::TransformMatrix>(assetWithMatrix->nodes.back().transform));
REQUIRE(std::holds_alternative<fastgltf::Node::TRS>(assetDecomposed->nodes.back().transform));
REQUIRE(std::holds_alternative<fastgltf::TRS>(assetDecomposed->nodes.back().transform));

// Get the TRS components from the first node and use them as the test data for decomposing.
const auto* pDefaultTRS = std::get_if<fastgltf::Node::TRS>(&assetWithMatrix->nodes.front().transform);
const auto* pDefaultTRS = std::get_if<fastgltf::TRS>(&assetWithMatrix->nodes.front().transform);
REQUIRE(pDefaultTRS != nullptr);
auto translation = glm::make_vec3(pDefaultTRS->translation.data());
auto rotation = glm::make_quat(pDefaultTRS->rotation.data());
Expand All @@ -401,7 +401,7 @@ TEST_CASE("Test TRS parsing and optional decomposition", "[gltf-loader]") {
REQUIRE(glm::make_mat4x4(pMatrix->data()) == transform);

// Check if the decomposed components equal the original components.
const auto* pDecomposedTRS = std::get_if<fastgltf::Node::TRS>(&assetDecomposed->nodes.back().transform);
const auto* pDecomposedTRS = std::get_if<fastgltf::TRS>(&assetDecomposed->nodes.back().transform);
REQUIRE(glm::make_vec3(pDecomposedTRS->translation.data()) == translation);
REQUIRE(glm::make_quat(pDecomposedTRS->rotation.data()) == rotation);
REQUIRE(glm::make_vec3(pDecomposedTRS->scale.data()) == scale);
Expand Down

0 comments on commit 159f6fe

Please sign in to comment.