Fix T101775 : Remove double listing of grease pencil data in dopesheet summary
ClosedPublic
Actions

Authored by Amelie Fondevilla (afonde) on Nov 2 2022, 4:16 PM.

Details

Reviewers

Jeroen Bakker (jbakker)
Falk David (filedescriptor)

Commits

rBd46317cf3c5e: Fix T101775: grease pencil keyframe not filtered in dopesheet summary

Summary

Proposed fix for T101775

The issue is : when trying to filter out keyframes corresponding to animated grease pencil data (not layer keyframes, but data related to grease pencil object, e.g. layer opacity) using the 'Grease Pencil' filter, the keyframes remain visible in the summary.

This is caused by the call to animdata_filter_ds_obdata which creates an item while it should not. This function deals with animated data from all kind of objects, but does not deal properly with grease pencil objects (the case of OB_GPENCIL type object is not handled in the switch statement).

Actually, grease pencil data is already handled by another function : animdata_filter_ds_gpencil, called later in animdata_filter_dopesheet_ob.
So currently, the handling of these data in the summary is done twice.

This patch proposes an easy way of fixing the issue : not calling animdata_filter_ds_obdata in the case of grease pencil object, and leaving the specific 'animdata_filter_ds_gpencil` deal with it.

Diff Detail

Repository

rB Blender

Branch

fix-summary (branched from master)

Build Status

Buildable 24462
Build 24462: arc lint + arc unit

Event Timeline

Amelie Fondevilla (afonde) requested review of this revision.Nov 2 2022, 4:16 PM

Amelie Fondevilla (afonde) created this revision.

Harbormaster completed remote builds in B24462: Diff 57199.Nov 2 2022, 4:16 PM

Amelie Fondevilla (afonde) retitled this revision from Remove double listing of grease pencil data in dopesheet to Fix T101775 : Remove double listing of grease pencil data in dopesheet summary.Nov 2 2022, 4:18 PM

Amelie Fondevilla (afonde) edited the summary of this revision. (Show Details)

Note that there is a cleaner (but more complicated) way of dealing with this : remove the part of animdata_filter_ds_gpencil that is handling this data, and make room in animdata_filter_ds_obdata to handle grease pencil data.
Let me know your opinion on this, if you have one.

Amelie Fondevilla (afonde) edited the summary of this revision. (Show Details)Nov 2 2022, 4:29 PM

Amelie Fondevilla (afonde) added reviewers: Jeroen Bakker (jbakker), Falk David (filedescriptor).

I think it's ok to do it this way for the moment.

It's a nit-pick but a common pattern for spiting the code path for objects is to use a switch statement on the object type. So I would do something like:

/* object data */
if (ob->data) {
  switch (ob->type) {
    /* grease pencil */
    case OB_GPENCIL:
      if (!(ads->filterflag & ADS_FILTER_NOGPENCIL)) {
        tmp_items += animdata_filter_ds_gpencil(ac, &tmp_data, ads, base, filter_mode);
      }
      break;

    default:
      tmp_items += animdata_filter_ds_obdata(ac, &tmp_data, ads, ob, filter_mode);
      break;
  }
}

This revision is now accepted and ready to land.Nov 4 2022, 11:39 AM

Jeroen Bakker (jbakker) accepted this revision.Nov 14 2022, 11:21 AM

Closed by commit rBd46317cf3c5e: Fix T101775: grease pencil keyframe not filtered in dopesheet summary (authored by Amelie Fondevilla (afonde)). · Explain WhyNov 15 2022, 9:50 AM

This revision was automatically updated to reflect the committed changes.

Amelie Fondevilla (afonde) added a commit: rBd46317cf3c5e: Fix T101775: grease pencil keyframe not filtered in dopesheet summary.