[gimp] Bug 731279 - Tool Preset Editor not working correctly

[Michael Natterer <mitch src gnome org>]

commit d1e3d7c5c7615e080948bd8da00de6a50d8a0d6e
Author: Michael Natterer <mitch gimp org>
Date:   Wed Oct 12 20:12:08 2016 +0200

    Bug 731279 - Tool Preset Editor not working correctly
    
    This fixes restoring of brush properties (size, spacing angle etc.)
    from presets, which was utterly broken before. The fix consists of
    two parts:
    
    - In tool_manager_preset_changed(), always copy the brush properties
      again after setting the preview on the tool options, in order to
      override brush properites that get copied from a linked brush when
      that brush gets set on the tool options.
    
    But no amount of copying stuff again and again would help without:
    
    - In gimp_context_set_by_type(), don't use g_object_set() to set the
      object (brush, pattern etc.). Instead, build a GValue and call
      gimp_context_set_property(). This may seem odd, but avoids a
      g_object_freeze_notify()/thaw_notify() around the g_object_set(),
      which was causing "notify" to be emitted at the very end, after
      everything this context change has triggered. GimpContext is an
      essential core object and there is an expectation of a reasonable
      order of signal emissions and callbacks being called. The "notify"
      at the end was keeping any callbacks of the context's "foo-changed"
      signals to override anything an earlier callback had done, if a
      "notify" callback was overriding that overriding again.
    
    This was probably the reason for a lot of odd behavior observed over
    the years. In fact, I have been searching for this for at least 5
    years.

 app/core/gimpcontext.c   |   27 +++++++++++++++++++++++----
 app/tools/tool_manager.c |   20 ++++++++++----------
 2 files changed, 33 insertions(+), 14 deletions(-)
---
diff --git a/app/core/gimpcontext.c b/app/core/gimpcontext.c
index 0923d39..af5727d 100644
--- a/app/core/gimpcontext.c
+++ b/app/core/gimpcontext.c
@@ -1940,7 +1940,9 @@ gimp_context_set_by_type (GimpContext *context,
                           GType        type,
                           GimpObject  *object)
 {
-  GimpContextPropType prop;
+  GimpContextPropType  prop;
+  GParamSpec          *pspec;
+  GValue               value = G_VALUE_INIT;
 
   g_return_if_fail (GIMP_IS_CONTEXT (context));
 
@@ -1948,9 +1950,26 @@ gimp_context_set_by_type (GimpContext *context,
 
   g_return_if_fail (prop != -1);
 
-  g_object_set (context,
-                gimp_context_prop_names[prop], object,
-                NULL);
+  pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (context),
+                                        gimp_context_prop_names[prop]);
+
+  g_return_if_fail (pspec != NULL);
+
+  g_value_init (&value, pspec->value_type);
+  g_value_set_object (&value, object);
+
+  /*  we use gimp_context_set_property() (which in turn only calls
+   *  gimp_context_set_foo() functions) instead of the much more obvious
+   *  g_object_set(); this avoids g_object_freeze_notify()/thaw_notify()
+   *  around the g_object_set() and makes GimpContext callbacks being
+   *  called in a much more predictable order. See bug #731279.
+   */
+  gimp_context_set_property (G_OBJECT (context),
+                             pspec->param_id,
+                             (const GValue *) &value,
+                             pspec);
+
+  g_value_unset (&value);
 }
 
 void
diff --git a/app/tools/tool_manager.c b/app/tools/tool_manager.c
index d11010d..39c6887 100644
--- a/app/tools/tool_manager.c
+++ b/app/tools/tool_manager.c
@@ -115,7 +115,7 @@ tool_manager_init (Gimp *gimp)
 
   tool_manager->shared_paint_options = g_object_new (GIMP_TYPE_PAINT_OPTIONS,
                                                      "gimp", gimp,
-                                                     "name", "tmp",
+                                                     "name", "tool-manager-shared-paint-options",
                                                      NULL);
 
   g_signal_connect (user_context, "tool-changed",
@@ -756,23 +756,23 @@ tool_manager_preset_changed (GimpContext     *user_context,
 
   if (GIMP_IS_PAINT_OPTIONS (preset->tool_options))
     {
-      GimpCoreConfig  *config = user_context->gimp->config;
-      GimpToolOptions *src    = preset->tool_options;
-      GimpToolOptions *dest   = tool_manager->active_tool->tool_info->tool_options;
+      GimpToolOptions *src  = preset->tool_options;
+      GimpToolOptions *dest = tool_manager->active_tool->tool_info->tool_options;
 
-      /* if connect_options() did overwrite the brush options and the
-       * preset contains a brush, use the brush options from the
-       * preset
+      /*  copy various data objects' additional tool options again
+       *  manually, they might have been overwritten by e.g. the "link
+       *  brush stuff to brush defaults" logic in
+       *  gimptooloptions-gui.c
        */
-      if (config->global_brush && preset->use_brush)
+      if (preset->use_brush)
         gimp_paint_options_copy_brush_props (GIMP_PAINT_OPTIONS (src),
                                              GIMP_PAINT_OPTIONS (dest));
 
-      if (config->global_dynamics && preset->use_dynamics)
+      if (preset->use_dynamics)
         gimp_paint_options_copy_dynamics_props (GIMP_PAINT_OPTIONS (src),
                                                 GIMP_PAINT_OPTIONS (dest));
 
-      if (config->global_gradient && preset->use_gradient)
+      if (preset->use_gradient)
         gimp_paint_options_copy_gradient_props (GIMP_PAINT_OPTIONS (src),
                                                 GIMP_PAINT_OPTIONS (dest));
     }