From 9a2f5b0709cd0f721ed5b5ef6b137d2a5ec90e5a Mon Sep 17 00:00:00 2001 From: Jehan Date: Sat, 12 Nov 2022 18:28:58 +0100 Subject: [PATCH] app: rework and fix the logic for copy-pasting multiple drawables. There were a lot of incertainty of what should happen when we copy layers being descendant of each other (i.e. when you select a group layer and some of its children), then when you paste such data. So we sat down with Aryeom and tried to come up with some consistent behavior which is somewhat expectable, but also which would allow the most use-case. Otherwise it was making very weird result when pasting the data, duplicating some layers and whatnot, which was obviously a buggy behavior and never the expected result. We decided that if you select one leaf item, then even if you also selected a parent item, it would be as though the parent was not selected. This is very often what you expect anyway when you select a whole bunch of layers and would work well if, say, you shift-click over many layers in sub-groups. Then you wouldn't have to manually ctrl-click to unselect every group. Then what if you were instead expecting to copy many groups? Then you could shift-click the group arrow, closing all same-level groups. Once they are all closed, you can shift-click the groups to only select group layers, not their contents. This way, both use cases are still quite doable easily with this default choice. --- app/core/gimp-edit.c | 170 ++++++++++++++++++------------ app/core/gimpchannel-combine.c | 2 +- app/core/gimpchannel-select.c | 2 +- app/core/gimpimage-new.c | 98 +++++++++++------ app/core/gimpimage-new.h | 3 +- app/core/gimpimage-pick-color.c | 2 +- app/core/gimpselection.c | 2 +- app/file-data/file-data-pat.c | 2 +- app/paint/gimpperspectiveclone.c | 2 +- app/paint/gimpsourceoptions.c | 2 +- app/tools/gimpbycolorselecttool.c | 2 +- app/tools/gimpfuzzyselecttool.c | 2 +- 12 files changed, 180 insertions(+), 109 deletions(-) diff --git a/app/core/gimp-edit.c b/app/core/gimp-edit.c index 668a6310be..313ff04611 100644 --- a/app/core/gimp-edit.c +++ b/app/core/gimp-edit.c @@ -118,7 +118,7 @@ gimp_edit_cut (GimpImage *image, g_list_free (remove); /* Now copy all layers into the clipboard image. */ - clip_image = gimp_image_new_from_drawables (image->gimp, drawables, FALSE); + clip_image = gimp_image_new_from_drawables (image->gimp, drawables, FALSE, TRUE); gimp_container_remove (image->gimp->images, GIMP_OBJECT (clip_image)); gimp_set_clipboard_image (image->gimp, clip_image); g_object_unref (clip_image); @@ -193,7 +193,7 @@ gimp_edit_copy (GimpImage *image, GimpImage *clip_image; GimpChannel *clip_selection; - clip_image = gimp_image_new_from_drawables (image->gimp, drawables, TRUE); + clip_image = gimp_image_new_from_drawables (image->gimp, drawables, TRUE, TRUE); gimp_container_remove (image->gimp->images, GIMP_OBJECT (clip_image)); gimp_set_clipboard_image (image->gimp, clip_image); g_object_unref (clip_image); @@ -335,6 +335,101 @@ gimp_edit_paste_is_floating (GimpPasteType paste_type, g_return_val_if_reached (FALSE); } +static GList * +gimp_edit_paste_get_tagged_layers (GimpImage *image, + GList *layers, + GList *returned_layers, + const Babl *floating_format, + GimpImageBaseType base_type, + GimpPrecision precision, + GimpPasteType paste_type) +{ + GList *iter; + + for (iter = layers; iter; iter = iter->next) + { + GimpLayer *layer; + GType layer_type; + gboolean copied = TRUE; + + switch (paste_type) + { + case GIMP_PASTE_TYPE_FLOATING: + case GIMP_PASTE_TYPE_FLOATING_IN_PLACE: + case GIMP_PASTE_TYPE_FLOATING_INTO: + case GIMP_PASTE_TYPE_FLOATING_INTO_IN_PLACE: + /* when pasting as floating make sure gimp_item_convert() + * will turn group layers into normal layers, otherwise use + * the same layer type so e.g. text information gets + * preserved. See issue #2667. + */ + if (GIMP_IS_GROUP_LAYER (iter->data)) + layer_type = GIMP_TYPE_LAYER; + else + layer_type = G_TYPE_FROM_INSTANCE (iter->data); + break; + + case GIMP_PASTE_TYPE_NEW_LAYER: + case GIMP_PASTE_TYPE_NEW_LAYER_IN_PLACE: + layer_type = G_TYPE_FROM_INSTANCE (iter->data); + break; + + default: + g_return_val_if_reached (NULL); + } + + if (GIMP_IS_GROUP_LAYER (iter->data)) + copied = (gboolean) GPOINTER_TO_INT (g_object_get_data (G_OBJECT (iter->data), + "gimp-image-copied-layer")); + if (copied) + { + layer = GIMP_LAYER (gimp_item_convert (GIMP_ITEM (iter->data), + image, layer_type)); + returned_layers = g_list_prepend (returned_layers, layer); + + switch (paste_type) + { + case GIMP_PASTE_TYPE_FLOATING: + case GIMP_PASTE_TYPE_FLOATING_IN_PLACE: + case GIMP_PASTE_TYPE_FLOATING_INTO: + case GIMP_PASTE_TYPE_FLOATING_INTO_IN_PLACE: + /* when pasting as floating selection, get rid of the layer mask, + * and make sure the layer has the right format + */ + if (gimp_layer_get_mask (iter->data)) + gimp_layer_apply_mask (iter->data, GIMP_MASK_DISCARD, FALSE); + + if (gimp_drawable_get_format (GIMP_DRAWABLE (iter->data)) != + floating_format) + { + gimp_drawable_convert_type (GIMP_DRAWABLE (iter->data), image, + base_type, precision, + TRUE, NULL, NULL, + GEGL_DITHER_NONE, GEGL_DITHER_NONE, + FALSE, NULL); + } + break; + + default: + break; + } + } + else + { + GimpContainer *container; + + container = gimp_viewable_get_children (iter->data); + returned_layers = gimp_edit_paste_get_tagged_layers (image, + GIMP_LIST (container)->queue->head, + returned_layers, + floating_format, + base_type, precision, paste_type); + } + } + + return returned_layers; +} + static GList * gimp_edit_paste_get_layers (GimpImage *image, GList *drawables, @@ -367,9 +462,7 @@ gimp_edit_paste_get_layers (GimpImage *image, if (GIMP_IS_IMAGE (paste)) { - GList *iter; - - layers = g_list_copy (gimp_image_get_layer_iter (GIMP_IMAGE (paste))); + layers = gimp_image_get_layer_iter (GIMP_IMAGE (paste)); if (g_list_length (layers) > 1) { @@ -379,68 +472,11 @@ gimp_edit_paste_get_layers (GimpImage *image, *paste_type = GIMP_PASTE_TYPE_NEW_LAYER; } - for (iter = layers; iter; iter = iter->next) - { - GType layer_type; - - switch (*paste_type) - { - case GIMP_PASTE_TYPE_FLOATING: - case GIMP_PASTE_TYPE_FLOATING_IN_PLACE: - case GIMP_PASTE_TYPE_FLOATING_INTO: - case GIMP_PASTE_TYPE_FLOATING_INTO_IN_PLACE: - /* when pasting as floating make sure gimp_item_convert() - * will turn group layers into normal layers, otherwise use - * the same layer type so e.g. text information gets - * preserved. See issue #2667. - */ - if (GIMP_IS_GROUP_LAYER (iter->data)) - layer_type = GIMP_TYPE_LAYER; - else - layer_type = G_TYPE_FROM_INSTANCE (iter->data); - break; - - case GIMP_PASTE_TYPE_NEW_LAYER: - case GIMP_PASTE_TYPE_NEW_LAYER_IN_PLACE: - layer_type = G_TYPE_FROM_INSTANCE (iter->data); - break; - - default: - g_return_val_if_reached (NULL); - } - - iter->data = GIMP_LAYER (gimp_item_convert (GIMP_ITEM (iter->data), - image, layer_type)); - - switch (*paste_type) - { - case GIMP_PASTE_TYPE_FLOATING: - case GIMP_PASTE_TYPE_FLOATING_IN_PLACE: - case GIMP_PASTE_TYPE_FLOATING_INTO: - case GIMP_PASTE_TYPE_FLOATING_INTO_IN_PLACE: - /* when pasting as floating selection, get rid of the layer mask, - * and make sure the layer has the right format - */ - if (gimp_layer_get_mask (iter->data)) - gimp_layer_apply_mask (iter->data, GIMP_MASK_DISCARD, FALSE); - - if (gimp_drawable_get_format (GIMP_DRAWABLE (iter->data)) != - floating_format) - { - gimp_drawable_convert_type (GIMP_DRAWABLE (iter->data), image, - gimp_drawable_get_base_type (drawables->data), - gimp_drawable_get_precision (drawables->data), - TRUE, - NULL, NULL, - GEGL_DITHER_NONE, GEGL_DITHER_NONE, - FALSE, NULL); - } - break; - - default: - break; - } - } + layers = gimp_edit_paste_get_tagged_layers (image, layers, NULL, floating_format, + gimp_drawable_get_base_type (drawables->data), + gimp_drawable_get_precision (drawables->data), + *paste_type); + layers = g_list_reverse (layers); } else if (GIMP_IS_BUFFER (paste)) { diff --git a/app/core/gimpchannel-combine.c b/app/core/gimpchannel-combine.c index 24ea70ec1f..b31bf4468d 100644 --- a/app/core/gimpchannel-combine.c +++ b/app/core/gimpchannel-combine.c @@ -534,7 +534,7 @@ gimp_channel_combine_items (GimpChannel *mask, { GList *merged_layers; - temp_image = gimp_image_new_from_drawables (image->gimp, items, FALSE); + temp_image = gimp_image_new_from_drawables (image->gimp, items, FALSE, FALSE); merged_layers = gimp_image_merge_visible_layers (temp_image, gimp_get_user_context (temp_image->gimp), GIMP_CLIP_TO_IMAGE, diff --git a/app/core/gimpchannel-select.c b/app/core/gimpchannel-select.c index 2201faa851..f881e30369 100644 --- a/app/core/gimpchannel-select.c +++ b/app/core/gimpchannel-select.c @@ -577,7 +577,7 @@ gimp_channel_select_by_color (GimpChannel *channel, } else { - sel_image = gimp_image_new_from_drawables (image->gimp, drawables, FALSE); + sel_image = gimp_image_new_from_drawables (image->gimp, drawables, FALSE, FALSE); gimp_container_remove (image->gimp->images, GIMP_OBJECT (sel_image)); pickable = GIMP_PICKABLE (sel_image); diff --git a/app/core/gimpimage-new.c b/app/core/gimpimage-new.c index 2c89c15661..fd93705fd9 100644 --- a/app/core/gimpimage-new.c +++ b/app/core/gimpimage-new.c @@ -37,6 +37,7 @@ #include "gimpchannel.h" #include "gimpcontext.h" #include "gimpdrawable-fill.h" +#include "gimpgrouplayer.h" #include "gimpimage.h" #include "gimpimage-color-profile.h" #include "gimpimage-colormap.h" @@ -246,8 +247,12 @@ gimp_image_new_from_drawable (Gimp *gimp, /** * gimp_image_new_copy_drawables: - * @image: - * @drawables: the drawables to insert into @image. + * @image: the image where @drawables belong to. + * @drawables: the drawables to copy into @new_image. + * @new_image: the image to insert to. + * @tag_copies: tag copies of @drawable with "gimp-image-copied-layer". + * @copied_drawables: + * @tagged_drawables: * @parent: * @new_parent: * @@ -259,13 +264,17 @@ gimp_image_new_from_drawable (Gimp *gimp, * full opacity and default layer mode. Otherwise, visibility, opacity * and layer mode will be copied as-is, allowing proper compositing. * - * The @parent and @new_parent arguments are only used internally for - * recursive calls and must be set to NULL for the initial call. + * The @copied_drawables, @tagged_drawables, @parent and @new_parent arguments + * are only used internally for recursive calls and must be set to NULL for the + * initial call. */ static void gimp_image_new_copy_drawables (GimpImage *image, GList *drawables, GimpImage *new_image, + gboolean tag_copies, + GList *copied_drawables, + GList *tagged_drawables, GimpLayer *parent, GimpLayer *new_parent) { @@ -277,26 +286,32 @@ gimp_image_new_copy_drawables (GimpImage *image, n_drawables = g_list_length (drawables); if (parent == NULL) { - if (n_drawables == 1) - { - layers = drawables; - } - else - { - /* Root layers. */ - layers = gimp_image_get_layer_iter (image); + /* Root layers. */ + layers = gimp_image_get_layer_iter (image); - /* Add any item parent. */ - drawables = g_list_copy (drawables); - for (iter = drawables; iter; iter = iter->next) - { - GimpItem *item = iter->data; - while ((item = gimp_item_get_parent (item))) - { - if (! g_list_find (drawables, item)) - drawables = g_list_prepend (drawables, item); - } - } + copied_drawables = g_list_copy (drawables); + for (iter = copied_drawables; iter; iter = iter->next) + { + /* Tagged drawables are the explicitly copied drawables which have no + * explicitly copied descendant items. + */ + GList *iter2; + + for (iter2 = iter; iter2; iter2 = iter2->next) + if (gimp_viewable_is_ancestor (iter->data, iter2->data)) + break; + + if (iter2 == NULL) + tagged_drawables = g_list_prepend (tagged_drawables, iter->data); + } + + /* Add any item parent. */ + for (iter = copied_drawables; iter; iter = iter->next) + { + GimpItem *item = iter->data; + while ((item = gimp_item_get_parent (item))) + if (! g_list_find (copied_drawables, item)) + copied_drawables = g_list_prepend (copied_drawables, item); } } else @@ -310,18 +325,31 @@ gimp_image_new_copy_drawables (GimpImage *image, index = 0; for (iter = layers; iter; iter = iter->next) { - if (g_list_find (drawables, iter->data)) + if (g_list_find (copied_drawables, iter->data)) { GimpLayer *new_layer; GType new_type; + gboolean is_group; + gboolean is_tagged; if (GIMP_IS_LAYER (iter->data)) new_type = G_TYPE_FROM_INSTANCE (iter->data); else new_type = GIMP_TYPE_LAYER; - new_layer = GIMP_LAYER (gimp_item_convert (GIMP_ITEM (iter->data), - new_image, new_type)); + is_group = (gimp_viewable_get_children (iter->data) != NULL); + is_tagged = (g_list_find (tagged_drawables, iter->data) != NULL); + + if (is_group && ! is_tagged) + new_layer = gimp_group_layer_new (new_image); + else + new_layer = GIMP_LAYER (gimp_item_convert (GIMP_ITEM (iter->data), + new_image, new_type)); + + if (tag_copies && is_tagged) + g_object_set_data (G_OBJECT (new_layer), + "gimp-image-copied-layer", + GINT_TO_POINTER (TRUE)); gimp_object_set_name (GIMP_OBJECT (new_layer), gimp_object_get_name (iter->data)); @@ -349,19 +377,25 @@ gimp_image_new_copy_drawables (GimpImage *image, gimp_image_add_layer (new_image, new_layer, new_parent, index++, TRUE); /* If a group, loop through children. */ - if (n_drawables > 1 && gimp_viewable_get_children (iter->data)) - gimp_image_new_copy_drawables (image, drawables, new_image, iter->data, new_layer); + if (is_group && ! is_tagged) + gimp_image_new_copy_drawables (image, drawables, new_image, tag_copies, + copied_drawables, tagged_drawables, + iter->data, new_layer); } } - if (parent == NULL && n_drawables != 1) - g_list_free (drawables); + if (parent == NULL) + { + g_list_free (copied_drawables); + g_list_free (tagged_drawables); + } } GimpImage * gimp_image_new_from_drawables (Gimp *gimp, GList *drawables, - gboolean copy_selection) + gboolean copy_selection, + gboolean tag_copies) { GimpImage *image = NULL; GimpImage *new_image; @@ -426,7 +460,7 @@ gimp_image_new_from_drawables (Gimp *gimp, } } - gimp_image_new_copy_drawables (image, drawables, new_image, NULL, NULL); + gimp_image_new_copy_drawables (image, drawables, new_image, tag_copies, NULL, NULL, NULL, NULL); gimp_image_undo_enable (new_image); return new_image; diff --git a/app/core/gimpimage-new.h b/app/core/gimpimage-new.h index 5b08a402a9..5622ccd3ef 100644 --- a/app/core/gimpimage-new.h +++ b/app/core/gimpimage-new.h @@ -31,7 +31,8 @@ GimpImage * gimp_image_new_from_drawable (Gimp *gimp, GimpDrawable *drawable); GimpImage * gimp_image_new_from_drawables (Gimp *gimp, GList *drawables, - gboolean copy_selection); + gboolean copy_selection, + gboolean tag_copies); GimpImage * gimp_image_new_from_component (Gimp *gimp, GimpImage *image, GimpChannelType component); diff --git a/app/core/gimpimage-pick-color.c b/app/core/gimpimage-pick-color.c index f158942054..90726eb989 100644 --- a/app/core/gimpimage-pick-color.c +++ b/app/core/gimpimage-pick-color.c @@ -115,7 +115,7 @@ gimp_image_pick_color (GimpImage *image, } else /* length > 1 */ { - pick_image = gimp_image_new_from_drawables (image->gimp, drawables, FALSE); + pick_image = gimp_image_new_from_drawables (image->gimp, drawables, FALSE, FALSE); gimp_container_remove (image->gimp->images, GIMP_OBJECT (pick_image)); if (! show_all) diff --git a/app/core/gimpselection.c b/app/core/gimpselection.c index cc8fda49ad..c5224f9dea 100644 --- a/app/core/gimpselection.c +++ b/app/core/gimpselection.c @@ -708,7 +708,7 @@ gimp_selection_extract (GimpSelection *selection, for (iter = pickables; iter; iter = iter->next) g_return_val_if_fail (GIMP_IS_DRAWABLE (iter->data), NULL); - temp_image = gimp_image_new_from_drawables (image->gimp, pickables, TRUE); + temp_image = gimp_image_new_from_drawables (image->gimp, pickables, TRUE, FALSE); selection = GIMP_SELECTION (gimp_image_get_mask (temp_image)); pickable = GIMP_PICKABLE (temp_image); diff --git a/app/file-data/file-data-pat.c b/app/file-data/file-data-pat.c index a3b62e68b4..fac6cf2f72 100644 --- a/app/file-data/file-data-pat.c +++ b/app/file-data/file-data-pat.c @@ -264,7 +264,7 @@ file_pat_image_to_pattern (GimpImage *image, for (gint i = 0; i < n_drawables; i++) drawable_list = g_list_prepend (drawable_list, drawables[i]); - subimage = gimp_image_new_from_drawables (image->gimp, drawable_list, FALSE); + subimage = gimp_image_new_from_drawables (image->gimp, drawable_list, FALSE, FALSE); g_list_free (drawable_list); gimp_container_remove (image->gimp->images, GIMP_OBJECT (subimage)); gimp_image_resize_to_layers (subimage, context, diff --git a/app/paint/gimpperspectiveclone.c b/app/paint/gimpperspectiveclone.c index b8432e7a36..83a3791fb0 100644 --- a/app/paint/gimpperspectiveclone.c +++ b/app/paint/gimpperspectiveclone.c @@ -231,7 +231,7 @@ gimp_perspective_clone_paint (GimpPaintCore *paint_core, { /* A composited image of the drawables */ del_image = gimp_image_new_from_drawables (src_image->gimp, drawables, - FALSE); + FALSE, FALSE); gimp_container_remove (src_image->gimp->images, GIMP_OBJECT (del_image)); src_pickable = GIMP_PICKABLE (del_image); diff --git a/app/paint/gimpsourceoptions.c b/app/paint/gimpsourceoptions.c index 8f4653293e..aca018dcc8 100644 --- a/app/paint/gimpsourceoptions.c +++ b/app/paint/gimpsourceoptions.c @@ -279,7 +279,7 @@ gimp_source_options_make_pickable (GimpSourceOptions *options) * to these drawables. */ options->src_image = gimp_image_new_from_drawables (image->gimp, options->src_drawables, - FALSE); + FALSE, FALSE); gimp_container_remove (image->gimp->images, GIMP_OBJECT (options->src_image)); options->src_pickable = GIMP_PICKABLE (options->src_image); diff --git a/app/tools/gimpbycolorselecttool.c b/app/tools/gimpbycolorselecttool.c index ce5cfc126e..f6ee21c521 100644 --- a/app/tools/gimpbycolorselecttool.c +++ b/app/tools/gimpbycolorselecttool.c @@ -127,7 +127,7 @@ gimp_by_color_select_tool_get_mask (GimpRegionSelectTool *region_select, } else { - select_image = gimp_image_new_from_drawables (image->gimp, drawables, FALSE); + select_image = gimp_image_new_from_drawables (image->gimp, drawables, FALSE, FALSE); gimp_container_remove (image->gimp->images, GIMP_OBJECT (select_image)); pickable = GIMP_PICKABLE (select_image); diff --git a/app/tools/gimpfuzzyselecttool.c b/app/tools/gimpfuzzyselecttool.c index d48d77fbb6..fdbb04f1ff 100644 --- a/app/tools/gimpfuzzyselecttool.c +++ b/app/tools/gimpfuzzyselecttool.c @@ -126,7 +126,7 @@ gimp_fuzzy_select_tool_get_mask (GimpRegionSelectTool *region_select, } else { - select_image = gimp_image_new_from_drawables (image->gimp, drawables, FALSE); + select_image = gimp_image_new_from_drawables (image->gimp, drawables, FALSE, FALSE); gimp_container_remove (image->gimp->images, GIMP_OBJECT (select_image)); pickable = GIMP_PICKABLE (select_image);