Skip to content

Add BlendSpace BlendPoint weight transitions by velocity limiting #105923

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ZeEndy
Copy link

@ZeEndy ZeEndy commented Apr 29, 2025

Closes godotengine/godot-proposals#12335
Still need more input on the proposal to improve usability but its pretty much done for the most part. This is my first PR did the best I could but I'm sure some things can defiantly be improved.

Implementation of blend point weight smoothing

  • Weight smoothing to AnimationBlendSpace2D
  • Weight smoothing to AnimationBlendSpace1D
  • Editor UI (change smoothing to only appear when using sync and continuous)

Demo project file

@Calinou Calinou added this to the 4.x milestone Apr 29, 2025
@ZeEndy ZeEndy closed this Apr 30, 2025
@ZeEndy ZeEndy reopened this Apr 30, 2025
@ZeEndy ZeEndy force-pushed the blendspace-sample-smoothing branch 6 times, most recently from 9cdd42e to 9b20f94 Compare April 30, 2025 20:46
@ZeEndy ZeEndy marked this pull request as ready for review April 30, 2025 20:48
@ZeEndy ZeEndy requested review from a team as code owners April 30, 2025 20:48
@fire fire requested a review from TokageItLab May 1, 2025 00:56
@TokageItLab
Copy link
Member

TokageItLab commented May 1, 2025

It looks like you have multiple implementations mixed up (weight implementation and smooth implementation can be separated). But from a quick look, I think it makes sense to have the BlendPoint have weight.

However, I am quite skeptical of the need for time-dependent smoothing using move_toward(). This should obviously be done before passing Vector2 to BlendSpace2D and be sufficient.

If you want to adjust the slope of the blendmap, you should not use move_toward(), but rather a mathematical implementation (without time delta) that maps the linear interplation to a smoothstep or tanh S-curve and adjusts the rapidity of that slope (like adjusting the contrast of an image).

Or, if it matches the your usage, I think we could welcome the approach of adding a new AnimationNode like GradientSpace2D that places a number of circles with blur.

@ZeEndy ZeEndy force-pushed the blendspace-sample-smoothing branch from 9b20f94 to 88f473e Compare May 2, 2025 11:26
@ZeEndy
Copy link
Author

ZeEndy commented May 2, 2025

@TokageItLab

However, I am quite skeptical of the need for time-dependent smoothing using move_toward(). This should obviously be done before passing Vector2 to BlendSpace2D and be sufficient.

I changed the delta time to not be based on the playback's delta time and updated the demo to be playable, If you switch with tab while playing you can see there is a major difference between smoothness when just interpolating the blend_position vector2 and using this method. It is usually common practice to have a blendspace to have 1 axis responsible for rotation and the other for speed, If you tried to interpolate that the character would have to go through each rotation before being able animate in a different direction even if you delay the interpolation of speed.

This is why this PR and feature proposal was made, due to a lack of a way to actually smoothly move between 2 radically different (in position) points with many points in between inside the blendspace with delta, without having to remake the blendspace in a script with way too many parameters to take care of or remake the entire blendspace system in gdextension to achieve the same effect as shown in the demo.

I dont mind making this a different animation node but I think there is a massive misunderstanding between the 2 methods of smoothing. I've stated in the proposal before why the method you suggested is not sufficient when it comes to large blendspaces. I've tried said method on multiple projects in different configurations and in none of them was it good enough as it still required the blend_position to cross unnecessary animations points causing jank.

I can however agree that just using move_toward() is not the solution and it would be better to figure out what would be a good way to allow users to modify the smoothing calculation themselves or what options could be included that would suffice for most users.

Or, if it matches the your usage, I think we could welcome the approach of adding a new AnimationNode like GradientSpace2D that places a number of circles with blur.

I'm not sure about the specific implementation of this animation node as I dont know if the GradientSpace2D/1D with a bunch of circles with a blur would be a good idea. Maybe if it was made to look like heatmap that changes color depending on the weight of the animation, but I'd have to look into to how to achieve that effect while clamped to the the blendspace "mesh" that wouldn't be insanely expensive.
Heres a quick mock up of where a user would have made the blend_position quickly move left to right.
image

@TokageItLab
Copy link
Member

TokageItLab commented May 2, 2025

For explanation, first assume that the vertex weights are all 1 to simplify the calculation.

Consider the current point P in triangle ABC.

Draw a line from vertex A through the current point to intersect line BC. The intersection with line BC is A', and the weight of the A can be calculated as follows:

weight_A = remap(A.distance_to(P), 0, A.distance_to(A'), 1, 0)

Here the weights are linearly interpolated in the range 0 to 1. Then, you can remap it to an arbitrary curve interpolation (smooth_speed is used as the abruptness of the curve). This is also done for B and C. Finally, each point is multiplied by its weight and divided by the total weight to normalize and you should be able to remove the move_toward and time base delta.

BTW, the implicitly normalized weight can be confused with the weights in Deterministic, which is the AnimationTree default. This is because weight can exceed 1, as allowed by NodeAdd and XfadeCurve.

So you should probably either "make normalization optional (default is true)" or "force normalization and rename it to height".

@ZeEndy
Copy link
Author

ZeEndy commented May 2, 2025

I'm not removing time delta since that's not the point of the PR. The point of the PR isn't to change how the weights are applied across 3 points in a triangle, the point of the PR is across the entire blendspace. I'm not here to change the curvature of weights I'm here to change the fact there is no way to quickly transition from triangle ABC to triangle GHI without going through DEF and applying the weights fully causing animation jank.

Just smoothing the blend_position Vector2 wont fix that and its not sufficient at all. If you try demo(or just look at the video) you'll see I put in both your suggested method from before with the Vector2 and my smoothing that you can hot swap with mine on or off while walking around and its a night and day difference.

BlendPoint.Smoothing.mp4

@TokageItLab
Copy link
Member

TokageItLab commented May 2, 2025

So are you saying that you want to discretely divide the triangle and crossfade the invalid / valid vertices when the current point cross the boundary? If so, delta might make sense, but at least it sounds that it should be a property called transition duration or blend time, not smooth. And in that case, I think that fading property can be enabled in DC mode as well.

Also, I think it would be clearer to have a fade in / fade out time parameter for each vertex, instead of having a single master speed parameter for BlendSpace2D and a weight for each vertex.

If you want to set the blend time for all vertices at once, you can use AnimationPlayer as a reference to set the default_blend_time in BlendSpace2D.

In this case, instead of having a bool smooth in BlendSpace2D, we could have a bool override_blend checkbox at each vertex, so that only the vertices with this checkbox enabled use their own fade in / fade out time instead of the default blend time.

@ZeEndy
Copy link
Author

ZeEndy commented May 2, 2025

Yea it's something among those lines. DC can definitely be done as well I just did continuas as that seemed like the logical first place. I only named it smoothing since that's what it was called in UE and I have no real other reference for such a feature. I'll definitely look into implementing your suggestions when I can.

@fire fire moved this to Work in progress in Animation Team Issue Triage May 4, 2025
@ZeEndy ZeEndy changed the title Add BlendSpace BlendPoint weight smoothing Add BlendSpace BlendPoint weight fading May 4, 2025
@ZeEndy ZeEndy changed the title Add BlendSpace BlendPoint weight fading Add BlendSpace BlendPoint weight transitions May 6, 2025
@ZeEndy ZeEndy force-pushed the blendspace-sample-smoothing branch 2 times, most recently from 4cc14d6 to 457810c Compare May 12, 2025 11:45
@ZeEndy ZeEndy changed the title Add BlendSpace BlendPoint weight transitions Add BlendSpace BlendPoint weight transitions by velocity limiting May 13, 2025
@ZeEndy ZeEndy force-pushed the blendspace-sample-smoothing branch 4 times, most recently from 16fe32c to 615eb2e Compare May 17, 2025 23:50
@ZeEndy ZeEndy force-pushed the blendspace-sample-smoothing branch 2 times, most recently from 4726f46 to ba22d29 Compare May 21, 2025 07:59
@TokageItLab TokageItLab marked this pull request as draft May 21, 2025 16:55
@TokageItLab
Copy link
Member

TokageItLab commented May 21, 2025

Once I made it a draft, so please mark as ready to review when finish working. If you have any other questions, you can mention or DM me.

@fire fire requested a review from a team May 21, 2025 17:02
@ZeEndy ZeEndy force-pushed the blendspace-sample-smoothing branch 2 times, most recently from aeb86e3 to e8c9ff9 Compare May 22, 2025 07:58
@ZeEndy ZeEndy force-pushed the blendspace-sample-smoothing branch from e8c9ff9 to 5252abb Compare May 24, 2025 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Work in progress
Development

Successfully merging this pull request may close these issues.

Add BlendSpace BlendPoint weight transitions
3 participants