griefith/test2
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix #115813: Handles get broken when scaling annotation keyframes in Dope Sheet

Browse Source 
Apparently this problem has existed at least since b6b61681ef.

When converting pointers to generic structs in
`transform_convert_action.cc`, elements of type `TransData` and
`TransData2D` must align by maintaining synchronized indices within
their respective arrays.

Despite both arrays having equivalent lengths, some `TransData2D`
instances were omitted when they come from types `ANIMTYPE_GPLAYER` or
`ANIMTYPE_MASKLAYER`.

This misalignment resulted in `TransData` elements not properly
corresponding with their `TransData2D` counterparts.

A potential fix could be incrementing `td2d++` for each
`ANIMTYPE_GPLAYER` or `ANIMTYPE_MASKLAYER` occurrence. This approach,
while introducing blank `TransData2D` entries, would preserve index
alignment with `TransData`.

However, I opted for a less workaround-centric approach by converting
`tGPFtransdata` elements into `TransData2D`.This solution, albeit
slightly fragile due to the lack of a dedicated member in `TransData2D`
for integer value flushing, appears better than allowing blank
`TransData2D` fields.
This commit is contained in:
Germano Cavalcante
2023-12-05 18:12:36 -03:00
parent 7ec59b05ff
commit 7f626e08e1
1 changed files with 40 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files

101
source/blender/editors/transform/transform_convert_action.cc
View File

@@ -37,17 +37,13 @@
#include "transform_convert.hh" #include "transform_convert.hh"
/** Helper struct for GP-frame transforms. */ /* Weak way of identifying whether TransData was set by #GPLayerToTransData or
struct tGPFtransdata { * #MaskLayerToTransData. This way we can identify whether the #td->extra is a pointer to an
union { * integer value and we can correctly flush the #recalcData_actedit. */
/** Where `transdata` writes transform. */ static bool data_is_gp_or_mask(TransData *td, TransData2D *td2d)
float val; {
/** #td->val and #td->loc share the same pointer. */ return td->extra && !td2d->loc2d;
float loc[3]; }
};
/** Pointer to `gpf->framenum` */
int *sdata;
};
/* -------------------------------------------------------------------- */ /* -------------------------------------------------------------------- */
/** \name Grease Pencil Transform helpers /** \name Grease Pencil Transform helpers
@@ -352,6 +348,8 @@ static void TimeToTransData(
/* Set flags to move handles as necessary. */ /* Set flags to move handles as necessary. */
td->flag |= TD_MOVEHANDLE1 | TD_MOVEHANDLE2; td->flag |= TD_MOVEHANDLE1 | TD_MOVEHANDLE2;
BLI_assert(!data_is_gp_or_mask(td, td2d));
} }
/* This function advances the address to which td points to, so it must return /* This function advances the address to which td points to, so it must return
@@ -406,7 +404,7 @@ static TransData *ActionFCurveToTransData(TransData *td,
* 'R'/'L' mean only data on the named side are used. * 'R'/'L' mean only data on the named side are used.
*/ */
static int GPLayerToTransData(TransData *td, static int GPLayerToTransData(TransData *td,
tGPFtransdata *tfd, TransData2D *td2d,
bGPDlayer *gpl, bGPDlayer *gpl,
char side, char side,
float cfra, float cfra,
@@ -420,26 +418,27 @@ static int GPLayerToTransData(TransData *td,
const bool is_selected = (gpf->flag & GP_FRAME_SELECT) != 0; const bool is_selected = (gpf->flag & GP_FRAME_SELECT) != 0;
if (is_prop_edit || is_selected) { if (is_prop_edit || is_selected) {
if (FrameOnMouseSide(side, float(gpf->framenum), cfra)) { if (FrameOnMouseSide(side, float(gpf->framenum), cfra)) {
tfd->val = float(gpf->framenum); td2d->loc[0] = float(gpf->framenum);
tfd->sdata = &gpf->framenum;
td->loc = tfd->loc; td->loc = td->val = td2d->loc;
td->iloc[0] = tfd->loc[0]; td->iloc[0] = td->ival = td2d->loc[0];
td->val = &tfd->val;
td->ival = tfd->val;
td->center[0] = td->ival; td->center[0] = td->ival;
td->center[1] = ypos; td->center[1] = ypos;
/* Store the int value in #td->extra, so we can flush later. */
td->extra = &gpf->framenum;
if (is_selected) { if (is_selected) {
td->flag = TD_SELECTED; td->flag = TD_SELECTED;
} }
/* Advance `td` now. */ /* Advance `td` now. */
td++; td++;
tfd++; td2d++;
count++; count++;
BLI_assert(data_is_gp_or_mask(td, td2d));
} }
} }
} }
@@ -498,6 +497,8 @@ static int GreasePencilLayerToTransData(TransData *td,
td2d++; td2d++;
total_trans_frames++; total_trans_frames++;
any_frame_affected = true; any_frame_affected = true;
BLI_assert(!data_is_gp_or_mask(td, td2d));
}; };
const blender::Map<int, GreasePencilFrame> &frame_map = const blender::Map<int, GreasePencilFrame> &frame_map =
@@ -522,7 +523,7 @@ static int GreasePencilLayerToTransData(TransData *td,
/* refer to comment above #GPLayerToTransData, this is the same but for masks */ /* refer to comment above #GPLayerToTransData, this is the same but for masks */
static int MaskLayerToTransData(TransData *td, static int MaskLayerToTransData(TransData *td,
tGPFtransdata *tfd, TransData2D *td2d,
MaskLayer *masklay, MaskLayer *masklay,
char side, char side,
float cfra, float cfra,
@@ -535,22 +536,23 @@ static int MaskLayerToTransData(TransData *td,
LISTBASE_FOREACH (MaskLayerShape *, masklay_shape, &masklay->splines_shapes) { LISTBASE_FOREACH (MaskLayerShape *, masklay_shape, &masklay->splines_shapes) {
if (is_prop_edit || (masklay_shape->flag & MASK_SHAPE_SELECT)) { if (is_prop_edit || (masklay_shape->flag & MASK_SHAPE_SELECT)) {
if (FrameOnMouseSide(side, float(masklay_shape->frame), cfra)) { if (FrameOnMouseSide(side, float(masklay_shape->frame), cfra)) {
tfd->val = float(masklay_shape->frame); td2d->loc[0] = float(masklay_shape->frame);
tfd->sdata = &masklay_shape->frame;
td->loc = tfd->loc; td->loc = td->val = td2d->loc;
td->iloc[0] = tfd->loc[0]; td->iloc[0] = td->ival = td2d->loc[0];
td->val = &tfd->val;
td->ival = tfd->val;
td->center[0] = td->ival; td->center[0] = td->ival;
td->center[1] = ypos; td->center[1] = ypos;
/* Store the int value in #td->extra, so we can flush later. */
td->extra = &masklay_shape->frame;
/* advance td now */ /* advance td now */
td++; td++;
tfd++; td2d++;
count++; count++;
BLI_assert(data_is_gp_or_mask(td, td2d));
} }
} }
} }
@@ -563,7 +565,6 @@ static void createTransActionData(bContext *C, TransInfo *t)
Scene *scene = t->scene; Scene *scene = t->scene;
TransData *td = nullptr; TransData *td = nullptr;
TransData2D *td2d = nullptr; TransData2D *td2d = nullptr;
tGPFtransdata *tfd = nullptr;
/* The T_DUPLICATED_KEYFRAMES flag is only set if we made some duplicates of the selected frames, /* The T_DUPLICATED_KEYFRAMES flag is only set if we made some duplicates of the selected frames,
* and they are the ones that are being transformed. */ * and they are the ones that are being transformed. */
@@ -669,13 +670,6 @@ static void createTransActionData(bContext *C, TransInfo *t)
td = tc->data; td = tc->data;
td2d = tc->data_2d; td2d = tc->data_2d;
if (ELEM(ac.datatype, ANIMCONT_GPENCIL, ANIMCONT_MASK, ANIMCONT_DOPESHEET, ANIMCONT_TIMELINE)) {
tc->data_gpf_len = gpf_count;
tc->custom.type.data = tfd = static_cast<tGPFtransdata *>(
MEM_callocN(sizeof(tGPFtransdata) * gpf_count, "tGPFtransdata"));
tc->custom.type.use_free = true;
}
/* loop 2: build transdata array */ /* loop 2: build transdata array */
LISTBASE_FOREACH (bAnimListElem *, ale, &anim_data) { LISTBASE_FOREACH (bAnimListElem *, ale, &anim_data) {
@@ -697,9 +691,9 @@ static void createTransActionData(bContext *C, TransInfo *t)
bGPDlayer *gpl = (bGPDlayer *)ale->data; bGPDlayer *gpl = (bGPDlayer *)ale->data;
int i; int i;
i = GPLayerToTransData(td, tfd, gpl, t->frame_side, cfra, is_prop_edit, ypos); i = GPLayerToTransData(td, td2d, gpl, t->frame_side, cfra, is_prop_edit, ypos);
td += i; td += i;
tfd += i; td2d += i;
} }
else if (ale->type == ANIMTYPE_GREASE_PENCIL_LAYER) { else if (ale->type == ANIMTYPE_GREASE_PENCIL_LAYER) {
using namespace blender::bke::greasepencil; using namespace blender::bke::greasepencil;
@@ -715,9 +709,9 @@ static void createTransActionData(bContext *C, TransInfo *t)
MaskLayer *masklay = (MaskLayer *)ale->data; MaskLayer *masklay = (MaskLayer *)ale->data;
int i; int i;
i = MaskLayerToTransData(td, tfd, masklay, t->frame_side, cfra, is_prop_edit, ypos); i = MaskLayerToTransData(td, td2d, masklay, t->frame_side, cfra, is_prop_edit, ypos);
td += i; td += i;
tfd += i; td2d += i;
} }
else { else {
AnimData *adt = ANIM_nla_mapping_get(&ac, ale); AnimData *adt = ANIM_nla_mapping_get(&ac, ale);
@@ -879,20 +873,6 @@ static void createTransActionData(bContext *C, TransInfo *t)
/** \name Action Transform Flush /** \name Action Transform Flush
* \{ */ * \{ */
/* (Grease Pencil Legacy)
* This function helps flush transdata written to tempdata into the gp-frames. */
static void flushTransIntFrameActionData(TransInfo *t)
{
TransDataContainer *tc = TRANS_DATA_CONTAINER_FIRST_SINGLE(t);
tGPFtransdata *tfd = static_cast<tGPFtransdata *>(tc->custom.type.data);
/* flush data!
* Expects data_gpf_len to be set in the data container. */
for (int i = 0; i < tc->data_gpf_len; i++, tfd++) {
*(tfd->sdata) = round_fl_to_int(tfd->val);
}
}
static void invert_snap(eSnapMode &snap_mode) static void invert_snap(eSnapMode &snap_mode)
{ {
/* Make snapping work like before 4.0 where pressing CTRL will switch between snapping to seconds /* Make snapping work like before 4.0 where pressing CTRL will switch between snapping to seconds
@@ -932,12 +912,6 @@ static void recalcData_actedit(TransInfo *t)
ANIM_animdata_context_getdata(&ac); ANIM_animdata_context_getdata(&ac);
/* perform flush */
if (ELEM(ac.datatype, ANIMCONT_GPENCIL, ANIMCONT_MASK, ANIMCONT_DOPESHEET, ANIMCONT_TIMELINE)) {
/* flush transform values back to actual coordinates */
flushTransIntFrameActionData(t);
}
/* Flush 2d vector. */ /* Flush 2d vector. */
TransDataContainer *tc = TRANS_DATA_CONTAINER_FIRST_SINGLE(t); TransDataContainer *tc = TRANS_DATA_CONTAINER_FIRST_SINGLE(t);
eSnapMode snap_mode = t->tsnap.mode; eSnapMode snap_mode = t->tsnap.mode;
@@ -966,6 +940,11 @@ static void recalcData_actedit(TransInfo *t)
round_fl_to_int(td2d->loc[0]), round_fl_to_int(td2d->loc[0]),
use_duplicated); use_duplicated);
} }
else if (data_is_gp_or_mask(td, td2d)) {
/* (Grease Pencil Legacy)
* This helps flush transdata written to tempdata into the gp-frames. */
*reinterpret_cast<int *>(td->extra) = round_fl_to_int(td2d->loc[0]);
}
} }
if (ac.datatype != ANIMCONT_MASK) { if (ac.datatype != ANIMCONT_MASK) {