Anim: Add new keyframe type 'GENERATED' #120564

Sybren A. Stüvel · 2024-04-12T12:42:50+02:00

Sybren A. Stüvel commented

2024-04-12 12:42:50 +02:00

Add a new keyframe type named 'generated', which is meant to indicate that the key was set by some automated tool (like an add-on), rather than manually by an animator.

This is meant for tooling that needs to create keys in a repeatable way. With this new key type, the tool can know which keys it generated before, and thus those can be removed and re-generated.

Here are some screenshots with different themes. In the middle there are some generated keys, and some of those are selected. The 'generated' keys are intentionally shown dimly, so that they don't distract from the animator-set keys.

Theme	Screenshot
Blender Dark
Blender Light
Deep Grey
Maya
Minimal Dark	The blue outline comes from the existing theme
Modo
Print Friendly
White
XSI

Note that the themes (except the first two) are part of the addons repository, and thus can't be included in this PR. I'll commit those separately.

Question for the reviewers: what shall we do with the graph editor? Blender currently draws all key types the same there. For this type, we might want to make an exception? That can be done in a later PR though.

Add a new keyframe type named 'generated', which is meant to indicate that the key was set by some automated tool (like an add-on), rather than manually by an animator. This is meant for tooling that needs to create keys in a repeatable way. With this new key type, the tool can know which keys it generated before, and thus those can be removed and re-generated. Here are some screenshots with different themes. In the middle there are some generated keys, and some of those are selected. The 'generated' keys are intentionally shown dimly, so that they don't distract from the animator-set keys. | Theme | Screenshot | | -------------- | -------------------------------------------------------------------------------------------------------------- | | Blender Dark | ![image](/attachments/ad637714-b763-4ebe-8a1d-0e236d054783) | | Blender Light | ![image](/attachments/fc116ab9-0c44-4977-a546-2ec1510508ae) | | Deep Grey | ![image](/attachments/28991fcf-e20d-4600-bdc5-f8c499b2e304) | | Maya | ![image](/attachments/44a24fae-6931-4484-bddb-c4011781ef73) | | Minimal Dark | ![image](/attachments/deebd72c-c173-42db-8481-be645a834c6d) <br>The blue outline comes from the existing theme | | Modo | ![image](/attachments/b8c1c413-3c90-4124-b0f9-9ce7cc16f9bf) | | Print Friendly | ![image](/attachments/cd78f08d-1f27-4939-86e0-877887e939c2) | | White | ![image](/attachments/dff5fdf8-aece-4200-9192-30820a7db47a) | | XSI | ![image](/attachments/f1a17149-7f61-422c-bd43-7777e1584e83) | Note that the themes (except the first two) are part of the addons repository, and thus can't be included in this PR. I'll commit those separately. ------------ Question for the reviewers: what shall we do with the graph editor? Blender currently draws all key types the same there. For this type, we might want to make an exception? That can be done in a later PR though.

image.png

9.1 KiB

image.png

10 KiB

image.png

8.7 KiB

image.png

9.8 KiB

image.png

8.8 KiB

image.png

15 KiB

image.png

11 KiB

image.png

9.5 KiB

image.png

8.6 KiB

image.png

8.7 KiB

❤️ 1

Sybren A. Stüvel added 1 commit 2024-04-12 12:42:58 +02:00

Anim: Add new keyframe type 'GENERATED' 218138a287

Add a new keyframe type named 'generated', which is meant to indicate
that the key was set by some automated tool, rather than manually by an
animator.

This is meant for tooling that needs to create keys in a repeatable way.
With this new key type, the tool can know which keys it generated
before, and thus those can be removed and re-generated.

Sybren A. Stüvel added the

Module

Animation & Rigging

label 2024-04-12 12:43:22 +02:00

Sybren A. Stüvel requested review from Nathan Vegdahl 2024-04-12 12:44:28 +02:00

Sybren A. Stüvel requested review from Christoph Lendenfeld 2024-04-12 12:44:28 +02:00

Sybren A. Stüvel requested review from Pablo Vazquez 2024-04-12 12:44:35 +02:00

Pablo Vazquez reviewed 2024-04-12 12:52:36 +02:00

source/blender/makesrna/intern/rna_fcurve.cc Outdated

						
				@ -113,0 +114,4 @@

				     "GENERATED",

				     ICON_KEYTYPE_GENERATED_VEC,

				     "Generated",

				     "A key generated by some generated, instead of a human animator"},

Pablo Vazquez commented

2024-04-12 12:52:36 +02:00

The tooltip A key generated by some generated, instead of a human animator is a bit confusing, maybe something like:

A key created automatically by a generative operation
A key created by a generative tool, not manually created
A key created by a generator operator, not manually

The tooltip `A key generated by some generated, instead of a human animator` is a bit confusing, maybe something like: 0. `A key created automatically by a generative operation` 1. `A key created by a generative tool, not manually created` 2. `A key created by a generator operator, not manually`

Sybren A. Stüvel commented

2024-04-12 15:18:00 +02:00

Hah, over-zealous search & replace, the key type was originally called "automation", but I changed that to "generated". This description became downright stupid. It used to be:

A key generated by some automation, instead of a human animator

In any case, I like a combination of your suggestions, A key generated automatically by a tool, not manually created.

Hah, over-zealous search & replace, the key type was originally called "automation", but I changed that to "generated". This description became downright stupid. It used to be: > A key generated by some automation, instead of a human animator In any case, I like a combination of your suggestions, `A key generated automatically by a tool, not manually created`.

👍 1

dr.sybren marked this conversation as resolved

Christoph Lendenfeld commented

2024-04-12 14:33:12 +02:00

Code wise I think this works.
The bigger issue I see is with UX. The user can set keyframes as "generated" by pressing R in the Dope Sheet.
That's confusing and also unsafe if the goal is for those key types to be marked as "generate by code"

I am not sure what the solution is. If we hide this key type from the artists its also a bit confusing. The artists would have no way of knowing what that key type means.

The following is my train of thought that led nowhere.

I think a better approach would be to separate the concern of "animator that wants to organize his keys" and "developer that wants to tag keys for later use". We could add a flag to BezTriple doesn't even make the struct bigger since we'd just be eating into the padding.

But as I am writing this I realize that by using a flag the artists will not see that keys have been marked.
So the following can happen.

Artist uses a tool that generates keys and tags them.

Artist modifies those keys, then executes the tool.

The tool rebuilds keys that have been modified by the artists who is confused why his animation changed.

Code wise I think this works. The bigger issue I see is with UX. The user can set keyframes as "generated" by pressing `R` in the Dope Sheet. That's confusing and also unsafe if the goal is for those key types to be marked as "generate by code" I am not sure what the solution is. If we hide this key type from the artists its also a bit confusing. The artists would have no way of knowing what that key type means. The following is my train of thought that led nowhere. > I think a better approach would be to separate the concern of "animator that wants to organize his keys" and "developer that wants to tag keys for later use". We could add a flag to `BezTriple` doesn't even make the struct bigger since we'd just be eating into the padding. > > But as I am writing this I realize that by using a flag the artists will not see that keys have been marked. > So the following can happen. > * Artist uses a tool that generates keys and tags them. > * Artist modifies those keys, then executes the tool. > * The tool rebuilds keys that have been modified by the artists who is confused why his animation changed.

Sybren A. Stüvel added 1 commit 2024-04-12 15:19:19 +02:00

Feedback Pablo

buildbot/vexp-code-patch-linux-x86_64 Build done.

Details

buildbot/vexp-code-patch-darwin-arm64 Build done.

Details

buildbot/vexp-code-patch-windows-amd64 Build done.

Details

buildbot/vexp-code-patch-coordinator Build done.

Details

40fe9b5203

Sybren A. Stüvel commented

2024-04-12 15:25:04 +02:00

The bigger issue I see is with UX. The user can set keyframes as "generated" by pressing R in the Dope Sheet.
That's confusing and also unsafe if the goal is for those key types to be marked as "generate by code"

I see your point. I think this is fine, though. It gives control to animators, where they can choose themselves whether to change generated keys to manual ones (opening the door for the next tool to create other keys), or vice versa.

I wouldn't mind having a separator between the currently-existing types and the 'generated' one, though, to visualise a bit better that it's a different one. But that's hard, given that this popup is just a generic "show the enum" call.

I think a better approach would be to separate the concern of "animator that wants to organize his keys" and "developer that wants to tag keys for later use". We could add a flag to BezTriple doesn't even make the struct bigger since we'd just be eating into the padding.

But as I am writing this I realize that by using a flag the artists will not see that keys have been marked.
So the following can happen.

Artist uses a tool that generates keys and tags them.

Artist modifies those keys, then executes the tool.

The tool rebuilds keys that have been modified by the artists who is confused why his animation changed.

I think it's not a good idea to have hidden flags, and we don't have enough colours & shapes to make this obvious in the dopesheet. That's why I got to the most simple approach, and simply add another key type.

> The bigger issue I see is with UX. The user can set keyframes as "generated" by pressing `R` in the Dope Sheet. > That's confusing and also unsafe if the goal is for those key types to be marked as "generate by code" I see your point. I think this is fine, though. It gives control to animators, where they can choose themselves whether to change generated keys to manual ones (opening the door for the next tool to create other keys), or vice versa. I wouldn't mind having a separator between the currently-existing types and the 'generated' one, though, to visualise a bit better that it's a different one. But that's hard, given that this popup is just a generic "show the enum" call. > > I think a better approach would be to separate the concern of "animator that wants to organize his keys" and "developer that wants to tag keys for later use". We could add a flag to `BezTriple` doesn't even make the struct bigger since we'd just be eating into the padding. > > > > But as I am writing this I realize that by using a flag the artists will not see that keys have been marked. > > So the following can happen. > > * Artist uses a tool that generates keys and tags them. > > * Artist modifies those keys, then executes the tool. > > * The tool rebuilds keys that have been modified by the artists who is confused why his animation changed. I think it's not a good idea to have hidden flags, and we don't have enough colours & shapes to make this obvious in the dopesheet. That's why I got to the most simple approach, and simply add another key type.

Nathan Vegdahl approved these changes 2024-04-12 15:57:40 +02:00

Nathan Vegdahl left a comment

Looks good to me.

source/blender/editors/animation/keyframes_draw.cc

						
				@ -41,3 +41,3 @@

				                         const bool sel,

				                         const eBezTriple_KeyframeType key_type,

				                         const short mode,

				                         const eKeyframeShapeDrawOpts mode,

Nathan Vegdahl commented

2024-04-12 15:47:59 +02:00

Thank you.

❤️ 1

dr.sybren marked this conversation as resolved

Sybren A. Stüvel commented

2024-04-12 16:25:06 +02:00

@blender-bot build

Sybren A. Stüvel referenced this pull request

2024-04-12 16:26:56 +02:00

Anim: add keytype argument to `keyframe_insert()` RNA function #120578

Christoph Lendenfeld approved these changes 2024-04-12 16:29:46 +02:00

Christoph Lendenfeld left a comment

I think it's not a good idea to have hidden flags, and we don't have enough colours & shapes to make this obvious in the dopesheet. That's why I got to the most simple approach, and simply add another key type.

I agree, let's land it then

> I think it's not a good idea to have hidden flags, and we don't have enough colours & shapes to make this obvious in the dopesheet. That's why I got to the most simple approach, and simply add another key type. I agree, let's land it then

🎉 1

Sybren A. Stüvel referenced this issue from a commit

2024-04-15 10:43:13 +02:00

Anim: Add new keyframe type 'GENERATED'

Sybren A. Stüvel referenced this issue from a commit

2024-04-15 10:46:06 +02:00

Anim: add theme colors for new 'GENERATED' key type

Sybren A. Stüvel manually merged commit 51e1f29a68 into main

2024-04-15 11:38:05 +02:00

Dalai Felinto referenced this issue from a commit

2024-05-14 21:59:52 +02:00

Anim: add theme colors for new 'GENERATED' key type