From 5e1d4714fef72407e3497e70ddac3f1caa959f39 Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Wed, 22 Feb 2017 21:20:50 +0100 Subject: 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). --- source/blender/editors/curve/editcurve.c | 88 +++++++++++++++++--------------- 1 file changed, 48 insertions(+), 40 deletions(-) (limited to 'source/blender/editors/curve/editcurve.c') diff --git a/source/blender/editors/curve/editcurve.c b/source/blender/editors/curve/editcurve.c index 5686cacd4e5..47f42ab5321 100644 --- a/source/blender/editors/curve/editcurve.c +++ b/source/blender/editors/curve/editcurve.c @@ -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); -- cgit v1.2.3