Graph Editor: Fix selection and View > Selected with 'View Only Selected Curve Keyframes' option enabled
ClosedPublic
Actions

Authored by Philipp Oeser (lichtwerk) on Nov 12 2019, 7:41 PM.

Details

Reviewers

Sybren A. Stüvel (sybren)
Sebastian Parborg (zeddb)
Luciano Muñoz Sessarego (looch)
Pablo Vazquez (pablovazquez)
Julian Eisel (Severin)

Maniphest Tasks

T67873: View Selected in Graph Editor (Only Selected Curve Keyframes) takes hidden keys into account when calculating zoom

Commits

rB355905cb0278: Fix T67873: View Selected in Graph Editor (Only Selected Curve Keyframes) takes…

Summary

With 'View Only Selected Curve Keyframes' option enabled, it was
possible to select [box/circle/lasso] hidden/non-visible keyframes.
Think this should never happen.

Also 'View > Selected' was taking the hidden/non-visible keyframes into
account when calculating the keyframe extends for zooming.

related to T68962
Fixes T67873

P.S.: please test thouroughly (I'm still doing as well -- animfilter code is hairy on first sight...)

Diff Detail

Repository

rB Blender

Branch

T67873_ (branched from master)

Build Status

Buildable 5647
Build 5647: arc lint + arc unit

Event Timeline

Philipp Oeser (lichtwerk) created this revision.Nov 12 2019, 7:41 PM

Harbormaster completed remote builds in B5647: Diff 19576.Nov 12 2019, 7:41 PM

Philipp Oeser (lichtwerk) mentioned this in T68962: Graph Editor: Link Visible and Selected states.Nov 12 2019, 7:43 PM

Roman (roman13) awarded a token.Nov 13 2019, 7:41 AM

Sybren A. Stüvel (sybren) added inline comments.Nov 13 2019, 10:35 AM

source/blender/editors/space_graph/graph_edit.c
95–98	This snippet is duplicated twice now, and something similar maybe occurs in other places as well. Would it be good to have a function for this? Or is it really only used here, and is this a duplication we can live with?

Philipp Oeser (lichtwerk) added inline comments.Nov 13 2019, 11:52 AM

source/blender/editors/space_graph/graph_edit.c
95–98	I think it is best to decouple the selection behavior (second case below) from the actual "fix" for `View Selected`. See my comment in T68962. That would leave us with only one case in this Diff, and only later (if we decide to tweak all the selection operators as well), we can use this in more places and dedouplicate into own function. Right? If you think the same, I will update the Diff and remove the change to `box_select_graphkeys` for now...

Philipp Oeser (lichtwerk) added a reviewer: Julian Eisel (Severin).Nov 15 2019, 9:39 PM

Sybren A. Stüvel (sybren) added inline comments.Nov 21 2019, 11:55 AM

source/blender/editors/space_graph/graph_edit.c
95–98	If you think the same I think the same.

The effective changes done here should be fine. Don't really care about how the patch is split, we're just nit-picking a bit here :)

source/blender/editors/space_graph/graph_edit.c
95–98	Although I don't think this minor code duplication is a big deal at all, something like `graph_get_animdata_for_select()` that handles the common select filters and calls `ANIM_animdata_filter()` would be nice.