Page MenuHome

Fix T76595: Indicate the Active Keyframe in Graph Editor
Needs ReviewPublic

Authored by Hans Goudey (HooglyBoogly) on Thu, May 14, 11:06 PM.

Details

Summary

Currently there is a panel that says "Active Keyframe" for numerically editing one keyframe's values, but in the code there is no concept of the "active keyframe."

This patch adds an "active keyframe index" to each FCurve, and displays it with a theme color for the active vertex (which didn't exist before) if the FCurve is active.

The active keyframe is not currently set for select operations other than basic click-select, which mirrors the behavior in the 3D view.

Diff Detail

Repository
rB Blender
Branch
active-fcurve-keyframe (branched from master)
Build Status
Buildable 8122
Build 8122: arc lint + arc unit

Event Timeline

Hans Goudey (HooglyBoogly) requested review of this revision.Thu, May 14, 11:06 PM
Hans Goudey (HooglyBoogly) created this revision.
  • Remove unintentional change
Hans Goudey (HooglyBoogly) retitled this revision from Fix T76595: No Indication of Active Keyframe in Graph Editor to Fix T76595: Indicate the Active Keyframe in Graph Editor.Fri, May 15, 3:42 AM
Julian Eisel (Severin) requested changes to this revision.Sun, May 17, 8:03 PM

This makes sense to me in principle. With a bit more polish this can go in I think.

  • There's a drawing glitch: After deleting the active keyframe, the white dot is still there.
  • Shift+selecting an already selected point to deselect it activates the triple. Elsewhere we activate it on first click, and deselect on second one.
  • If multiple points are selected, dragging one makes it active. That's not how we do it usually, e.g. see Node Editor. I think the logic should be: Only set active if either the point wasn't selected before or other points got deselected. See wait_to_deselect_others and already_selected.

Just a thought:

  • In the Dopesheet we don't have an active keyframe I think. But the selection is always the same as in the Graph Editor (they access the same FCurve data). For consistency I think the Dopesheet should also display the active keyframe. To visualize it, the outline could be drawn white (unselected keyframes are already drawn white, so that's not an option).
source/blender/editors/space_graph/graph_draw.c
229–238

Would stay consistent with the functions above and move this into its own function. Then higher and lower level code isn't mixed as much, and code reads more or less like English text.
Same for the handles below.

This revision now requires changes to proceed.Sun, May 17, 8:03 PM

This makes sense to me in principle. With a bit more polish this can go in I think.

  • There's a drawing glitch: After deleting the active keyframe, the white dot is still there.

I can't replicate this. Is this what you meant?

  • Shift+selecting an already selected point to deselect it activates the triple. Elsewhere we activate it on first click, and deselect on second one.

True, deselecting an active point shouldn't activate its handles.

  • If multiple points are selected, dragging one makes it active. That's not how we do it usually, e.g. see Node Editor. I think the logic should be: Only set active if either the point wasn't selected before or other points got deselected. See wait_to_deselect_others and already_selected.

This check seems to work pretty well:

// AFTER CLICK SELECT
if (BEZT_ISSEL_ANY(bezt) && (select_mode == SELECT_ADD || !already_selected)) {
  // SET ACTIVE
}

Just a thought:

  • In the Dopesheet we don't have an active keyframe I think. But the selection is always the same as in the Graph Editor (they access the same FCurve data). For consistency I think the Dopesheet should also display the active keyframe. To visualize it, the outline could be drawn white (unselected keyframes are already drawn white, so that's not an option).

The code for drawing the dopesheet keyframes is quite a bit more complicated, so this doesn't fit in quite as well there, but I got different drawing to work.


I'm not convinced this is the right thing to do though. Having an "active vertex" is mainly to make it clear which vertex is being edited in the "Active Keyframe" panel. With no such panel in the dopesheet the different drawing feels more like noise to me.

  • Change set active logic
  • Move active graph vertex drawing to separate functions
  • Draw active keyframe differently in dopesheet

Clicking in the dopesheet doesn't set the active vertex yet. I'd like to decide whether that's useful before spending time implementing it.

Hans Goudey (HooglyBoogly) marked an inline comment as done.Mon, May 18, 7:00 PM

Having an "active vertex" is mainly to make it clear which vertex is being edited in the "Active Keyframe" panel. With no such panel in the dopesheet the different drawing feels more like noise to me.

I agree. Let's get this working right in the graph editor first.

  • Merge branch 'master' into active-fcurve-keyframe
  • Remove dopesheet drawing changes