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.
I'm seeing an assert for null string data not being correctly thrown in
a SceneTools API on ARM64 and an overflow of this 48-bit offset seems to
be a culprit. So better have that covered in the constructor already.
It'll be soon needed by other tools and library users as well. The main
difference compared to the original internal API is that the
detection for shared mapping views now takes also sizes and strides into
account, without relying on just the data pointer.
Some additional robustness checks are needed regarding fields that have
mapping sharing enforced (as otherwise it'll fail in the SceneData
constructor which isn't nice), that'll be done in subsequent commits.
In some cases it's needed to release (or copy) the data first and
only then access the field properties through the SceneData to
(optionally) re-route the field views to a new data location. But since
releaseData() was implicitly erasing field data as well, this wasn't
possible and the only other option was to release the field data first
and then access them through the low-level SceneFieldData API with all
convenience lost.
This makes the release*Data() APIs a bit dangerous to use, but that
should be fine -- those aren't meant to be used by regular code anyway.
Similar caveat is with MaterialData already.
Especially the part about non-owned data was lacking, with basically no
information about what are offset-only attributes and fields actually
good for.
Every time I push an innocent change it ends up breaking the
non-deprecated build, such as 34a91cf458.
Sigh, I really need to clean up the backwards compatibility ifdefs...
Uses StridedBitArrayView underneath. Waited for this container to exist,
because implementing this using bools and wasting 8x more memory wasn't
a good option. Plus, being able to address single bits opens a
possibility to describe individual bits in enum flags, whereas the only
other option would be to take the whole flag as an opaque type
containing "some bit values".
Apparently I forgot to actually test these -- in order to fit the string
data pointer without making SceneFieldData too large, it'đ stored as an
offset from fieldData. And that's something not expressible in a
constexpr context. Thus the only way to create a constexpr string field
is by using the offset-only constructor (which is now appropriately
tested).
This change at least allowed me to move the constructor to the cpp file,
saving on header size and using more lightweight assertions.
Somehow I forgot to cross-reference the SceneFieldData constructor and
mention which are usable for strings and which not, and that no builtin
fields are strings.
Finally got an idea how to provide various options store these
efficiently, so it's implemented now. Five different storage variants
times four different type sizes.
With the upcoming string field support, field<UnsignedShort>() will be
allowed to peek into SceneFieldType::UnsignedShort, but also
SceneFieldType::StringOffset16 or
SceneFieldType::StringRangeNullTerminated16. Most types still have a 1:1
mapping, which is why this delegates to the existing
SceneFieldTypeFor<T> trait (which is also still needed to autodetect the
SceneFieldType in a SceneFieldData constructor), but certain type will
be able to provide a specialized handling.
These APIs are mostly just for debugging purposes, not widely used, so
it doesn't make sense to have them as constexpr in the header. (Plus the
returned void view is useless in a constexpr context anyway.)
This header size is getting out of hand, so every stripped bit counts.
Also now that they're no longer constexpr, I can go back to using
regular assertions. The reinterpret_cast<> wasn't needed either.
The MeshData and SceneData find APIs had the complexity documented, do
the same for MaterialData too. Plus cross-link this API from
hasAttribute() / hasField() so it's easier to find.
I.e., with the intention to be implemented in the best way possible,
without relying on some 3rd party library with murky corner cases and
questionable tradeoffs.
I realized those are too annoying when writing a glTF exporter which
contains a lot of switches over enums. And as further shown by the diff,
those were only inflicting additional pain in *all* switch statements,
nothing else, no other added value. And everywhere else the helpers are
the designated way to deal with those, so there's no point in having an
explicit enum value denoting start of a "custom range".
It wasn't even any convenient to have it in the enum, as the extra
effort needed for casting actually made it *exactly* the same length as
if I'd just use a separately-defined constant.
Of course, discovering bugs RIGHT AFTER I merge to master. At least now
I can't have the OCD urge to squash that back to
7dc2dea45b and
dd9f4747b7, saving some time.
Mostly just to avoid the return types changing to incompatible types in
the future, breaking existing code. The internals are currently not
fully ready to operate with 64-bit object IDs, especially the AsArray()
APIs -- those I will have to solve in the future somehow. Returning
64-bit values in the pairs would add four byte padding after basically
each value, which is way too wasteful for the common case.
The Into() APIs could eventually get 64-bit overloads though.
Currently used by the per-object access APIs to make the lookup
constant- or logarithmic-time instead of linear, available for use by
external data consumers as well.
Now it's a field and its corresponding object mapping, instead of
field and "objects":
- Goes better with the concept that there's not really any materialized
"object" anywhere, just fields mapped to them.
- No more weird singular/plural difference between field() and
objects(), it's field() and mapping() now.
- The objectCount() that actually wasn't really object count is now a
mappingBound(), an upper bound for object IDs contained in the object
mapping views. Which is quite self-explanatory without having to
mention every time that the range may be sparse.
This got originally added as some sort of a kludge to make it easy to go
to the parent transformation, assuming Parent and Transformation share
the same object mapping:
parentTransformation = transformations[parents[i]]
But after some ACTUAL REAL WORLD use, I realized that there's often a
set of objects that have a Parent defined, and then another, completely
disjoint, set of objects that have a transformation (for example certain
nodes having no transformation at all because it's an identity). And so
this parent indirection is not only useless, but in fact an additional
complication. Let's say we make a map of the transformations, where
transformationMap[i] is a transformation for object i:
transformationMap = {}
for j in range(len(transformations)):
transformationMap[transformationObject[j]] = transformation[j]
Then, with *no* assumptions about shared object mapping, the indirection
would cause parent transformation retrieval to look like this:
parentTransformation = transformationMap[parentObjects[parents[i]]
While *without* the indirection, it'd be just
parentTransformation = transformationMap[parents[i]]
Because that way one can query a field with *AsArray() and iterate
through it in a single expression. This also resolves the pending issue
where it was more than annoying to fetch object mapping for TRS fields
when only a subset of the fields is available.
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.
Another thought I had was about allowing a 2x2 / 3x3 matrix to be used
for rotation, but there's the ambiguity with it possibly containing
scaling / shear / whatever, which would then cause EXPLODING HEADACHES
when converting to quaternions / complex numbers.
Using hasField() + fieldId() was a bad usage pattern leading to a double
linear lookup, so there's now findFieldId() returning an Optional which
covers both. Similarly for finding an object offset in an field, there's
a findFieldObjectOffset() returning an Optional, fieldObjectOffset()
asserting if an object is not found (for convenience to avoid explicit
error handling on user side) and hasFieldObject().
The internal helpers were also renamed and the offset argument moved to
be last for consistency.
Same as with MeshData2D/3D, the original ObjectData API and plugin
interfaces are preserved to keep existing code as well as existing
importer implementations working. As Magnum's own importers will get
updated to the new SceneData workflow, a backward compatibility layer
provided that translates it to the subset that the legacy ObjectData
understands.
With this commit, both existing plugin code can build (and test against)
the new workflow, and any ports to the new workflow can test against the
legacy interfaces. Except that for now the compatibility layer doesn't
deal with objects that have more than one mesh or for example a light
and a camera attached, this will be done in a separate step.
Support utilities needed for SceneTools that will operate on arbitrary
SceneData (adding/removing fields, objects...) without having to know
what each field means and how it needs to be treated.
Honestly the same would make sense for the VertexFormat enum as well.
But not now, later.
Vladimír Vondruš