diff --git a/src/Magnum/Animation/Track.h b/src/Magnum/Animation/Track.h index c7f716964..b7ec6257f 100644 --- a/src/Magnum/Animation/Track.h +++ b/src/Magnum/Animation/Track.h @@ -427,6 +427,15 @@ template class TrackViewStorage { private: template friend class TrackView; + #ifndef DOXYGEN_GENERATING_OUTPUT + /* This one creates TrackViewStorage instances on the fly using the + below constructor */ + friend Trade::AnimationTrackData; + #endif explicit TrackViewStorage(const Containers::StridedArrayView1D& keys, const Containers::StridedArrayView1D::value, const void, void>::type>& values, Interpolation interpolation, void(*interpolator)(), Extrapolation before, Extrapolation after) noexcept: _keys{keys}, _values{values}, _interpolator{interpolator}, _interpolation{interpolation}, _before{before}, _after{after} { CORRADE_ASSERT(keys.size() == values.size(), "Animation::TrackView: expected key and value view to have the same size but got" << keys.size() << "and" << values.size(), ); diff --git a/src/Magnum/Trade/AnimationData.cpp b/src/Magnum/Trade/AnimationData.cpp index f8fa56134..d10995562 100644 --- a/src/Magnum/Trade/AnimationData.cpp +++ b/src/Magnum/Trade/AnimationData.cpp @@ -33,6 +33,25 @@ namespace Magnum { namespace Trade { +AnimationTrackData::AnimationTrackData(const AnimationTrackType type, const AnimationTrackType resultType, const AnimationTrackTarget targetName, const UnsignedLong target, const Animation::TrackViewStorage& 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()} { + #ifndef CORRADE_TARGET_32BIT + CORRADE_ASSERT(view.size() <= 0xffffffffu, + "Trade::AnimationTrackData: expected keyframe count to fit into 32 bits but got" << view.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(), ); +} + +Animation::TrackViewStorage AnimationTrackData::track() const { + return Animation::TrackViewStorage{ + /* We're sure the views are correct, circumventing the asserts */ + Containers::StridedArrayView1D{{nullptr, ~std::size_t{}}, static_cast(_keysData), _size, _keysStride}, + Containers::StridedArrayView1D{{nullptr, ~std::size_t{}}, static_cast(_valuesData), _size, _valuesStride}, + _interpolation, _interpolator, _before, _after}; +} + AnimationData::AnimationData(Containers::Array&& data, Containers::Array&& tracks, const Range1D& duration, const void* importerState) noexcept: _dataFlags{DataFlag::Owned|DataFlag::Mutable}, _duration{duration}, _data{std::move(data)}, _tracks{std::move(tracks)}, _importerState{importerState} {} AnimationData::AnimationData(Containers::Array&& data, std::initializer_list tracks, const Range1D& duration, const void* importerState): AnimationData{std::move(data), Implementation::initializerListToArrayWithDefaultDeleter(tracks), duration, importerState} {} @@ -47,11 +66,19 @@ AnimationData::AnimationData(const DataFlags dataFlags, const Containers::ArrayV AnimationData::AnimationData(Containers::Array&& data, Containers::Array&& tracks, const void* importerState) noexcept: _dataFlags{DataFlag::Owned|DataFlag::Mutable}, _data{std::move(data)}, _tracks{std::move(tracks)}, _importerState{importerState} { if(!_tracks.isEmpty()) { + const auto duration = [](const AnimationTrackData& track) { + if(!track._size) return Range1D{}; + + /* We're sure the view is correct, circumventing the assert */ + Containers::StridedArrayView1D keys{{nullptr, ~std::size_t{}}, static_cast(track._keysData), track._size, track._keysStride}; + return Range1D{keys.front(), keys.back()}; + }; + /* Reset duration to duration of the first track so it properly support cases where tracks don't start at 0 */ - _duration = _tracks.front()._view.duration(); + _duration = duration(_tracks.front()); for(std::size_t i = 1; i != _tracks.size(); ++i) - _duration = Math::join(_duration, _tracks[i]._view.duration()); + _duration = Math::join(_duration, duration(_tracks[i])); } } @@ -101,20 +128,21 @@ UnsignedLong AnimationData::trackTarget(UnsignedInt id) const { return _tracks[id]._target; } -const Animation::TrackViewStorage& AnimationData::track(UnsignedInt id) const { +Animation::TrackViewStorage AnimationData::track(UnsignedInt id) const { CORRADE_ASSERT(id < _tracks.size(), - "Trade::AnimationData::track(): index" << id << "out of range for" << _tracks.size() << "tracks", _tracks[0]._view); - return _tracks[id]._view; + "Trade::AnimationData::track(): index" << id << "out of range for" << _tracks.size() << "tracks", _tracks[0].track()); + return _tracks[id].track(); } -const Animation::TrackViewStorage& AnimationData::mutableTrack(UnsignedInt id) { +Animation::TrackViewStorage AnimationData::mutableTrack(UnsignedInt id) { + /* EW those casts! */ CORRADE_ASSERT(_dataFlags & DataFlag::Mutable, "Trade::AnimationData::mutableTrack(): the animation is not mutable", - reinterpret_cast&>(_tracks[id]._view)); + reinterpret_cast&>(static_cast&>(_tracks[0].track()))); CORRADE_ASSERT(id < _tracks.size(), "Trade::AnimationData::mutableTrack(): index" << id << "out of range for" << _tracks.size() << "tracks", - reinterpret_cast&>(_tracks[0]._view)); - return reinterpret_cast&>(_tracks[id]._view); + reinterpret_cast&>(static_cast&>(_tracks[0].track()))); + return reinterpret_cast&>(static_cast&>(_tracks[id].track())); } Containers::Array AnimationData::release() { diff --git a/src/Magnum/Trade/AnimationData.h b/src/Magnum/Trade/AnimationData.h index 816fec790..6590ed075 100644 --- a/src/Magnum/Trade/AnimationData.h +++ b/src/Magnum/Trade/AnimationData.h @@ -338,7 +338,7 @@ Convenience type for populating @ref AnimationData. Has no accessors, as the data are then accessible through @ref AnimationData APIs. @experimental */ -class AnimationTrackData { +class MAGNUM_TRADE_EXPORT AnimationTrackData { public: /** * @brief Default constructor @@ -347,7 +347,7 @@ class AnimationTrackData { * initialization of the track array for @ref AnimationData, expected * to be replaced with concrete values later. */ - explicit AnimationTrackData() noexcept: _type{}, _resultType{}, _targetName{}, _target{}, _view{} {} + explicit AnimationTrackData() noexcept: _type{}, _resultType{}, _targetName{}, _interpolation{}, _before{}, _after{}, _target{}, _size{}, _keysStride{}, _valuesStride{}, _keysData{}, _valuesData{}, _interpolator{} {} /** * @brief Type-erased constructor @@ -356,11 +356,14 @@ class AnimationTrackData { * @param targetName Track target name * @param target Track target ID * @param view Type-erased @ref Animation::TrackView instance + * + * Expects that @p view key and value strides both fit into signed + * 16-bit values and that keyframe count fits into 32 bits. */ /** @todo stop taking TrackViewStorage and instead directly take the key/value views, interpolator/interpolation and extrapolation -- it's just 6 overloads and makes usage much better */ - explicit AnimationTrackData(AnimationTrackType type, AnimationTrackType resultType, AnimationTrackTarget targetName, UnsignedLong target, const Animation::TrackViewStorage& view) noexcept: _type{type}, _resultType{resultType}, _targetName{targetName}, _target{target}, _view{view} {} + explicit AnimationTrackData(AnimationTrackType type, AnimationTrackType resultType, AnimationTrackTarget targetName, UnsignedLong target, const Animation::TrackViewStorage& view) noexcept; /** @overload * @@ -409,16 +412,23 @@ class AnimationTrackData { * @brief Type-erased @ref Animation::TrackView instance * @m_since_latest */ - Animation::TrackViewStorage track() const { return _view; } + Animation::TrackViewStorage track() const; private: friend AnimationData; AnimationTrackType _type, _resultType; AnimationTrackTarget _targetName; - /* 4-byte padding */ + Animation::Interpolation _interpolation; + Animation::Extrapolation _before, _after; + /* 1 byte padding */ UnsignedLong _target; - Animation::TrackViewStorage _view; + UnsignedInt _size; + Short _keysStride; + Short _valuesStride; + const void* _keysData; + const void* _valuesData; + void(*_interpolator)(); }; /** @@ -683,7 +693,7 @@ class MAGNUM_TRADE_EXPORT AnimationData { * checked version below to access a concrete @ref Animation::TrackView * type. */ - const Animation::TrackViewStorage& track(UnsignedInt id) const; + Animation::TrackViewStorage track(UnsignedInt id) const; /** * @brief Mutable track data storage @@ -693,7 +703,7 @@ class MAGNUM_TRADE_EXPORT AnimationData { * animation is mutable. * @see @ref dataFlags() */ - const Animation::TrackViewStorage& mutableTrack(UnsignedInt id); + Animation::TrackViewStorage mutableTrack(UnsignedInt id); /** * @brief Track data @@ -707,7 +717,7 @@ class MAGNUM_TRADE_EXPORT AnimationData { * use the view or you need to release the data array using * @ref release() and manage its lifetime yourself. */ - template> const Animation::TrackView& track(UnsignedInt id) const; + template> Animation::TrackView track(UnsignedInt id) const; /** * @brief Mutable track data @@ -717,7 +727,7 @@ class MAGNUM_TRADE_EXPORT AnimationData { * animation is mutable. * @see @ref dataFlags() */ - template> const Animation::TrackView& mutableTrack(UnsignedInt id); + template> Animation::TrackView mutableTrack(UnsignedInt id); /** * @brief Release data storage @@ -812,15 +822,15 @@ namespace Implementation { template inline AnimationTrackData::AnimationTrackData(AnimationTrackTarget targetName, UnsignedLong target, const Animation::TrackView& view) noexcept: AnimationTrackData{Implementation::animationTypeFor(), Implementation::animationTypeFor(), targetName, target, view} {} -template const Animation::TrackView& AnimationData::track(UnsignedInt id) const { - const Animation::TrackViewStorage& storage = track(id); +template Animation::TrackView AnimationData::track(UnsignedInt id) const { + const Animation::TrackViewStorage storage = track(id); CORRADE_ASSERT(Implementation::animationTypeFor() == _tracks[id]._type, "Trade::AnimationData::track(): improper type requested for" << _tracks[id]._type, (static_cast&>(storage))); CORRADE_ASSERT(Implementation::animationTypeFor() == _tracks[id]._resultType, "Trade::AnimationData::track(): improper result type requested for" << _tracks[id]._resultType, (static_cast&>(storage))); return static_cast&>(storage); } -template const Animation::TrackView& AnimationData::mutableTrack(UnsignedInt id) { - const Animation::TrackViewStorage& storage = mutableTrack(id); +template Animation::TrackView AnimationData::mutableTrack(UnsignedInt id) { + const Animation::TrackViewStorage storage = mutableTrack(id); CORRADE_ASSERT(Implementation::animationTypeFor() == _tracks[id]._type, "Trade::AnimationData::mutableTrack(): improper type requested for" << _tracks[id]._type, (static_cast&>(storage))); CORRADE_ASSERT(Implementation::animationTypeFor() == _tracks[id]._resultType, "Trade::AnimationData::mutableTrack(): improper result type requested for" << _tracks[id]._resultType, (static_cast&>(storage))); return static_cast&>(storage); diff --git a/src/Magnum/Trade/Test/AnimationDataTest.cpp b/src/Magnum/Trade/Test/AnimationDataTest.cpp index 78ffeb23b..ecbb5638d 100644 --- a/src/Magnum/Trade/Test/AnimationDataTest.cpp +++ b/src/Magnum/Trade/Test/AnimationDataTest.cpp @@ -49,6 +49,10 @@ struct AnimationDataTest: TestSuite::Tester { void constructTrackResultType(); void constructTrackTemplate(); void constructTrackDefault(); + #ifndef CORRADE_TARGET_32BIT + void constructTrackWrongSize(); + #endif + void constructTrackWrongStride(); void construct(); void constructNotOwned(); @@ -93,6 +97,10 @@ AnimationDataTest::AnimationDataTest() { &AnimationDataTest::constructTrackResultType, &AnimationDataTest::constructTrackTemplate, &AnimationDataTest::constructTrackDefault, + #ifndef CORRADE_TARGET_32BIT + &AnimationDataTest::constructTrackWrongSize, + #endif + &AnimationDataTest::constructTrackWrongStride, &AnimationDataTest::construct, &AnimationDataTest::constructImplicitDuration, @@ -249,6 +257,75 @@ void AnimationDataTest::constructTrackDefault() { CORRADE_COMPARE(data.track().values().data(), nullptr); } +#ifndef CORRADE_TARGET_32BIT +void AnimationDataTest::constructTrackWrongSize() { + CORRADE_SKIP_IF_NO_ASSERT(); + + /* This should be fine */ + AnimationTrackData{AnimationTrackTarget::Rotation3D, 16, Animation::TrackView{ + {nullptr, 0xffffffffu}, + {nullptr, 0xffffffffu}, + Animation::Interpolation::Constant + }}; + + std::ostringstream out; + Error redirectError{&out}; + AnimationTrackData{AnimationTrackTarget::Rotation3D, 16, Animation::TrackView{ + {nullptr, 0x100000000ull}, + {nullptr, 0x100000000ull}, + Animation::Interpolation::Constant + }}; + CORRADE_COMPARE(out.str(), + "Trade::AnimationTrackData: expected keyframe count to fit into 32 bits but got 4294967296\n"); +} +#endif + +void AnimationDataTest::constructTrackWrongStride() { + CORRADE_SKIP_IF_NO_ASSERT(); + + char toomuch[2*(32768 + sizeof(Vector2))]; + + /* These should be fine */ + AnimationTrackData{AnimationTrackTarget::Scaling2D, 1, Animation::TrackView{ + Containers::StridedArrayView1D{Containers::arrayCast(toomuch), 2, 32767}, + Containers::StridedArrayView1D{Containers::arrayCast(toomuch), 2, 32768}.flipped<0>(), + Animation::Interpolation::Constant + }}; + AnimationTrackData{AnimationTrackTarget::Scaling2D, 1, Animation::TrackView{ + Containers::StridedArrayView1D{Containers::arrayCast(toomuch), 2, 32768}.flipped<0>(), + Containers::StridedArrayView1D{Containers::arrayCast(toomuch), 2, 32767}, + Animation::Interpolation::Constant + }}; + + std::ostringstream out; + Error redirectError{&out}; + AnimationTrackData{AnimationTrackTarget::Scaling2D, 1, Animation::TrackView{ + Containers::StridedArrayView1D{Containers::arrayCast(toomuch), 2, 32768}, + Containers::StridedArrayView1D{Containers::arrayCast(toomuch), 2, 32767}, + Animation::Interpolation::Constant + }}; + AnimationTrackData{AnimationTrackTarget::Scaling2D, 1, Animation::TrackView{ + Containers::StridedArrayView1D{Containers::arrayCast(toomuch), 2, 32769}.flipped<0>(), + Containers::StridedArrayView1D{Containers::arrayCast(toomuch), 2, 32767}, + Animation::Interpolation::Constant + }}; + AnimationTrackData{AnimationTrackTarget::Scaling2D, 1, Animation::TrackView{ + Containers::StridedArrayView1D{Containers::arrayCast(toomuch), 2, 32767}, + Containers::StridedArrayView1D{Containers::arrayCast(toomuch), 2, 32768}, + Animation::Interpolation::Constant + }}; + AnimationTrackData{AnimationTrackTarget::Scaling2D, 1, Animation::TrackView{ + Containers::StridedArrayView1D{Containers::arrayCast(toomuch), 2, 32767}, + Containers::StridedArrayView1D{Containers::arrayCast(toomuch), 2, 32769}.flipped<0>(), + Animation::Interpolation::Constant + }}; + CORRADE_COMPARE(out.str(), + "Trade::AnimationTrackData: expected key stride to fit into 16 bits but got 32768\n" + "Trade::AnimationTrackData: expected key stride to fit into 16 bits but got -32769\n" + "Trade::AnimationTrackData: expected value stride to fit into 16 bits but got 32768\n" + "Trade::AnimationTrackData: expected value stride to fit into 16 bits but got -32769\n"); +} + void AnimationDataTest::construct() { struct Data { Float time;