Reduce cubic interpolated animation hitch #54730

robfram · 2021-11-07T17:35:29Z

Reduce cubic interpolated animation hitch

Cubic interpolation didn't wrap around first and last keys, clamping
pre and post keys to first and last before interpolating values.

Fixes #20087

Calinou · 2021-11-07T18:18:42Z

Thanks for opening a pull request 🙂

Pull requests should be opened against the master branch instead of 3.x (as long as the feature/fix is relevant for master). This is because feature development occurs in the master branch. We can then cherry-pick specific PRs to the 3.x branch if they don't break backwards compatibility.

robfram · 2021-11-07T18:31:30Z

Pull requests should be opened against the master branch instead of 3.x (as long as the feature/fix is relevant for master). This is because feature development occurs in the master branch. We can then cherry-pick specific PRs to the 3.x branch if they don't break backwards compatibility.

Sorry, I can't PR against master branch because I can't test changes as my machine is not vulkan capable (and not even sure it compiles at all). I was looking for bugs in the 3.x branch for that reason, but I forgot this bug was still present in master.
My apologies.

Calinou · 2021-11-07T18:37:41Z

I can't PR against master branch because I can't test changes as my machine is not vulkan capable (and not even sure it compiles at all).

The master branch now has OpenGL support for 2D rendering, which you can enable by using the --rendering-driver opengl3 --single-window command line arguments.

You can create a new project from the command line as follows:

# Create an empty `project.godot` file within a folder.
mkdir -p /tmp/test
touch /tmp/test/project.godot

# Replace "~/godot/bin/godot.linuxbsd.tools.64" with the path to your compiled Godot executable.
~/godot/bin/godot.linuxbsd.tools.64 --rendering-driver opengl3 --single-window /tmp/test/project.godot

The 3D editor viewport won't display anything, but you should be able to switch to the 2D tab and see sprites added there.

robfram · 2021-11-07T18:50:56Z

The master branch now has OpenGL support for 2D rendering, which you can enable by using the --rendering-driver opengl3 --single-window command line arguments.

You can create a new project from the command line as follows:
# Create an empty `project.godot` file within a folder.
mkdir -p /tmp/test
touch /tmp/test/project.godot

# Replace "~/godot/bin/godot.linuxbsd.tools.64" with the path to your compiled Godot executable.
~/godot/bin/godot.linuxbsd.tools.64 --rendering-driver opengl3 --single-window /tmp/test/project.godot
The 3D editor viewport won't display anything, but you should be able to switch to the 2D tab and see sprites added there.

Amazing! I'm trying to compile it right now, disabling all third party modules that throw errors. if I'm able to make it run, I could replace this PR with a new one against master, and try to use it for future ones.
Thanks!

robfram · 2021-11-09T18:27:15Z

The PR for master is #54811

I was able to compile it patching 2 thirdparty libraries (icu4c and basis_universal). Should I open an issue/PR for them or they are outside Godot scope being thirdparty libs?

Although the editor works (in 2D as you said), I can't run the scenes. I got this message:

Your video card driver does not support any of the supported Vulkan or OpenGL versions.
Please update your drivers or if you have a very old or integrated GPU, upgrade it.
If you have updated your graphics drivers recently, try rebooting.

For this PR this was not a problem, as the hitch was noticeable directly in the editor.

akien-mga · 2021-11-09T21:00:01Z

I was able to compile it patching 2 thirdparty libraries (icu4c and basis_universal). Should I open an issue/PR for them or they are outside Godot scope being thirdparty libs?

We don't modify thirdparty libs unless we really can't fix issues differently. I'm not aware of any build issue for those libs on any platforms supported in our official builds though, I assume you're on a more exotic system? Please open issues for these.

robfram · 2021-11-10T17:55:21Z

Note: Perhaps this whole text should be it's own issue entry. I can move it to one if you want and delete it here. Thanks in advance, and I know this is not a common-enough scenario to be really attendend.

I assume you're on a more exotic system? Please open issues for these.

Yes, I'm on FreeBSD. Before opening issues for this (for basis_universal is only add one more defined comparison to get the same #include as MacOS), I want to confirm the building module system is working as expected.

I mean, I understand that they are options and modules (mainly talking about icu4c here) that can be enabled or disabled because:

They are optional and not needed to compile a working version of Godot, and/or
They can be provided system wide (aldreay present in the OS) instead as a Godot module when compiling.

When I tried to compile with builtin_icu=no, compilation fails due to others thirdparty modules dependences on it:

[ 29%] build_gdnative_api_struct(["modules/gdnative/include/gdnative_api_struct.gen.h", "modules/gdnative/gdnative_api_struct.gen.cpp"], ["modules/gdnative/gdnative_api.json"])
[ 32%] modules/text_server_adv/text_server_adv.cpp:38:10: fatal error: 'thirdparty/icu4c/icudata.gen.h' file not found
#include "thirdparty/icu4c/icudata.gen.h"
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[ 34%] 1 error generated.
[ 37%] tza_to_cpp(["thirdparty/oidn/weights/rtlightmap_hdr.gen.cpp"], ["thirdparty/oidn/weights/rtlightmap_hdr.tza"])
scons: *** [modules/text_server_adv/text_server_adv.linuxbsd.tools.64.llvm.o] Error 1
scons: building terminated because of errors.
[Time elapsed: 00:00:28.468]

So, I proceed with module_text_server_adv_enabled=no, and then it's the turn for basis_universal to fail. Again, no problem, time to disable that module too with module_basis_universal_enabled=no, and continue compiling.

And then, finally, when we try to execute it following the previous indication for OpenGL, we have a core dumped:

$ godot.linuxbsd.tools.64.llvm --rendering-driver opengl3 --single-window -e .
Godot Engine v4.0.dev.custom_build.e8870ddef - https://godotengine.org
ERROR: Property not found: rendering/quality/2d/use_nvidia_rect_flicker_workaround
   at: _get (core/config/project_settings.cpp:246)
ERROR: Property not found: rendering/quality/shading/force_lambert_over_burley
   at: _get (core/config/project_settings.cpp:246)
ERROR: Property not found: rendering/quality/shading/force_blinn_over_ggx
   at: _get (core/config/project_settings.cpp:246)
OpenGL Renderer: Mesa DRI Intel(R) HD Graphics 3000 (SNB GT2)
ERROR: Condition "!props.has(p_prop)" is true.
   at: set_custom_property_info (core/config/project_settings.cpp:977)
ERROR: Condition "!props.has(p_prop)" is true.
   at: set_custom_property_info (core/config/project_settings.cpp:977)
ERROR: Property not found: rendering/quality/2d/use_software_skinning
   at: _get (core/config/project_settings.cpp:246)
ERROR: Property not found: rendering/quality/2d/ninepatch_mode
   at: _get (core/config/project_settings.cpp:246)
  Undefined symbol "snd_dlpath"
  Undefined symbol "snd_config_is_array"
W: [(null)] caps.c: Normally all extra capabilities would be dropped now, but that's impossible because PulseAudio was built without capabilities support.

ERROR: TextServer: Unable to create TextServer interface.
   at: setup2 (main/main.cpp:1859)
ERROR: A Thread object has been destroyed without wait_to_finish() having been called on it. Please do so to ensure correct cleanup of the thread.
   at: ~Thread (core/os/thread.cpp:124)
Segmentation fault (core dumped)

First of all, compiled with the patched modules (with all the default modules enabled at compilation time) works good enough. Secondly, we can really ignore the core dump, as the important part is the ERROR: TextServer: Unable to create TextServer interface. message I think. Is module_text_server_adv mandatory?

Perhaps if the text_server_adv module could use system wide ICU, then no additional module modification would be needed at all. Otherwise, disabling the ICU module should disable text_server_adv automatically.

I don't mind trying to generate elaborated patches to thirdparty modules if it will fix the compilation problem to a runnable executable.

akien-mga · 2021-11-15T22:30:27Z

If you disable text_server_adv, you need to enable text_server_fb (fallback), which has less features but should give you a functional editor.

But yes, please open issues about the compilation problems for basis_universal and icu4c, they should be fixed.

Edit: Closed by mistake.

akien-mga · 2022-01-12T16:06:23Z

Closing as superseded by #54811 so we keep reviews in a single place (the master PR should be trivial to cherry-pick to 3.x once approved and merged).

akien-mga · 2022-01-12T16:07:14Z

I was too fast, the master PR says that things likely need a different implementation for 4.0 so it definitely makes sense to keep the 3.x implementation around.

TokageItLab

I think it's fine for 3.x. However, for 4.0 #54811 (comment), there is a problem as I mentioned there, ~~so please let me postpone it~~ see #58344.

TokageItLab · 2022-02-20T07:30:08Z

scene/resources/animation.cpp

@@ -1718,11 +1718,19 @@ T Animation::_interpolate(const Vector<TKey<T>> &p_keys, float p_time, Interpola
 		case INTERPOLATION_CUBIC: {
 			int pre = idx - 1;
 			if (pre < 0) {
-				pre = 0;
+				if (p_loop_wrap) {


Suggested change

if (p_loop_wrap) {

if (loop && p_loop_wrap) {

These keys should not be looked up when not looping.

TokageItLab · 2022-02-20T07:30:24Z

scene/resources/animation.cpp

 			}
 			int post = next + 1;
 			if (post >= len) {
-				post = next;
+				if (p_loop_wrap) {


Suggested change

if (p_loop_wrap) {

if (loop && p_loop_wrap) {

TokageItLab · 2022-03-01T10:18:38Z

Superseded by #58651.

Reduce cubic interpolated animation hitch

e0ae57d

Calinou added bug topic:animation labels Nov 7, 2021

Calinou added the cherrypick:3.4 label Nov 7, 2021

robfram mentioned this pull request Nov 9, 2021

Reduce cubic interpolated animation hitch #54811

Closed

akien-mga added this to the 3.5 milestone Nov 15, 2021

akien-mga closed this Nov 15, 2021

akien-mga reopened this Nov 15, 2021

akien-mga force-pushed the 3.x branch 2 times, most recently from 71cb8d3 to c58391c Compare January 6, 2022 22:40

akien-mga closed this Jan 12, 2022

akien-mga added archived and removed cherrypick:3.4 labels Jan 12, 2022

akien-mga reopened this Jan 12, 2022

akien-mga added cherrypick:3.4 and removed archived labels Jan 12, 2022

TokageItLab approved these changes Feb 20, 2022

View reviewed changes

TokageItLab requested changes Feb 20, 2022

View reviewed changes

TokageItLab mentioned this pull request Mar 1, 2022

[3.x] Fix cubic interpolate when looping #58651

Merged

TokageItLab closed this Mar 1, 2022

TokageItLab added archived and removed cherrypick:3.4 labels Mar 1, 2022

TokageItLab removed this from the 3.5 milestone Mar 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reduce cubic interpolated animation hitch #54730

Reduce cubic interpolated animation hitch #54730

robfram commented Nov 7, 2021

Calinou commented Nov 7, 2021 •

edited

Loading

robfram commented Nov 7, 2021

Calinou commented Nov 7, 2021 •

edited

Loading

robfram commented Nov 7, 2021

robfram commented Nov 9, 2021

akien-mga commented Nov 9, 2021

robfram commented Nov 10, 2021

akien-mga commented Nov 15, 2021 •

edited

Loading

akien-mga commented Jan 12, 2022

akien-mga commented Jan 12, 2022

TokageItLab left a comment •

edited

Loading

TokageItLab Feb 20, 2022 •

edited

Loading

TokageItLab Feb 20, 2022

TokageItLab commented Mar 1, 2022

Reduce cubic interpolated animation hitch #54730

Reduce cubic interpolated animation hitch #54730

Conversation

robfram commented Nov 7, 2021

Calinou commented Nov 7, 2021 • edited Loading

robfram commented Nov 7, 2021

Calinou commented Nov 7, 2021 • edited Loading

robfram commented Nov 7, 2021

robfram commented Nov 9, 2021

akien-mga commented Nov 9, 2021

robfram commented Nov 10, 2021

akien-mga commented Nov 15, 2021 • edited Loading

akien-mga commented Jan 12, 2022

akien-mga commented Jan 12, 2022

TokageItLab left a comment • edited Loading

Choose a reason for hiding this comment

TokageItLab Feb 20, 2022 • edited Loading

Choose a reason for hiding this comment

TokageItLab Feb 20, 2022

Choose a reason for hiding this comment

TokageItLab commented Mar 1, 2022

Calinou commented Nov 7, 2021 •

edited

Loading

Calinou commented Nov 7, 2021 •

edited

Loading

akien-mga commented Nov 15, 2021 •

edited

Loading

TokageItLab left a comment •

edited

Loading

TokageItLab Feb 20, 2022 •

edited

Loading