From 89d4164f54d49e5ffeff6ce0ec46556a3b638a8d Mon Sep 17 00:00:00 2001 From: Joshua Leung Date: Wed, 7 Jun 2017 19:07:21 +1200 Subject: GPencil Copy/Paste Fix: Copying/Pasting strokes between datablocks would crash The problem was that newly pasted strokes were still using colours from the original datablock. As a result, you'd either get an immediate crash, or if you managed to save the file before it crashed, each stroke would get reloaded with a dummy colour. This commit fixes makes it possible to copy/paste strokes between datablocks again. However, there are still problems when trying to paste across file boundaries (i.e. copy strokes in one file, paste in another), which the next commit will address. --- source/blender/editors/gpencil/gpencil_edit.c | 62 ++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) (limited to 'source/blender/editors/gpencil') diff --git a/source/blender/editors/gpencil/gpencil_edit.c b/source/blender/editors/gpencil/gpencil_edit.c index 2df4b2cae54..f2a0476a9e7 100644 --- a/source/blender/editors/gpencil/gpencil_edit.c +++ b/source/blender/editors/gpencil/gpencil_edit.c @@ -38,8 +38,11 @@ #include "MEM_guardedalloc.h" -#include "BLI_math.h" #include "BLI_blenlib.h" +#include "BLI_ghash.h" +#include "BLI_math.h" +#include "BLI_string.h" +#include "BLI_string_utils.h" #include "BLI_utildefines.h" #include "BLT_translation.h" @@ -455,6 +458,7 @@ static int gp_strokes_paste_exec(bContext *C, wmOperator *op) Scene *scene = CTX_data_scene(C); bGPdata *gpd = ED_gpencil_data_get_active(C); bGPDlayer *gpl = CTX_data_active_gpencil_layer(C); /* only use active for copy merge */ + bGPDpalette *palette = CTX_data_active_gpencil_palette(C); bGPDframe *gpf; eGP_PasteMode type = RNA_enum_get(op->ptr, "type"); @@ -515,6 +519,48 @@ static int gp_strokes_paste_exec(bContext *C, wmOperator *op) } CTX_DATA_END; + + /* First Pass Over Buffer: Identifiy the colors needed */ + GHash *src_colors = BLI_ghash_str_new("GPencil Paste Src Colors"); + GHash *dst_colors = BLI_ghash_str_new("GPencil Paste Dst Colors"); + GHashIterator gh_iter; + + for (bGPDstroke *gps = gp_strokes_copypastebuf.first; gps; gps = gps->next) { + if (ED_gpencil_stroke_can_use(C, gps)) { + if (BLI_ghash_haskey(src_colors, gps->colorname) == false) { + BLI_ghash_insert(src_colors, gps->colorname, gps->palcolor); + } + } + } + + /* Ensure that all the necessary colors exist */ + if (palette == NULL) { + palette = BKE_gpencil_palette_addnew(gpd, "Pasted Palette", true); + } + + GHASH_ITER(gh_iter, src_colors) { + bGPDpalettecolor *palcolor; + char *name = BLI_ghashIterator_getKey(&gh_iter); + + /* Look for existing color to map to */ + /* XXX: What to do if same name but different color? Behaviour here should depend on a property? */ + palcolor = BKE_gpencil_palettecolor_getbyname(palette, name); + if (palcolor == NULL) { + /* Doesn't Exist - Create new matching color for this palette */ + /* XXX: This still doesn't fix the pasting across file boundaries problem... */ + bGPDpalettecolor *src_color = BLI_ghashIterator_getValue(&gh_iter); + + palcolor = MEM_dupallocN(src_color); + BLI_addtail(&palette->colors, palcolor); + + BLI_uniquename(&palette->colors, palcolor, DATA_("GP Color"), '.', offsetof(bGPDpalettecolor, info), sizeof(palcolor->info)); + } + + /* Store this mapping (for use later when pasting) */ + BLI_ghash_insert(dst_colors, name, palcolor); + } + + /* Second Pass Over Buffer: Actually copy over the strokes (and adjust the colors) */ for (bGPDstroke *gps = gp_strokes_copypastebuf.first; gps; gps = gps->next) { if (ED_gpencil_stroke_can_use(C, gps)) { /* Need to verify if layer exists */ @@ -533,6 +579,7 @@ static int gp_strokes_paste_exec(bContext *C, wmOperator *op) */ gpf = BKE_gpencil_layer_getframe(gpl, CFRA, true); if (gpf) { + /* Create new stroke */ bGPDstroke *new_stroke = MEM_dupallocN(gps); new_stroke->tmp_layerinfo[0] = '\0'; @@ -543,10 +590,23 @@ static int gp_strokes_paste_exec(bContext *C, wmOperator *op) new_stroke->next = new_stroke->prev = NULL; BLI_addtail(&gpf->strokes, new_stroke); + + /* Fix color references */ + BLI_assert(new_stroke->colorname[0] != '\0'); + new_stroke->palcolor = BLI_ghash_lookup(dst_colors, new_stroke->colorname); + + BLI_assert(new_stroke->palcolor != NULL); + BLI_strncpy(new_stroke->colorname, new_stroke->palcolor->info, sizeof(new_stroke->colorname)); + + /*new_stroke->flag |= GP_STROKE_RECALC_COLOR; */ } } } + /* free temp data*/ + BLI_ghash_free(src_colors, NULL, NULL); + BLI_ghash_free(dst_colors, NULL, NULL); + /* updates */ WM_event_add_notifier(C, NC_GPENCIL | ND_DATA | NA_EDITED, NULL); -- cgit v1.2.3 From 7667040dd023469f8feaf42467c6237177565cd2 Mon Sep 17 00:00:00 2001 From: Joshua Leung Date: Wed, 7 Jun 2017 19:07:57 +1200 Subject: GP Copy/Paste Fix: Paste button doesn't update after copying strokes using Ctrl-C --- source/blender/editors/gpencil/gpencil_edit.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'source/blender/editors/gpencil') diff --git a/source/blender/editors/gpencil/gpencil_edit.c b/source/blender/editors/gpencil/gpencil_edit.c index f2a0476a9e7..c90faba3761 100644 --- a/source/blender/editors/gpencil/gpencil_edit.c +++ b/source/blender/editors/gpencil/gpencil_edit.c @@ -416,7 +416,10 @@ static int gp_strokes_copy_exec(bContext *C, wmOperator *op) } CTX_DATA_END; - /* done - no updates needed */ + /* updates (to ensure operator buttons are refreshed, when used via hotkeys) */ + WM_event_add_notifier(C, NC_GPENCIL | ND_DATA, NULL); // XXX? + + /* done */ return OPERATOR_FINISHED; } -- cgit v1.2.3 From 2bd49785f9d8bad39bd5687dfff022022cccc63f Mon Sep 17 00:00:00 2001 From: Joshua Leung Date: Thu, 8 Jun 2017 02:42:04 +1200 Subject: Fix: Pasting GP strokes files between files (or when the original colors were deleted) would crash The problem was that the strokes in the copy-paste buffer could be keeping dangling pointers to colors that were already freed. Therefore, this commit makes it so that when copying the strokes, we now make copies of the colors and put them in a hashtable beside the stroke buffer. This is convenient, as it saves us having to look up what colours need to be copied over each time when pasting. --- source/blender/editors/gpencil/gpencil_edit.c | 62 ++++++++++++++++++--------- 1 file changed, 42 insertions(+), 20 deletions(-) (limited to 'source/blender/editors/gpencil') diff --git a/source/blender/editors/gpencil/gpencil_edit.c b/source/blender/editors/gpencil/gpencil_edit.c index c90faba3761..697095ffb85 100644 --- a/source/blender/editors/gpencil/gpencil_edit.c +++ b/source/blender/editors/gpencil/gpencil_edit.c @@ -338,11 +338,27 @@ void GPENCIL_OT_duplicate(wmOperatorType *ot) /* NOTE: is exposed within the editors/gpencil module so that other tools can use it too */ ListBase gp_strokes_copypastebuf = {NULL, NULL}; +/* Hash for hanging on to all the palette colors used by strokes in the buffer + * + * This is needed to prevent dangling and unsafe pointers when pasting across datablocks, + * or after a color used by a stroke in the buffer gets deleted (via user action or undo). + */ +GHash *gp_strokes_copypastebuf_colors = NULL; + /* Free copy/paste buffer data */ void ED_gpencil_strokes_copybuf_free(void) { bGPDstroke *gps, *gpsn; + /* Free the palettes buffer + * NOTE: This is done before the strokes so that the name ptrs (keys) are still safe + */ + if (gp_strokes_copypastebuf_colors) { + BLI_ghash_free(gp_strokes_copypastebuf_colors, NULL, MEM_freeN); + gp_strokes_copypastebuf_colors = NULL; + } + + /* Free the stroke buffer */ for (gps = gp_strokes_copypastebuf.first; gps; gps = gpsn) { gpsn = gps->next; @@ -416,6 +432,22 @@ static int gp_strokes_copy_exec(bContext *C, wmOperator *op) } CTX_DATA_END; + /* Build up hash of colors used in these strokes, making copies of these to protect against dangling pointers */ + if (gp_strokes_copypastebuf.first) { + gp_strokes_copypastebuf_colors = BLI_ghash_str_new("GPencil CopyBuf Colors"); + + for (bGPDstroke *gps = gp_strokes_copypastebuf.first; gps; gps = gps->next) { + if (ED_gpencil_stroke_can_use(C, gps)) { + if (BLI_ghash_haskey(gp_strokes_copypastebuf_colors, gps->colorname) == false) { + bGPDpalettecolor *color = MEM_dupallocN(gps->palcolor); + + BLI_ghash_insert(gp_strokes_copypastebuf_colors, gps->colorname, color); + gps->palcolor = color; + } + } + } + } + /* updates (to ensure operator buttons are refreshed, when used via hotkeys) */ WM_event_add_notifier(C, NC_GPENCIL | ND_DATA, NULL); // XXX? @@ -464,6 +496,8 @@ static int gp_strokes_paste_exec(bContext *C, wmOperator *op) bGPDpalette *palette = CTX_data_active_gpencil_palette(C); bGPDframe *gpf; + GHash *new_colors; + GHashIterator gh_iter; eGP_PasteMode type = RNA_enum_get(op->ptr, "type"); /* check for various error conditions */ @@ -523,25 +557,14 @@ static int gp_strokes_paste_exec(bContext *C, wmOperator *op) CTX_DATA_END; - /* First Pass Over Buffer: Identifiy the colors needed */ - GHash *src_colors = BLI_ghash_str_new("GPencil Paste Src Colors"); - GHash *dst_colors = BLI_ghash_str_new("GPencil Paste Dst Colors"); - GHashIterator gh_iter; - - for (bGPDstroke *gps = gp_strokes_copypastebuf.first; gps; gps = gps->next) { - if (ED_gpencil_stroke_can_use(C, gps)) { - if (BLI_ghash_haskey(src_colors, gps->colorname) == false) { - BLI_ghash_insert(src_colors, gps->colorname, gps->palcolor); - } - } - } - /* Ensure that all the necessary colors exist */ + // TODO: Split this out into a helper function if (palette == NULL) { palette = BKE_gpencil_palette_addnew(gpd, "Pasted Palette", true); } + new_colors = BLI_ghash_str_new("GPencil Paste Dst Colors"); - GHASH_ITER(gh_iter, src_colors) { + GHASH_ITER(gh_iter, gp_strokes_copypastebuf_colors) { bGPDpalettecolor *palcolor; char *name = BLI_ghashIterator_getKey(&gh_iter); @@ -560,10 +583,10 @@ static int gp_strokes_paste_exec(bContext *C, wmOperator *op) } /* Store this mapping (for use later when pasting) */ - BLI_ghash_insert(dst_colors, name, palcolor); + BLI_ghash_insert(new_colors, name, palcolor); } - /* Second Pass Over Buffer: Actually copy over the strokes (and adjust the colors) */ + /* Copy over the strokes from the buffer (and adjust the colors) */ for (bGPDstroke *gps = gp_strokes_copypastebuf.first; gps; gps = gps->next) { if (ED_gpencil_stroke_can_use(C, gps)) { /* Need to verify if layer exists */ @@ -596,7 +619,7 @@ static int gp_strokes_paste_exec(bContext *C, wmOperator *op) /* Fix color references */ BLI_assert(new_stroke->colorname[0] != '\0'); - new_stroke->palcolor = BLI_ghash_lookup(dst_colors, new_stroke->colorname); + new_stroke->palcolor = BLI_ghash_lookup(new_colors, new_stroke->colorname); BLI_assert(new_stroke->palcolor != NULL); BLI_strncpy(new_stroke->colorname, new_stroke->palcolor->info, sizeof(new_stroke->colorname)); @@ -606,9 +629,8 @@ static int gp_strokes_paste_exec(bContext *C, wmOperator *op) } } - /* free temp data*/ - BLI_ghash_free(src_colors, NULL, NULL); - BLI_ghash_free(dst_colors, NULL, NULL); + /* free temp data */ + BLI_ghash_free(new_colors, NULL, NULL); /* updates */ WM_event_add_notifier(C, NC_GPENCIL | ND_DATA | NA_EDITED, NULL); -- cgit v1.2.3 From d25ab3fcc44ef2e58a1cca197948f3f362b4cc80 Mon Sep 17 00:00:00 2001 From: Joshua Leung Date: Thu, 8 Jun 2017 03:07:57 +1200 Subject: Fix: GP Clone brush was not correcting color references for pasted strokes either --- source/blender/editors/gpencil/gpencil_brush.c | 21 ++++++++ source/blender/editors/gpencil/gpencil_edit.c | 72 ++++++++++++++----------- source/blender/editors/gpencil/gpencil_intern.h | 5 ++ 3 files changed, 68 insertions(+), 30 deletions(-) (limited to 'source/blender/editors/gpencil') diff --git a/source/blender/editors/gpencil/gpencil_brush.c b/source/blender/editors/gpencil/gpencil_brush.c index 8ddf3a22517..5332bec0c64 100644 --- a/source/blender/editors/gpencil/gpencil_brush.c +++ b/source/blender/editors/gpencil/gpencil_brush.c @@ -773,6 +773,9 @@ typedef struct tGPSB_CloneBrushData { /* for "stamp" mode, the currently pasted brushes */ bGPDstroke **new_strokes; + + /* mapping from colors referenced per stroke, to the new colours in the "pasted" strokes */ + GHash *new_colors; } tGPSB_CloneBrushData; /* Initialise "clone" brush data */ @@ -816,6 +819,11 @@ static void gp_brush_clone_init(bContext *C, tGP_BrushEditData *gso) if (1 /*gso->brush->mode == GP_EDITBRUSH_CLONE_MODE_STAMP*/) { data->new_strokes = MEM_callocN(sizeof(bGPDstroke *) * data->totitems, "cloned strokes ptr array"); } + + /* Init colormap for mapping between the pasted stroke's source colour(names) + * and the final colours that will be used here instead... + */ + data->new_colors = gp_copybuf_validate_colormap(gso->gpd); } /* Free custom data used for "clone" brush */ @@ -829,6 +837,12 @@ static void gp_brush_clone_free(tGP_BrushEditData *gso) data->new_strokes = NULL; } + /* free copybuf colormap */ + if (data->new_colors) { + BLI_ghash_free(data->new_colors, NULL, NULL); + data->new_colors = NULL; + } + /* free the customdata itself */ MEM_freeN(data); gso->customdata = NULL; @@ -869,6 +883,13 @@ static void gp_brush_clone_add(bContext *C, tGP_BrushEditData *gso) new_stroke->next = new_stroke->prev = NULL; BLI_addtail(&gpf->strokes, new_stroke); + /* Fix color references */ + BLI_assert(new_stroke->colorname[0] != '\0'); + new_stroke->palcolor = BLI_ghash_lookup(data->new_colors, new_stroke->colorname); + + BLI_assert(new_stroke->palcolor != NULL); + BLI_strncpy(new_stroke->colorname, new_stroke->palcolor->info, sizeof(new_stroke->colorname)); + /* Adjust all the stroke's points, so that the strokes * get pasted relative to where the cursor is now */ diff --git a/source/blender/editors/gpencil/gpencil_edit.c b/source/blender/editors/gpencil/gpencil_edit.c index 697095ffb85..4b0ba6942e4 100644 --- a/source/blender/editors/gpencil/gpencil_edit.c +++ b/source/blender/editors/gpencil/gpencil_edit.c @@ -371,6 +371,46 @@ void ED_gpencil_strokes_copybuf_free(void) gp_strokes_copypastebuf.first = gp_strokes_copypastebuf.last = NULL; } +/* Ensure that destination datablock has all the colours the pasted strokes need + * Helper function for copy-pasting strokes + */ +GHash *gp_copybuf_validate_colormap(bGPdata *gpd) +{ + GHash *new_colors = BLI_ghash_str_new("GPencil Paste Dst Colors"); + GHashIterator gh_iter; + + /* If there's no active palette yet (i.e. new datablock), add one */ + bGPDpalette *palette = BKE_gpencil_palette_getactive(gpd); + if (palette == NULL) { + palette = BKE_gpencil_palette_addnew(gpd, "Pasted Palette", true); + } + + /* For each color, figure out what to map to... */ + GHASH_ITER(gh_iter, gp_strokes_copypastebuf_colors) { + bGPDpalettecolor *palcolor; + char *name = BLI_ghashIterator_getKey(&gh_iter); + + /* Look for existing color to map to */ + /* XXX: What to do if same name but different color? Behaviour here should depend on a property? */ + palcolor = BKE_gpencil_palettecolor_getbyname(palette, name); + if (palcolor == NULL) { + /* Doesn't Exist - Create new matching color for this palette */ + /* XXX: This still doesn't fix the pasting across file boundaries problem... */ + bGPDpalettecolor *src_color = BLI_ghashIterator_getValue(&gh_iter); + + palcolor = MEM_dupallocN(src_color); + BLI_addtail(&palette->colors, palcolor); + + BLI_uniquename(&palette->colors, palcolor, DATA_("GP Color"), '.', offsetof(bGPDpalettecolor, info), sizeof(palcolor->info)); + } + + /* Store this mapping (for use later when pasting) */ + BLI_ghash_insert(new_colors, name, palcolor); + } + + return new_colors; +} + /* --------------------- */ /* Copy selected strokes */ @@ -496,9 +536,8 @@ static int gp_strokes_paste_exec(bContext *C, wmOperator *op) bGPDpalette *palette = CTX_data_active_gpencil_palette(C); bGPDframe *gpf; - GHash *new_colors; - GHashIterator gh_iter; eGP_PasteMode type = RNA_enum_get(op->ptr, "type"); + GHash *new_colors; /* check for various error conditions */ if (gpd == NULL) { @@ -556,36 +595,9 @@ static int gp_strokes_paste_exec(bContext *C, wmOperator *op) } CTX_DATA_END; - /* Ensure that all the necessary colors exist */ - // TODO: Split this out into a helper function - if (palette == NULL) { - palette = BKE_gpencil_palette_addnew(gpd, "Pasted Palette", true); - } - new_colors = BLI_ghash_str_new("GPencil Paste Dst Colors"); - - GHASH_ITER(gh_iter, gp_strokes_copypastebuf_colors) { - bGPDpalettecolor *palcolor; - char *name = BLI_ghashIterator_getKey(&gh_iter); + new_colors = gp_copybuf_validate_colormap(gpd); - /* Look for existing color to map to */ - /* XXX: What to do if same name but different color? Behaviour here should depend on a property? */ - palcolor = BKE_gpencil_palettecolor_getbyname(palette, name); - if (palcolor == NULL) { - /* Doesn't Exist - Create new matching color for this palette */ - /* XXX: This still doesn't fix the pasting across file boundaries problem... */ - bGPDpalettecolor *src_color = BLI_ghashIterator_getValue(&gh_iter); - - palcolor = MEM_dupallocN(src_color); - BLI_addtail(&palette->colors, palcolor); - - BLI_uniquename(&palette->colors, palcolor, DATA_("GP Color"), '.', offsetof(bGPDpalettecolor, info), sizeof(palcolor->info)); - } - - /* Store this mapping (for use later when pasting) */ - BLI_ghash_insert(new_colors, name, palcolor); - } - /* Copy over the strokes from the buffer (and adjust the colors) */ for (bGPDstroke *gps = gp_strokes_copypastebuf.first; gps; gps = gps->next) { if (ED_gpencil_stroke_can_use(C, gps)) { diff --git a/source/blender/editors/gpencil/gpencil_intern.h b/source/blender/editors/gpencil/gpencil_intern.h index 5c7c9b84adb..13af06fd4fd 100644 --- a/source/blender/editors/gpencil/gpencil_intern.h +++ b/source/blender/editors/gpencil/gpencil_intern.h @@ -40,6 +40,8 @@ struct bGPdata; struct bGPDstroke; struct bGPDspoint; +struct GHash; + struct ARegion; struct View2D; struct wmOperatorType; @@ -155,6 +157,9 @@ int gp_brush_crt_presets_poll(bContext *C); extern ListBase gp_strokes_copypastebuf; +/* Build a map for converting between old colornames and destination-color-refs */ +struct GHash *gp_copybuf_validate_colormap(bGPdata *gpd); + /* Stroke Editing ------------------------------------ */ void gp_stroke_delete_tagged_points(bGPDframe *gpf, bGPDstroke *gps, bGPDstroke *next_stroke, int tag_flags); -- cgit v1.2.3 From 46c073e4ac38fae5209b82284313b740b132fd66 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Fri, 9 Jun 2017 06:45:21 +1000 Subject: Cleanup: cmake indentation, missing include --- source/blender/editors/gpencil/gpencil_edit.c | 1 - 1 file changed, 1 deletion(-) (limited to 'source/blender/editors/gpencil') diff --git a/source/blender/editors/gpencil/gpencil_edit.c b/source/blender/editors/gpencil/gpencil_edit.c index 4b0ba6942e4..174feec0e22 100644 --- a/source/blender/editors/gpencil/gpencil_edit.c +++ b/source/blender/editors/gpencil/gpencil_edit.c @@ -533,7 +533,6 @@ static int gp_strokes_paste_exec(bContext *C, wmOperator *op) Scene *scene = CTX_data_scene(C); bGPdata *gpd = ED_gpencil_data_get_active(C); bGPDlayer *gpl = CTX_data_active_gpencil_layer(C); /* only use active for copy merge */ - bGPDpalette *palette = CTX_data_active_gpencil_palette(C); bGPDframe *gpf; eGP_PasteMode type = RNA_enum_get(op->ptr, "type"); -- cgit v1.2.3