From 3e5fce27cb6a45d315feb3aa9163c32baadb53f0 Mon Sep 17 00:00:00 2001 From: Jehan Date: Sat, 9 Aug 2025 23:26:13 +0200 Subject: [PATCH] app: move logic to know if a link layer is being monitored to GimpLink. Instead of keeping a "modified" property in GimpLinkLayer, we just check if the link is being monitored. It's also a better wording because we may "discard the link information" without actually modifying the layer pixels. Also I now actually shut down the file monitoring process. This can be a bit expensive so when we don't need it, let's really free the GFileMonitor. This commit also fixes scaling of link layer (which got broken along the way) and improves the undo code. Note: I'll probably want to modify the XCF flags, but let's do this in the end, depending on further changes too. --- app/actions/layers-actions.c | 3 +- app/core/gimplink.c | 55 ++++++++++-- app/core/gimplink.h | 5 +- app/core/gimplinklayer.c | 135 ++++++++++++++-------------- app/core/gimplinklayer.h | 20 ++--- app/core/gimplinklayerundo.c | 44 +++++---- app/core/gimplinklayerundo.h | 1 - app/widgets/gimpviewrendererlayer.c | 3 +- 8 files changed, 152 insertions(+), 114 deletions(-) diff --git a/app/actions/layers-actions.c b/app/actions/layers-actions.c index 265aa6e37d..62fe9a598f 100644 --- a/app/actions/layers-actions.c +++ b/app/actions/layers-actions.c @@ -1014,7 +1014,8 @@ layers_actions_update (GimpActionGroup *group, text_layer = gimp_item_is_text_layer (GIMP_ITEM (layer)); vector_layer = gimp_item_is_vector_layer (GIMP_ITEM (layer)); - link_layer = gimp_item_is_link_layer (GIMP_ITEM (layer)); + if (GIMP_IS_LINK_LAYER (layer)) + link_layer = gimp_link_layer_is_monitored (GIMP_LINK_LAYER (layer)); } } diff --git a/app/core/gimplink.c b/app/core/gimplink.c index 170a1b3987..58678cdb65 100644 --- a/app/core/gimplink.c +++ b/app/core/gimplink.c @@ -97,6 +97,7 @@ static gboolean gimp_link_emit_changed (gpointer data); static void gimp_link_update_buffer (GimpLink *link, GimpProgress *progress, GError **error); +static void gimp_link_start_monitoring (GimpLink *link); static gchar * gimp_link_get_relative_path (GimpLink *link, GFile *parent, gint n_back); @@ -361,6 +362,17 @@ gimp_link_update_buffer (GimpLink *link, } } +static void +gimp_link_start_monitoring (GimpLink *link) +{ + link->p->monitor = g_file_monitor_file (link->p->file, + G_FILE_MONITOR_WATCH_HARD_LINKS, + NULL, NULL); + g_signal_connect (link->p->monitor, "changed", + G_CALLBACK (gimp_link_file_changed), + link); +} + /** * gimp_link_get_relative_path: * @link: the image this link is associated with. @@ -505,12 +517,11 @@ gimp_link_duplicate (GimpLink *link) gchar *basename; basename = g_file_get_basename (new_link->p->file); - new_link->p->monitor = g_file_monitor_file (new_link->p->file, G_FILE_MONITOR_NONE, NULL, NULL); - g_signal_connect (new_link->p->monitor, "changed", - G_CALLBACK (gimp_link_file_changed), - new_link); gimp_object_set_name_safe (GIMP_OBJECT (new_link), basename); g_free (basename); + + if (gimp_link_is_monitored (link)) + gimp_link_start_monitoring (new_link); } return new_link; @@ -579,12 +590,10 @@ gimp_link_set_file (GimpLink *link, gchar *basename; basename = g_file_get_basename (link->p->file); - link->p->monitor = g_file_monitor_file (link->p->file, G_FILE_MONITOR_NONE, NULL, NULL); - g_signal_connect (link->p->monitor, "changed", - G_CALLBACK (gimp_link_file_changed), - link); gimp_object_set_name_safe (GIMP_OBJECT (link), basename); g_free (basename); + + gimp_link_start_monitoring (link); } g_object_notify_by_pspec (G_OBJECT (link), link_props[PROP_FILE]); @@ -607,6 +616,33 @@ gimp_link_set_absolute_path (GimpLink *link, link->p->absolute_path = absolute_path; } +void +gimp_link_freeze (GimpLink *link) +{ + g_return_if_fail (GIMP_IS_LINK (link)); + g_return_if_fail (link->p->monitor != NULL); + + g_clear_object (&link->p->monitor); +} + +void +gimp_link_thaw (GimpLink *link) +{ + g_return_if_fail (GIMP_IS_LINK (link)); + g_return_if_fail (G_IS_FILE (link->p->file) && link->p->monitor == NULL); + + gimp_link_update_buffer (link, NULL, NULL); + gimp_link_start_monitoring (link); +} + +gboolean +gimp_link_is_monitored (GimpLink *link) +{ + g_return_val_if_fail (GIMP_IS_LINK (link), FALSE); + + return (link->p->monitor != NULL); +} + gboolean gimp_link_is_broken (GimpLink *link) { @@ -624,6 +660,9 @@ gimp_link_set_size (GimpLink *link, link->p->width = width; link->p->height = height; + + if (link->p->monitor && link->p->is_vector) + gimp_link_update_buffer (link, NULL, NULL); } void diff --git a/app/core/gimplink.h b/app/core/gimplink.h index 168c3c344d..30ba21653a 100644 --- a/app/core/gimplink.h +++ b/app/core/gimplink.h @@ -67,8 +67,11 @@ void gimp_link_set_file (GimpLink *layer, GimpProgress *progress, GError **error); gboolean gimp_link_get_absolute_path (GimpLink *link); -void gimp_link_set_absolute_path (GimpLink *layer, +void gimp_link_set_absolute_path (GimpLink *link, gboolean absolute_path); +void gimp_link_freeze (GimpLink *link); +void gimp_link_thaw (GimpLink *link); +gboolean gimp_link_is_monitored (GimpLink *link); gboolean gimp_link_is_broken (GimpLink *link); diff --git a/app/core/gimplinklayer.c b/app/core/gimplinklayer.c index d0cc44bc12..ebee4f7256 100644 --- a/app/core/gimplinklayer.c +++ b/app/core/gimplinklayer.c @@ -63,14 +63,13 @@ enum PROP_0, PROP_LINK, PROP_AUTO_RENAME, - PROP_MODIFIED, - PROP_SCALED_ONLY + PROP_SCALED_ONLY, + N_PROPS }; struct _GimpLinkLayerPrivate { GimpLink *link; - gboolean modified; gboolean scaled_only; gboolean auto_rename; }; @@ -125,7 +124,6 @@ static void gimp_link_layer_convert_type (GimpLayer *layer, gboolean push_undo, GimpProgress *progress); -static void gimp_link_layer_link_changed (GimpLinkLayer *layer); static gboolean gimp_link_layer_render (GimpLinkLayer *layer); static void gimp_link_layer_set_xcf_flags (GimpLinkLayer *layer, @@ -135,6 +133,8 @@ G_DEFINE_TYPE_WITH_PRIVATE (GimpLinkLayer, gimp_link_layer, GIMP_TYPE_LAYER) #define parent_class gimp_link_layer_parent_class +static GParamSpec *link_layer_props[N_PROPS] = { NULL, }; + static void gimp_link_layer_class_init (GimpLinkLayerClass *klass) @@ -172,29 +172,25 @@ gimp_link_layer_class_init (GimpLinkLayerClass *klass) layer_class->convert_type = gimp_link_layer_convert_type; - g_object_class_install_property (object_class, PROP_LINK, - g_param_spec_object ("link", - NULL, NULL, - GIMP_TYPE_LINK, - GIMP_PARAM_READWRITE)); + link_layer_props[PROP_LINK] = g_param_spec_object ("link", + NULL, NULL, + GIMP_TYPE_LINK, + GIMP_PARAM_READWRITE | + GIMP_PARAM_STATIC_STRINGS); - GIMP_CONFIG_PROP_BOOLEAN (object_class, PROP_AUTO_RENAME, - "auto-rename", - NULL, NULL, - TRUE, - GIMP_PARAM_STATIC_STRINGS); + link_layer_props[PROP_AUTO_RENAME] = g_param_spec_boolean ("auto-rename", + NULL, NULL, + TRUE, + GIMP_PARAM_READWRITE | + GIMP_PARAM_STATIC_STRINGS); - GIMP_CONFIG_PROP_BOOLEAN (object_class, PROP_MODIFIED, - "modified", - NULL, NULL, - FALSE, - GIMP_PARAM_STATIC_STRINGS); + link_layer_props[PROP_SCALED_ONLY] = g_param_spec_boolean ("scaled-only", + NULL, NULL, + FALSE, + GIMP_PARAM_READWRITE | + GIMP_PARAM_STATIC_STRINGS); - GIMP_CONFIG_PROP_BOOLEAN (object_class, PROP_SCALED_ONLY, - "scaled-only", - NULL, NULL, - FALSE, - GIMP_PARAM_STATIC_STRINGS); + g_object_class_install_properties (object_class, N_PROPS, link_layer_props); } static void @@ -230,9 +226,6 @@ gimp_link_layer_get_property (GObject *object, case PROP_AUTO_RENAME: g_value_set_boolean (value, layer->p->auto_rename); break; - case PROP_MODIFIED: - g_value_set_boolean (value, layer->p->modified); - break; case PROP_SCALED_ONLY: g_value_set_boolean (value, layer->p->scaled_only); break; @@ -259,9 +252,6 @@ gimp_link_layer_set_property (GObject *object, case PROP_AUTO_RENAME: layer->p->auto_rename = g_value_get_boolean (value); break; - case PROP_MODIFIED: - layer->p->modified = g_value_get_boolean (value); - break; case PROP_SCALED_ONLY: layer->p->scaled_only = g_value_get_boolean (value); break; @@ -300,23 +290,21 @@ gimp_link_layer_duplicate (GimpItem *item, { GimpLinkLayer *layer = GIMP_LINK_LAYER (item); GimpLinkLayer *new_layer = GIMP_LINK_LAYER (new_item); + GimpLink *link = NULL; if (layer->p->link) { - GimpLink *link = gimp_link_duplicate (layer->p->link); - + link = gimp_link_duplicate (layer->p->link); gimp_link_layer_set_link (new_layer, link, FALSE); - - g_object_unref (link); } gimp_config_sync (G_OBJECT (layer), G_OBJECT (new_layer), 0); - if (layer->p->modified) + if (! link || ! gimp_link_is_monitored (link)) { GeglBuffer *buffer; - gint width; - gint height; + gint width; + gint height; buffer = gimp_drawable_get_buffer (GIMP_DRAWABLE (layer)); width = gegl_buffer_get_width (buffer); @@ -337,8 +325,10 @@ gimp_link_layer_duplicate (GimpItem *item, gimp_drawable_get_buffer (GIMP_DRAWABLE (new_layer)), NULL); } - new_layer->p->modified = layer->p->modified; new_layer->p->scaled_only = layer->p->scaled_only; + new_layer->p->auto_rename = layer->p->auto_rename; + + g_clear_object (&link); } return new_item; @@ -395,7 +385,8 @@ gimp_link_layer_scale (GimpItem *item, if (queue) gimp_object_queue_pop (queue); - if (gimp_link_is_vector (link_layer->p->link) && ! link_layer->p->modified) + if (gimp_link_is_vector (link_layer->p->link) && + gimp_link_is_monitored (link_layer->p->link)) { /* Non-modified vector images are always recomputed from the * source file and therefore are always sharp. @@ -407,7 +398,7 @@ gimp_link_layer_scale (GimpItem *item, { gboolean scaled_only = FALSE; - if (! link_layer->p->modified || link_layer->p->scaled_only) + if (gimp_link_is_monitored (link_layer->p->link) || link_layer->p->scaled_only) { /* Raster images whose only modification are previous scaling * are scaled back from the source file. Though they are still @@ -451,7 +442,7 @@ gimp_link_layer_set_buffer (GimpDrawable *drawable, GimpLinkLayer *layer = GIMP_LINK_LAYER (drawable); GimpImage *image = gimp_item_get_image (GIMP_ITEM (layer)); - if (push_undo && ! layer->p->modified) + if (push_undo && gimp_link_is_monitored (layer->p->link)) gimp_image_undo_group_start (image, GIMP_UNDO_GROUP_DRAWABLE_MOD, undo_desc); @@ -460,11 +451,11 @@ gimp_link_layer_set_buffer (GimpDrawable *drawable, buffer, bounds); - if (push_undo && ! layer->p->modified) + if (push_undo && gimp_link_is_monitored (layer->p->link)) { gimp_image_undo_push_link_layer (image, NULL, layer); - g_object_set (drawable, "modified", TRUE, NULL); + gimp_link_freeze (layer->p->link); g_object_set (drawable, "scaled-only", FALSE, NULL); gimp_image_undo_group_end (image); @@ -482,20 +473,24 @@ gimp_link_layer_push_undo (GimpDrawable *drawable, { GimpLinkLayer *layer = GIMP_LINK_LAYER (drawable); GimpImage *image = gimp_item_get_image (GIMP_ITEM (layer)); + gboolean monitored; - if (! layer->p->modified) - gimp_image_undo_group_start (image, GIMP_UNDO_GROUP_DRAWABLE, undo_desc); + monitored = gimp_link_is_monitored (layer->p->link); + + if (monitored) + { + gimp_image_undo_group_start (image, GIMP_UNDO_GROUP_DRAWABLE, undo_desc); + + gimp_image_undo_push_link_layer (image, NULL, layer); + gimp_link_freeze (layer->p->link); + } GIMP_DRAWABLE_CLASS (parent_class)->push_undo (drawable, undo_desc, buffer, x, y, width, height); - if (! layer->p->modified) + if (monitored) { - gimp_image_undo_push_link_layer (image, NULL, layer); - - g_object_set (drawable, "modified", TRUE, NULL); - gimp_image_undo_group_end (image); } } @@ -515,7 +510,7 @@ gimp_link_layer_convert_type (GimpLayer *layer, GimpImage *image = gimp_item_get_image (GIMP_ITEM (link_layer)); if (! link_layer->p->link || - link_layer->p->modified || + ! gimp_link_is_monitored (link_layer->p->link) || layer_dither_type != GEGL_DITHER_NONE) { GIMP_LAYER_CLASS (parent_class)->convert_type (layer, dest_image, @@ -597,7 +592,7 @@ gimp_link_layer_set_link (GimpLinkLayer *layer, if (layer->p->link) { g_signal_handlers_disconnect_by_func (layer->p->link, - G_CALLBACK (gimp_link_layer_link_changed), + G_CALLBACK (gimp_link_layer_render), layer); } @@ -607,11 +602,11 @@ gimp_link_layer_set_link (GimpLinkLayer *layer, if (link) { g_signal_connect_object (link, "changed", - G_CALLBACK (gimp_link_layer_link_changed), + G_CALLBACK (gimp_link_layer_render), layer, G_CONNECT_SWAPPED); - g_object_set (layer, "modified", FALSE, NULL); - rendered = gimp_link_layer_render (layer); + if (gimp_link_is_monitored (link)) + rendered = gimp_link_layer_render (layer); } g_object_notify (G_OBJECT (layer), "link"); @@ -636,9 +631,12 @@ gimp_link_layer_discard (GimpLinkLayer *layer) gimp_image_undo_push_link_layer (gimp_item_get_image (GIMP_ITEM (layer)), _("Discard Link"), layer); - layer->p->modified = TRUE; + gimp_link_freeze (layer->p->link); + /* Triggers thumbnail update. */ gimp_drawable_update_all (GIMP_DRAWABLE (layer)); + /* Triggers contextual menu update. */ + gimp_image_flush (gimp_item_get_image (GIMP_ITEM (layer))); } void @@ -650,15 +648,17 @@ gimp_link_layer_monitor (GimpLinkLayer *layer) gimp_image_undo_push_link_layer (gimp_item_get_image (GIMP_ITEM (layer)), _("Monitor Link"), layer); - layer->p->modified = FALSE; + gimp_link_thaw (layer->p->link); gimp_link_layer_render (layer); } gboolean -gimp_item_is_link_layer (GimpItem *item) +gimp_link_layer_is_monitored (GimpLinkLayer *layer) { - return (GIMP_IS_LINK_LAYER (item) && - ! GIMP_LINK_LAYER (item)->p->modified); + g_return_val_if_fail (GIMP_IS_LINK_LAYER (layer), FALSE); + + return (GIMP_LINK_LAYER (layer)->p->link && + gimp_link_is_monitored (GIMP_LINK_LAYER (layer)->p->link)); } guint32 @@ -671,7 +671,7 @@ gimp_link_layer_get_xcf_flags (GimpLinkLayer *link_layer) if (! link_layer->p->auto_rename) flags |= LINK_LAYER_XCF_DONT_AUTO_RENAME; - if (link_layer->p->modified) + if (! gimp_link_is_monitored (link_layer->p->link)) flags |= LINK_LAYER_XCF_MODIFIED; return flags; @@ -737,13 +737,6 @@ gimp_link_layer_from_layer (GimpLayer **layer, /* private functions */ -static void -gimp_link_layer_link_changed (GimpLinkLayer *layer) -{ - if (! layer->p->modified) - gimp_link_layer_render (layer); -} - static gboolean gimp_link_layer_render (GimpLinkLayer *layer) { @@ -849,6 +842,10 @@ gimp_link_layer_set_xcf_flags (GimpLinkLayer *layer, g_object_set (layer, "auto-rename", (flags & LINK_LAYER_XCF_DONT_AUTO_RENAME) == 0, - "modified", (flags & LINK_LAYER_XCF_MODIFIED) != 0, NULL); + + if ((flags & LINK_LAYER_XCF_MODIFIED) != 0) + gimp_link_freeze (layer->p->link); + else + gimp_link_thaw (layer->p->link); } diff --git a/app/core/gimplinklayer.h b/app/core/gimplinklayer.h index 379d7959c0..8550a7866b 100644 --- a/app/core/gimplinklayer.h +++ b/app/core/gimplinklayer.h @@ -49,19 +49,19 @@ struct _GimpLinkLayerClass }; -GType gimp_link_layer_get_type (void) G_GNUC_CONST; +GType gimp_link_layer_get_type (void) G_GNUC_CONST; -GimpLayer * gimp_link_layer_new (GimpImage *image, - GimpLink *link); +GimpLayer * gimp_link_layer_new (GimpImage *image, + GimpLink *link); -GimpLink * gimp_link_layer_get_link (GimpLinkLayer *layer); -gboolean gimp_link_layer_set_link (GimpLinkLayer *layer, - GimpLink *link, - gboolean push_undo); +GimpLink * gimp_link_layer_get_link (GimpLinkLayer *layer); +gboolean gimp_link_layer_set_link (GimpLinkLayer *layer, + GimpLink *link, + gboolean push_undo); -void gimp_link_layer_discard (GimpLinkLayer *layer); -void gimp_link_layer_monitor (GimpLinkLayer *layer); -gboolean gimp_item_is_link_layer (GimpItem *item); +void gimp_link_layer_discard (GimpLinkLayer *layer); +void gimp_link_layer_monitor (GimpLinkLayer *layer); +gboolean gimp_link_layer_is_monitored (GimpLinkLayer *layer); /* Only to be used for XCF loading/saving. */ diff --git a/app/core/gimplinklayerundo.c b/app/core/gimplinklayerundo.c index 6e1ca05afc..fbe0fb87f0 100644 --- a/app/core/gimplinklayerundo.c +++ b/app/core/gimplinklayerundo.c @@ -36,12 +36,12 @@ enum { PROP_0, - PROP_PREV_MODIFIED, PROP_PREV_LINK }; static void gimp_link_layer_undo_constructed (GObject *object); +static void gimp_link_layer_undo_finalize (GObject *object); static void gimp_link_layer_undo_set_property (GObject *object, guint property_id, const GValue *value, @@ -72,6 +72,7 @@ gimp_link_layer_undo_class_init (GimpLinkLayerUndoClass *klass) GimpUndoClass *undo_class = GIMP_UNDO_CLASS (klass); object_class->constructed = gimp_link_layer_undo_constructed; + object_class->finalize = gimp_link_layer_undo_finalize; object_class->set_property = gimp_link_layer_undo_set_property; object_class->get_property = gimp_link_layer_undo_get_property; @@ -79,12 +80,6 @@ gimp_link_layer_undo_class_init (GimpLinkLayerUndoClass *klass) undo_class->pop = gimp_link_layer_undo_pop; - g_object_class_install_property (object_class, PROP_PREV_MODIFIED, - g_param_spec_boolean ("prev-modified", NULL, NULL, - TRUE, - GIMP_PARAM_READWRITE | - G_PARAM_CONSTRUCT_ONLY)); - g_object_class_install_property (object_class, PROP_PREV_LINK, g_param_spec_object ("prev-link", NULL, NULL, @@ -103,6 +98,7 @@ gimp_link_layer_undo_constructed (GObject *object) { GimpLinkLayerUndo *undo = GIMP_LINK_LAYER_UNDO (object); GimpLinkLayer *layer; + GimpLink *link; G_OBJECT_CLASS (parent_class)->constructed (object); @@ -110,8 +106,18 @@ gimp_link_layer_undo_constructed (GObject *object) layer = GIMP_LINK_LAYER (GIMP_ITEM_UNDO (undo)->item); - undo->link = gimp_link_layer_get_link (layer); - undo->modified = ! gimp_item_is_link_layer (GIMP_ITEM (layer)); + link = gimp_link_layer_get_link (layer); + undo->link = link ? gimp_link_duplicate (link) : NULL; +} + +static void +gimp_link_layer_undo_finalize (GObject *object) +{ + GimpLinkLayerUndo *undo = GIMP_LINK_LAYER_UNDO (object); + + g_clear_object (&undo->link); + + G_OBJECT_CLASS (parent_class)->finalize (object); } static void @@ -124,11 +130,9 @@ gimp_link_layer_undo_set_property (GObject *object, switch (property_id) { - case PROP_PREV_MODIFIED: - undo->modified = g_value_get_boolean (value); - break; case PROP_PREV_LINK: - undo->link = g_value_get_object (value); + g_clear_object (&undo->link); + undo->link = g_value_get_object (value) ? gimp_link_duplicate (g_value_get_object (value)) : NULL; break; default: @@ -147,9 +151,6 @@ gimp_link_layer_undo_get_property (GObject *object, switch (property_id) { - case PROP_PREV_MODIFIED: - g_value_set_boolean (value, undo->modified); - break; case PROP_PREV_LINK: g_value_set_object (value, undo->link); break; @@ -183,18 +184,15 @@ gimp_link_layer_undo_pop (GimpUndo *undo, GimpLinkLayerUndo *layer_undo = GIMP_LINK_LAYER_UNDO (undo); GimpLinkLayer *layer = GIMP_LINK_LAYER (GIMP_ITEM_UNDO (undo)->item); GimpLink *link; - gboolean modified; GIMP_UNDO_CLASS (parent_class)->pop (undo, undo_mode, accum); - modified = ! gimp_item_is_link_layer (GIMP_ITEM (layer)); - link = gimp_link_layer_get_link (layer); + link = gimp_link_layer_get_link (layer); + link = link ? g_object_ref (link) : NULL; gimp_link_layer_set_link (layer, layer_undo->link, FALSE); - g_object_set (layer, "modified", layer_undo->modified, NULL); - layer_undo->modified = modified; - layer_undo->link = link; + g_clear_object (&layer_undo->link); - gimp_drawable_update_all (GIMP_DRAWABLE (layer)); + layer_undo->link = link; } diff --git a/app/core/gimplinklayerundo.h b/app/core/gimplinklayerundo.h index 7428bce3bf..acad93f1dd 100644 --- a/app/core/gimplinklayerundo.h +++ b/app/core/gimplinklayerundo.h @@ -39,7 +39,6 @@ struct _GimpLinkLayerUndo { GimpItemUndo parent_instance; - gboolean modified; GimpLink *link; }; diff --git a/app/widgets/gimpviewrendererlayer.c b/app/widgets/gimpviewrendererlayer.c index 0552e91812..467476cb0c 100644 --- a/app/widgets/gimpviewrendererlayer.c +++ b/app/widgets/gimpviewrendererlayer.c @@ -74,7 +74,8 @@ gimp_view_renderer_layer_render (GimpViewRenderer *renderer, icon_name = GIMP_ICON_LAYER_FLOATING_SELECTION; } else if (gimp_item_is_text_layer (GIMP_ITEM (renderer->viewable)) || - gimp_item_is_link_layer (GIMP_ITEM (renderer->viewable))) + (GIMP_IS_LINK_LAYER (renderer->viewable) && + gimp_link_layer_is_monitored (GIMP_LINK_LAYER (renderer->viewable)))) { icon_name = gimp_viewable_get_icon_name (renderer->viewable); }