From f7354119cc8089dd7a788bacec0259db14bf07b8 Mon Sep 17 00:00:00 2001 From: Joshua Leung Date: Thu, 15 Feb 2018 15:45:08 +1300 Subject: Fix more missing ID remapping in animation editor callbacks Applying the same fixes as introduced in 98d797b67c07e85889768bf8ecde292e9e6f70e9 this time, for the Graph and NLA editors --- source/blender/editors/space_graph/space_graph.c | 7 +++---- source/blender/editors/space_nla/space_nla.c | 7 +++---- 2 files changed, 6 insertions(+), 8 deletions(-) (limited to 'source/blender/editors') diff --git a/source/blender/editors/space_graph/space_graph.c b/source/blender/editors/space_graph/space_graph.c index f12db310856..df6de83336d 100644 --- a/source/blender/editors/space_graph/space_graph.c +++ b/source/blender/editors/space_graph/space_graph.c @@ -683,13 +683,12 @@ static void graph_id_remap(ScrArea *UNUSED(sa), SpaceLink *slink, ID *old_id, ID { SpaceIpo *sgraph = (SpaceIpo *)slink; - if (!ELEM(GS(old_id->name), ID_GR)) { - return; - } - if (sgraph->ads && (ID *)sgraph->ads->filter_grp == old_id) { sgraph->ads->filter_grp = (Group *)new_id; } + if ((ID *)sgraph->ads->source == old_id) { + sgraph->ads->source = new_id; + } } /* only called once, from space/spacetypes.c */ diff --git a/source/blender/editors/space_nla/space_nla.c b/source/blender/editors/space_nla/space_nla.c index 3b5604087b9..020b3a41f4d 100644 --- a/source/blender/editors/space_nla/space_nla.c +++ b/source/blender/editors/space_nla/space_nla.c @@ -506,13 +506,12 @@ static void nla_id_remap(ScrArea *UNUSED(sa), SpaceLink *slink, ID *old_id, ID * { SpaceNla *snla = (SpaceNla *)slink; - if (!ELEM(GS(old_id->name), ID_GR)) { - return; - } - if ((ID *)snla->ads->filter_grp == old_id) { snla->ads->filter_grp = (Group *)new_id; } + if ((ID *)snla->ads->source == old_id) { + snla->ads->source = new_id; + } } /* only called once, from space/spacetypes.c */ -- cgit v1.2.3 From 4d966aa19c36d86a407f419f88dc713f454e74a0 Mon Sep 17 00:00:00 2001 From: Joshua Leung Date: Thu, 15 Feb 2018 16:39:46 +1300 Subject: Fix T54065: NLA-Strip Control Curves would get disabled when name-based-filtering is enabled This bug took a while to track down. In the test file with this report, the Nla-Strip Control Curve for strip time would get disabled if you changed the NLA Editor to a second Graph Editor instance. It turns out that because this second Graph Editor would have the "filter fcurves by name" option enabled, this would trigger a lookup of the referenced property's name (in order to test whether it matched the filtering criteria). However, since that filtering code was written before the introduction of these curves, it still assumed that the names for these Control Curves should be handled the same as for standard FCurves. Unfortunately, that doesn't work, as the property lookups fail if the standard method is used - when the lookups fail, the F-Curves get tagged as being invalid/disabled (and need to be reset using the "Revive Disabled FCurves" operator). Note: The changes in this patch look complicated, as I've had to shuffle a bit of code around so that the name-filtering check can have access to the additional info it needs. In the process, I've also removed the earlier (hacky) approach where the control curves were getting added to a temp buffer to get changed from normal FCurves to special ANIMTYPE_NLACURVES. --- source/blender/editors/animation/anim_filter.c | 72 +++++++++++--------------- 1 file changed, 30 insertions(+), 42 deletions(-) (limited to 'source/blender/editors') diff --git a/source/blender/editors/animation/anim_filter.c b/source/blender/editors/animation/anim_filter.c index 2ac305dbc30..0d51dc8dc76 100644 --- a/source/blender/editors/animation/anim_filter.c +++ b/source/blender/editors/animation/anim_filter.c @@ -877,6 +877,7 @@ static bAnimListElem *make_new_animlistelem(void *data, short datatype, ID *owne break; } case ANIMTYPE_FCURVE: + case ANIMTYPE_NLACURVE: /* practically the same as ANIMTYPE_FCURVE. Differences are applied post-creation */ { FCurve *fcu = (FCurve *)data; @@ -1086,13 +1087,14 @@ static bool name_matches_dopesheet_filter(bDopeSheet *ads, char *name) /* (Display-)Name-based F-Curve filtering * NOTE: when this function returns true, the F-Curve is to be skipped */ -static bool skip_fcurve_with_name(bDopeSheet *ads, FCurve *fcu, ID *owner_id) +static bool skip_fcurve_with_name(bDopeSheet *ads, FCurve *fcu, eAnim_ChannelType channel_type, void *owner, ID *owner_id) { bAnimListElem ale_dummy = {NULL}; const bAnimChannelType *acf; - /* create a dummy wrapper for the F-Curve */ - ale_dummy.type = ANIMTYPE_FCURVE; + /* create a dummy wrapper for the F-Curve, so we can get typeinfo for it */ + ale_dummy.type = channel_type; + ale_dummy.owner = owner; ale_dummy.id = owner_id; ale_dummy.data = fcu; @@ -1155,8 +1157,9 @@ static bool fcurve_has_errors(FCurve *fcu) } /* find the next F-Curve that is usable for inclusion */ -static FCurve *animfilter_fcurve_next(bDopeSheet *ads, FCurve *first, bActionGroup *grp, int filter_mode, ID *owner_id) +static FCurve *animfilter_fcurve_next(bDopeSheet *ads, FCurve *first, eAnim_ChannelType channel_type, int filter_mode, void *owner, ID *owner_id) { + bActionGroup *grp = (channel_type == ANIMTYPE_FCURVE) ? owner : NULL; FCurve *fcu = NULL; /* loop over F-Curves - assume that the caller of this has already checked that these should be included @@ -1190,7 +1193,7 @@ static FCurve *animfilter_fcurve_next(bDopeSheet *ads, FCurve *first, bActionGro if (!(filter_mode & ANIMFILTER_ACTIVE) || (fcu->flag & FCURVE_ACTIVE)) { /* name based filtering... */ if ( ((ads) && (ads->filterflag & ADS_FILTER_BY_FCU_NAME)) && (owner_id) ) { - if (skip_fcurve_with_name(ads, fcu, owner_id)) + if (skip_fcurve_with_name(ads, fcu, channel_type, owner, owner_id)) continue; } @@ -1213,7 +1216,10 @@ static FCurve *animfilter_fcurve_next(bDopeSheet *ads, FCurve *first, bActionGro return NULL; } -static size_t animfilter_fcurves(ListBase *anim_data, bDopeSheet *ads, FCurve *first, bActionGroup *grp, int filter_mode, ID *owner_id) +static size_t animfilter_fcurves(ListBase *anim_data, bDopeSheet *ads, + FCurve *first, eAnim_ChannelType fcurve_type, + int filter_mode, + void *owner, ID *owner_id) { FCurve *fcu; size_t items = 0; @@ -1227,8 +1233,18 @@ static size_t animfilter_fcurves(ListBase *anim_data, bDopeSheet *ads, FCurve *f * 4) the fcu pointer is set to the F-Curve after the one we just added, so that we can keep going through * the rest of the F-Curve list without an eternal loop. Back to step 2 :) */ - for (fcu = first; ( (fcu = animfilter_fcurve_next(ads, fcu, grp, filter_mode, owner_id)) ); fcu = fcu->next) { - ANIMCHANNEL_NEW_CHANNEL(fcu, ANIMTYPE_FCURVE, owner_id); + for (fcu = first; ( (fcu = animfilter_fcurve_next(ads, fcu, fcurve_type, filter_mode, owner, owner_id)) ); fcu = fcu->next) { + if (UNLIKELY(fcurve_type == ANIMTYPE_NLACURVE)) { + /* NLA Control Curve - Basically the same as normal F-Curves, except we need to set some stuff differently */ + ANIMCHANNEL_NEW_CHANNEL_FULL(fcu, ANIMTYPE_NLACURVE, owner_id, { + ale->owner = owner; /* strip */ + ale->adt = NULL; /* to prevent time mapping from causing problems */ + }); + } + else { + /* Normal FCurve */ + ANIMCHANNEL_NEW_CHANNEL(fcu, ANIMTYPE_FCURVE, owner_id); + } } /* return the number of items added to the list */ @@ -1279,10 +1295,10 @@ static size_t animfilter_act_group(bAnimContext *ac, ListBase *anim_data, bDopeS /* group must be editable for its children to be editable (if we care about this) */ if (!(filter_mode & ANIMFILTER_FOREDIT) || EDITABLE_AGRP(agrp)) { /* get first F-Curve which can be used here */ - FCurve *first_fcu = animfilter_fcurve_next(ads, agrp->channels.first, agrp, filter_mode, owner_id); + FCurve *first_fcu = animfilter_fcurve_next(ads, agrp->channels.first, ANIMTYPE_FCURVE, filter_mode, agrp, owner_id); /* filter list, starting from this F-Curve */ - tmp_items += animfilter_fcurves(&tmp_data, ads, first_fcu, agrp, filter_mode, owner_id); + tmp_items += animfilter_fcurves(&tmp_data, ads, first_fcu, ANIMTYPE_FCURVE, filter_mode, agrp, owner_id); } } } @@ -1338,7 +1354,7 @@ static size_t animfilter_action(bAnimContext *ac, ListBase *anim_data, bDopeShee /* un-grouped F-Curves (only if we're not only considering those channels in the active group) */ if (!(filter_mode & ANIMFILTER_ACTGROUPED)) { FCurve *firstfcu = (lastchan) ? (lastchan->next) : (act->curves.first); - items += animfilter_fcurves(anim_data, ads, firstfcu, NULL, filter_mode, owner_id); + items += animfilter_fcurves(anim_data, ads, firstfcu, ANIMTYPE_FCURVE, filter_mode, NULL, owner_id); } /* return the number of items added to the list */ @@ -1460,36 +1476,8 @@ static size_t animfilter_nla_controls(ListBase *anim_data, bDopeSheet *ads, Anim /* for now, we only go one level deep - so controls on grouped FCurves are not handled */ for (nlt = adt->nla_tracks.first; nlt; nlt = nlt->next) { for (strip = nlt->strips.first; strip; strip = strip->next) { - ListBase strip_curves = {NULL, NULL}; - size_t strip_items = 0; - - /* create the raw items */ - strip_items += animfilter_fcurves(&strip_curves, ads, strip->fcurves.first, NULL, filter_mode, owner_id); - - /* change their types and add extra data - * - There is no point making a separate copy of animfilter_fcurves for this now/yet, - * unless we later get per-element control curves for other stuff too - */ - if (strip_items) { - bAnimListElem *ale, *ale_n = NULL; - - for (ale = strip_curves.first; ale; ale = ale_n) { - ale_n = ale->next; - - /* change the type to being a FCurve for editing NLA strip controls */ - BLI_assert(ale->type == ANIMTYPE_FCURVE); - - ale->type = ANIMTYPE_NLACURVE; - ale->owner = strip; - - ale->adt = NULL; /* XXX: This way, there are no problems with time mapping errors */ - } - } - - /* add strip curves to the set of channels inside the group being collected */ - BLI_movelisttolist(&tmp_data, &strip_curves); - BLI_assert(BLI_listbase_is_empty(&strip_curves)); - tmp_items += strip_items; + /* pass strip as the "owner", so that the name lookups (used while filtering) will resolve */ + tmp_items += animfilter_fcurves(&tmp_data, ads, strip->fcurves.first, ANIMTYPE_NLACURVE, filter_mode, strip, owner_id); } } } @@ -1540,7 +1528,7 @@ static size_t animfilter_block_data(bAnimContext *ac, ListBase *anim_data, bDope items += animfilter_nla(ac, anim_data, ads, adt, filter_mode, id); }, { /* Drivers */ - items += animfilter_fcurves(anim_data, ads, adt->drivers.first, NULL, filter_mode, id); + items += animfilter_fcurves(anim_data, ads, adt->drivers.first, ANIMTYPE_FCURVE, filter_mode, NULL, id); }, { /* NLA Control Keyframes */ items += animfilter_nla_controls(anim_data, ads, adt, filter_mode, id); -- cgit v1.2.3 From ac717928ad027b3a048e6aba7e4f64e4a617d8cc Mon Sep 17 00:00:00 2001 From: Joshua Leung Date: Wed, 21 Feb 2018 17:53:54 +1300 Subject: Feature Request T54106: When scaling keyframes, merge (by-averaging) selected keys that end up on the same frame Currently, when scaling keyframes in the Dopesheet, if multiple selected keyframes end up on the same frame post-scaling, they would not get removed by the "Automerge" setting that normally removes duplicates on the same frame. This commit changes the behaviour so that when multiple selected keyframes end up on the same frame, instead of keeping all these around on the same frame (e.g. resulting in a column of keyframes on different values), we will instead merge them into a single keyframe (by averaging the values). This should result in a smoother F-Curve with fewer "stair-steps" that need to be carefully cleaned out afterwards. Requested by @hjalti --- .../editors/transform/transform_conversions.c | 119 +++++++++++++++++++++ 1 file changed, 119 insertions(+) (limited to 'source/blender/editors') diff --git a/source/blender/editors/transform/transform_conversions.c b/source/blender/editors/transform/transform_conversions.c index 34cc143f3a9..7b18ee09df8 100644 --- a/source/blender/editors/transform/transform_conversions.c +++ b/source/blender/editors/transform/transform_conversions.c @@ -3512,10 +3512,129 @@ static void posttrans_mask_clean(Mask *mask) } } +/* Time + Average value */ +typedef struct tRetainedKeyframe { + struct tRetainedKeyframe *next, *prev; + float frame; /* frame to cluster around */ + float val; /* average value */ + + size_t tot_count; /* number of keyframes that have been averaged */ + size_t del_count; /* number of keyframes of this sort that have been deleted so far */ +} tRetainedKeyframe; + /* Called during special_aftertrans_update to make sure selected keyframes replace * any other keyframes which may reside on that frame (that is not selected). */ static void posttrans_fcurve_clean(FCurve *fcu, const bool use_handle) +{ + /* NOTE: We assume that all keys are sorted */ + ListBase retained_keys = {NULL, NULL}; + tRetainedKeyframe *last_rk = NULL; + + /* sanity checks */ + if ((fcu->totvert == 0) || (fcu->bezt == NULL)) + return; + printf("cleaning fcurve = '%s' (%p)\n", fcu->rna_path, fcu); + + /* 1) Identify selected keyframes, and average the values on those + * in case there are collisions due to multiple keys getting scaled + * to all end up on the same frame + */ + for (int i = 0; i < fcu->totvert; i++) { + BezTriple *bezt = &fcu->bezt[i]; + + if (BEZT_ISSEL_ANY(bezt)) { + bool found = false; + + /* If there's another selected frame here, merge it */ + for (tRetainedKeyframe *rk = retained_keys.last; rk; rk = rk->prev) { + if (IS_EQT(rk->frame, bezt->vec[1][0], BEZT_BINARYSEARCH_THRESH)) { + rk->val += (bezt->vec[1][1] - rk->val) / ((float)rk->tot_count); + rk->tot_count++; + + found = true; + break; + } + else if (rk->frame < bezt->vec[1][0]) { + /* XXX: terminate early if have passed the supposed insertion point? */ + printf(" %f: rk %f (@ %p) is earlier (last = %p)\n", bezt->vec[1][0], rk->frame, rk, retained_keys.last); + } + } + + /* If nothing found yet, create a new one */ + if (found == false) { + tRetainedKeyframe *rk = MEM_callocN(sizeof(tRetainedKeyframe), "tRetainedKeyframe"); + + rk->frame = bezt->vec[1][0]; + rk->val = bezt->vec[1][1]; + rk->tot_count = 1; + + BLI_addtail(&retained_keys, rk); + } + } + } + + if (BLI_listbase_is_empty(&retained_keys)) { + /* This may happen if none of the points were selected... */ + printf("%s: nothing to do for FCurve %p (rna_path = '%s')\n", __func__, fcu, fcu->rna_path); + return; + } + else { + // XXX: Debug - Print out all the retained keys so we can check if they're in order! + int rk_index = 0; + for (tRetainedKeyframe *rk = retained_keys.first; rk; rk = rk->next) { + printf(" %d: f = %f, v = %f (n = %d)\n", rk_index, rk->frame, rk->val, rk->tot_count); + rk_index++; + } + + /* "last_rk" is the last one we need to search (assuming everything is sorted) */ + last_rk = retained_keys.last; + } + + /* 2) Delete all keyframes duplicating the "retained keys" found above + * - Most of these will be unselected keyframes + * - Some will be selected keyframes though. For those, we only keep the last one + * (or else everything is gone), and replace its value with the averaged value. + */ + for (int i = fcu->totvert - 1; i >= 0; i--) { + BezTriple *bezt = &fcu->bezt[i]; + + /* Is this a candidate for deletion? */ + // TODO: Replace loop with an O(1) lookup instead + // TODO: update last_rk on each deletion + for (tRetainedKeyframe *rk = last_rk; rk; rk = rk->prev) { + if (IS_EQT(bezt->vec[1][0], rk->frame, BEZT_BINARYSEARCH_THRESH)) { + /* Delete this keyframe, unless it's the last selected one on this frame, + * in which case, we'll update its value instead + */ + if (BEZT_ISSEL_ANY(bezt) && (rk->del_count == rk->tot_count - 1)) { + /* Update keyframe */ + // XXX: update handles too... + bezt->vec[1][1] = rk->val; + + /* Adjust last_rk, since we don't need to use this one anymore */ + last_rk = rk->prev; + } + else { + /* Delete keyframe */ + delete_fcurve_key(fcu, i, 0); + } + + /* Stop searching for matching RK's */ + rk->del_count++; + break; + } + else if (rk->frame < bezt->vec[1][0]) { + /* Terminate search early - There shouldn't be anything */ + } + } + } + + /* cleanup */ + BLI_freelistN(&retained_keys); +} + +static void UNUSED_FUNCTION(posttrans_fcurve_clean__OLD)(FCurve *fcu, const bool use_handle) { float *selcache; /* cache for frame numbers of selected frames (fcu->totvert*sizeof(float)) */ int len, index, i; /* number of frames in cache, item index */ -- cgit v1.2.3 From 63da3b79edc630d1a5e311d24bfca6506ecfdac8 Mon Sep 17 00:00:00 2001 From: Joshua Leung Date: Wed, 21 Feb 2018 18:59:15 +1300 Subject: Minor Optimisation: Terminate early if we've passed the insertion point for tRetainedKeyframes --- source/blender/editors/transform/transform_conversions.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'source/blender/editors') diff --git a/source/blender/editors/transform/transform_conversions.c b/source/blender/editors/transform/transform_conversions.c index 7b18ee09df8..3693c3e40fb 100644 --- a/source/blender/editors/transform/transform_conversions.c +++ b/source/blender/editors/transform/transform_conversions.c @@ -3556,8 +3556,9 @@ static void posttrans_fcurve_clean(FCurve *fcu, const bool use_handle) break; } else if (rk->frame < bezt->vec[1][0]) { - /* XXX: terminate early if have passed the supposed insertion point? */ + /* Terminate early if have passed the supposed insertion point? */ printf(" %f: rk %f (@ %p) is earlier (last = %p)\n", bezt->vec[1][0], rk->frame, rk, retained_keys.last); + break; } } @@ -3626,6 +3627,7 @@ static void posttrans_fcurve_clean(FCurve *fcu, const bool use_handle) } else if (rk->frame < bezt->vec[1][0]) { /* Terminate search early - There shouldn't be anything */ + break; } } } -- cgit v1.2.3 From f2cdb1c7ccb1893db4c093d49b13204dadc7e535 Mon Sep 17 00:00:00 2001 From: Joshua Leung Date: Wed, 21 Feb 2018 19:14:33 +1300 Subject: Tweak/Fix for T54106 - Moving multiple selected keyframes on top of an unselected one would not merge the keys This commit removes an earlier attempt at optimising the lookups for duplicates of a particular tRetainedKeyframe once we'd already deleted all the selected copies. The problem was that now, instead of getting rid of the unselected keys (i.e. the basic function here), we were only getting rid of the selected duplicates. With this fix, unselected keyframes will now get removed (as expected) again. However, we currently don't take their values into account when merging keyframes, since it is assumed that we don't care so much about their values when overriding. --- source/blender/editors/transform/transform_conversions.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) (limited to 'source/blender/editors') diff --git a/source/blender/editors/transform/transform_conversions.c b/source/blender/editors/transform/transform_conversions.c index 3693c3e40fb..35fa373854a 100644 --- a/source/blender/editors/transform/transform_conversions.c +++ b/source/blender/editors/transform/transform_conversions.c @@ -3529,7 +3529,6 @@ static void posttrans_fcurve_clean(FCurve *fcu, const bool use_handle) { /* NOTE: We assume that all keys are sorted */ ListBase retained_keys = {NULL, NULL}; - tRetainedKeyframe *last_rk = NULL; /* sanity checks */ if ((fcu->totvert == 0) || (fcu->bezt == NULL)) @@ -3587,9 +3586,6 @@ static void posttrans_fcurve_clean(FCurve *fcu, const bool use_handle) printf(" %d: f = %f, v = %f (n = %d)\n", rk_index, rk->frame, rk->val, rk->tot_count); rk_index++; } - - /* "last_rk" is the last one we need to search (assuming everything is sorted) */ - last_rk = retained_keys.last; } /* 2) Delete all keyframes duplicating the "retained keys" found above @@ -3602,8 +3598,7 @@ static void posttrans_fcurve_clean(FCurve *fcu, const bool use_handle) /* Is this a candidate for deletion? */ // TODO: Replace loop with an O(1) lookup instead - // TODO: update last_rk on each deletion - for (tRetainedKeyframe *rk = last_rk; rk; rk = rk->prev) { + for (tRetainedKeyframe *rk = retained_keys.last; rk; rk = rk->prev) { if (IS_EQT(bezt->vec[1][0], rk->frame, BEZT_BINARYSEARCH_THRESH)) { /* Delete this keyframe, unless it's the last selected one on this frame, * in which case, we'll update its value instead @@ -3612,9 +3607,6 @@ static void posttrans_fcurve_clean(FCurve *fcu, const bool use_handle) /* Update keyframe */ // XXX: update handles too... bezt->vec[1][1] = rk->val; - - /* Adjust last_rk, since we don't need to use this one anymore */ - last_rk = rk->prev; } else { /* Delete keyframe */ @@ -3625,10 +3617,6 @@ static void posttrans_fcurve_clean(FCurve *fcu, const bool use_handle) rk->del_count++; break; } - else if (rk->frame < bezt->vec[1][0]) { - /* Terminate search early - There shouldn't be anything */ - break; - } } } -- cgit v1.2.3 From dd75211d83b81212a433ee9b872a1da3f1192abb Mon Sep 17 00:00:00 2001 From: Joshua Leung Date: Wed, 21 Feb 2018 19:15:41 +1300 Subject: Cleanup: Delete debugging code and the old version of the automerge code --- .../editors/transform/transform_conversions.c | 75 ++-------------------- 1 file changed, 5 insertions(+), 70 deletions(-) (limited to 'source/blender/editors') diff --git a/source/blender/editors/transform/transform_conversions.c b/source/blender/editors/transform/transform_conversions.c index 35fa373854a..9d44bf50b02 100644 --- a/source/blender/editors/transform/transform_conversions.c +++ b/source/blender/editors/transform/transform_conversions.c @@ -3533,7 +3533,6 @@ static void posttrans_fcurve_clean(FCurve *fcu, const bool use_handle) /* sanity checks */ if ((fcu->totvert == 0) || (fcu->bezt == NULL)) return; - printf("cleaning fcurve = '%s' (%p)\n", fcu->rna_path, fcu); /* 1) Identify selected keyframes, and average the values on those * in case there are collisions due to multiple keys getting scaled @@ -3556,7 +3555,6 @@ static void posttrans_fcurve_clean(FCurve *fcu, const bool use_handle) } else if (rk->frame < bezt->vec[1][0]) { /* Terminate early if have passed the supposed insertion point? */ - printf(" %f: rk %f (@ %p) is earlier (last = %p)\n", bezt->vec[1][0], rk->frame, rk, retained_keys.last); break; } } @@ -3576,16 +3574,10 @@ static void posttrans_fcurve_clean(FCurve *fcu, const bool use_handle) if (BLI_listbase_is_empty(&retained_keys)) { /* This may happen if none of the points were selected... */ - printf("%s: nothing to do for FCurve %p (rna_path = '%s')\n", __func__, fcu, fcu->rna_path); - return; - } - else { - // XXX: Debug - Print out all the retained keys so we can check if they're in order! - int rk_index = 0; - for (tRetainedKeyframe *rk = retained_keys.first; rk; rk = rk->next) { - printf(" %d: f = %f, v = %f (n = %d)\n", rk_index, rk->frame, rk->val, rk->tot_count); - rk_index++; + if (G.debug & G_DEBUG) { + printf("%s: nothing to do for FCurve %p (rna_path = '%s')\n", __func__, fcu, fcu->rna_path); } + return; } /* 2) Delete all keyframes duplicating the "retained keys" found above @@ -3597,7 +3589,7 @@ static void posttrans_fcurve_clean(FCurve *fcu, const bool use_handle) BezTriple *bezt = &fcu->bezt[i]; /* Is this a candidate for deletion? */ - // TODO: Replace loop with an O(1) lookup instead + /* TODO: Replace loop with an O(1) lookup instead */ for (tRetainedKeyframe *rk = retained_keys.last; rk; rk = rk->prev) { if (IS_EQT(bezt->vec[1][0], rk->frame, BEZT_BINARYSEARCH_THRESH)) { /* Delete this keyframe, unless it's the last selected one on this frame, @@ -3605,7 +3597,7 @@ static void posttrans_fcurve_clean(FCurve *fcu, const bool use_handle) */ if (BEZT_ISSEL_ANY(bezt) && (rk->del_count == rk->tot_count - 1)) { /* Update keyframe */ - // XXX: update handles too... + /* TODO: update handles too? */ bezt->vec[1][1] = rk->val; } else { @@ -3624,63 +3616,6 @@ static void posttrans_fcurve_clean(FCurve *fcu, const bool use_handle) BLI_freelistN(&retained_keys); } -static void UNUSED_FUNCTION(posttrans_fcurve_clean__OLD)(FCurve *fcu, const bool use_handle) -{ - float *selcache; /* cache for frame numbers of selected frames (fcu->totvert*sizeof(float)) */ - int len, index, i; /* number of frames in cache, item index */ - - /* allocate memory for the cache */ - // TODO: investigate using BezTriple columns instead? - if (fcu->totvert == 0 || fcu->bezt == NULL) - return; - selcache = MEM_callocN(sizeof(float) * fcu->totvert, "FCurveSelFrameNums"); - len = 0; - index = 0; - - /* We do 2 loops, 1 for marking keyframes for deletion, one for deleting - * as there is no guarantee what order the keyframes are exactly, even though - * they have been sorted by time. - */ - - /* Loop 1: find selected keyframes */ - for (i = 0; i < fcu->totvert; i++) { - BezTriple *bezt = &fcu->bezt[i]; - - if (BEZT_ISSEL_ANY(bezt)) { - selcache[index] = bezt->vec[1][0]; - index++; - len++; - } - } - - /* Loop 2: delete unselected keyframes on the same frames - * (if any keyframes were found, or the whole curve wasn't affected) - */ - if ((len) && (len != fcu->totvert)) { - for (i = fcu->totvert - 1; i >= 0; i--) { - BezTriple *bezt = &fcu->bezt[i]; - - if (BEZT_ISSEL_ANY(bezt) == 0) { - /* check beztriple should be removed according to cache */ - for (index = 0; index < len; index++) { - if (IS_EQF(bezt->vec[1][0], selcache[index])) { - delete_fcurve_key(fcu, i, 0); - break; - } - else if (bezt->vec[1][0] < selcache[index]) - break; - } - } - } - - testhandles_fcurve(fcu, use_handle); - } - - /* free cache */ - MEM_freeN(selcache); -} - - /* Called by special_aftertrans_update to make sure selected keyframes replace * any other keyframes which may reside on that frame (that is not selected). -- cgit v1.2.3 From bba112011692d38829f7291d05b173418d8ba91a Mon Sep 17 00:00:00 2001 From: Joshua Leung Date: Wed, 21 Feb 2018 22:37:48 +1300 Subject: Fix: Don't average keyframe values if FCurve can only have int/discrete values This is to prevent problems with integer/enum properties getting invalid values set. --- source/blender/editors/transform/transform_conversions.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'source/blender/editors') diff --git a/source/blender/editors/transform/transform_conversions.c b/source/blender/editors/transform/transform_conversions.c index 9d44bf50b02..41511015d61 100644 --- a/source/blender/editors/transform/transform_conversions.c +++ b/source/blender/editors/transform/transform_conversions.c @@ -3529,6 +3529,7 @@ static void posttrans_fcurve_clean(FCurve *fcu, const bool use_handle) { /* NOTE: We assume that all keys are sorted */ ListBase retained_keys = {NULL, NULL}; + const bool can_average_points = ((fcu->flag & (FCURVE_INT_VALUES | FCURVE_DISCRETE_VALUES)) == 0); /* sanity checks */ if ((fcu->totvert == 0) || (fcu->bezt == NULL)) @@ -3597,8 +3598,10 @@ static void posttrans_fcurve_clean(FCurve *fcu, const bool use_handle) */ if (BEZT_ISSEL_ANY(bezt) && (rk->del_count == rk->tot_count - 1)) { /* Update keyframe */ - /* TODO: update handles too? */ - bezt->vec[1][1] = rk->val; + if (can_average_points) { + /* TODO: update handles too? */ + bezt->vec[1][1] = rk->val; + } } else { /* Delete keyframe */ -- cgit v1.2.3 From 915d120c3629868f93ddf2a10acfcafbeff5a327 Mon Sep 17 00:00:00 2001 From: Joshua Leung Date: Wed, 21 Feb 2018 23:21:56 +1300 Subject: Fix: Forgot to recalculate handles after deleting keyframes --- source/blender/editors/transform/transform_conversions.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'source/blender/editors') diff --git a/source/blender/editors/transform/transform_conversions.c b/source/blender/editors/transform/transform_conversions.c index 41511015d61..fc6865c3ed1 100644 --- a/source/blender/editors/transform/transform_conversions.c +++ b/source/blender/editors/transform/transform_conversions.c @@ -3615,6 +3615,9 @@ static void posttrans_fcurve_clean(FCurve *fcu, const bool use_handle) } } + /* 3) Recalculate handles */ + testhandles_fcurve(fcu, use_handle); + /* cleanup */ BLI_freelistN(&retained_keys); } -- cgit v1.2.3 From cab608066a9ac85c60956ebf94bdca67c01a76c7 Mon Sep 17 00:00:00 2001 From: Joshua Leung Date: Thu, 22 Feb 2018 00:44:03 +1300 Subject: Fix: Return back to conventional way of averaging points for keyframe de-dup The other approach was causing too much error in some cases (e.g. favouring the lower-valued keyframes). This fix should make the resulting curves less bumpy/jagged. --- source/blender/editors/transform/transform_conversions.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'source/blender/editors') diff --git a/source/blender/editors/transform/transform_conversions.c b/source/blender/editors/transform/transform_conversions.c index fc6865c3ed1..42a15939aee 100644 --- a/source/blender/editors/transform/transform_conversions.c +++ b/source/blender/editors/transform/transform_conversions.c @@ -3548,7 +3548,7 @@ static void posttrans_fcurve_clean(FCurve *fcu, const bool use_handle) /* If there's another selected frame here, merge it */ for (tRetainedKeyframe *rk = retained_keys.last; rk; rk = rk->prev) { if (IS_EQT(rk->frame, bezt->vec[1][0], BEZT_BINARYSEARCH_THRESH)) { - rk->val += (bezt->vec[1][1] - rk->val) / ((float)rk->tot_count); + rk->val += bezt->vec[1][1]; rk->tot_count++; found = true; @@ -3580,6 +3580,12 @@ static void posttrans_fcurve_clean(FCurve *fcu, const bool use_handle) } return; } + else { + /* Compute the average values for each retained keyframe */ + for (tRetainedKeyframe *rk = retained_keys.first; rk; rk = rk->next) { + rk->val = rk->val / (float)rk->tot_count; + } + } /* 2) Delete all keyframes duplicating the "retained keys" found above * - Most of these will be unselected keyframes -- cgit v1.2.3 From 7de387f4b56de3e069cce784c89e520fe77e5f1e Mon Sep 17 00:00:00 2001 From: Joshua Leung Date: Thu, 22 Feb 2018 00:59:15 +1300 Subject: Cleanup: Don't perform borderselect on channels that aren't visible --- source/blender/editors/space_action/action_select.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'source/blender/editors') diff --git a/source/blender/editors/space_action/action_select.c b/source/blender/editors/space_action/action_select.c index 3c3a71d00fd..3346c628336 100644 --- a/source/blender/editors/space_action/action_select.c +++ b/source/blender/editors/space_action/action_select.c @@ -262,7 +262,7 @@ static void borderselect_action(bAnimContext *ac, const rcti rect, short mode, s { /* loop over data selecting */ switch (ale->type) { -#if 0 /* XXXX: Keyframes are not currently shown here */ +#if 0 /* XXX: Keyframes are not currently shown here */ case ANIMTYPE_GPDATABLOCK: { bGPdata *gpd = ale->data; @@ -466,6 +466,7 @@ static void region_select_action_keys(bAnimContext *ac, const rctf *rectf_view, { /* loop over data selecting */ switch (ale->type) { +#if 0 /* XXX: Keyframes are not currently shown here */ case ANIMTYPE_GPDATABLOCK: { bGPdata *gpd = ale->data; @@ -475,6 +476,7 @@ static void region_select_action_keys(bAnimContext *ac, const rctf *rectf_view, } break; } +#endif case ANIMTYPE_GPLAYER: { ED_gplayer_frames_select_region(&ked, ale->data, mode, selectmode); @@ -718,8 +720,6 @@ static void columnselect_action_keys(bAnimContext *ac, short mode) KeyframeEditFunc select_cb, ok_cb; KeyframeEditData ked = {{NULL}}; - /* initialize keyframe editing data */ - /* build list of columns */ switch (mode) { case ACTKEYS_COLUMNSEL_KEYS: /* list of selected keys */ -- cgit v1.2.3