Fix #112085: Not able to insert keys on subframes #112131

Christoph Lendenfeld · 2023-09-08T12:59:09+02:00

2023-09-08 12:59:09 +02:00

When subframes were enabled, and the time cursor was at a subframe position, the keying would still insert the key at the full frame.

This was because the value passed to the key insertion functions was just the full frame value and not the subframe part.
Fixing it by using BKE_scene_frame_get() which returns the floating point frame including the subframe part.

I've added unit tests to ensure that inserting keys at subframe values doesn't cause issues.

When subframes were enabled, and the time cursor was at a subframe position, the keying would still insert the key at the full frame. This was because the value passed to the key insertion functions was just the full frame value and not the subframe part. Fixing it by using `BKE_scene_frame_get()` which returns the floating point frame including the subframe part. I've added unit tests to ensure that inserting keys at subframe values doesn't cause issues.

Christoph Lendenfeld added the

Module

Animation & Rigging

label 2023-09-08 12:59:09 +02:00

Christoph Lendenfeld added 1 commit 2023-09-08 12:59:26 +02:00

d661c0bf36 the fix

Christoph Lendenfeld added this to the Animation & Rigging project 2023-09-08 13:01:38 +02:00

Christoph Lendenfeld added 3 commits 2023-09-12 13:21:13 +02:00

d0b5a3fa48 Merge branch 'main' into fix_subframe_keyframing

95990561a0 Merge branch 'main' into fix_subframe_keyframing

ed06bda997 add keyframe insertion test

Sybren A. Stüvel commented

2023-09-12 14:44:29 +02:00

I think a million frames is a bit much, realistically. Even at 60FPS that would be a 4.6 hour shot. DNA_scene_types.h has this though:

#define MAXFRAME 1048574

so yeah, even though in practice it'll hardly happen, testing with that high frame count is actually good :)

As for the precision, BEZT_BINARYSEARCH_THRESH = 0.01f, which is currently what defines the practical resolution of subframes. Testing with less than 7 decimal places would be acceptable for sure.

I think a million frames is a bit much, realistically. Even at 60FPS that would be a 4.6 hour shot. `DNA_scene_types.h` has this though: ```c #define MAXFRAME 1048574 ``` so yeah, even though in practice it'll hardly happen, testing with that high frame count is actually good :) As for the precision, `BEZT_BINARYSEARCH_THRESH = 0.01f`, which is currently what defines the practical resolution of subframes. Testing with less than 7 decimal places would be acceptable for sure.

Christoph Lendenfeld commented

2023-09-12 15:15:14 +02:00

thanks that got me unstuck
I've now realized why the test failed. Floating point numbers don't have sufficient resolution at 1.000.000
I've used this calculator for this https://www.h-schmidt.net/FloatConverter/IEEE754.html
and also confirmed in blender. When you go to that frame and enable subframes, the steps the time cursor takes correspond to the floating point precision steps.

Now the question is what to do.
Soften up the test some more or have it fail correctly.
By the latter I mean insert 50 keys but only expect them to be at the floating point precision steps and also check the amount of keys is not larger than the steps available.
I tend to the latter but am open to suggestions

EDIT: pushed a change that fails correctly to better explain what I mean by that

thanks that got me unstuck I've now realized why the test failed. Floating point numbers don't have sufficient resolution at 1.000.000 I've used this calculator for this https://www.h-schmidt.net/FloatConverter/IEEE754.html and also confirmed in blender. When you go to that frame and enable subframes, the steps the time cursor takes correspond to the floating point precision steps. Now the question is what to do. Soften up the test some more or have it fail correctly. By the latter I mean insert 50 keys but only expect them to be at the floating point precision steps and also check the amount of keys is not larger than the steps available. I tend to the latter but am open to suggestions ---- EDIT: pushed a change that fails correctly to better explain what I mean by that

Christoph Lendenfeld added 1 commit 2023-09-12 15:31:32 +02:00

4d0b805eab change high frame number check

takes floating point resolution into account

Christoph Lendenfeld added 1 commit 2023-09-12 16:51:39 +02:00

3504292fb3 keyframe deletion test

Christoph Lendenfeld changed title from ~~WIP: Fix #112085: Not able to insert keys on subframes~~ to Fix #112085: Not able to insert keys on subframes

2023-09-12 17:02:55 +02:00

Christoph Lendenfeld requested review from Sybren A. Stüvel 2023-09-12 17:03:03 +02:00

Sybren A. Stüvel approved these changes 2023-09-14 14:58:12 +02:00

Sybren A. Stüvel left a comment

Excellent! Yes, I prefer this approach too.

Christoph Lendenfeld merged commit 8f1bd71992 into main

2023-09-14 15:07:15 +02:00

Christoph Lendenfeld deleted branch fix_subframe_keyframing

2023-09-14 15:07:20 +02:00

Christoph Lendenfeld referenced this issue from a commit

2023-09-14 15:07:22 +02:00

Fix #112085: Not able to insert keys on subframes

Christoph Lendenfeld removed this from the Animation & Rigging project 2023-11-21 14:38:27 +01: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

Fix #112085: Not able to insert keys on subframes #112131