Taking Animation::TrackViewStorage wasn't really a good idea, as it
wasn't solving anything -- in order to create it, there needs to be a
TrackView of a concrete type first anyway, and even then it required a
lot of additional verbose typing.
The new constructors take basically what TrackView takes, plus there's
additionally a constructor that takes a typeless value view together
with explicitly specified value and result types, allowing a truly
type-erased usage. On the other hand, the templated variants directly
deduce the types without any additional typing, making the construction
similarly straightforward to e.g. SceneFieldData.
In case of the type-erased constructors, if the interpolator function
isn't supplied explicitly, an implicit one is picked based on the value
type, result type and interpolation. Not all combinations make sense of
course, so this is a new assertion point, however compared to the
previous way where the interpolator was picked from a *typed* TrackView,
this doesn't add any new restriction -- what asserted there, asserts now
as well, and additionally you can't have e.g. a Quaternion track with a
boolean result. I may also be eventually adding assertions to check that
the target name matches the result type -- so e.g. a rotation isn't
specified as an integer and such. Compared to newer APIs like MeshData,
MaterialData or SceneData the AnimationData API has a significant lack
of sanity asserts like this.
Instead of storing Animation::TrackViewStorage directly it now contains
the view pointers, strides and size (where the size is shared by both
keys and values) together with packing the non-pointer values into
existing paddings. Together with reducing the keyframe count to 32 bits
and strides to 16 bits (which is consistent with MeshData and
SceneData), this reduces the size from 80 bytes to 48.
Not using TrackViewStorage also means we can directly accept the
key/value views in constructors, significantly improving the usability.
This also makes it possible to add support for (constexpr) offset-only
track data and thus easy serializability, again similarly to
MeshAttributeData and SceneFieldData.
Again, similarly to what's done for custom MeshAttribute and SceneField
values already. I'm bumping the importer interface version as adding new
virtual functions is a silent ABI breakage, but it's good to do in any
case as the AnimationTrackTarget enum was extended to 16 bits and the
values got shifted.
For consistency with what's already done for MeshAttribute and
SceneField. The ::Custom enum value is deprecated in favor of these, the
only actually breaking change is that the debug printer now subtracts
32768 for custom values (consistently with custom mesh attributes and
scene fields), while it printed the absolute enum value before.
The `Type` was suggesting it'd be some C++ type, definitely not values
like Scaling3D or Translation2D, resulting in a significant "brain
autocompletion error" every time I was using that type.
Unfortunately on AnimationData the trackTargetType() couldn't similarly
get renamed to trackTarget() as there's already trackTarget() that
contains the node ID the target points to, so it's trackTargetName()
instead. Renaming trackTarget() to trackTargetId() wasn't an option as
that would be inconsistent with everything else (TextureTools::image(),
MaterialAttribute::BaseColorTexture, SceneField::Mesh are all IDs but
they don't have an `Id` suffix); renaming to AnimationTrackTargetName
would keep it insanely long and wouldn't make it consistent either
(MeshAttribute, SceneFIeld, MaterialAttribute are all referred to as
"names" yet they don't have a `Name` suffix).
So it has 32k values for custom targets, instead of just 127. This
makes it consistent with MeshAttribute, which also provides 32k values,
while SceneField has a whole 31-bit range to make it possible to store
arbitrary ECS identifiers as well.
Since this is an ABI break, I'm also shifting the values by 1 to have
zero used for an invalid value, consistently with SceneField,
MeshAttribute etc.
Because it somewhat confusingly may have implied that it's really
composed of 8-bit bools, and not bits. The same reasoning was used to
pick the name for Corrade's Containers::BitArray.
Backwards compatibility aliases are in place as usual, however the
internal BoolVectorConverter is now BitVectorConverter and there
unfortunately cannot be any backwards compatibility. This breaks only
GLM and Eigen integration in the magnum-integration repo, which I'm
fixing immediately. I don't expect any user code to use this internal
helper. For regular vectors maybe, for this one definitely not.
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.
Doxygen 1.8.17 *finally* fixes enum value linking and so linking to
Enum::Value via TotallyArbitraryString::Value no longer works. That
uncovered a ton of documentation errors.
A welcome improvement, for once.
This is in line with how the other APIs are named (for example
ObjectDataXD have instance type and instance). This would be very hard
to change later without breaking backwards compatibility, so I'm doing
it now, until the animation APIs get widely used.
Note that I don't mean all possible underlying types, just all possible
vector classes. This is needed for splines etc. which may be templated
and have result type a generic vector, for example.
This reduces the templated code a bit, as I moved the index assertion to
the *.cpp file. Also the function now returns a reference to avoid
needless copies -- it's a view, but still quite a heavy view.
Moreover, this will prevent from passing e.g. integer-based keys to
Trade::AnimationData. And this also now allows me to add duration() to
Trade::AnimationData. I also moved all accessors that don't need a
concrete value type to this base class.
Vladimír Vondruš