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...
Such an unnecessary footgun -- I was already checking the other case,
having attribute data too short, but this I thought is fine because it's
not leading to any crash. Well it's leading to needless pain and
suffering, that's what it is doing!
And of course already found FIVE such bugs in just Magnum tests alone.
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.
This got probably implemented long before the change in
c74b4c6b90. Or actually maybe not at all.
In any case, it'd cause an ambiguity with the 2D char view constructor
when the "updimensioning" StridedArrayView constructor gets introduced.
I spent some time wondering if as<PbrClearCoatMaterialData>() can be
used on a material that doesn't neccessarily have that layer. It can, so
hint that in the docs.
To match the order (and thus also verbose log) in which they're added
inside addImporterContents(), and subsequently also the order
magnum-sceneconverter processes them manually. There's no reason to have
it different and cause confusion.
Because, when it fails, it'd attempt to print them as strings,
inevitably leading to a crash or garbage in the terminal. Moreover, with
the upcoming fix that makes StringView *actually* convertible from an
ArrayView, it would become ambiguous.
Stone age APIs used here again, I can't fathom how I could live without
member slicing for so long. This also fixes an OOB access which trips up
the new ArrayView assertions -- accessing element 0 in the constructor
isn't a good thing to do if there's no data at all.
This still contained the original message which didn't say the index.
Times changed significantly since then, and the printed index really
proved to be useful.
Also fixing a OOB assertion in the test triggered by the new ArrayView
element access assertions -- the graceful assertion returns the first
element, so we have to have at least one there to return.
This is the first builtin array attribute, with one of the objectives
being an ability to support an arbitrary count of per-vertex weights in
a single contiguous attribute without the complexity of having to go
through several four-component attributes.
On the shader side it still needs to get cut into at most four
components per attribute, but there's no reason for such limitation to
get propagated here as well.
Co-authored-by: Vladimír Vondruš <mosra@centrum.cz>
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.
I'm not sure why this restriction was there as nothing was preventing
them from being used. The attribute is only accessible through the
typeless attribute(), which gives back
`Containers::StridedArrayView2D<const char>` with second dimension size
being set to the full stride. And there it doesn't matter if the format
is an array or not.
This will be useful for joint IDs and weights, for example doing crazy
things like packing the IDs into an array of 8 4-bit numbers, saving
half the memory compared to the smallest builtin representation using
UnsignedByte[8].
Vladimír Vondruš