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.

Sign up for GitHub

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

SceneData field specification optimizations #542

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

SceneData field specification optimizations #542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1254c8c
Trade: allow omitting implicit SceneFieldData object mapping.
mosra Dec 6, 2021
3e28d8f
Trade: allow omitting trivial SceneField::Parent data.
mosra Dec 7, 2021
da27aac
[wip] the null fields/mappings are a PAIN
mosra Dec 7, 2021
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
21 changes: 21 additions & 0 deletions doc/snippets/MagnumTrade.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -915,6 +915,27 @@ Trade::SceneData{Trade::SceneMappingType::UnsignedInt, 120, std::move(data),
/* [SceneFieldData-usage-offset-only] */
}

{
/* [SceneFieldData-usage-implicit-mapping] */
Containers::ArrayView<Matrix4> transformations = DOXYGEN_ELLIPSIS({});

Trade::SceneFieldData field{Trade::SceneField::Transformation,
Containers::ArrayView<UnsignedInt>{nullptr, transformations.size()},
transformations,
Trade::SceneFieldFlag::ImplicitMapping};
/* [SceneFieldData-usage-implicit-mapping] */
}

{
std::size_t objectCount{};
/* [SceneFieldData-usage-trivial-parent] */
Trade::SceneFieldData field{Trade::SceneField::Parent,
Containers::ArrayView<UnsignedInt>{nullptr, objectCount},
Containers::ArrayView<Int>{nullptr, objectCount},
Trade::SceneFieldFlag::ImplicitMapping|Trade::SceneFieldFlag::TrivialField};
/* [SceneFieldData-usage-trivial-parent] */
}

{
typedef SceneGraph::Scene<SceneGraph::MatrixTransformation3D> Scene3D;
typedef SceneGraph::Object<SceneGraph::MatrixTransformation3D> Object3D;
Expand Down
2 changes: 2 additions & 0 deletions src/Magnum/SceneTools/Implementation/convertToSingleFunctionObjects.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ inline Trade::SceneData convertToSingleFunctionObjects(const Trade::SceneData& s
- fields that don't actually get their object mapping touched
during the process (and then all fields that share object
mapping with them) */
#warning removing implicit mapping from here will mean the null will get treated as a placeholder by copy(), not wanted
#warning it needs to restore the field instead
} else fields[i] = Trade::SceneFieldData{field.name(), field.mappingType(), field.mappingData(), field.fieldType(), field.fieldData(), field.fieldArraySize(), field.flags() & ~Trade::SceneFieldFlag::ImplicitMapping};
}

Expand Down
49 changes: 48 additions & 1 deletion src/Magnum/SceneTools/Test/CombineTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ struct CombineTest: TestSuite::Tester {
void objectsShared();
void objectsPlaceholderFieldPlaceholder();
void objectSharedFieldPlaceholder();

void implicitNullMapping();
void trivialNullParent();
};

struct {
Expand All @@ -60,7 +63,10 @@ CombineTest::CombineTest() {
addTests({&CombineTest::alignment,
&CombineTest::objectsShared,
&CombineTest::objectsPlaceholderFieldPlaceholder,
&CombineTest::objectSharedFieldPlaceholder});
&CombineTest::objectSharedFieldPlaceholder,

&CombineTest::implicitNullMapping,
&CombineTest::trivialNullParent});
}

using namespace Math::Literals;
Expand Down Expand Up @@ -324,6 +330,47 @@ void CombineTest::objectSharedFieldPlaceholder() {
CORRADE_COMPARE(scene.field(Trade::SceneField::MeshMaterial).stride()[0], 4);
}

void CombineTest::implicitNullMapping() {
const Short parentFieldData[]{-1, 0, 0};
const UnsignedByte meshFieldData[]{3, 5};

Trade::SceneData scene = Implementation::combine(Trade::SceneMappingType::UnsignedShort, 167, Containers::arrayView({
/* If the field has any flags, it shouldn't be treated as a
placeholder */
#warning or maybe it should be preserved? yeah
Trade::SceneFieldData{Trade::SceneField::Mesh, Containers::ArrayView<UnsignedByte>{nullptr, Containers::arraySize(meshFieldData)}, Containers::arrayView(meshFieldData), Trade::SceneFieldFlag::ImplicitMapping},
Trade::SceneFieldData{Trade::SceneField::Parent, Containers::ArrayView<UnsignedShort>{nullptr, Containers::arraySize(parentFieldData)}, Containers::arrayView(parentFieldData), Trade::SceneFieldFlag::ImplicitMapping}
}));

CORRADE_COMPARE(scene.mappingBound(), 167);
CORRADE_COMPARE(scene.fieldCount(), 2);

CORRADE_COMPARE(scene.fieldName(0), Trade::SceneField::Mesh);
CORRADE_COMPARE(scene.fieldFlags(0), Trade::SceneFieldFlag::ImplicitMapping);
CORRADE_COMPARE(scene.fieldType(0), Trade::SceneFieldType::UnsignedByte);
CORRADE_COMPARE(scene.fieldArraySize(0), 0);
CORRADE_COMPARE_AS(scene.mapping<UnsignedShort>(0), Containers::arrayView<UnsignedShort>({
0, 1, 2
}), TestSuite::Compare::Container);
CORRADE_COMPARE_AS(scene.field<UnsignedByte>(0),
Containers::arrayView(meshFieldData),
TestSuite::Compare::Container);

CORRADE_COMPARE(scene.fieldName(1), Trade::SceneField::Parent);
CORRADE_COMPARE(scene.fieldFlags(1), Trade::SceneFieldFlag::ImplicitMapping);
CORRADE_COMPARE(scene.fieldType(1), Trade::SceneFieldType::Short);
CORRADE_COMPARE(scene.fieldArraySize(1), 0);
CORRADE_COMPARE_AS(scene.mapping<UnsignedShort>(1), Containers::arrayView<UnsignedShort>({
0, 1
}), TestSuite::Compare::Container);
CORRADE_COMPARE_AS(scene.field<UnsignedByte>(1),
Containers::arrayView(meshFieldData),
TestSuite::Compare::Container);
}

void CombineTest::trivialNullParent() {
}

}}}}

CORRADE_TEST_MAIN(Magnum::SceneTools::Test::CombineTest)
88 changes: 73 additions & 15 deletions src/Magnum/Trade/SceneData.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,7 @@ Debug& operator<<(Debug& debug, const SceneFieldFlag value) {
_c(OffsetOnly)
_c(ImplicitMapping)
_c(OrderedMapping)
_c(TrivialField)
#undef _c
/* LCOV_EXCL_STOP */
}
Expand All @@ -497,7 +498,8 @@ Debug& operator<<(Debug& debug, const SceneFieldFlags value) {
SceneFieldFlag::OffsetOnly,
SceneFieldFlag::ImplicitMapping,
/* This one is implied by ImplicitMapping, so has to be after */
SceneFieldFlag::OrderedMapping
SceneFieldFlag::OrderedMapping,
SceneFieldFlag::TrivialField
});
}

Expand Down Expand Up @@ -594,21 +596,42 @@ SceneData::SceneData(const SceneMappingType mappingType, const UnsignedLong mapp
const UnsignedInt fieldTypeSize = sceneFieldTypeSize(field._fieldType)*
(field._fieldArraySize ? field._fieldArraySize : 1);
if(field._flags & SceneFieldFlag::OffsetOnly) {
const std::size_t mappingSize = field._mappingData.offset + (field._size - 1)*field._mappingStride + mappingTypeSize;
const std::size_t fieldSize = field._fieldData.offset + (field._size - 1)*field._fieldStride + fieldTypeSize;
CORRADE_ASSERT(mappingSize <= _data.size(),
"Trade::SceneData: offset-only mapping data of field" << i << "span" << mappingSize << "bytes but passed data array has only" << _data.size(), );
CORRADE_ASSERT(fieldSize <= _data.size(),
"Trade::SceneData: offset-only field data of field" << i << "span" << fieldSize << "bytes but passed data array has only" << _data.size(), );
/* If an offset-only field has an implicit mapping, we ignore
the offset / size completely */
if(!(field._flags >= SceneFieldFlag::ImplicitMapping)) {
const std::size_t mappingSize = field._mappingData.offset + (field._size - 1)*field._mappingStride + mappingTypeSize;
CORRADE_ASSERT(mappingSize <= _data.size(),
"Trade::SceneData: offset-only mapping data of field" << i << "span" << mappingSize << "bytes but passed data array has only" << _data.size(), );
}

/* If an offset-only field is trivial, we ignore the offset /
size completely. Trivial fields are whitelisted, which was
already checked in SceneFieldData constructor. */
if(!(field._flags >= SceneFieldFlag::TrivialField)) {
const std::size_t fieldSize = field._fieldData.offset + (field._size - 1)*field._fieldStride + fieldTypeSize;
CORRADE_ASSERT(fieldSize <= _data.size(),
"Trade::SceneData: offset-only field data of field" << i << "span" << fieldSize << "bytes but passed data array has only" << _data.size(), );
}

} else {
const void* const mappingBegin = field._mappingData.pointer;
const void* const fieldBegin = field._fieldData.pointer;
const void* const mappingEnd = static_cast<const char*>(field._mappingData.pointer) + (field._size - 1)*field._mappingStride + mappingTypeSize;
const void* const fieldEnd = static_cast<const char*>(field._fieldData.pointer) + (field._size - 1)*field._fieldStride + fieldTypeSize;
CORRADE_ASSERT(mappingBegin >= _data.begin() && mappingEnd <= _data.end(),
"Trade::SceneData: mapping data [" << Debug::nospace << mappingBegin << Debug::nospace << ":" << Debug::nospace << mappingEnd << Debug::nospace << "] of field" << i << "are not contained in passed data array [" << Debug::nospace << static_cast<const void*>(_data.begin()) << Debug::nospace << ":" << Debug::nospace << static_cast<const void*>(_data.end()) << Debug::nospace << "]", );
CORRADE_ASSERT(fieldBegin >= _data.begin() && fieldEnd <= _data.end(),
"Trade::SceneData: field data [" << Debug::nospace << fieldBegin << Debug::nospace << ":" << Debug::nospace << fieldEnd << Debug::nospace << "] of field" << i << "are not contained in passed data array [" << Debug::nospace << static_cast<const void*>(_data.begin()) << Debug::nospace << ":" << Debug::nospace << static_cast<const void*>(_data.end()) << Debug::nospace << "]", );
/* If a field has an implicit mapping, we allow it to be
nullptr */
if(!(field._flags >= SceneFieldFlag::ImplicitMapping && !field._mappingData.pointer)) {
const void* const mappingBegin = field._mappingData.pointer;
const void* const mappingEnd = static_cast<const char*>(field._mappingData.pointer) + (field._size - 1)*field._mappingStride + mappingTypeSize;
CORRADE_ASSERT(mappingBegin >= _data.begin() && mappingEnd <= _data.end(),
"Trade::SceneData: mapping data [" << Debug::nospace << mappingBegin << Debug::nospace << ":" << Debug::nospace << mappingEnd << Debug::nospace << "] of field" << i << "are not contained in passed data array [" << Debug::nospace << static_cast<const void*>(_data.begin()) << Debug::nospace << ":" << Debug::nospace << static_cast<const void*>(_data.end()) << Debug::nospace << "]", );
}

/* Trivial fields are allowed to be nullptr. Trivial fields are
whitelisted, which was already checked in SceneFieldData
constructor. */
if(!(field._flags >= SceneFieldFlag::TrivialField && !field._fieldData.pointer)) {
const void* const fieldBegin = field._fieldData.pointer;
const void* const fieldEnd = static_cast<const char*>(field._fieldData.pointer) + (field._size - 1)*field._fieldStride + fieldTypeSize;
CORRADE_ASSERT(fieldBegin >= _data.begin() && fieldEnd <= _data.end(),
"Trade::SceneData: field data [" << Debug::nospace << fieldBegin << Debug::nospace << ":" << Debug::nospace << fieldEnd << Debug::nospace << "] of field" << i << "are not contained in passed data array [" << Debug::nospace << static_cast<const void*>(_data.begin()) << Debug::nospace << ":" << Debug::nospace << static_cast<const void*>(_data.end()) << Debug::nospace << "]", );
}
}
}
#endif
Expand Down Expand Up @@ -806,6 +829,12 @@ Containers::ArrayView<char> SceneData::mutableData() & {

Containers::StridedArrayView1D<const void> SceneData::fieldDataMappingViewInternal(const SceneFieldData& field, const std::size_t offset, const std::size_t size) const {
CORRADE_INTERNAL_ASSERT(offset + size <= field._size);

/* If this is a offset-only field with implicit mapping, ignore the
offset/stride and always assume it's not present */
if(field._flags >= (SceneFieldFlag::OffsetOnly|SceneFieldFlag::ImplicitMapping))
return {{nullptr, ~std::size_t{}}, size, field._mappingStride};

return Containers::StridedArrayView1D<const void>{
/* We're *sure* the view is correct, so faking the view size */
{static_cast<const char*>(field._flags & SceneFieldFlag::OffsetOnly ?
Expand All @@ -821,6 +850,12 @@ Containers::StridedArrayView1D<const void> SceneData::fieldDataMappingViewIntern

Containers::StridedArrayView1D<const void> SceneData::fieldDataFieldViewInternal(const SceneFieldData& field, const std::size_t offset, const std::size_t size) const {
CORRADE_INTERNAL_ASSERT(offset + size <= field._size);

/* If this is a offset-only field that's trivial, ignore the offset/stride
and always assume it's not present */
if(field._flags >= (SceneFieldFlag::OffsetOnly|SceneFieldFlag::TrivialField))
return {{nullptr, ~std::size_t{}}, size, field._fieldStride};

return Containers::StridedArrayView1D<const void>{
/* We're *sure* the view is correct, so faking the view size */
{static_cast<const char*>(field._flags & SceneFieldFlag::OffsetOnly ?
Expand Down Expand Up @@ -1140,6 +1175,18 @@ void SceneData::mappingIntoInternal(const UnsignedInt fieldId, const std::size_t
checked by the callers */

const SceneFieldData& field = _fields[fieldId];

/* If we don't have any data for an implicit mapping or the implicit
mapping is offset-only (where we always assume there's no data),
generate the sequence */
if((field._flags >= SceneFieldFlag::ImplicitMapping && !field._mappingData.pointer) ||
(field._flags >= (SceneFieldFlag::ImplicitMapping|SceneFieldFlag::OffsetOnly)))
{
for(std::size_t i = 0; i != destination.size(); ++i)
destination[i] = offset + i;
return;
}

const Containers::StridedArrayView1D<const void> mappingData = fieldDataMappingViewInternal(field, offset, destination.size());
const auto destination1ui = Containers::arrayCast<2, UnsignedInt>(destination);

Expand Down Expand Up @@ -1210,6 +1257,17 @@ void SceneData::parentsIntoInternal(const UnsignedInt fieldId, const std::size_t
checked by the callers */

const SceneFieldData& field = _fields[fieldId];

/* If we don't have any data for a trivial parent or the trivial parent is
offset-only (where we always assume there's no data), generate the data */
if((field._flags >= SceneFieldFlag::TrivialField && !field._fieldData.pointer) ||
(field._flags >= (SceneFieldFlag::TrivialField|SceneFieldFlag::OffsetOnly)))
{
for(std::size_t i = 0; i != destination.size(); ++i)
destination[i] = -1;
return;
}

const Containers::StridedArrayView1D<const void> fieldData = fieldDataFieldViewInternal(field, offset, destination.size());
const auto destination1i = Containers::arrayCast<2, Int>(destination);

Expand Down
Loading