From b20ab3acd62ef9f3f8bd7fc037c25fb0c78f5293 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 27 Mar 2023 19:31:34 +0200 Subject: [PATCH] Trade: expand AnimationTrackTargetType to 16 bits. 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. --- doc/changelog.dox | 3 +++ src/Magnum/Trade/AnimationData.cpp | 6 +++--- src/Magnum/Trade/AnimationData.h | 9 +++++--- src/Magnum/Trade/Test/AnimationDataTest.cpp | 24 ++++++++++----------- src/Magnum/Trade/Trade.h | 2 +- 5 files changed, 25 insertions(+), 19 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index e200b6edd..c7fcb4ade 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -1338,6 +1338,9 @@ See also: and @ref Shaders::DistanceFieldVectorGL is removed, as its benefits were rather questionable --- on the contrary, it made subclass implementation more verbose and less clear +- The @ref Trade::AnimationTrackTargetType enum was extended from 8 to 16 + bits to provide more room for custom targets, consistently with + @ref Trade::MeshAttribute. - Mutable access to @ref Trade::PhongMaterialData color and texture information, deprecated in 2020.06, is now removed, as it's impossible to implement through the redesigned @ref Trade::MaterialData APIs. However diff --git a/src/Magnum/Trade/AnimationData.cpp b/src/Magnum/Trade/AnimationData.cpp index f9fbd005d..7a6073de8 100644 --- a/src/Magnum/Trade/AnimationData.cpp +++ b/src/Magnum/Trade/AnimationData.cpp @@ -198,8 +198,8 @@ Debug& operator<<(Debug& debug, const AnimationTrackTargetType value) { if(!packed) debug << "Trade::AnimationTrackTargetType" << Debug::nospace; - if(UnsignedByte(value) >= UnsignedByte(AnimationTrackTargetType::Custom)) - return debug << (packed ? "Custom(" : "::Custom(") << Debug::nospace << UnsignedByte(value) << Debug::nospace << ")"; + if(UnsignedShort(value) >= UnsignedShort(AnimationTrackTargetType::Custom)) + return debug << (packed ? "Custom(" : "::Custom(") << Debug::nospace << UnsignedShort(value) << Debug::nospace << ")"; switch(value) { /* LCOV_EXCL_START */ @@ -217,7 +217,7 @@ Debug& operator<<(Debug& debug, const AnimationTrackTargetType value) { case AnimationTrackTargetType::Custom: CORRADE_INTERNAL_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */ } - return debug << (packed ? "" : "(") << Debug::nospace << reinterpret_cast(UnsignedByte(value)) << Debug::nospace << (packed ? "" : ")"); + return debug << (packed ? "" : "(") << Debug::nospace << reinterpret_cast(UnsignedShort(value)) << Debug::nospace << (packed ? "" : ")"); } #endif diff --git a/src/Magnum/Trade/AnimationData.h b/src/Magnum/Trade/AnimationData.h index 7967705ad..406a3d169 100644 --- a/src/Magnum/Trade/AnimationData.h +++ b/src/Magnum/Trade/AnimationData.h @@ -169,7 +169,9 @@ MAGNUM_TRADE_EXPORT Debug& operator<<(Debug& debug, AnimationTrackType value); @see @ref AnimationData @experimental */ -enum class AnimationTrackTargetType: UnsignedByte { +enum class AnimationTrackTargetType: UnsignedShort { + /* Zero used for an invalid value */ + /** * Modifies 2D object translation. Type is usually * @ref Magnum::Vector2 "Vector2" or @@ -180,7 +182,7 @@ enum class AnimationTrackTargetType: UnsignedByte { * @ref AnimationTrackType::CubicHermite2D, * @ref SceneField::Translation, @ref SceneData::is2D() */ - Translation2D, + Translation2D = 1, /** * Modifies 3D object translation. Type is usually @@ -248,7 +250,7 @@ enum class AnimationTrackTargetType: UnsignedByte { * an existing object. See documentation of a particular importer for * details. */ - Custom = 128 + Custom = 32768 }; /** @debugoperatorenum{AnimationTrackTargetType} */ @@ -309,6 +311,7 @@ class AnimationTrackData { AnimationTrackType _type, _resultType; AnimationTrackTargetType _targetType; + /* 4-byte padding */ UnsignedLong _target; Animation::TrackViewStorage _view; }; diff --git a/src/Magnum/Trade/Test/AnimationDataTest.cpp b/src/Magnum/Trade/Test/AnimationDataTest.cpp index 57900aa76..13f577979 100644 --- a/src/Magnum/Trade/Test/AnimationDataTest.cpp +++ b/src/Magnum/Trade/Test/AnimationDataTest.cpp @@ -249,12 +249,12 @@ void AnimationDataTest::constructImplicitDuration() { const int state = 5; AnimationData data{std::move(buffer), { - AnimationTrackData{AnimationTrackTargetType(129), 0, + AnimationTrackData{AnimationTrackTargetType(32769), 0, Animation::TrackView{ {view, &view[0].time, 2, sizeof(Data)}, {view, &view[0].value, 2, sizeof(Data)}, Animation::Interpolation::Constant}}, - AnimationTrackData{AnimationTrackTargetType(130), 1, + AnimationTrackData{AnimationTrackTargetType(32770), 1, Animation::TrackView{ {view, &view[2].time, 2, sizeof(Data)}, {view, &view[2].value, 2, sizeof(Data)}, @@ -268,7 +268,7 @@ void AnimationDataTest::constructImplicitDuration() { { CORRADE_COMPARE(data.trackType(0), AnimationTrackType::Bool); CORRADE_COMPARE(data.trackResultType(0), AnimationTrackType::Bool); - CORRADE_COMPARE(data.trackTargetType(0), AnimationTrackTargetType(129)); + CORRADE_COMPARE(data.trackTargetType(0), AnimationTrackTargetType(32769)); CORRADE_COMPARE(data.trackTarget(0), 0); Animation::TrackView track = data.track(0); @@ -287,7 +287,7 @@ void AnimationDataTest::constructImplicitDuration() { } { CORRADE_COMPARE(data.trackType(1), AnimationTrackType::Bool); CORRADE_COMPARE(data.trackResultType(1), AnimationTrackType::Bool); - CORRADE_COMPARE(data.trackTargetType(1), AnimationTrackTargetType(130)); + CORRADE_COMPARE(data.trackTargetType(1), AnimationTrackTargetType(32770)); CORRADE_COMPARE(data.trackTarget(1), 1); Animation::TrackView track = data.track(1); @@ -365,7 +365,7 @@ void AnimationDataTest::constructImplicitDurationNotOwned() { const int state = 5; AnimationData data{instanceData.dataFlags, keyframes, { - AnimationTrackData{AnimationTrackTargetType(129), 0, + AnimationTrackData{AnimationTrackTargetType(32769), 0, Animation::TrackView{keyframes, Animation::Interpolation::Constant}}, }, &state}; @@ -380,7 +380,7 @@ void AnimationDataTest::constructImplicitDurationNotOwned() { { CORRADE_COMPARE(data.trackType(0), AnimationTrackType::Bool); CORRADE_COMPARE(data.trackResultType(0), AnimationTrackType::Bool); - CORRADE_COMPARE(data.trackTargetType(0), AnimationTrackTargetType(129)); + CORRADE_COMPARE(data.trackTargetType(0), AnimationTrackTargetType(32769)); CORRADE_COMPARE(data.trackTarget(0), 0); Animation::TrackView track = data.track(0); @@ -532,7 +532,7 @@ void AnimationDataTest::mutableAccessNotAllowed() { }; AnimationData data{{}, keyframes, { - AnimationTrackData{AnimationTrackTargetType(129), 0, + AnimationTrackData{AnimationTrackTargetType(32769), 0, Animation::TrackView{keyframes, Animation::Interpolation::Constant}}, }}; CORRADE_COMPARE(data.dataFlags(), DataFlags{}); @@ -640,7 +640,7 @@ void AnimationDataTest::release() { }; AnimationData data{{}, keyframes, { - AnimationTrackData{AnimationTrackTargetType(129), 0, + AnimationTrackData{AnimationTrackTargetType(32769), 0, Animation::TrackView{keyframes, Animation::Interpolation::Constant}}, }}; CORRADE_COMPARE(data.trackCount(), 1); @@ -668,15 +668,15 @@ void AnimationDataTest::debugAnimationTrackTypePacked() { void AnimationDataTest::debugAnimationTrackTargetType() { std::ostringstream out; - Debug{&out} << AnimationTrackTargetType::Rotation3D << AnimationTrackTargetType(135) << AnimationTrackTargetType(0x42); - CORRADE_COMPARE(out.str(), "Trade::AnimationTrackTargetType::Rotation3D Trade::AnimationTrackTargetType::Custom(135) Trade::AnimationTrackTargetType(0x42)\n"); + Debug{&out} << AnimationTrackTargetType::Rotation3D << AnimationTrackTargetType(32777) << AnimationTrackTargetType(0x4242); + CORRADE_COMPARE(out.str(), "Trade::AnimationTrackTargetType::Rotation3D Trade::AnimationTrackTargetType::Custom(32777) Trade::AnimationTrackTargetType(0x4242)\n"); } void AnimationDataTest::debugAnimationTrackTargetTypePacked() { std::ostringstream out; /* Last is not packed, ones before should not make any flags persistent */ - Debug{&out} << Debug::packed << AnimationTrackTargetType::Rotation3D << Debug::packed << AnimationTrackTargetType(135) << Debug::packed << AnimationTrackTargetType(0x42) << AnimationTrackType::Float; - CORRADE_COMPARE(out.str(), "Rotation3D Custom(135) 0x42 Trade::AnimationTrackType::Float\n"); + Debug{&out} << Debug::packed << AnimationTrackTargetType::Rotation3D << Debug::packed << AnimationTrackTargetType(32888) << Debug::packed << AnimationTrackTargetType(0x4242) << AnimationTrackType::Float; + CORRADE_COMPARE(out.str(), "Rotation3D Custom(32888) 0x4242 Trade::AnimationTrackType::Float\n"); } }}}} diff --git a/src/Magnum/Trade/Trade.h b/src/Magnum/Trade/Trade.h index cdb7ee518..b9abf643e 100644 --- a/src/Magnum/Trade/Trade.h +++ b/src/Magnum/Trade/Trade.h @@ -61,7 +61,7 @@ class MaterialData; typedef CORRADE_DEPRECATED("use MaterialData instead") MaterialData AbstractMaterialData; #endif -enum class AnimationTrackTargetType: UnsignedByte; +enum class AnimationTrackTargetType: UnsignedShort; enum class AnimationTrackType: UnsignedByte; class AnimationTrackData; class AnimationData;