Commit d1e3d7c5 authored by Michael Natterer's avatar Michael Natterer 😴
Browse files

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.
parent cecdbb77
......@@ -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
......
......@@ -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));
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment