After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 693797 - Sample colorize fails when you choose current gradient as target.
Sample colorize fails when you choose current gradient as target.
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: libgimp
2.8.4
Other All
: Normal normal
: 2.8
Assigned To: GIMP Bugs
GIMP Bugs
: 693719 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2013-02-14 14:15 UTC by Lyle Kroll
Modified: 2013-02-17 00:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
screen capture of error. (32.17 KB, image/x-png)
2013-02-14 14:15 UTC, Lyle Kroll
  Details
patch for gimpitemcombobox.c (1.15 KB, patch)
2013-02-15 15:49 UTC, Hartmut Kuhse
none Details | Review
initializing value to value below zero -WRONG- (94.43 KB, patch)
2013-02-15 22:20 UTC, Hartmut Kuhse
none Details | Review
initializing value to value below zero (517 bytes, patch)
2013-02-15 22:46 UTC, Hartmut Kuhse
none Details | Review

Description Lyle Kroll 2013-02-14 14:15:11 UTC
Created attachment 236060 [details]
screen capture of error.

Not sure when this one popped up since I don't use sample colorize that often, but did install 2.8.4 and it shows up there.   Had someone else confirm this too at GIMPChat.  http://gimpchat.com/viewtopic.php?f=4&t=6449

It may be platform specific since someone there says they don't see it in Linux; using Windows 64-bit version win Win7.
Comment 1 Michael Schumacher 2013-02-14 14:21:31 UTC
And this causes a crash, or just this error message?
Comment 2 Lyle Kroll 2013-02-14 14:33:09 UTC
It doesn't crash GIMP; the filter just doesn't execute but displays the error message.   If you choose a target image as the source, Sample Colorize still works properly.   :)
Comment 3 Hartmut Kuhse 2013-02-15 15:49:36 UTC
Created attachment 236258 [details] [review]
patch for gimpitemcombobox.c

Changes callback function gimp_item_combo_box_changed, called, when a new Item is selected.
The layer combo box as in the "sample-colorize" plugin can be extended by own values. These are not valid items, so the check for validity leads to complete erasure of the items with a new proposal of all (valid) layers in the box. This action will also erase the non-item entries of the box.
My suggestion:
Delete only the invalid items from the box.
The programmer of plug-ins must asure, that the ID-value of appended entries are below zero (as in the plugin "sample-colorize", the value for "gradient" is -444 and for "inverted gradient" is -445). Those entries will not be tested for validity but will be still present after deleting invalid items.
Comment 4 Michael Natterer 2013-02-15 17:48:59 UTC
*** Bug 693719 has been marked as a duplicate of this bug. ***
Comment 5 Michael Natterer 2013-02-15 17:56:51 UTC
Thanks Hartmut, fixed in master and gimp-2-8. Changed to patch to
reinitialize all items, because this is currently our only (broken)
way to update plug-in combo boxes when images change.

commit 36cf018f8bc302b9f661c50b90b4b1ee19c7e09d
Author: Michael Natterer <mitch@gimp.org>
Date:   Fri Feb 15 18:52:02 2013 +0100

    Bug 693797 - Sample colorize fails when you choose current gradient as target
    
    Based on a patch from Hartmut Kuhse, make sure we don't remove
    custom-added items from GimpItemComboBoxes when repopulating it.
    (cherry picked from commit 73cb32c36ef9ed47046fd98e51af2da1aaf131be)

 libgimp/gimpitemcombobox.c |   24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)
Comment 6 Hartmut Kuhse 2013-02-15 22:20:04 UTC
Created attachment 236314 [details] [review]
initializing value to value below zero -WRONG-

I forgot:
For avoiding further errors, the value for the active item should be initialized with a negative number.
At least in windows, the changed signal callback in gimpitemcombobox is called before the callback in the plugin, so the value can stay in an uninitialized condition.
Comment 7 Hartmut Kuhse 2013-02-15 22:41:44 UTC
My patch works somewhat better...
My patch removes invalid items from the combobox. (This is the situation where the funcion returns without changing "value" so it stays in a uninitialized condition.)
You made that validity test for the situation, that the plugin is open, and the combobox is filled with all available lets say layers. With the plugin open, a layer is deleted from the layer stack. This is not recognized by the plugin, so the already deleted layer is still in the combobox. If that layer is selected, the plugin crashes.
Now the patch does:
I have three layers.
Open sample-colorize.
All layers are in the combobox.
I delete a layer from the layer stack.
I select the just deleted layer in the combobox.

Expected: The layer is removed from the combobox. What now happens to the selection (gimp_int_combo_box_get_active), is undefined.

What actually happens: The layer is not removed from the combobox. The plugin behaves, as there was selected nothing. Selecting the already deleted layer again, the item in the combo box changes to the other, still valid layer, so now there are two identical layers in the combobox. They bahave exactly the same way.
Comment 8 Hartmut Kuhse 2013-02-15 22:46:01 UTC
Created attachment 236317 [details] [review]
initializing value to value below zero
Comment 9 Michael Natterer 2013-02-16 09:19:50 UTC
Are you saying the "changed" signal is being emitted anyway while removing
the layers? That shouldn't happen, but I didn't test it :) Will look
this afternoon.
Comment 10 Hartmut Kuhse 2013-02-16 11:37:19 UTC
No. This doesn't happen.
Butt it seems, that not all entries are removed from the combobox.
I'm still testing, but I have some troubles with my developing environment...
Comment 11 Michael Natterer 2013-02-17 00:37:58 UTC
Argh, I have no clue what I tested, but what I pushed is entirely broken
and removes nothing from the store, will fix.
Comment 12 Michael Natterer 2013-02-17 00:49:25 UTC
Proper fix this time, master and gimp-2-8:

commit 76452a01aae5f8c0639f3a758002d72abb320845
Author: Michael Natterer <mitch@gimp.org>
Date:   Sun Feb 17 01:45:20 2013 +0100

    Bug 693797 - Sample colorize fails when you choose current gradient as target
    
    Fix my last commit and don't delete items in the foreach() callback,
    because it's impossible to delete items in foreach(). Instead, collect
    them in a list and remove them after foreach().
    (cherry picked from commit c91fbd54ad04d9b4e87690a794df5f15d9e8d0be)

 libgimp/gimpitemcombobox.c |   15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)