godot icon indicating copy to clipboard operation
godot copied to clipboard

Convert Vector to LocalVector in animation system

Open YYF233333 opened this issue 1 year ago • 2 comments

Implement godotengine/godot-proposals#10594

convert all Vector I found in animation_tree.h and animation_blend_tree.h except:

  • connections : they involve a copy which may benifit from COW
  • get_editable_animation_list : seems like an interface

Tested with godot-benchmark, no noticeable performance change. Is this change acceptable as a codestyle improvement?

Build with scons target=template_release production=yes, run with .\godot.windows.template_release.x86_64.console.exe .\godot-benchmarks\project.godot -- --run-benchmarks --include-benchmarks="animation/animated_models/*"

Test Result:

Master Branch:
{"engine":{"version":"v4.4.dev.custom_build","version_hash":"e3213aaef5e0e72b8272e65d989d3d8222be17ca"},"system":{"cpu_architecture":"x86_64","cpu_name":"12th Gen Intel(R) Core(TM) i5-12400","os":"Windows","cpu_count":12}}
{"benchmarks":[{"name":"Animation Blend Tree 100","category":"Animation > Animated Models","results":{"render_cpu":6.455,"render_gpu":64.95,"time":0.036}},{"name":"Animation State Machine 1000","category":"Animation > Animated Models","results":{"render_cpu":6.371,"render_gpu":64.87,"time":0.017}}]}
{"benchmarks":[{"name":"Animation Blend Tree 100","category":"Animation > Animated Models","results":{"render_cpu":6.462,"render_gpu":64.97,"time":0.023}},{"name":"Animation State Machine 1000","category":"Animation > Animated Models","results":{"render_cpu":6.378,"render_gpu":64.82,"time":0.014}}]}
{"benchmarks":[{"name":"Animation Blend Tree 100","category":"Animation > Animated Models","results":{"render_cpu":6.457,"render_gpu":65.08,"time":0.023}},{"name":"Animation State Machine 1000","category":"Animation > Animated Models","results":{"render_cpu":6.313,"render_gpu":64.85,"time":0.017}}]}
{"benchmarks":[{"name":"Animation Blend Tree 100","category":"Animation > Animated Models","results":{"render_cpu":6.451,"render_gpu":65.03,"time":0.032}},{"name":"Animation State Machine 1000","category":"Animation > Animated Models","results":{"render_cpu":6.358,"render_gpu":64.81,"time":0.015}}]}
{"benchmarks":[{"name":"Animation Blend Tree 100","category":"Animation > Animated Models","results":{"render_cpu":6.482,"render_gpu":64.93,"time":0.026}},{"name":"Animation State Machine 1000","category":"Animation > Animated Models","results":{"render_cpu":6.369,"render_gpu":64.82,"time":0.016}}]}

This PR:
{"engine":{"version_hash":"5d3d68dd405a13a4de63b8c39c000fc67bb5dea8","version":"v4.4.dev.custom_build"},"system":{"os":"Windows","cpu_count":12,"cpu_name":"12th Gen Intel(R) Core(TM) i5-12400","cpu_architecture":"x86_64"}}
{"benchmarks":[{"name":"Animation Blend Tree 100","category":"Animation > Animated Models","results":{"render_cpu":6.599,"render_gpu":65.46,"time":0.028}},{"name":"Animation State Machine 1000","category":"Animation > Animated Models","results":{"render_cpu":6.37,"render_gpu":65.55,"time":0.016}}]}
{"benchmarks":[{"name":"Animation Blend Tree 100","category":"Animation > Animated Models","results":{"render_cpu":6.562,"render_gpu":65.04,"time":0.022}},{"name":"Animation State Machine 1000","category":"Animation > Animated Models","results":{"render_cpu":6.434,"render_gpu":64.74,"time":0.026}}]}
{"benchmarks":[{"name":"Animation Blend Tree 100","category":"Animation > Animated Models","results":{"render_cpu":6.562,"render_gpu":66.36,"time":0.024}},{"name":"Animation State Machine 1000","category":"Animation > Animated Models","results":{"render_cpu":6.427,"render_gpu":64.78,"time":0.029}}]}
{"benchmarks":[{"name":"Animation Blend Tree 100","category":"Animation > Animated Models","results":{"render_cpu":6.236,"render_gpu":63.03,"time":0.023}},{"name":"Animation State Machine 1000","category":"Animation > Animated Models","results":{"render_cpu":6.425,"render_gpu":62.2,"time":0.017}}]}
{"benchmarks":[{"name":"Animation Blend Tree 100","category":"Animation > Animated Models","results":{"render_cpu":6.927,"render_gpu":65.25,"time":0.025}},{"name":"Animation State Machine 1000","category":"Animation > Animated Models","results":{"render_cpu":6.874,"render_gpu":65.07,"time":0.017}}]}

# Animation Blend Tree 100 is changed to Animation Blend Tree 1000 because it takes too little time

YYF233333 avatar Oct 01 '24 11:10 YYF233333

oops, I forget to check dev_mode, now tested locally.

YYF233333 avatar Oct 01 '24 14:10 YYF233333

I didn't see any major problems, but I haven't tested.

fire avatar Oct 02 '24 04:10 fire

Is this PR still needed? It has been no feedback for several months. I'm going to close this if no body want.

YYF233333 avatar Dec 17 '24 09:12 YYF233333

Thanks!

Repiteo avatar Dec 23 '24 17:12 Repiteo