blender
/
blender
136
407
Fork 589
Code Issues 6.3k Pull Requests 767 Projects 19 Wiki Activity

Allow select range in animation editor #104565

Merged
Pratik Borhade merged 19 commits from PratikPB2123/blender:T103855-anim-select-extend into main 2023-05-05 17:46:14 +02:00
Conversation 21 Commits 19 Files Changed 7 +63 -94
2 changed files with 63 additions and 94 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit b3c299277e - Show all commits

152
source/blender/editors/animation/anim_channels_edit.c
View File

@ -214,6 +214,59 @@ void ANIM_set_active_channel(bAnimContext *ac,
ANIM_animdata_freelist(&anim_data);
}
bool ANIM_is_active_channel(bAnimListElem *ale)
Sybren A. Stüvel commented 2023-05-01 16:32:08 +02:00
Outdated
Review
Copy Link

Document what the change_active parameter does. Booleans are notoriously tricky to understand.

Document what the `change_active` parameter does. Booleans are notoriously tricky to understand.
{
bool is_active_found = false;
dr.sybren marked this conversation as resolved
Sybren A. Stüvel commented 2023-05-04 16:11:02 +02:00
Review
Copy Link

You don't need this bool variable. Since it's only set once, never changes, and then is used to return a value, you can replace all the is_active_found = ... with return ....

You don't need this `bool` variable. Since it's only set once, never changes, and then is used to return a value, you can replace all the `is_active_found = ...` with `return ...`.
switch (ale->type) {
case ANIMTYPE_FILLACTD: /* Action Expander */
case ANIMTYPE_DSMAT: /* Datablock AnimData Expanders */
case ANIMTYPE_DSLAM:
case ANIMTYPE_DSCAM:
case ANIMTYPE_DSCACHEFILE:
case ANIMTYPE_DSCUR:
case ANIMTYPE_DSSKEY:
case ANIMTYPE_DSWOR:
case ANIMTYPE_DSPART:
case ANIMTYPE_DSMBALL:
case ANIMTYPE_DSARM:
case ANIMTYPE_DSMESH:
case ANIMTYPE_DSNTREE:
case ANIMTYPE_DSTEX:
case ANIMTYPE_DSLAT:
case ANIMTYPE_DSLINESTYLE:
case ANIMTYPE_DSSPK:
case ANIMTYPE_DSGPENCIL:
case ANIMTYPE_DSMCLIP:
case ANIMTYPE_DSHAIR:
case ANIMTYPE_DSPOINTCLOUD:
case ANIMTYPE_DSVOLUME:
case ANIMTYPE_NLAACTION:
case ANIMTYPE_DSSIMULATION: {
if (ale->adt) {
is_active_found = ale->adt->flag & ADT_UI_ACTIVE;
}
break;
}
case ANIMTYPE_GROUP: {
bActionGroup *argp = (bActionGroup *)ale->data;
is_active_found = argp->flag & AGRP_ACTIVE;
break;
}
case ANIMTYPE_FCURVE:
case ANIMTYPE_NLACURVE: {
FCurve *fcu = (FCurve *)ale->data;
is_active_found = fcu->flag & FCURVE_ACTIVE;
break;
}
case ANIMTYPE_GPLAYER: {
Sybren A. Stüvel commented 2023-05-05 12:52:11 +02:00
Outdated
Review
Copy Link

This return is still in the wrong place...

This `return` is still in the wrong place...
Sybren A. Stüvel commented 2023-05-05 13:00:46 +02:00
Review
Copy Link

👍 now it's instantly clear that these were not forgotten.

👍 now it's instantly clear that these were not forgotten.
bGPDlayer *gpl = (bGPDlayer *)ale->data;
is_active_found = gpl->flag & GP_LAYER_ACTIVE;
break;
}
return is_active_found;
Sybren A. Stüvel commented 2023-05-04 16:11:45 +02:00
Outdated
Review
Copy Link

This line is never reached, and the function has no return statement for when there is no matching case.

Just end the function with return false;

This line is never reached, and the function has no `return` statement for when there is no matching `case`. Just end the function with `return false;`
}
}
/* change_active determines whether to change the active bone of the armature when selecting pose
* channels. It is false during range selection otherwise true. */
static void select_pchan_for_action_group(bAnimContext *ac,
@ -3043,53 +3096,9 @@ static bool animchannel_has_active_of_type(bAnimContext *ac, const eAnim_Channel
if (ale->type != type) {
continue;
}
switch (ale->type) {
case ANIMTYPE_FILLACTD: /* Action Expander */
case ANIMTYPE_DSMAT: /* Datablock AnimData Expanders */
case ANIMTYPE_DSLAM:
case ANIMTYPE_DSCAM:
case ANIMTYPE_DSCACHEFILE:
case ANIMTYPE_DSCUR:
case ANIMTYPE_DSSKEY:
case ANIMTYPE_DSWOR:
case ANIMTYPE_DSPART:
case ANIMTYPE_DSMBALL:
case ANIMTYPE_DSARM:
case ANIMTYPE_DSMESH:
case ANIMTYPE_DSNTREE:
case ANIMTYPE_DSTEX:
case ANIMTYPE_DSLAT:
case ANIMTYPE_DSLINESTYLE:
case ANIMTYPE_DSSPK:
case ANIMTYPE_DSGPENCIL:
case ANIMTYPE_DSMCLIP:
case ANIMTYPE_DSHAIR:
case ANIMTYPE_DSPOINTCLOUD:
case ANIMTYPE_DSVOLUME:
case ANIMTYPE_NLAACTION:
case ANIMTYPE_DSSIMULATION: {
if (ale->adt) {
is_active_found |= ale->adt->flag & ADT_UI_ACTIVE;
}
break;
}
case ANIMTYPE_GROUP: {
bActionGroup *argp = (bActionGroup *)ale->data;
is_active_found |= argp->flag & AGRP_ACTIVE;
break;
}
case ANIMTYPE_FCURVE:
case ANIMTYPE_NLACURVE: {
FCurve *fcu = (FCurve *)ale->data;
is_active_found |= fcu->flag & FCURVE_ACTIVE;
break;
}
case ANIMTYPE_GPLAYER: {
bGPDlayer *gpl = (bGPDlayer *)ale->data;
is_active_found |= gpl->flag & GP_LAYER_ACTIVE;
break;
}
is_active_found = ANIM_is_active_channel(ale);
if (is_active_found) {
break;
}
}
@ -3170,53 +3179,8 @@ static void animchannel_select_range(bAnimContext *ac, bAnimListElem *cursor_ele
continue;
}
switch (ale->type) {
case ANIMTYPE_FILLACTD: /* Action Expander */
case ANIMTYPE_DSMAT: /* Datablock AnimData Expanders */
case ANIMTYPE_DSLAM:
case ANIMTYPE_DSCAM:
case ANIMTYPE_DSCACHEFILE:
case ANIMTYPE_DSCUR:
case ANIMTYPE_DSSKEY:
case ANIMTYPE_DSWOR:
case ANIMTYPE_DSPART:
case ANIMTYPE_DSMBALL:
case ANIMTYPE_DSARM:
case ANIMTYPE_DSMESH:
case ANIMTYPE_DSNTREE:
case ANIMTYPE_DSTEX:
case ANIMTYPE_DSLAT:
case ANIMTYPE_DSLINESTYLE:
case ANIMTYPE_DSSPK:
case ANIMTYPE_DSGPENCIL:
case ANIMTYPE_DSMCLIP:
case ANIMTYPE_DSHAIR:
case ANIMTYPE_DSPOINTCLOUD:
case ANIMTYPE_DSVOLUME:
case ANIMTYPE_NLAACTION:
case ANIMTYPE_DSSIMULATION: {
if (ale->adt) {
is_active_elem = ale->adt->flag & ADT_UI_ACTIVE;
}
break;
}
case ANIMTYPE_GROUP: {
bActionGroup *argp = (bActionGroup *)ale->data;
is_active_elem = argp->flag & AGRP_ACTIVE;
break;
}
case ANIMTYPE_FCURVE:
case ANIMTYPE_NLACURVE: {
FCurve *fcu = (FCurve *)ale->data;
is_active_elem = fcu->flag & FCURVE_ACTIVE;
break;
}
case ANIMTYPE_GPLAYER: {
bGPDlayer *gpl = (bGPDlayer *)ale->data;
is_active_elem = gpl->flag & GP_LAYER_ACTIVE;
break;
}
}
is_active_elem = ANIM_is_active_channel(ale);
Sybren A. Stüvel commented 2023-05-04 16:31:20 +02:00
Outdated
Review
Copy Link

is_active_elem isn't used before this point, so just declare it as const bool is_active_elem = ANIM_is_active_channel(ale);

`is_active_elem` isn't used before this point, so just declare it as `const bool is_active_elem = ANIM_is_active_channel(ale);`
/* Restrict selection when active element is not found and group-channels are excluded from the
* selection. */
if (is_active_elem || is_cursor_elem) {

5
source/blender/editors/include/ED_anim_api.h
View File

@ -696,6 +696,11 @@ void ANIM_set_active_channel(bAnimContext *ac,
void *channel_data,
eAnim_ChannelType channel_type);
/**
* Return whether channel is active.
*/
bool ANIM_is_active_channel(bAnimListElem *ale);
/**
* Delete the F-Curve from the given AnimData block (if possible),
* as appropriate according to animation context.