ic
/
magnum
mirror of https://github.com/mosra/magnum.git
6
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

Trade: drop assertions for 64-bit object/parent values from SceneData. 

This has to be solved on a more systematic level, perhaps even by
switching all types to be 64-bit. In the following commit all *AsArray()
and *Int() functions will output the object IDs as well, meaning this
would need to be handled in each and every API, which is a huge
maintenance burden.

As it's very unlikely that there actually will *ever* be >4G objects,
one possible option would be to introduce some "object ID hash" field
that would provide (contiguous?) remapping of the object ID to 32-bit
values, and the Into() and AsArray() accessors would return this
remapping instead of the original. But then again it'd cause issues with
for example animation references that would still reference the original
64-bit value.
pull/525/head
Vladimír Vondruš 5 years ago
parent
98e023b0b5
commit
2b409ee0ba
3 changed files with 4 additions and 94 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 2
      src/Magnum/Trade/SceneData.cpp
  2. 28
      src/Magnum/Trade/SceneData.h
  3. 68
      src/Magnum/Trade/Test/SceneDataTest.cpp

2
src/Magnum/Trade/SceneData.cpp
Unescape View File

@ -1088,7 +1088,6 @@ void SceneData::objectsIntoInternal(const UnsignedInt fieldId, const std::size_t
else if(field._objectType == SceneObjectType::UnsignedByte)
Math::castInto(Containers::arrayCast<2, const UnsignedByte>(objectData, 1), destination1ui);
else if(field._objectType == SceneObjectType::UnsignedLong) {
CORRADE_ASSERT(_objectCount <= 0xffffffffull, "Trade::SceneData::objectsInto(): indices for up to" << _objectCount << "objects can't fit into a 32-bit type, access them directly via objects() instead", );
Math::castInto(Containers::arrayCast<2, const UnsignedLong>(objectData, 1), destination1ui);
} else CORRADE_INTERNAL_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */
}
@ -1159,7 +1158,6 @@ void SceneData::parentsIntoInternal(const UnsignedInt fieldId, const std::size_t
else if(field._fieldType == SceneFieldType::Byte)
Math::castInto(Containers::arrayCast<2, const Byte>(fieldData, 1), destination1i);
else if(field._fieldType == SceneFieldType::Long) {
CORRADE_ASSERT(field._size <= 0xffffffffull, "Trade::SceneData::parentsInto(): parent indices for up to" << field._size << "objects can't fit into a 32-bit type, access them directly via field() instead", );
Math::castInto(Containers::arrayCast<2, const Long>(fieldData, 1), destination1i);
} else CORRADE_INTERNAL_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */
}

28
src/Magnum/Trade/SceneData.h
Unescape View File

@ -60,16 +60,7 @@ enum class SceneObjectType: UnsignedByte {
UnsignedByte = 1, /**< @relativeref{Magnum,UnsignedByte} */
UnsignedShort, /**< @relativeref{Magnum,UnsignedShort} */
UnsignedInt, /**< @relativeref{Magnum,UnsignedInt} */
/**
* @relativeref{Magnum,UnsignedLong}. Meant to be used only in rare cases
* for *really huge* scenes. If this type is used and
* @ref SceneData::objectCount() is larger than the max representable
* 32-bit value, the indices can't be retrieved using
* @ref SceneData::objectsAsArray(), but only using appropriately typed
* @ref SceneData::objects().
*/
UnsignedLong
UnsignedLong /**< @relativeref{Magnum,UnsignedLong} */
};
/**
@ -109,7 +100,7 @@ enum class SceneField: UnsignedInt {
/**
* Parent index. Type is usually @ref SceneFieldType::Int, but can be also
* any of @relativeref{SceneFieldType,Byte},
* @relativeref{SceneFieldType,Short} or, rarely, a
* @relativeref{SceneFieldType,Short} or a
* @relativeref{SceneFieldType,Long}. A value of @cpp -1 @ce means there's
* no parent. An object should have only one parent, altough this isn't
* enforced in any way, and which of the duplicate fields gets used is not
@ -1483,11 +1474,6 @@ class MAGNUM_TRADE_EXPORT SceneData {
* @ref objects(UnsignedInt) const that converts the field from an
* arbitrary underlying type and returns it in a newly-allocated array.
* The @p fieldId is expected to be smaller than @ref fieldCount().
* @attention In the rare case when @ref objectType() is
* @ref SceneObjectType::UnsignedLong and @ref objectCount() is
* larger than the max representable 32-bit value, this function
* can't be used, only an appropriately typed
* @ref objects(UnsignedInt) const.
* @see @ref objectsInto(UnsignedInt, const Containers::StridedArrayView1D<UnsignedInt>&) const
*/
Containers::Array<UnsignedInt> objectsAsArray(UnsignedInt fieldId) const;
@ -1525,11 +1511,6 @@ class MAGNUM_TRADE_EXPORT SceneData {
* @ref objects(SceneField) const that converts the field from an
* arbitrary underlying type and returns it in a newly-allocated array.
* The @p fieldName is expected to exist.
* @attention In the rare case when @ref objectType() is
* @ref SceneObjectType::UnsignedLong and @ref objectCount() is
* larger than the max representable 32-bit value, this function
* can't be used, only an appropriately typed
* @ref objects(SceneField) const.
* @see @ref objectsInto(SceneField, const Containers::StridedArrayView1D<UnsignedInt>&) const,
* @ref hasField()
*/
@ -1568,11 +1549,6 @@ class MAGNUM_TRADE_EXPORT SceneData {
* @ref SceneField::Parent as the argument that converts the field from
* an arbitrary underlying type and returns it in a newly-allocated
* array. The field is expected to exist.
* @attention In the rare case when @ref fieldType(SceneField) const is
* @ref SceneFieldType::Long and @ref fieldSize(SceneField) const
* is larger than the max representable 32-bit value, this
* function can't be used, only an appropriately typed
* @ref field(SceneField) const.
* @see @ref parentsInto(), @ref hasField(), @ref parentFor(),
* @ref childrenFor()
*/

68
src/Magnum/Trade/Test/SceneDataTest.cpp
Unescape View File

@ -119,14 +119,10 @@ struct SceneDataTest: TestSuite::Tester {
template<class T> void objectsAsArrayByIndex();
template<class T> void objectsAsArrayByName();
void objectsAsArrayLongType();
void objectsIntoArrayByIndex();
void objectsIntoArrayByName();
void objectsIntoArrayInvalidSizeOrOffset();
template<class T> void parentsAsArray();
#ifndef CORRADE_TARGET_32BIT
void parentsAsArrayLongType();
#endif
void parentsIntoArray();
void parentsIntoArrayInvalidSizeOrOffset();
template<class T> void transformations2DAsArray();
@ -316,8 +312,7 @@ SceneDataTest::SceneDataTest() {
&SceneDataTest::objectsAsArrayByName<UnsignedByte>,
&SceneDataTest::objectsAsArrayByName<UnsignedShort>,
&SceneDataTest::objectsAsArrayByName<UnsignedInt>,
&SceneDataTest::objectsAsArrayByName<UnsignedLong>,
&SceneDataTest::objectsAsArrayLongType});
&SceneDataTest::objectsAsArrayByName<UnsignedLong>});
addInstancedTests({&SceneDataTest::objectsIntoArrayByIndex,
&SceneDataTest::objectsIntoArrayByName},
@ -327,11 +322,7 @@ SceneDataTest::SceneDataTest() {
&SceneDataTest::parentsAsArray<Byte>,
&SceneDataTest::parentsAsArray<Short>,
&SceneDataTest::parentsAsArray<Int>,
&SceneDataTest::parentsAsArray<Long>,
#ifndef CORRADE_TARGET_32BIT
&SceneDataTest::parentsAsArrayLongType,
#endif
});
&SceneDataTest::parentsAsArray<Long>});
addInstancedTests({&SceneDataTest::parentsIntoArray},
Containers::arraySize(IntoArrayOffsetData));
@ -2233,33 +2224,6 @@ template<class T> void SceneDataTest::objectsAsArrayByName() {
TestSuite::Compare::Container);
}
void SceneDataTest::objectsAsArrayLongType() {
#ifdef CORRADE_NO_ASSERT
CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
#endif
struct Field {
UnsignedLong object;
UnsignedByte mesh;
} fields[3]{};
Containers::StridedArrayView1D<Field> view = fields;
SceneData scene{SceneObjectType::UnsignedLong, 0x100000000ull, {}, fields, {
SceneFieldData{SceneField::Mesh, view.slice(&Field::object), view.slice(&Field::mesh)}
}};
/* AsArray calls into IntoArray, which then has the assert, so this tests
both */
std::ostringstream out;
Error redirectError{&out};
scene.objectsAsArray(0);
scene.objectsAsArray(SceneField::Mesh);
CORRADE_COMPARE(out.str(),
"Trade::SceneData::objectsInto(): indices for up to 4294967296 objects can't fit into a 32-bit type, access them directly via objects() instead\n"
"Trade::SceneData::objectsInto(): indices for up to 4294967296 objects can't fit into a 32-bit type, access them directly via objects() instead\n");
}
void SceneDataTest::objectsIntoArrayByIndex() {
auto&& data = IntoArrayOffsetData[testCaseInstanceId()];
setTestCaseDescription(data.name);
@ -2407,34 +2371,6 @@ template<class T> void SceneDataTest::parentsAsArray() {
TestSuite::Compare::Container);
}
#ifndef CORRADE_TARGET_32BIT
void SceneDataTest::parentsAsArrayLongType() {
#ifdef CORRADE_NO_ASSERT
CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
#endif
struct Field {
UnsignedLong object;
Long parent;
};
Containers::Array<char> data{nullptr, 0x100000000ull*sizeof(Field), [](char*, std::size_t) {}};
Containers::StridedArrayView1D<Field> view = Containers::arrayCast<Field>(data);
SceneData scene{SceneObjectType::UnsignedLong, 0x100000000ull, std::move(data), {
SceneFieldData{SceneField::Parent, view.slice(&Field::object), view.slice(&Field::parent)}
}};
/* AsArray calls into IntoArray, which then has the assert, so this tests
both */
std::ostringstream out;
Error redirectError{&out};
scene.parentsAsArray();
CORRADE_COMPARE(out.str(),
"Trade::SceneData::parentsInto(): parent indices for up to 4294967296 objects can't fit into a 32-bit type, access them directly via field() instead\n");
}
#endif
void SceneDataTest::parentsIntoArray() {
auto&& data = IntoArrayOffsetData[testCaseInstanceId()];
setTestCaseDescription(data.name);

Write Preview
Loading…
Cancel
Save