Anim: don't include handles when calculating FCurve bounds in Graph Editor #120519

Christoph Lendenfeld · 2024-04-11T13:30:21+02:00

2024-04-11 13:30:21 +02:00

On heavy scenes, the function get_graph_keyframe_extents takes up a considerable amount of the time (~50%) it takes to draw the Graph Editor.
This function is called to set the View2D extents which makes the sliders work properly.
I'd argue that level of precision isn't needed just to display sliders.
Passing the flag as false speeds up the drawing code by 22%.

Timings for graph_main_region_draw

Before	After
32ms	25ms

Timings for get_graph_keyframe_extents

Before	After
~16ms	~8ms

Test file used
https://download.blender.org/ftp/sybren/animation-rigging/heavy_mocap_test.blend

Before

After:

Notice the size of the sliders on the bottom and right has changed as a result of no longer factoring the handles in.
Test file attached

On heavy scenes, the function `get_graph_keyframe_extents` takes up a considerable amount of the time (~50%) it takes to draw the Graph Editor. This function is called to set the `View2D` extents which makes the sliders work properly. I'd argue that level of precision isn't needed just to display sliders. Passing the flag as `false` speeds up the drawing code by 22%. Timings for `graph_main_region_draw` | Before | After | | - | - | | 32ms | 25ms | Timings for `get_graph_keyframe_extents` | Before | After | | - | - | | ~16ms | ~8ms | Test file used https://download.blender.org/ftp/sybren/animation-rigging/heavy_mocap_test.blend ------ Before ![image](/attachments/4af14b61-84f1-41da-a326-6d978faca9c6) After: ![image](/attachments/10bd2504-f79b-4a0d-819a-ce07762a83ba) Notice the size of the sliders on the bottom and right has changed as a result of no longer factoring the handles in. Test file attached

image.png

52 KiB

image.png

52 KiB

ge_slider_handles.blend

910 KiB

Christoph Lendenfeld added the

Module

Animation & Rigging

label 2024-04-11 13:30:21 +02:00

Christoph Lendenfeld added 1 commit 2024-04-11 13:30:32 +02:00

6e464e7e81 don't calc handles

Christoph Lendenfeld added 2 commits 2024-04-12 09:43:09 +02:00

81de18c1b4 Merge branch 'main' into graph_dont_calc_handles

ba1b1454be add comment

Nathan Vegdahl approved these changes 2024-04-12 11:07:51 +02:00

Nathan Vegdahl left a comment

Thanks for the pictures! Now I understand what you were talking about.

To me this seems totally fine:

In practice I don't think many people use the scroll bars for navigation. But even for people that do, the scroll bar extents don't actually affect how far you can drag them (just tested this). In other words, they're purely a visual indication of data bounds, with no effect on interaction.
Changing that visual indication to mean "the keys, without handles" seems just as useful to me as "the keys, with handles". It might actually be even more useful.

Thanks for the pictures! Now I understand what you were talking about. To me this seems totally fine: - In practice I don't think many people use the scroll bars for navigation. But even for people that do, the scroll bar extents don't actually affect how far you can drag them (just tested this). In other words, they're purely a visual indication of data bounds, with no effect on interaction. - Changing that visual indication to mean "the keys, without handles" seems just as useful to me as "the keys, with handles". It might actually be even more useful.

Sybren A. Stüvel commented

2024-04-12 11:14:58 +02:00

This function is called to set the View2D extents which makes the sliders work properly.

Given that there are also the "slider operators", maybe it's clearer to refer to those as the scrollbars? Or the scrollbar sliders?

And 👍 for the functionality, I agree that computing these extents is not the best use of CPU time.

> This function is called to set the `View2D` extents which makes the sliders work properly. Given that there are also the "slider operators", maybe it's clearer to refer to those as the scrollbars? Or the scrollbar sliders? And :+1: for the functionality, I agree that computing these extents is not the best use of CPU time.

Christoph Lendenfeld commented

2024-04-12 11:26:04 +02:00

Given that there are also the "slider operators", maybe it's clearer to refer to those as the scrollbars? Or the scrollbar sliders?

Ha yeah sorry was led astray by Tons comment which mentions sliders. Scrollbars is indeed the correct word to use

> Given that there are also the "slider operators", maybe it's clearer to refer to those as the scrollbars? Or the scrollbar sliders? Ha yeah sorry was led astray by Tons comment which mentions sliders. Scrollbars is indeed the correct word to use

Christoph Lendenfeld referenced this issue from a commit

2024-04-12 11:27:45 +02:00

Anim: don't include handles when calculating FCurve bounds in Graph Editor

Christoph Lendenfeld merged commit 0157fede80 into main

2024-04-12 11:27:45 +02:00

Christoph Lendenfeld deleted branch graph_dont_calc_handles

2024-04-12 11:27:48 +02:00

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Anim: don't include handles when calculating FCurve bounds in Graph Editor #120519