Bugfix [#27157] keyframing a constrained bone does not work as before
authorJoshua Leung <aligorith@gmail.com>
Sat, 16 Jul 2011 06:46:39 +0000 (06:46 +0000)
committerJoshua Leung <aligorith@gmail.com>
Sat, 16 Jul 2011 06:46:39 +0000 (06:46 +0000)
Visual Keyframing was broken by r.34685, which used another method
which, at the time, appeared to work perfectly fine. Apparently not.

Also, extend/fixed visual keying to work for axis-angle rotations too.
Needs some testing, but should probably work

1  2 
source/blender/editors/animation/keyframing.c

index 1ba695209b2e9f672950c086ff8f616d4b6b4e8c,8dc813d940b023723d8bbdd3e072bd2329057029..109da669ce6e3fac118192ec39c0c88855bc17c4
@@@ -1,4 -1,4 +1,4 @@@
 -/**
 +/*
   * $Id$
   *
   * ***** BEGIN GPL LICENSE BLOCK *****
   *
   * ***** END GPL LICENSE BLOCK *****
   */
 +
 +/** \file blender/editors/animation/keyframing.c
 + *  \ingroup edanimation
 + */
 +
   
  #include <stdio.h>
  #include <stddef.h>
@@@ -55,7 -50,6 +55,7 @@@
  
  #include "BKE_animsys.h"
  #include "BKE_action.h"
 +#include "BKE_armature.h"
  #include "BKE_constraint.h"
  #include "BKE_depsgraph.h"
  #include "BKE_fcurve.h"
@@@ -300,7 -294,6 +300,7 @@@ int insert_bezt_fcurve (FCurve *fcu, Be
  int insert_vert_fcurve (FCurve *fcu, float x, float y, short flag)
  {
        BezTriple beztr= {{{0}}};
 +      unsigned int oldTot = fcu->totvert;
        int a;
        
        /* set all three points, for nicer start position 
        if ((fcu->totvert > 2) && (flag & INSERTKEY_REPLACE)==0) {
                BezTriple *bezt= (fcu->bezt + a);
                
 -              /* set interpolation from previous (if available) */
 -              // FIXME: this doesn't work if user tweaked the interpolation specifically, and they were just overwriting some existing key in the process...
 -              if (a > 0) bezt->ipo= (bezt-1)->ipo;
 -              else if (a < fcu->totvert-1) bezt->ipo= (bezt+1)->ipo;
 +              /* set interpolation from previous (if available), but only if we didn't just replace some keyframe 
 +               *      - replacement is indicated by no-change in number of verts
 +               *      - when replacing, the user may have specified some interpolation that should be kept
 +               */
 +              if (fcu->totvert > oldTot) {
 +                      if (a > 0) 
 +                              bezt->ipo= (bezt-1)->ipo;
 +                      else if (a < fcu->totvert-1) 
 +                              bezt->ipo= (bezt+1)->ipo;
 +              }
                        
                /* don't recalculate handles if fast is set
                 *      - this is a hack to make importers faster
@@@ -366,7 -353,7 +366,7 @@@ enum 
        KEYNEEDED_JUSTADD,
        KEYNEEDED_DELPREV,
        KEYNEEDED_DELNEXT
 -} eKeyNeededStatus;
 +} /*eKeyNeededStatus*/;
  
  /* This helper function determines whether a new keyframe is needed */
  /* Cases where keyframes should not be added:
@@@ -403,14 -390,14 +403,14 @@@ static short new_key_needed (FCurve *fc
                        prevVal= prev->vec[1][1];
                        
                        /* keyframe to be added at point where there are already two similar points? */
 -                      if (IS_EQ(prevPosi, cFrame) && IS_EQ(beztPosi, cFrame) && IS_EQ(beztPosi, prevPosi)) {
 +                      if (IS_EQF(prevPosi, cFrame) && IS_EQF(beztPosi, cFrame) && IS_EQF(beztPosi, prevPosi)) {
                                return KEYNEEDED_DONTADD;
                        }
                        
                        /* keyframe between prev+current points ? */
                        if ((prevPosi <= cFrame) && (cFrame <= beztPosi)) {
                                /* is the value of keyframe to be added the same as keyframes on either side ? */
 -                              if (IS_EQ(prevVal, nValue) && IS_EQ(beztVal, nValue) && IS_EQ(prevVal, beztVal)) {
 +                              if (IS_EQF(prevVal, nValue) && IS_EQF(beztVal, nValue) && IS_EQF(prevVal, beztVal)) {
                                        return KEYNEEDED_DONTADD;
                                }
                                else {
                                        realVal= evaluate_fcurve(fcu, cFrame);
                                        
                                        /* compare whether it's the same as proposed */
 -                                      if (IS_EQ(realVal, nValue)) 
 +                                      if (IS_EQF(realVal, nValue))
                                                return KEYNEEDED_DONTADD;
                                        else 
                                                return KEYNEEDED_JUSTADD;
                                 * stays around or not depends on whether the values of previous/current
                                 * beztriples and new keyframe are the same.
                                 */
 -                              if (IS_EQ(prevVal, nValue) && IS_EQ(beztVal, nValue) && IS_EQ(prevVal, beztVal))
 +                              if (IS_EQF(prevVal, nValue) && IS_EQF(beztVal, nValue) && IS_EQF(prevVal, beztVal))
                                        return KEYNEEDED_DELNEXT;
                                else 
                                        return KEYNEEDED_JUSTADD;
        else 
                valB= bezt->vec[1][1] + 1.0f; 
                
 -      if (IS_EQ(valA, nValue) && IS_EQ(valA, valB)) 
 +      if (IS_EQF(valA, nValue) && IS_EQF(valA, valB))
                return KEYNEEDED_DELPREV;
        else 
                return KEYNEEDED_JUSTADD;
@@@ -661,41 -648,41 +661,72 @@@ static float visualkey_get_value (Point
                                mat4_to_eulO(eul, ob->rotmode, ob->obmat);
                                return eul[array_index];
                        }
-                       // FIXME: other types of rotation don't work
++                      else if (strstr(identifier, "rotation_quaternion")) {
++                              float trimat[3][3], quat[4];
++                              
++                              copy_m3_m4(trimat, ob->obmat);
++                              mat3_to_quat_is_ok(quat, trimat);
++                              
++                              return quat[array_index];
++                      }
++                      else if (strstr(identifier, "rotation_axis_angle")) {
++                              float axis[3], angle;
++                              
++                              mat4_to_axis_angle(axis, &angle, ob->obmat);
++                              
++                              /* w = 0, x,y,z = 1,2,3 */
++                              if (array_index == 0)
++                                      return angle;
++                              else
++                                      return axis[array_index - 1];
++                      }
                }
        }
        else if (ptr->type == &RNA_PoseBone) {
 -              Object *ob= (Object *)ptr->id.data; /* we assume that this is always set, and is an object */
++              Object *ob = (Object *)ptr->id.data; /* we assume that this is always set, and is an object */
                bPoseChannel *pchan= (bPoseChannel *)ptr->data;
--              bPoseChannel tchan;
++              float tmat[4][4];
                
--              /* make a copy of pchan so that we can apply and decompose its chan_mat, thus getting the 
--               * rest-pose to pose-mode transform that got stored there at the end of posing calculations
--               * for B-Bone deforms to use
--               *      - it should be safe to just make a local copy like this, since we're not doing anything with the copied pointers
++              /* Although it is not strictly required for this particular space conversion, 
++               * arg1 must not be null, as there is a null check for the other conversions to
++               * be safe. Therefore, the active object is passed here, and in many cases, this
++               * will be what owns the pose-channel that is getting this anyway.
                 */
--              memcpy(&tchan, pchan, sizeof(bPoseChannel));
-               pchan_apply_mat4(&tchan, pchan->chan_mat, TRUE);
 -              pchan_apply_mat4(&tchan, pchan->chan_mat);
++              copy_m4_m4(tmat, pchan->pose_mat);
++              constraint_mat_convertspace(ob, pchan, tmat, CONSTRAINT_SPACE_POSE, CONSTRAINT_SPACE_LOCAL);
                
                /* Loc, Rot/Quat keyframes are supported... */
                if (strstr(identifier, "location")) {
                        /* only use for non-connected bones */
                        if ((pchan->bone->parent) && !(pchan->bone->flag & BONE_CONNECTED))
--                              return tchan.loc[array_index];
++                              return tmat[3][array_index];
                        else if (pchan->bone->parent == NULL)
--                              return tchan.loc[array_index];
++                              return tmat[3][array_index];
                }
                else if (strstr(identifier, "rotation_euler")) {
--                      return tchan.eul[array_index];
++                      float eul[3];
++                      
++                      mat4_to_eulO(eul, pchan->rotmode, tmat);
++                      return eul[array_index];
                }
                else if (strstr(identifier, "rotation_quaternion")) {
--                      return tchan.quat[array_index];
++                      float trimat[3][3], quat[4];
++                      
++                      copy_m3_m4(trimat, tmat);
++                      mat3_to_quat_is_ok(quat, trimat);
++                      
++                      return quat[array_index];
                }
--              else if (strstr(identifier, "rotation_axisangle")) {
++              else if (strstr(identifier, "rotation_axis_angle")) {
++                      float axis[3], angle;
++                      
++                      mat4_to_axis_angle(axis, &angle, tmat);
++                      
                        /* w = 0, x,y,z = 1,2,3 */
                        if (array_index == 0)
--                              return tchan.rotAngle;
++                              return angle;
                        else
--                              return tchan.rotAxis[array_index - 1];
++                              return axis[array_index - 1];
                }
        }
        
@@@ -916,7 -903,7 +947,7 @@@ short insert_keyframe (ReportList *repo
                                /* for Loc/Rot/Scale and also Color F-Curves, the color of the F-Curve in the Graph Editor,
                                 * is determined by the array index for the F-Curve
                                 */
 -                              if (ELEM4(RNA_property_subtype(prop), PROP_TRANSLATION, PROP_XYZ, PROP_EULER, PROP_COLOR)) {
 +                              if (ELEM5(RNA_property_subtype(prop), PROP_TRANSLATION, PROP_XYZ, PROP_EULER, PROP_COLOR, PROP_COORDS)) {
                                        fcu->color_mode= FCURVE_COLOR_AUTO_RGB;
                                }
                        }
@@@ -1049,7 -1036,7 +1080,7 @@@ short delete_keyframe (ReportList *repo
  enum {
        COMMONKEY_MODE_INSERT = 0,
        COMMONKEY_MODE_DELETE,
 -} eCommonModifyKey_Modes;
 +} /*eCommonModifyKey_Modes*/;
  
  /* Polling callback for use with ANIM_*_keyframe() operators
   * This is based on the standard ED_operator_areaactive callback,
@@@ -1287,7 -1274,7 +1318,7 @@@ void ANIM_OT_keyframe_delete (wmOperato
        PropertyRNA *prop;
        
        /* identifiers */
 -      ot->name= "Delete Keyframe";
 +      ot->name= "Delete Keying-Set Keyframe";
        ot->idname= "ANIM_OT_keyframe_delete";
        ot->description= "Delete keyframes on the current frame for all properties in the specified Keying Set";
        
@@@ -1332,7 -1319,7 +1363,7 @@@ static int delete_key_v3d_exec (bContex
                short success= 0;
                
                /* loop through all curves in animdata and delete keys on this frame */
 -              if (ob->adt) {
 +              if ((ob->adt) && (ob->adt->action)) {
                        AnimData *adt= ob->adt;
                        bAction *act= adt->action;
                        
                        }
                }
                
 -              BKE_reportf(op->reports, RPT_INFO, "Ob '%s' - Successfully removed %d keyframes \n", id->name+2, success);
 +              BKE_reportf(op->reports, RPT_INFO, "Ob '%s' - Successfully had %d keyframes removed", id->name+2, success);
                
                ob->recalc |= OB_RECALC_OB;
        }
@@@ -1360,7 -1347,6 +1391,7 @@@ void ANIM_OT_keyframe_delete_v3d (wmOpe
  {
        /* identifiers */
        ot->name= "Delete Keyframe";
 +      ot->description= "Remove keyframes on current frame for selected object";
        ot->idname= "ANIM_OT_keyframe_delete_v3d";
        
        /* callbacks */
@@@ -1380,7 -1366,7 +1411,7 @@@ static int insert_key_button_exec (bCon
  {
        Main *bmain= CTX_data_main(C);
        Scene *scene= CTX_data_scene(C);
 -      PointerRNA ptr= {{0}};
 +      PointerRNA ptr= {{NULL}};
        PropertyRNA *prop= NULL;
        char *path;
        float cfra= (float)CFRA; // XXX for now, don't bother about all the yucky offset crap
@@@ -1469,7 -1455,7 +1500,7 @@@ static int delete_key_button_exec (bCon
  {
        Main *bmain= CTX_data_main(C);
        Scene *scene= CTX_data_scene(C);
 -      PointerRNA ptr= {{0}};
 +      PointerRNA ptr= {{NULL}};
        PropertyRNA *prop= NULL;
        char *path;
        float cfra= (float)CFRA; // XXX for now, don't bother about all the yucky offset crap
@@@ -1591,7 -1577,7 +1622,7 @@@ short fcurve_frame_has_keyframe (FCurv
  /* Checks whether an Action has a keyframe for a given frame 
   * Since we're only concerned whether a keyframe exists, we can simply loop until a match is found...
   */
 -short action_frame_has_keyframe (bAction *act, float frame, short filter)
 +static short action_frame_has_keyframe (bAction *act, float frame, short filter)
  {
        FCurve *fcu;
        
  }
  
  /* Checks whether an Object has a keyframe for a given frame */
 -short object_frame_has_keyframe (Object *ob, float frame, short filter)
 +static short object_frame_has_keyframe (Object *ob, float frame, short filter)
  {
        /* error checking */
        if (ob == NULL)