blender
/
blender
137
418
Fork 600
Code Issues 6.3k Pull Requests 778 Projects 19 Wiki Activity

Anim: View FCurve of Property in the Graph Editor #114407

Merged
Christoph Lendenfeld merged 40 commits from ChrisLend/blender:focus_in_ge into main 2023-11-21 14:07:03 +01:00
Conversation 35 Commits 40 Files Changed 2 +126 -114
2 changed files with 126 additions and 114 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 1be313ab89 - Show all commits

210
source/blender/editors/animation/anim_channels_edit.cc
View File

@ -853,112 +853,6 @@ void ANIM_frame_channel_y_extents(bContext *C, bAnimContext *ac)
ANIM_animdata_freelist(&anim_data);
}
static int prop_view_exec(bContext *C, wmOperator *op)
{
PointerRNA ptr = {nullptr};
PropertyRNA *prop = nullptr;
uiBut *but;
int index;
if (!(but = UI_context_active_but_prop_get(C, &ptr, &prop, &index))) {
/* pass event on if no active button found */
return (OPERATOR_CANCELLED | OPERATOR_PASS_THROUGH);
}
ListBase selection = {nullptr, nullptr};
if (CTX_data_mode_enum(C) == CTX_MODE_POSE) {
CTX_data_selected_pose_bones(C, &selection);
}
else {
CTX_data_selected_objects(C, &selection);
}
rctf bounds{};
bounds.xmin = INFINITY;
bounds.xmax = -INFINITY;
bounds.ymin = INFINITY;
bounds.ymax = -INFINITY;
const bool include_handles = RNA_boolean_get(op->ptr, "include_handles");
LISTBASE_FOREACH (CollectionPointerLink *, selected, &selection) {
ID *selected_id = selected->ptr.owner_id;
if (!BKE_animdata_id_is_animated(selected_id)) {
continue;
}
AnimData *anim_data = BKE_animdata_from_id(selected_id);
char *path = RNA_path_from_ID_to_property(&selected->ptr, prop);
bAction *action;
bool driven;
FCurve *fcurve = BKE_animadata_fcurve_find_by_rna_path(
anim_data, path, index, &action, &driven);
fcurve->flag |= (FCURVE_SELECTED | FCURVE_VISIBLE);
rctf fcu_bounds;
BKE_fcurve_calc_bounds(fcurve, false, include_handles, nullptr, &fcu_bounds);
BLI_rctf_union(&bounds, &fcu_bounds);
MEM_freeN(path);
}
BLI_freelistN(&selection);
if (!BLI_rctf_is_valid(&bounds)) {
return OPERATOR_CANCELLED;
}
LISTBASE_FOREACH (wmWindow *, win, &CTX_wm_manager(C)->windows) {
const bScreen *screen = BKE_workspace_active_screen_get(win->workspace_hook);
LISTBASE_FOREACH (ScrArea *, area, &screen->areabase) {
if (area->spacetype != SPACE_GRAPH) {
continue;
}
ARegion *window_region = BKE_area_find_region_type(area, RGN_TYPE_WINDOW);
if (!window_region) {
continue;
}
add_region_padding(C, window_region, &bounds);
const int smooth_viewtx = WM_operator_smooth_viewtx_get(op);
UI_view2d_smooth_view(C, window_region, &bounds, smooth_viewtx);
ED_area_tag_redraw(area);
break;
}
}
return OPERATOR_FINISHED;
}
static bool prop_view_poll(bContext *C)
{
return true;
}
static void ANIM_OT_prop_view(wmOperatorType *ot)
{
/* Identifiers */
ot->name = "Frame In Graph Editor";
ot->idname = "ANIM_OT_prop_view";
ot->description = "Frame the property under the cursor in the Graph Editor";
/* API callbacks */
ot->exec = prop_view_exec;
ot->poll = prop_view_poll;
ot->flag = 0;
ot->prop = RNA_def_boolean(ot->srna,
"include_handles",
true,
"Include Handles",
"Include handles of keyframes when calculating extents");
ot->prop = RNA_def_boolean(ot->srna,
"use_preview_range",
true,
"Use Preview Range",
"Ignore frames outside of the preview range");
}
/** \} */
/* -------------------------------------------------------------------- */
@ -4445,6 +4339,110 @@ static void ANIM_OT_channel_view_pick(wmOperatorType *ot)
"Ignore frames outside of the preview range");
}
static int prop_view_exec(bContext *C, wmOperator *op)
{
PointerRNA ptr = {nullptr};
PropertyRNA *prop = nullptr;
uiBut *but;
int index;
if (!(but = UI_context_active_but_prop_get(C, &ptr, &prop, &index))) {
/* pass event on if no active button found */
return (OPERATOR_CANCELLED | OPERATOR_PASS_THROUGH);
}
ListBase selection = {nullptr, nullptr};
if (CTX_data_mode_enum(C) == CTX_MODE_POSE) {
CTX_data_selected_pose_bones(C, &selection);
}
else {
CTX_data_selected_objects(C, &selection);
}
rctf bounds{};
bounds.xmin = INFINITY;
bounds.xmax = -INFINITY;
bounds.ymin = INFINITY;
bounds.ymax = -INFINITY;
const bool include_handles = RNA_boolean_get(op->ptr, "include_handles");
Scene *scene = CTX_data_scene(C);
float frame_range[2];
get_view_range(scene, true, frame_range);
LISTBASE_FOREACH (CollectionPointerLink *, selected, &selection) {
ID *selected_id = selected->ptr.owner_id;
if (!BKE_animdata_id_is_animated(selected_id)) {
continue;
}
AnimData *anim_data = BKE_animdata_from_id(selected_id);
char *path = RNA_path_from_ID_to_property(&selected->ptr, prop);
bAction *action;
bool driven;
FCurve *fcurve = BKE_animadata_fcurve_find_by_rna_path(
anim_data, path, index, &action, &driven);
fcurve->flag |= (FCURVE_SELECTED | FCURVE_VISIBLE);
rctf fcu_bounds;
nathanvegdahl marked this conversation as resolved Outdated
Nathan Vegdahl commented 2023-11-13 12:36:54 +01:00
Outdated
Review
Copy Link

The name of this function led me to believe that it was side-effect free, but in fact it's unhiding the fcurves as well. So I'm wondering if it makes sense to split this out into functions that serve as clearer vocabulary. For example:

  1. A function that finds the relevant fcurves and returns a Vector of them.
  2. A function that computes the bounds of a vector of fcurves.
  3. A function that unhides a vector of fcurves.

And then you can call them individually. In fact, 3 is probably short enough to just inline rather than making a function for it.

The name of this function led me to believe that it was side-effect free, but in fact it's unhiding the fcurves as well. So I'm wondering if it makes sense to split this out into functions that serve as clearer vocabulary. For example: 1. A function that finds the relevant fcurves and returns a `Vector` of them. 2. A function that computes the bounds of a vector of fcurves. 3. A function that unhides a vector of fcurves. And then you can call them individually. In fact, 3 is probably short enough to just inline rather than making a function for it.
Christoph Lendenfeld commented 2023-11-14 11:31:37 +01:00
Outdated
Review
Copy Link

I am not sure I can do that.
To do the NLA mapping, I need the AnimData struct
to get that, I need the ID

If I split the functions to handle a vector of FCurve * I no longer know which ID each FCurve belongs to.

I could make a struct that encapsulates the FCurve * with the AnimData and make a Vector of that.
Or rename the function to something more appropriate.
Let me know what you think

I am not sure I can do that. To do the NLA mapping, I need the `AnimData` struct to get that, I need the `ID` If I split the functions to handle a vector of `FCurve *` I no longer know which ID each FCurve belongs to. I could make a struct that encapsulates the `FCurve *` with the `AnimData` and make a Vector of that. Or rename the function to something more appropriate. Let me know what you think
👍 1
Nathan Vegdahl commented 2023-11-16 13:04:27 +01:00
Outdated
Review
Copy Link

Ah, that's a good point. Yeah, splitting this up could be tricky.

The functional programmer in me wants to split out the idea of filtering fcurves (i.e. finding the fcurves that satisfy a given criteria) from doing things with those fcurves (e.g. computing their total bounds, or unhiding them, etc.). But that would need to be part of a larger refactor of how a larger chunk of the animation code works, I think, and certainly doesn't belong in this PR even if we want to do it.

So yeah, for now let's just change the name. calculate_selection_fcurve_bounds_and_unhide is fine, I think. It's awkward, but I think that awkwardness is reflective of the multiple things the function is doing. And it's at least descriptive, so people won't accidentally reuse it thinking that all it does is compute bounds.

Ah, that's a good point. Yeah, splitting this up could be tricky. The functional programmer in me wants to split out the idea of filtering fcurves (i.e. finding the fcurves that satisfy a given criteria) from doing things with those fcurves (e.g. computing their total bounds, or unhiding them, etc.). But that would need to be part of a larger refactor of how a larger chunk of the animation code works, I think, and certainly doesn't belong in this PR even if we want to do it. So yeah, for now let's just change the name. `calculate_selection_fcurve_bounds_and_unhide` is fine, I think. It's awkward, but I think that awkwardness is reflective of the multiple things the function is doing. And it's at least descriptive, so people won't accidentally reuse it thinking that all it does is compute bounds.
Christoph Lendenfeld commented 2023-11-16 14:18:16 +01:00
Outdated
Review
Copy Link

good idea done that
at some point refactoring all the animation functions to check if they actually need AnimData would be good.
I am pretty sure we can simplify a lot of these functions

good idea done that at some point refactoring all the animation functions to check if they actually need `AnimData` would be good. I am pretty sure we can simplify a lot of these functions
👍 1
BKE_fcurve_calc_bounds(fcurve, false, include_handles, frame_range, &fcu_bounds);
BLI_rctf_union(&bounds, &fcu_bounds);
MEM_freeN(path);
}
BLI_freelistN(&selection);
if (!BLI_rctf_is_valid(&bounds)) {
return OPERATOR_CANCELLED;
}
LISTBASE_FOREACH (wmWindow *, win, &CTX_wm_manager(C)->windows) {
const bScreen *screen = BKE_workspace_active_screen_get(win->workspace_hook);
LISTBASE_FOREACH (ScrArea *, area, &screen->areabase) {
if (area->spacetype != SPACE_GRAPH) {
continue;
}
ARegion *window_region = BKE_area_find_region_type(area, RGN_TYPE_WINDOW);
if (!window_region) {
continue;
}
add_region_padding(C, window_region, &bounds);
const int smooth_viewtx = WM_operator_smooth_viewtx_get(op);
UI_view2d_smooth_view(C, window_region, &bounds, smooth_viewtx);
ED_area_tag_redraw(area);
break;
}
}
return OPERATOR_FINISHED;
}
static void ANIM_OT_prop_view(wmOperatorType *ot)
{
/* Identifiers */
ot->name = "Frame In Graph Editor";
ot->idname = "ANIM_OT_prop_view";
ot->description = "Frame the property under the cursor in the Graph Editor";
/* API callbacks */
ot->exec = prop_view_exec;
ot->flag = 0;
RNA_def_boolean(ot->srna,
"include_handles",
true,
"Include Handles",
"Include handles of keyframes when calculating extents");
RNA_def_boolean(ot->srna,
"all",
false,
"Show All",
"Frame the whole array property instead of only the index under the cursor");
}
/** \} */
/* -------------------------------------------------------------------- */

30
source/blender/editors/interface/interface_context_menu.cc
View File

@ -558,10 +558,6 @@ bool ui_popup_context_menu_for_button(bContext *C, uiBut *but, const wmEvent *ev
/* Keyframes */
if (but->flag & UI_BUT_ANIMATED_KEY) {
/* Replace/delete keyframes. */
uiItemO(layout,
CTX_IFACE_(BLT_I18NCONTEXT_OPERATOR_DEFAULT, "Show in Graph Editor"),
ICON_NONE,
"ANIM_OT_prop_view");
if (is_array_component) {
uiItemBooleanO(layout,
CTX_IFACE_(BLT_I18NCONTEXT_OPERATOR_DEFAULT, "Replace Keyframes"),
@ -610,10 +606,6 @@ bool ui_popup_context_menu_for_button(bContext *C, uiBut *but, const wmEvent *ev
/* pass */
}
else if (is_anim) {
uiItemO(layout,
CTX_IFACE_(BLT_I18NCONTEXT_OPERATOR_DEFAULT, "Show in Graph Editor"),
ICON_NONE,
"ANIM_OT_prop_view");
if (is_array_component) {
uiItemBooleanO(layout,
CTX_IFACE_(BLT_I18NCONTEXT_OPERATOR_DEFAULT, "Insert Keyframes"),
@ -663,6 +655,28 @@ bool ui_popup_context_menu_for_button(bContext *C, uiBut *but, const wmEvent *ev
}
}
if (is_anim || but->flag & UI_BUT_ANIMATED_KEY) {
uiItemS(layout);
if (is_array_component) {
uiItemO(layout,
CTX_IFACE_(BLT_I18NCONTEXT_OPERATOR_DEFAULT, "Show Single in Graph Editor"),
ICON_NONE,
"ANIM_OT_prop_view");
uiItemBooleanO(layout,
CTX_IFACE_(BLT_I18NCONTEXT_OPERATOR_DEFAULT, "Show All in Graph Editor"),
ICON_NONE,
"ANIM_OT_prop_view",
"all",
true);
}
else {
uiItemO(layout,
CTX_IFACE_(BLT_I18NCONTEXT_OPERATOR_DEFAULT, "Show in Graph Editor"),
ICON_NONE,
"ANIM_OT_prop_view");
}
}
/* Drivers */
if (but->flag & UI_BUT_DRIVEN) {
uiItemS(layout);