From 857a71ad66367eae68d985da9fd175ba7c7bbe61 Mon Sep 17 00:00:00 2001 From: sean Date: Fri, 2 Feb 2024 17:26:57 +0100 Subject: [PATCH] Fix minor issues --- include/fastgltf/types.hpp | 4 +- src/fastgltf.cpp | 110 +++++++++++++++++++------------------ 2 files changed, 59 insertions(+), 55 deletions(-) diff --git a/include/fastgltf/types.hpp b/include/fastgltf/types.hpp index 3d6ee2ff7..c4405c9b5 100644 --- a/include/fastgltf/types.hpp +++ b/include/fastgltf/types.hpp @@ -990,7 +990,7 @@ namespace fastgltf { } template , int> = 0> - OptionalWithFlagValue(const OptionalWithFlagValue& other) { + OptionalWithFlagValue(const OptionalWithFlagValue& other) noexcept(std::is_nothrow_copy_constructible_v) { if (other.has_value()) { new (std::addressof(_value)) T(*other); } else { @@ -999,7 +999,7 @@ namespace fastgltf { } template , int> = 0> - OptionalWithFlagValue(OptionalWithFlagValue&& other) { + OptionalWithFlagValue(OptionalWithFlagValue&& other) noexcept(std::is_nothrow_move_constructible_v) { if (other.has_value()) { new (std::addressof(_value)) T(std::move(*other)); } else { diff --git a/src/fastgltf.cpp b/src/fastgltf.cpp index 6fdd62a0e..f4ad16369 100644 --- a/src/fastgltf.cpp +++ b/src/fastgltf.cpp @@ -205,7 +205,7 @@ namespace fastgltf { } template - [[nodiscard, gnu::always_inline]] Error copyNumericalJsonArray(simdjson::ondemand::array& array, std::array& target) { + [[nodiscard, gnu::always_inline]] inline Error copyNumericalJsonArray(simdjson::ondemand::array& array, std::array& target) { std::size_t i = 0; for (auto value : array) { if (i >= target.size()) { @@ -1327,9 +1327,8 @@ fg::Error fg::Parser::parseAccessors(simdjson::ondemand::array& accessors, Asset accessor.byteOffset = 0UL; accessor.normalized = false; - // Function for parsing the min and max arrays for accessors + // Function for parsing the min and max arrays for accessors auto parseMinMax = [&](simdjson::ondemand::array& array, decltype(Accessor::max)& ref) -> fastgltf::Error { - return Error::None; decltype(Accessor::max) variant; using double_vec = std::variant_alternative_t<1, decltype(Accessor::max)>; @@ -1338,62 +1337,67 @@ fg::Error fg::Parser::parseAccessors(simdjson::ondemand::array& accessors, Asset // It's possible the min/max fields come before the accessor type is declared, in which case we don't know the size. // This single line is actually incredibly important as this function without it takes up roughly 15% of the entire // parsing process on average due to vector resizing. - auto initialCount = accessor.type == AccessorType::Invalid ? 0 : getNumComponents(accessor.type); + // When no exact count is known (accessor type field comes after min/max fields) we'll just count them and take the penalty. + auto initialCount = accessor.type == AccessorType::Invalid ? array.count_elements().value() : getNumComponents(accessor.type); if (accessor.componentType == ComponentType::Float || accessor.componentType == ComponentType::Double) { - variant = FASTGLTF_CONSTRUCT_PMR_RESOURCE(double_vec, resourceAllocator.get(), 0); + auto vec = FASTGLTF_CONSTRUCT_PMR_RESOURCE(double_vec, resourceAllocator.get(), 0); + vec.reserve(initialCount); + variant = std::move(vec); } else { - variant = FASTGLTF_CONSTRUCT_PMR_RESOURCE(int64_vec, resourceAllocator.get(), 0); + auto vec = FASTGLTF_CONSTRUCT_PMR_RESOURCE(int64_vec, resourceAllocator.get(), 0); + vec.reserve(initialCount); + variant = std::move(vec); } - for (auto element : array) { - ondemand::number num; - if (auto error = element.get_number().get(num); error != SUCCESS) { - return error == INCORRECT_TYPE ? Error::InvalidGltf : Error::InvalidJson; - } - - switch (num.get_number_type()) { - case ondemand::number_type::floating_point_number: { - // We can't safely promote double to ints. Therefore, if the element is a double, - // but our component type is not a floating point, that's invalid. - if (accessor.componentType != ComponentType::Float && accessor.componentType != ComponentType::Double) { - return Error::InvalidGltf; - } + for (auto element : array) { + ondemand::number num; + if (auto error = element.get_number().get(num); error != SUCCESS) { + return error == INCORRECT_TYPE ? Error::InvalidGltf : Error::InvalidJson; + } - if (!std::holds_alternative(variant)) { - return Error::InvalidGltf; - } - std::get(variant).emplace_back(num.get_double()); - break; - } - case ondemand::number_type::signed_integer: { - std::int64_t value = num.get_int64(); + switch (num.get_number_type()) { + case ondemand::number_type::floating_point_number: { + // We can't safely promote double to ints. Therefore, if the element is a double, + // but our component type is not a floating point, that's invalid. + if (accessor.componentType != ComponentType::Float && accessor.componentType != ComponentType::Double) { + return Error::InvalidGltf; + } - if (auto* doubles = std::get_if(&variant); doubles != nullptr) { - (*doubles).emplace_back(static_cast(value)); - } else if (auto* ints = std::get_if(&variant); ints != nullptr) { - (*ints).emplace_back(static_cast(value)); - } else { - return Error::InvalidGltf; - } - break; - } - case ondemand::number_type::unsigned_integer: { - // Note that the glTF spec doesn't care about any integer larger than 32-bits, so - // truncating uint64 to int64 wouldn't make any difference, as those large values - // aren't allowed anyway. - std::uint64_t value = num.get_uint64(); - - if (auto* doubles = std::get_if(&variant); doubles != nullptr) { - (*doubles).emplace_back(static_cast(value)); - } else if (auto* ints = std::get_if(&variant); ints != nullptr) { - (*ints).emplace_back(static_cast(value)); - } else { - return Error::InvalidGltf; - } - break; - } - } - } + if (!std::holds_alternative(variant)) { + return Error::InvalidGltf; + } + std::get(variant).emplace_back(num.get_double()); + break; + } + case ondemand::number_type::signed_integer: { + std::int64_t value = num.get_int64(); + + if (auto* doubles = std::get_if(&variant); doubles != nullptr) { + (*doubles).emplace_back(static_cast(value)); + } else if (auto* ints = std::get_if(&variant); ints != nullptr) { + (*ints).emplace_back(static_cast(value)); + } else { + return Error::InvalidGltf; + } + break; + } + case ondemand::number_type::unsigned_integer: { + // Note that the glTF spec doesn't care about any integer larger than 32-bits, so + // truncating uint64 to int64 wouldn't make any difference, as those large values + // aren't allowed anyway. + std::uint64_t value = num.get_uint64(); + + if (auto* doubles = std::get_if(&variant); doubles != nullptr) { + (*doubles).emplace_back(static_cast(value)); + } else if (auto* ints = std::get_if(&variant); ints != nullptr) { + (*ints).emplace_back(static_cast(value)); + } else { + return Error::InvalidGltf; + } + break; + } + } + } ref = std::move(variant); return Error::None;