Fix T50745: Shape key editing on bezier objects broken with Rendered Viewport Shading

So... Curve+shapekey was even more broken than it looked, this report was
actually a nice crasher (immediate crash in an ASAN build when trying to
edit a curve shapekey with some viewport rendering enabled).

There were actually two different issues here.

I) The less critical: rB6f1493f68fe was not fully fixing issues from
T50614. More specifically, if you updated obdata from editnurb
*without* freeing editnurb afterwards, you had a 'restored' (to
original curve) editnurb, without the edited shapekey modifications
anymore. This was fixed by tweaking again `calc_shapeKeys()` behavior in
`ED_curve_editnurb_load()`.

II) The crasher: in `ED_curve_editnurb_make()`, the call to
`init_editNurb_keyIndex()` was directly storing pointers of obdata
nurbs. Since those get freed every time `ED_curve_editnurb_load()` is
executed, it easily ended up being pointers to freed memory. This was
fixed by copying those data, which implied more complex handling code
for editnurbs->keyindex, and some reshuffling of a few functions to
avoid duplicating things between editor's editcurve.c and BKE's curve.c

Note that the separation of functions between editors and BKE area for
curve could use a serious update, it's currently messy to say the least.
Then again, that area is due to rework since a long time now... :/

Finally, aligned 'for_render' curve evaluation to mesh one - now
editing a shapekey will show in rendered viewports, if it does have some
weight (exactly as with shapekeys of meshes).
This commit is contained in:
Bastien Montagne 2017-02-22 21:20:50 +01:00
parent b637db2a7a
commit 5e1d4714fe
4 changed files with 77 additions and 47 deletions

@ -36,6 +36,7 @@
struct BezTriple;
struct Curve;
struct EditNurb;
struct GHash;
struct ListBase;
struct Main;
struct Nurb;
@ -52,6 +53,13 @@ typedef struct CurveCache {
struct Path *path;
} CurveCache;
/* Definitions needed for shape keys */
typedef struct CVKeyIndex {
void *orig_cv;
int key_index, nu_index, pt_index, vertex_index;
bool switched;
} CVKeyIndex;
#define KNOTSU(nu) ( (nu)->orderu + (nu)->pntsu + (((nu)->flagu & CU_NURB_CYCLIC) ? ((nu)->orderu - 1) : 0) )
#define KNOTSV(nu) ( (nu)->orderv + (nu)->pntsv + (((nu)->flagv & CU_NURB_CYCLIC) ? ((nu)->orderv - 1) : 0) )
@ -108,7 +116,8 @@ void BK_curve_nurbs_vertexCos_apply(struct ListBase *lb, float (*vertexCos)[3]);
float (*BKE_curve_nurbs_keyVertexCos_get(struct ListBase *lb, float *key))[3];
void BKE_curve_nurbs_keyVertexTilts_apply(struct ListBase *lb, float *key);
void BKE_curve_editNurb_keyIndex_free(struct EditNurb *editnurb);
void BKE_curve_editNurb_keyIndex_delCV(struct GHash *keyindex, const void *cv);
void BKE_curve_editNurb_keyIndex_free(struct GHash **keyindex);
void BKE_curve_editNurb_free(struct Curve *cu);
struct ListBase *BKE_curve_editNurbs_get(struct Curve *cu);

@ -89,20 +89,33 @@ void BKE_curve_editfont_free(Curve *cu)
}
}
void BKE_curve_editNurb_keyIndex_free(EditNurb *editnurb)
static void curve_editNurb_keyIndex_cv_free_cb(void *val)
{
if (!editnurb->keyindex) {
CVKeyIndex *index = val;
MEM_freeN(index->orig_cv);
MEM_freeN(val);
}
void BKE_curve_editNurb_keyIndex_delCV(GHash *keyindex, const void *cv)
{
BLI_assert(keyindex != NULL);
BLI_ghash_remove(keyindex, cv, NULL, curve_editNurb_keyIndex_cv_free_cb);
}
void BKE_curve_editNurb_keyIndex_free(GHash **keyindex)
{
if (!(*keyindex)) {
return;
}
BLI_ghash_free(editnurb->keyindex, NULL, MEM_freeN);
editnurb->keyindex = NULL;
BLI_ghash_free(*keyindex, NULL, curve_editNurb_keyIndex_cv_free_cb);
*keyindex = NULL;
}
void BKE_curve_editNurb_free(Curve *cu)
{
if (cu->editnurb) {
BKE_nurbList_free(&cu->editnurb->nurbs);
BKE_curve_editNurb_keyIndex_free(cu->editnurb);
BKE_curve_editNurb_keyIndex_free(&cu->editnurb->keyindex);
MEM_freeN(cu->editnurb);
cu->editnurb = NULL;
}

@ -819,7 +819,7 @@ static void curve_calc_modifiers_pre(Scene *scene, Object *ob, ListBase *nurb,
if (editmode)
required_mode |= eModifierMode_Editmode;
if (cu->editnurb == NULL) {
if (!editmode) {
keyVerts = BKE_key_evaluate_object(ob, &numVerts);
if (keyVerts) {

@ -91,13 +91,6 @@ typedef struct {
int flag;
} UndoCurve;
/* Definitions needed for shape keys */
typedef struct {
void *orig_cv;
int key_index, nu_index, pt_index, vertex_index;
bool switched;
} CVKeyIndex;
void selectend_nurb(Object *obedit, enum eEndPoint_Types selfirst, bool doswap, bool selstatus);
static void adduplicateflagNurb(Object *obedit, ListBase *newnurb, const short flag, const bool split);
static int curve_delete_segments(Object *obedit, const bool split);
@ -139,7 +132,7 @@ void printknots(Object *obedit)
static CVKeyIndex *init_cvKeyIndex(void *cv, int key_index, int nu_index, int pt_index, int vertex_index)
{
CVKeyIndex *cvIndex = MEM_callocN(sizeof(CVKeyIndex), "init_cvKeyIndex");
CVKeyIndex *cvIndex = MEM_callocN(sizeof(CVKeyIndex), __func__);
cvIndex->orig_cv = cv;
cvIndex->key_index = key_index;
@ -172,7 +165,12 @@ static void init_editNurb_keyIndex(EditNurb *editnurb, ListBase *origBase)
origbezt = orignu->bezt;
pt_index = 0;
while (a--) {
keyIndex = init_cvKeyIndex(origbezt, key_index, nu_index, pt_index, vertex_index);
/* We cannot keep *any* reference to curve obdata,
* it might be replaced and freed while editcurve remain in use (in viewport render case e.g.).
* Note that we could use a pool to avoid lots of malloc's here, but... not really a problem for now. */
BezTriple *origbezt_cpy = MEM_mallocN(sizeof(*origbezt), __func__);
*origbezt_cpy = *origbezt;
keyIndex = init_cvKeyIndex(origbezt_cpy, key_index, nu_index, pt_index, vertex_index);
BLI_ghash_insert(gh, bezt, keyIndex);
key_index += 12;
vertex_index += 3;
@ -187,7 +185,12 @@ static void init_editNurb_keyIndex(EditNurb *editnurb, ListBase *origBase)
origbp = orignu->bp;
pt_index = 0;
while (a--) {
keyIndex = init_cvKeyIndex(origbp, key_index, nu_index, pt_index, vertex_index);
/* We cannot keep *any* reference to curve obdata,
* it might be replaced and freed while editcurve remain in use (in viewport render case e.g.).
* Note that we could use a pool to avoid lots of malloc's here, but... not really a problem for now. */
BPoint *origbp_cpy = MEM_mallocN(sizeof(*origbp_cpy), __func__);
*origbp_cpy = *origbp;
keyIndex = init_cvKeyIndex(origbp_cpy, key_index, nu_index, pt_index, vertex_index);
BLI_ghash_insert(gh, bp, keyIndex);
key_index += 4;
bp++;
@ -248,23 +251,22 @@ static int getKeyIndexOrig_keyIndex(EditNurb *editnurb, void *cv)
return index->key_index;
}
static void keyIndex_delCV(EditNurb *editnurb, const void *cv)
static void keyIndex_delBezt(EditNurb *editnurb, BezTriple *bezt)
{
if (!editnurb->keyindex) {
return;
}
BLI_ghash_remove(editnurb->keyindex, cv, NULL, MEM_freeN);
}
static void keyIndex_delBezt(EditNurb *editnurb, BezTriple *bezt)
{
keyIndex_delCV(editnurb, bezt);
BKE_curve_editNurb_keyIndex_delCV(editnurb->keyindex, bezt);
}
static void keyIndex_delBP(EditNurb *editnurb, BPoint *bp)
{
keyIndex_delCV(editnurb, bp);
if (!editnurb->keyindex) {
return;
}
BKE_curve_editNurb_keyIndex_delCV(editnurb->keyindex, bp);
}
static void keyIndex_delNurb(EditNurb *editnurb, Nurb *nu)
@ -280,7 +282,7 @@ static void keyIndex_delNurb(EditNurb *editnurb, Nurb *nu)
a = nu->pntsu;
while (a--) {
BLI_ghash_remove(editnurb->keyindex, bezt, NULL, MEM_freeN);
BKE_curve_editNurb_keyIndex_delCV(editnurb->keyindex, bezt);
bezt++;
}
}
@ -289,7 +291,7 @@ static void keyIndex_delNurb(EditNurb *editnurb, Nurb *nu)
a = nu->pntsu * nu->pntsv;
while (a--) {
BLI_ghash_remove(editnurb->keyindex, bp, NULL, MEM_freeN);
BKE_curve_editNurb_keyIndex_delCV(editnurb->keyindex, bp);
bp++;
}
}
@ -533,6 +535,7 @@ static GHash *dupli_keyIndexHash(GHash *keyindex)
CVKeyIndex *newIndex = MEM_mallocN(sizeof(CVKeyIndex), "dupli_keyIndexHash index");
memcpy(newIndex, index, sizeof(CVKeyIndex));
newIndex->orig_cv = MEM_dupallocN(index->orig_cv);
BLI_ghash_insert(gh, cv, newIndex);
}
@ -622,7 +625,7 @@ static void calc_keyHandles(ListBase *nurb, float *key)
}
}
static void calc_shapeKeys(Object *obedit)
static void calc_shapeKeys(Object *obedit, ListBase *newnurbs)
{
Curve *cu = (Curve *)obedit->data;
@ -634,7 +637,7 @@ static void calc_shapeKeys(Object *obedit)
KeyBlock *actkey = BLI_findlink(&cu->key->block, editnurb->shapenr - 1);
BezTriple *bezt, *oldbezt;
BPoint *bp, *oldbp;
Nurb *nu;
Nurb *nu, *newnu;
int totvert = BKE_nurbList_verts_count(&editnurb->nurbs);
float (*ofs)[3] = NULL;
@ -704,20 +707,25 @@ static void calc_shapeKeys(Object *obedit)
currkey = cu->key->block.first;
while (currkey) {
int apply_offset = (ofs && (currkey != actkey) && (editnurb->shapenr - 1 == currkey->relative));
const bool apply_offset = (ofs && (currkey != actkey) && (editnurb->shapenr - 1 == currkey->relative));
float *fp = newkey = MEM_callocN(cu->key->elemsize * totvert, "currkey->data");
ofp = oldkey = currkey->data;
nu = editnurb->nurbs.first;
/* We need to restore to original curve into newnurb, *not* editcurve's nurbs.
* Otherwise, in case we update obdata *without* leaving editmode (e.g. viewport render), we would
* invalidate editcurve. */
newnu = newnurbs->first;
i = 0;
while (nu) {
if (currkey == actkey) {
int restore = actkey != cu->key->refkey;
const bool restore = actkey != cu->key->refkey;
if (nu->bezt) {
bezt = nu->bezt;
a = nu->pntsu;
BezTriple *newbezt = newnu->bezt;
while (a--) {
int j;
oldbezt = getKeyIndexOrig_bezt(editnurb, bezt);
@ -726,7 +734,7 @@ static void calc_shapeKeys(Object *obedit)
copy_v3_v3(fp, bezt->vec[j]);
if (restore && oldbezt) {
copy_v3_v3(bezt->vec[j], oldbezt->vec[j]);
copy_v3_v3(newbezt->vec[j], oldbezt->vec[j]);
}
fp += 3;
@ -734,16 +742,18 @@ static void calc_shapeKeys(Object *obedit)
fp[0] = bezt->alfa;
if (restore && oldbezt) {
bezt->alfa = oldbezt->alfa;
newbezt->alfa = oldbezt->alfa;
}
fp += 3; ++i; /* alphas */
bezt++;
newbezt++;
}
}
else {
bp = nu->bp;
a = nu->pntsu * nu->pntsv;
BPoint *newbp = newnu->bp;
while (a--) {
oldbp = getKeyIndexOrig_bp(editnurb, bp);
@ -752,12 +762,13 @@ static void calc_shapeKeys(Object *obedit)
fp[3] = bp->alfa;
if (restore && oldbp) {
copy_v3_v3(bp->vec, oldbp->vec);
bp->alfa = oldbp->alfa;
copy_v3_v3(newbp->vec, oldbp->vec);
newbp->alfa = oldbp->alfa;
}
fp += 4;
bp++;
newbp++;
i += 2;
}
}
@ -1193,11 +1204,6 @@ void ED_curve_editnurb_load(Object *obedit)
remap_hooks_and_vertex_parents(obedit);
/* We have to apply shapekeys *before* copying nurbs into newnurb, otherwise the reset to
* refkey/original curve data that has to be done when editing non-refkey shapekey would be useless,
* only affecting editnurb and not ob->data. */
calc_shapeKeys(obedit);
for (nu = editnurb->first; nu; nu = nu->next) {
newnu = BKE_nurb_duplicate(nu);
BLI_addtail(&newnurb, newnu);
@ -1207,6 +1213,11 @@ void ED_curve_editnurb_load(Object *obedit)
}
}
/* We have to pass also new copied nurbs, since we want to restore original curve (without edited shapekey)
* on obdata, but *not* on editcurve itself (ED_curve_editnurb_load call does not always implies freeing
* of editcurve, e.g. when called to generate render data...). */
calc_shapeKeys(obedit, &newnurb);
cu->nurb = newnurb;
ED_curve_updateAnimPaths(obedit->data);
@ -1233,8 +1244,7 @@ void ED_curve_editnurb_make(Object *obedit)
if (editnurb) {
BKE_nurbList_free(&editnurb->nurbs);
BKE_curve_editNurb_keyIndex_free(editnurb);
editnurb->keyindex = NULL;
BKE_curve_editNurb_keyIndex_free(&editnurb->keyindex);
}
else {
editnurb = MEM_callocN(sizeof(EditNurb), "editnurb");
@ -1314,8 +1324,7 @@ static int separate_exec(bContext *C, wmOperator *op)
ED_curve_editnurb_make(newob);
newedit = newcu->editnurb;
BKE_nurbList_free(&newedit->nurbs);
BKE_curve_editNurb_keyIndex_free(newedit);
newedit->keyindex = NULL;
BKE_curve_editNurb_keyIndex_free(&newedit->keyindex);
BLI_movelisttolist(&newedit->nurbs, &newnurb);
/* 4. put old object out of editmode and delete separated geometry */
@ -6115,7 +6124,7 @@ static void undoCurve_to_editCurve(void *ucu, void *UNUSED(edata), void *cu_v)
BKE_nurbList_free(editbase);
if (undoCurve->undoIndex) {
BLI_ghash_free(editnurb->keyindex, NULL, MEM_freeN);
BKE_curve_editNurb_keyIndex_free(&editnurb->keyindex);
editnurb->keyindex = dupli_keyIndexHash(undoCurve->undoIndex);
}
@ -6193,8 +6202,7 @@ static void free_undoCurve(void *ucv)
BKE_nurbList_free(&undoCurve->nubase);
if (undoCurve->undoIndex)
BLI_ghash_free(undoCurve->undoIndex, NULL, MEM_freeN);
BKE_curve_editNurb_keyIndex_free(&undoCurve->undoIndex);
free_fcurves(&undoCurve->fcurves);
free_fcurves(&undoCurve->drivers);