From b34468146dbb3d6b1908911aad3082f1c73a5275 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 16 May 2023 12:05:58 +0200 Subject: [PATCH] Trade: SceneFieldData::size() should be a std::size_t, not UnsignedLong. Not sure what was I thinking here -- if a field size wouldn't fit into a 32bit number, it won't fit into memory of a 32bit system anyway, so there's no real use for the size to be returned as 64bit always. Internally it *is* stored as a 64bit number, yes, to have compatible binary layout on 32bit and 64bit systems, but that doesn't mean the public API should return that too. And SceneData::fieldSize() is std::size_t, so this feels really like an accidental brainfart. The changed return type also means a lot of existing code doesn't need to do any explicit casting to std::size_t anymore. Yay. --- src/Magnum/SceneTools/Implementation/combine.h | 10 +++++----- .../Implementation/convertToSingleFunctionObjects.h | 6 +++--- src/Magnum/Trade/SceneData.h | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Magnum/SceneTools/Implementation/combine.h b/src/Magnum/SceneTools/Implementation/combine.h index 2f6d7d52b..4a2bcd1ef 100644 --- a/src/Magnum/SceneTools/Implementation/combine.h +++ b/src/Magnum/SceneTools/Implementation/combine.h @@ -159,7 +159,7 @@ inline Trade::SceneData combineFields(const Trade::SceneMappingType mappingType, sharedTrsMapping = itemViewOffset; arrayAppend(items, InPlaceInit, NoInit, - std::size_t(fields[sharedSceneFieldIds.trs[0]].size()), + fields[sharedSceneFieldIds.trs[0]].size(), mappingTypeSize, mappingTypeAlignment, itemViews[itemViewOffset].types); @@ -170,7 +170,7 @@ inline Trade::SceneData combineFields(const Trade::SceneMappingType mappingType, sharedMeshMaterialMapping = itemViewOffset; arrayAppend(items, InPlaceInit, NoInit, - std::size_t(fields[sharedSceneFieldIds.meshMaterial[0]].size()), + fields[sharedSceneFieldIds.meshMaterial[0]].size(), mappingTypeSize, mappingTypeAlignment, itemViews[itemViewOffset].types); @@ -220,7 +220,7 @@ inline Trade::SceneData combineFields(const Trade::SceneMappingType mappingType, itemViewMappings[i].first() = itemViewOffset; arrayAppend(items, InPlaceInit, NoInit, - std::size_t(field.size()), + field.size(), mappingTypeSize, mappingTypeAlignment, itemViews[itemViewOffset].types); @@ -236,13 +236,13 @@ inline Trade::SceneData combineFields(const Trade::SceneMappingType mappingType, if(fieldType == Trade::SceneFieldType::Bit) { arrayAppend(items, InPlaceInit, NoInit, - Containers::Size2D{std::size_t(field.size()), field.fieldArraySize() ? field.fieldArraySize() : 1}, + Containers::Size2D{field.size(), field.fieldArraySize() ? field.fieldArraySize() : 1}, itemViews[itemViewOffset].bits); ++itemViewOffset; } else { arrayAppend(items, InPlaceInit, NoInit, - std::size_t(field.size()), sceneFieldTypeSize(fieldType)*(field.fieldArraySize() ? field.fieldArraySize() : 1), + field.size(), sceneFieldTypeSize(fieldType)*(field.fieldArraySize() ? field.fieldArraySize() : 1), sceneFieldTypeAlignment(fieldType), itemViews[itemViewOffset].types); ++itemViewOffset; diff --git a/src/Magnum/SceneTools/Implementation/convertToSingleFunctionObjects.h b/src/Magnum/SceneTools/Implementation/convertToSingleFunctionObjects.h index c73934338..e827abb5f 100644 --- a/src/Magnum/SceneTools/Implementation/convertToSingleFunctionObjects.h +++ b/src/Magnum/SceneTools/Implementation/convertToSingleFunctionObjects.h @@ -124,11 +124,11 @@ inline Trade::SceneData convertToSingleFunctionObjects(const Trade::SceneData& s /** @todo wow this placeholder construction is HIDEOUS */ fields[i] = Trade::SceneFieldData{field.name(), field.mappingType(), - Containers::ArrayView{nullptr, std::size_t(field.size() + fieldsToCopyAdditionCount[*fieldToCopy])}, + Containers::ArrayView{nullptr, field.size() + fieldsToCopyAdditionCount[*fieldToCopy]}, field.fieldType(), Containers::StridedArrayView1D{ {nullptr, ~std::size_t{}}, - std::size_t(field.size() + fieldsToCopyAdditionCount[*fieldToCopy]), + field.size() + fieldsToCopyAdditionCount[*fieldToCopy], std::ptrdiff_t((field.fieldArraySize() ? field.fieldArraySize() : 1)*sceneFieldTypeSize(field.fieldType())) }, field.fieldArraySize(), @@ -140,7 +140,7 @@ inline Trade::SceneData convertToSingleFunctionObjects(const Trade::SceneData& s } else if(field.name() == Trade::SceneField::Parent) { /** @todo some nicer constructor for placeholders once this is in public interest */ - fields[i] = Trade::SceneFieldData{Trade::SceneField::Parent, Containers::ArrayView{nullptr, std::size_t(field.size() + objectsToAdd)}, Containers::ArrayView{nullptr, std::size_t(field.size() + objectsToAdd)}, + fields[i] = Trade::SceneFieldData{Trade::SceneField::Parent, Containers::ArrayView{nullptr, field.size() + objectsToAdd}, Containers::ArrayView{nullptr, field.size() + objectsToAdd}, /* If the field is ordered, we preserve that. But if it's implicit, we can't. */ field.flags() & ~(Trade::SceneFieldFlag::ImplicitMapping & ~Trade::SceneFieldFlag::OrderedMapping) diff --git a/src/Magnum/Trade/SceneData.h b/src/Magnum/Trade/SceneData.h index 8eaf31cde..e514d7f14 100644 --- a/src/Magnum/Trade/SceneData.h +++ b/src/Magnum/Trade/SceneData.h @@ -1324,7 +1324,7 @@ class MAGNUM_TRADE_EXPORT SceneFieldData { constexpr SceneField name() const { return _name; } /** @brief Number of entries */ - constexpr UnsignedLong size() const { return _size; } + constexpr std::size_t size() const { return _size; } /** @brief Object mapping type */ constexpr SceneMappingType mappingType() const {