From c26ca2339afeaba47a00b5d130d2ad9a133ec549 Mon Sep 17 00:00:00 2001 From: Jehan Date: Mon, 23 Sep 2024 14:21:43 +0200 Subject: [PATCH] app: store GimpPalette in GimpImageUndo, not a raw colormap. Also make sure gimp_data_copy() also copies the proper palette format restriction and apply it when necessary (e.g. setting the stored colormap which is done when popping an image undo). Otherwise we were losing format/space information, which was corrupting color space when undoing then redoing. --- app/core/gimpimage-color-profile.c | 2 +- app/core/gimpimage-colormap.c | 2 ++ app/core/gimpimageundo.c | 31 ++++++++++++++++++++---------- app/core/gimpimageundo.h | 2 +- app/core/gimppalette-import.c | 4 ++-- app/core/gimppalette.c | 21 +++++++++++++++++--- app/core/gimppalette.h | 3 ++- 7 files changed, 47 insertions(+), 18 deletions(-) diff --git a/app/core/gimpimage-color-profile.c b/app/core/gimpimage-color-profile.c index 85c67b9c8b..a0c40c7bf3 100644 --- a/app/core/gimpimage-color-profile.c +++ b/app/core/gimpimage-color-profile.c @@ -1010,7 +1010,7 @@ gimp_image_convert_profile_colormap (GimpImage *image, palette = gimp_image_get_colormap_palette (image); space = gimp_image_get_layer_space (image); format = gimp_babl_format (GIMP_RGB, private->precision, FALSE, space); - gimp_palette_restrict_format (palette, format); + gimp_palette_restrict_format (palette, format, TRUE); } static void diff --git a/app/core/gimpimage-colormap.c b/app/core/gimpimage-colormap.c index d03aacbca9..ed31ecc360 100644 --- a/app/core/gimpimage-colormap.c +++ b/app/core/gimpimage-colormap.c @@ -237,6 +237,8 @@ gimp_image_set_colormap_palette (GimpImage *image, while ((entry = gimp_palette_get_entry (private->palette, 0))) gimp_palette_delete_entry (private->palette, entry); + gimp_palette_restrict_format (private->palette, palette->format, FALSE); + for (i = 0; i < n_colors; i++) { GimpPaletteEntry *entry = gimp_palette_get_entry (palette, i); diff --git a/app/core/gimpimageundo.c b/app/core/gimpimageundo.c index bf7bbca71e..ec4638f09c 100644 --- a/app/core/gimpimageundo.c +++ b/app/core/gimpimageundo.c @@ -39,6 +39,7 @@ #include "gimpimage-metadata.h" #include "gimpimage-private.h" #include "gimpimageundo.h" +#include "gimppalette.h" enum @@ -182,7 +183,13 @@ gimp_image_undo_constructed (GObject *object) break; case GIMP_UNDO_IMAGE_COLORMAP: - image_undo->colormap = _gimp_image_get_colormap (image, &image_undo->num_colors); + { + GimpPalette *palette; + + palette = gimp_image_get_colormap_palette (image); + image_undo->colormap = GIMP_PALETTE (gimp_palette_new (NULL, gimp_object_get_name (palette))); + gimp_data_copy (GIMP_DATA (image_undo->colormap), GIMP_DATA (palette)); + } break; case GIMP_UNDO_IMAGE_HIDDEN_PROFILE: @@ -448,22 +455,26 @@ gimp_image_undo_pop (GimpUndo *undo, case GIMP_UNDO_IMAGE_COLORMAP: { - guchar *colormap; - gint num_colors; + GimpPalette *colormap; - colormap = _gimp_image_get_colormap (image, &num_colors); + colormap = gimp_image_get_colormap_palette (image); + if (colormap) + { + GimpPalette *palette = colormap; + + colormap = GIMP_PALETTE (gimp_palette_new (NULL, gimp_object_get_name (palette))); + gimp_data_copy (GIMP_DATA (colormap), GIMP_DATA (palette)); + } if (image_undo->colormap) - _gimp_image_set_colormap (image, image_undo->colormap, - image_undo->num_colors, FALSE); + gimp_image_set_colormap_palette (image, image_undo->colormap, FALSE); else gimp_image_unset_colormap (image, FALSE); if (image_undo->colormap) - g_free (image_undo->colormap); + g_object_unref (image_undo->colormap); - image_undo->num_colors = num_colors; - image_undo->colormap = colormap; + image_undo->colormap = colormap; } break; @@ -522,7 +533,7 @@ gimp_image_undo_free (GimpUndo *undo, GimpImageUndo *image_undo = GIMP_IMAGE_UNDO (undo); g_clear_object (&image_undo->grid); - g_clear_pointer (&image_undo->colormap, g_free); + g_clear_object (&image_undo->colormap); g_clear_object (&image_undo->hidden_profile); g_clear_object (&image_undo->metadata); g_clear_pointer (&image_undo->parasite_name, g_free); diff --git a/app/core/gimpimageundo.h b/app/core/gimpimageundo.h index a3144e1428..08f7ef8e08 100644 --- a/app/core/gimpimageundo.h +++ b/app/core/gimpimageundo.h @@ -50,7 +50,7 @@ struct _GimpImageUndo GimpUnit *resolution_unit; GimpGrid *grid; gint num_colors; - guchar *colormap; + GimpPalette *colormap; GimpColorProfile *hidden_profile; GimpMetadata *metadata; gchar *parasite_name; diff --git a/app/core/gimppalette-import.c b/app/core/gimppalette-import.c index 3bd6f3fd50..5362b0d44b 100644 --- a/app/core/gimppalette-import.c +++ b/app/core/gimppalette-import.c @@ -253,7 +253,7 @@ gimp_palette_import_make_palette (GHashTable *table, if (! table) return palette; - gimp_palette_restrict_format (palette, format); + gimp_palette_restrict_format (palette, format, FALSE); g_hash_table_foreach (table, gimp_palette_import_create_list, &list); list = g_slist_sort (list, gimp_palette_import_sort_colors); @@ -275,7 +275,7 @@ gimp_palette_import_make_palette (GHashTable *table, g_slist_free (list); - gimp_palette_restrict_format (palette, NULL); + gimp_palette_restrict_format (palette, NULL, FALSE); return palette; } diff --git a/app/core/gimppalette.c b/app/core/gimppalette.c index 33eda09994..240eb76c3e 100644 --- a/app/core/gimppalette.c +++ b/app/core/gimppalette.c @@ -363,11 +363,18 @@ gimp_palette_copy (GimpData *data, palette->n_colors = 0; palette->n_columns = src_palette->n_columns; + palette->format = src_palette->format; + /* Note: we don't copy the image variable on purpose (same as we don't + * copy the one from the parent GimpData, nor the file), and we don't + * want to keep syncing our format with changes in this image. + */ for (list = src_palette->colors; list; list = g_list_next (list)) { GimpPaletteEntry *entry = list->data; + /* Small validity check. */ + g_return_if_fail (! palette->format || palette->format == gegl_color_get_format (entry->color)); gimp_palette_add_entry (palette, -1, entry->name, entry->color); } @@ -426,7 +433,7 @@ gimp_palette_image_space_updated (GimpImage *image, space = gimp_image_get_layer_space (image); format = gimp_babl_format (GIMP_RGB, gimp_image_get_precision (image), FALSE, space); - gimp_palette_restrict_format (palette, format); + gimp_palette_restrict_format (palette, format, FALSE); } static void @@ -465,7 +472,7 @@ gimp_palette_notify_image (GimpPalette *palette, } else { - gimp_palette_restrict_format (palette, NULL); + gimp_palette_restrict_format (palette, NULL, FALSE); } } @@ -474,7 +481,8 @@ gimp_palette_notify_image (GimpPalette *palette, void gimp_palette_restrict_format (GimpPalette *palette, - const Babl *format) + const Babl *format, + gboolean push_undo_if_image) { gint n_colors; @@ -483,6 +491,13 @@ gimp_palette_restrict_format (GimpPalette *palette, if (palette->format == format) return; + if (push_undo_if_image && gimp_data_get_image (GIMP_DATA (palette))) + gimp_image_undo_push_image_colormap (gimp_data_get_image (GIMP_DATA (palette)), + /* TODO: use localized string + * after string freeze. + */ + /*C_("undo-type", "Change Colormap format restriction"));*/ + "Change Colormap format restriction"); palette->format = format; if (palette->format == NULL) diff --git a/app/core/gimppalette.h b/app/core/gimppalette.h index b8e2efac8d..e8f2660437 100644 --- a/app/core/gimppalette.h +++ b/app/core/gimppalette.h @@ -77,7 +77,8 @@ GimpData * gimp_palette_new (GimpContext *context, GimpData * gimp_palette_get_standard (GimpContext *context); void gimp_palette_restrict_format (GimpPalette *palette, - const Babl *format); + const Babl *format, + gboolean push_undo_if_image); const Babl * gimp_palette_get_restriction (GimpPalette *palette); GList * gimp_palette_get_colors (GimpPalette *palette);