Skip to content

Commit

Permalink
Trade: provide better AnimationTrackData constructors.
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@mosra
mosra committed Apr 9, 2023
1 parent 5f1fc10 commit 94e9961
Show file tree
Hide file tree
Showing 5 changed files with 658 additions and 59 deletions.
8 changes: 8 additions & 0 deletions doc/changelog.dox
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1053,6 +1053,14 @@ See also:
- @cpp Trade::AbstractMaterialData @ce as well as its containing header
`Magnum/Trade/AbstractMaterialData.h` is now a deprecated alias to the new
and more flexible @ref Trade::MaterialData.
- @ref Trade::AnimationTrackData constructors taking
@ref Animation::TrackViewStorage are deprecated in favor of significantly
less verbose variants that take key/value views and other track parameters
directly, allowing them to either directly deduce the value type or be
truly type-erased, no longer requiring user code to manually handle all
possible cases. The variant taking an @ref Animation::TrackView is kept
however as it may be convenient when exporting already-populated animation
instances.
- @cpp Trade::LightData::Type::Infinite @ce, originally adapted from the
OpenGEX specification, is deprecated in favor of
@ref Trade::LightData::Type::Directional as that's the more commonly used
Expand Down
75 changes: 68 additions & 7 deletions src/Magnum/Trade/AnimationData.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,78 @@

namespace Magnum { namespace Trade {

AnimationTrackData::AnimationTrackData(const AnimationTrackType type, const AnimationTrackType resultType, const AnimationTrackTarget targetName, const UnsignedLong target, const Animation::TrackViewStorage<const Float>& view) noexcept: _type{type}, _resultType{resultType}, _targetName{targetName}, _interpolation{view.interpolation()}, _before{view.before()}, _after{view.after()}, _target{target}, _size{UnsignedInt(view.size())}, _keysStride{Short(view.keys().stride())}, _valuesStride{Short(view.values().stride())}, _keysData{view.keys().data()}, _valuesData{view.values().data()}, _interpolator{view.interpolator()} {
namespace {

auto animationInterpolatorFor(const Animation::Interpolation interpolation, const AnimationTrackType type, const AnimationTrackType resultType) -> void(*)() {
switch(type) {
/* LCOV_EXCL_START */
#define _ct(value, type_) \
case AnimationTrackType::value: \
if(type == resultType) \
return reinterpret_cast<void(*)()>(Trade::animationInterpolatorFor<type_, type_>(interpolation)); \
break;
#define _c(type_) _ct(type_, type_)
_ct(Bool, bool)
_c(Float)
_c(UnsignedInt)
_c(Int)
_c(BitVector2)
_c(BitVector3)
_c(BitVector4)
_c(Vector2)
_c(Vector2ui)
_c(Vector2i)
_c(Vector3)
_c(Vector3ui)
_c(Vector3i)
_c(Vector4)
_c(Vector4ui)
_c(Vector4i)
_c(Complex)
_c(Quaternion)
_c(DualQuaternion)
#undef _c
#undef _ct
/* LCOV_EXCL_STOP */

/* LCOV_EXCL_START */
#define _cr(type_, resultType_) \
case AnimationTrackType::type_: \
if(resultType == AnimationTrackType::resultType_) \
return reinterpret_cast<void(*)()>(Trade::animationInterpolatorFor<type_, resultType_>(interpolation)); \
break;
_cr(CubicHermite1D, Float)
_cr(CubicHermite2D, Vector2)
_cr(CubicHermite3D, Vector3)
_cr(CubicHermiteComplex, Complex)
_cr(CubicHermiteQuaternion, Quaternion)
#undef _cr
/* LCOV_EXCL_STOP */
}

/** @todo this doesn't print the types when e.g. a spline interpolation is
requested for bool, how to fix? */
CORRADE_ASSERT_UNREACHABLE("Trade::AnimationTrackData: can't deduce interpolator function for" << type << Debug::nospace << "," << resultType << "and" << interpolation, {});
}

}

AnimationTrackData::AnimationTrackData(const AnimationTrackTarget targetName, const UnsignedLong target, const AnimationTrackType type, const AnimationTrackType resultType, const Containers::StridedArrayView1D<const Float>& keys, const Containers::StridedArrayView1D<const void>& values, const Animation::Interpolation interpolation, void(*const interpolator)(), const Animation::Extrapolation before, const Animation::Extrapolation after) noexcept: _type{type}, _resultType{resultType}, _targetName{targetName}, _interpolation{interpolation}, _before{before}, _after{after}, _target{target}, _size{UnsignedInt(keys.size())}, _keysStride{Short(keys.stride())}, _valuesStride{Short(values.stride())}, _keysData{keys.data()}, _valuesData{values.data()}, _interpolator{interpolator} {
CORRADE_ASSERT(keys.size() == values.size(),
"Trade::AnimationTrackData: expected key and value view to have the same size but got" << keys.size() << "and" << values.size(), );
#ifndef CORRADE_TARGET_32BIT
CORRADE_ASSERT(view.size() <= 0xffffffffu,
"Trade::AnimationTrackData: expected keyframe count to fit into 32 bits but got" << view.size(), );
CORRADE_ASSERT(keys.size() <= 0xffffffffu,
"Trade::AnimationTrackData: expected keyframe count to fit into 32 bits but got" << keys.size(), );
#endif
CORRADE_ASSERT(view.keys().stride() >= -32768 && view.keys().stride() <= 32767,
"Trade::AnimationTrackData: expected key stride to fit into 16 bits but got" << view.keys().stride(), );
CORRADE_ASSERT(view.values().stride() >= -32768 && view.values().stride() <= 32767,
"Trade::AnimationTrackData: expected value stride to fit into 16 bits but got" << view.values().stride(), );
CORRADE_ASSERT(keys.stride() >= -32768 && keys.stride() <= 32767,
"Trade::AnimationTrackData: expected key stride to fit into 16 bits but got" << keys.stride(), );
CORRADE_ASSERT(values.stride() >= -32768 && values.stride() <= 32767,
"Trade::AnimationTrackData: expected value stride to fit into 16 bits but got" << values.stride(), );
}


AnimationTrackData::AnimationTrackData(const AnimationTrackTarget targetName, const UnsignedLong target, const AnimationTrackType type, const AnimationTrackType resultType, const Containers::StridedArrayView1D<const Float>& keys, const Containers::StridedArrayView1D<const void>& values, const Animation::Interpolation interpolation, const Animation::Extrapolation before, const Animation::Extrapolation after) noexcept: AnimationTrackData{targetName, target, type, resultType, keys, values, interpolation, animationInterpolatorFor(interpolation, type, resultType), before, after} {}

Animation::TrackViewStorage<const Float> AnimationTrackData::track() const {
return Animation::TrackViewStorage<const Float>{
/* We're sure the views are correct, circumventing the asserts */
Expand Down
Loading

0 comments on commit 94e9961

Please sign in to comment.