Implement animation compression #54050

reduz · 2021-10-20T23:56:19Z

Roughly based on godotengine/godot-proposals#3375 (used format is slightly different).

Implement bitwidth (compression of octahedral mapped rotation deltas) and page based animation compression (see animation.h for format).
Can compress imported animations up to 15-20% of their original size.
Compression format opens the door to streaming.
Works transparently (happens all inside animation.h)

Algorithm is described in the comments of animation.h

Why bitwidth over curve fitting?

Simpler
Faster compression
Faster decompression
Better all rounder, and better suited for mocap (jittery data makes curve fitting inefficient).
More easily allows paging, for better cache coherency and ability to stream.

I experimented with octahedral and spherical mapping. While spherical mapping has the advantage of less discontinuity when wrapping around, the compressor does a good job of isolating the transients (border crossings), so it matters little in practice, as they don't happen as often.

NOTE Seems to work in everything I tested, but try at your own risk.

scene/resources/animation.h

jordo · 2021-10-21T04:51:47Z

Thanks for documenting the compression format. I would put a version byte (or uint32_t) header at the start of the encoding (you could start with 0x1). If someone changes or updates the encoding format in the future (maybe you need a few extra bytes of precision sometime in the future), there is an easy(er) check and migration path for any data stored in a previous encoding version to a newer one.

core/math/vector3.h

reduz · 2021-10-21T11:46:12Z

@jordo

I would put a version byte (or uint32_t) header at the start of the encoding (you could start with 0x1).

Makes sense, but I suppose I can just put it in the Compression dictionary that gets serialized. Will add it before getting the PR out of draft.

jordo · 2021-10-21T16:19:50Z

scene/resources/animation.cpp

+Quaternion Animation::_uncompress_quaternion(const Vector3i &p_value) const {
+	Vector3 axis = Vector3::octahedron_decode(Vector2(float(p_value.x) / 65535.0, float(p_value.y) / 65535.0));
+	float angle = (float(p_value.z) / 65535.0) * 2.0 * Math_PI;
+	return Quaternion(axis, angle);


As another observation, this stuff may be useful in other places in the engine (encode / compress) a Quaternion to Vector3i.. (Networking for example). It may be worth considering organizing the generic compression/encoding funcs to be available & accessible outside of the Animation class.

I am not super sure about this, but be it the case at some point I suppose it can moved. Would wait until there is demand to avoid cluttering the core APIs.

I guess I'm just curious where the line is drawn on something being considered API clutter then? Vector3 (now) in the PR exposes a new octahedron_decode and octahedron_encode (Vector3->Vector2) in core API. Why is Quaternion->Vector3i considered API clutter when the former is not? The Quaternion->Vector3i seems very useful elsewhere, especially as it should be used for encoding Quaternions in say netcode or other serializations.

Because I am going to implement octahedral encoding and decoding in vertex arrays in a subsequent PR. If this were the case for the other encodes definitely they would be exposed in core. I just prefer the conservative approach of only providing commonly used functions in core API.

Because I am going to implement octahedral encoding and decoding in vertex arrays in a subsequent PR.

That's great, but why not design modularly now and then you won't have to refactor later? uncompress_quaternion just really shouldn't be a member method of Animation. It's PURELY functional, which is a good indication it shouldn't be a member method at all. Why would you need an Animation object to use this compression when it's a pure function and reads no data in the Animation object itself? :(

About exposing it publicly in the engine, I comment because we are already doing this in our network module (no one encodes orientations over the wire with 4 floats). But now when we migrate to 4.0, we will still need to duplicate this functionality because the functionality provided by the engine will be specifically encapsulated by the Animation object.

Well, its not super difficult to do this manually need it be, the main thing that matters is the octahedron compression, so it feels like for now this may be enough. May change my mind later if I see more demand.

To clarify, I am very obsessive of not putting in core stuff that is easy to workaround or that is not used frequently and I am not convinced about that. If I am mistaken on omitting something, generally time proves me wrong but if nobody uses something and it adds bloat, then time makes it very difficult to fix that bloat.

scene/resources/animation.h

scene/resources/animation.cpp

fire · 2021-10-21T17:01:12Z

On Windows 11. HighDPI is on. Noticed some clipped lock buttons.

reduz · 2021-10-21T17:08:55Z

@fire Its the same with the remove buttons, seems like a bug in ScrollContainer, unrelated to this PR.

fire · 2021-10-21T17:39:50Z

I saw that the compressed track is not on by default. Does this mean #51341 (default to 30fps) needs work?

reduz · 2021-10-21T17:47:04Z

@fire It would be nice to do some performance measurements on how fast the compressed track is vs the regular one, if the performance is not affected as much we could make it default, but I think this is likely better in a subsequent PR.

NOTE Sorry I misunderstood you, yes letting aside setting it on by default, it should be fine to default to 30fps with this PR, since if size starts becoming a problem, compression can turned on.

fire · 2021-10-21T17:52:05Z

The option can be difficult to find in menus to turn on compression. so it should be defaulted to off for the sake of people animating.
Maybe on the first keyframe [keying] destroy the compression and reimport? Then we can default on compression.
We should be able to default compress on in the default settings.
With this pr merged we can default to 30fps. 🦖

Roughly based on godotengine/godot-proposals#3375 (used format is slightly different). * Implement bitwidth based animation compression (see animation.h for format). * Can compress imported animations up to 10 times. * Compression format opens the door to streaming. * Works transparently (happens all inside animation.h)

lyuma

Reviewed together with @fire

I don't think there is anything we should do about this right now, since it's not called at runtime.... but _fetch_compressed_by_index appears to be linear time complexity.

It may be good to note that fetching all indices using a function like position_track_get_key in a loop on a compressed track would take O(N^2) time total.

Had a discussion in rocket chat that we should document how to calculate the tolerance and when to use uncompressed.

lyuma · 2021-10-21T18:53:36Z

core/math/quaternion.cpp

+	real_t r = ((real_t)1) / Math::sqrt(1 - w * w);
+	return Vector3(x * r, y * r, z * r);


Does this return NaN for the identity quaternion? Should it return 0,0,0 or an arbitrary axis instead?

I think w should never be negative, but it may be possible this is the case due to numerical precision, so a MAX(1-w*w,0.0) might be in order for a separate PR.

even if w is exactly +1.0 or -1.0, the issue is that (1-w*w) may become 0.0 which would cause a division by zero if Math::sqrt(1 - w * w)==0.
that is to say, r may become NaN
perhaps real_t r = MAX(0.0, ....); would avoid both issues.

(I'm a bit curious how identity quaternion is currently handled in this code / why this encoding doesn't cause trouble when converting such a NaN axis to octahedral compression)

I think the problem is that, if w is 1.0 (or -1.0), then whatever axis is does not really matter because upon rebuilding of axis/angle axis is multiplied by 0. I am not sure if we should add a special check to these functions for this case.

lyuma · 2021-10-22T00:27:03Z

scene/resources/animation.cpp

+	for (uint32_t i = 0; i < compression.pages.size(); i++) {
+		if (compression.pages[i].time_offset > p_time) {
+			break;


This should use binary search in case a given animation track has a large number of pages.

_fetch_compressed would be called in the inner loop of animation playback at runtime so it should be fast.

Note: unlike _fetch_compressed_by_index, I think this would be important to optimize since it's used heavily at runtime.

I think for the time being there will not be as many pages to make it worth implementing binary search. At some point, animation streaming should be implemented (for very large animations) and I think that will be the best time to do and test this optimization.

lyuma · 2021-10-22T00:43:53Z

scene/resources/animation.cpp

+		uint32_t frame = 0;
+	};
+
+	float split_tolerance = 1.5;


Should some of your test data from
https://chat.godotengine.org/channel/animation?msg=bY8KpPJ7tfkQNpGzv
be included here to explain how this magic number was chosen?

I just tried compressing a large animation to find the best value, but its not like it makes a huge difference.

akien-mga · 2021-10-25T12:28:06Z

Thanks!

Fixup to #54050, CI's GCC builds didn't catch it.

aaronfranke · 2021-10-29T16:23:16Z

core/math/vector3.h

+	_FORCE_INLINE_ Vector2 octahedron_encode() const {
+		Vector3 n = *this;
+		n /= Math::abs(n.x) + Math::abs(n.y) + Math::abs(n.z);
+		Vector2 o;
+		if (n.z >= 0.0) {
+			o.x = n.x;
+			o.y = n.y;
+		} else {
+			o.x = (1.0 - Math::abs(n.y)) * (n.x >= 0.0 ? 1.0 : -1.0);
+			o.y = (1.0 - Math::abs(n.x)) * (n.y >= 0.0 ? 1.0 : -1.0);
+		}
+		o.x = o.x * 0.5 + 0.5;
+		o.y = o.y * 0.5 + 0.5;
+		return o;
+	}
+
+	static _FORCE_INLINE_ Vector3 octahedron_decode(const Vector2 &p_oct) {
+		Vector2 f(p_oct.x * 2.0 - 1.0, p_oct.y * 2.0 - 1.0);
+		Vector3 n(f.x, f.y, 1.0f - Math::abs(f.x) - Math::abs(f.y));
+		float t = CLAMP(-n.z, 0.0, 1.0);
+		n.x += n.x >= 0 ? -t : t;
+		n.y += n.y >= 0 ? -t : t;
+		return n.normalized();
+	}


These methods seem pretty specific, is Vector3 the best place for them?

fire reviewed Oct 21, 2021

View reviewed changes

scene/resources/animation.h Outdated Show resolved Hide resolved

reduz force-pushed the animation-compression branch from 87a752f to 12feb72 Compare October 21, 2021 02:49

jordo reviewed Oct 21, 2021

View reviewed changes

core/math/vector3.h Show resolved Hide resolved

Chaosus added enhancement topic:animation topic:import labels Oct 21, 2021

Chaosus added this to the 4.0 milestone Oct 21, 2021

reduz force-pushed the animation-compression branch from 12feb72 to 587fbc9 Compare October 21, 2021 13:01

reduz marked this pull request as ready for review October 21, 2021 13:02

reduz requested review from a team as code owners October 21, 2021 13:02

reduz changed the title ~~Implement Animation Compression~~ Implement animation compression Oct 21, 2021

Calinou added the topic:3d label Oct 21, 2021

reduz force-pushed the animation-compression branch from 587fbc9 to 45e2ddb Compare October 21, 2021 13:30

jordo reviewed Oct 21, 2021

View reviewed changes

fire reviewed Oct 21, 2021

View reviewed changes

scene/resources/animation.h Outdated Show resolved Hide resolved

fire reviewed Oct 21, 2021

View reviewed changes

scene/resources/animation.cpp Show resolved Hide resolved

reduz force-pushed the animation-compression branch from 45e2ddb to a69541d Compare October 21, 2021 21:27

lyuma approved these changes Oct 22, 2021

View reviewed changes

akien-mga approved these changes Oct 25, 2021

View reviewed changes

akien-mga merged commit 24fdedf into godotengine:master Oct 25, 2021

akien-mga added a commit that referenced this pull request Oct 25, 2021

Fix GCC 12 -Werror=type-limits in animation compression code

5ff0624

Fixup to #54050, CI's GCC builds didn't catch it.

aaronfranke reviewed Oct 29, 2021

View reviewed changes

fire mentioned this pull request Feb 16, 2022

Add support for animation streaming (animation compression already implemented in 4.0) godotengine/godot-proposals#3375

Open

raulsntos mentioned this pull request Aug 25, 2022

Sync C# cubic interpolation with core #64860

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement animation compression #54050

Implement animation compression #54050

reduz commented Oct 20, 2021 •

edited

Loading

jordo commented Oct 21, 2021 •

edited

Loading

reduz commented Oct 21, 2021

jordo Oct 21, 2021

reduz Oct 21, 2021

jordo Oct 21, 2021

reduz Oct 21, 2021

jordo Oct 21, 2021 •

edited

Loading

reduz Oct 21, 2021

reduz Oct 21, 2021

fire commented Oct 21, 2021

reduz commented Oct 21, 2021

fire commented Oct 21, 2021 •

edited

Loading

reduz commented Oct 21, 2021 •

edited

Loading

fire commented Oct 21, 2021 •

edited

Loading

lyuma left a comment

lyuma Oct 21, 2021

reduz Oct 22, 2021

lyuma Oct 22, 2021

reduz Oct 24, 2021

lyuma Oct 22, 2021

lyuma Oct 22, 2021

reduz Oct 25, 2021

lyuma Oct 22, 2021

reduz Oct 22, 2021

akien-mga commented Oct 25, 2021

aaronfranke Oct 29, 2021

		real_t r = ((real_t)1) / Math::sqrt(1 - w * w);
		return Vector3(x * r, y * r, z * r);

Implement animation compression #54050

Implement animation compression #54050

Conversation

reduz commented Oct 20, 2021 • edited Loading

jordo commented Oct 21, 2021 • edited Loading

reduz commented Oct 21, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jordo Oct 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fire commented Oct 21, 2021

reduz commented Oct 21, 2021

fire commented Oct 21, 2021 • edited Loading

reduz commented Oct 21, 2021 • edited Loading

fire commented Oct 21, 2021 • edited Loading

lyuma left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akien-mga commented Oct 25, 2021

Choose a reason for hiding this comment

reduz commented Oct 20, 2021 •

edited

Loading

jordo commented Oct 21, 2021 •

edited

Loading

jordo Oct 21, 2021 •

edited

Loading

fire commented Oct 21, 2021 •

edited

Loading

reduz commented Oct 21, 2021 •

edited

Loading

fire commented Oct 21, 2021 •

edited

Loading