GNOME Bugzilla – Bug 356901
Gfig PDB error
Last modified: 2007-01-05 20:31:06 UTC
Please describe the problem: GIMP Message PDB calling error for procedure 'gimp-brushes-popup': Argument 'opacity' (#3, type gdouble) out of bounds (validation changed '-1.000000' to '0.000000') Steps to reproduce: 1. New Image 2. Filters > Render > Gfig 3. Click the "Browse" button for the brush dialog... Actual results: The described Gimp Message appears Expected results: Brush dialog to recieve focus Does this happen every time? For every brand new image yes, sometimes if you hit ok, close gfig, and then restart gfig (dont close the image) the brush dialog will pop-up correctly Other information: Tested CVS on ubuntu, xubuntu... Tested 2.3.11 on windows XP, SP2 - GTK+ 2.8.18
I can confirm this. The problem is that the function call in gfig-dialog.c looks like: gfig_context->brush_select = gimp_brush_select_button_new ("Brush", gfig_context->default_style.brush_name, -1.0, -1, -1); where the 3rd argument is the initial opacity. My guess is that the value "-1" used to mean "not specified", but in any case, it does not fall into the valid range specified in the pdb arg description for brushes_popup, which is 0 to 100. I'm not sure what the theoretically correct fix is here -- it seems like changing -1.0 to 100.0 will work around the problem and give the correct result.
Negative values used to mean "not specified". Even though this is ugly, we need to restore this API.
Err, ignore that comment. I had a look at the GIMP 2.2 source code and a negative opacity was never allowed and also never documented to have a special meaning. This call probably just silently failed.
2006-09-21 Sven Neumann <sven@gimp.org> * plug-ins/gfig/gfig-dialog.c: pass correct parameters to gimp_brush_select_button_new(). Fixes bug #356901. * plug-ins/script-fu/scripts/test-sphere.scm: use 100.0 as the default opacity for SF-BRUSH.
I also discovered the use of -1's in the call to gimp_brush_select_button_new() as per comment #1. I found the function in libgimp/gimpbrushselectbutton.c. The comment block for that function can be found starting at line 297. The comments clearly state that -1 is a valid value. The comment lines are: * @opacity: Initial opacity. -1 means to use current opacity. * @spacing: Initial spacing. -1 means to use current spacing. * @paint_mode: Initial paint mode. -1 means to use current paint mode. I didn't know where the numeric values for the function are validated or else I would have tried to fix the problem.
This is a new function, so we could fix this by changing the documentation. We could also try to implement the documented behaviour. This would be different to the gimp_brush_select_new() call that gimp_brush_select_button_new() is supposed to replace. But if it makes sense... Does it?
Allowing -1 could be useful in some circumstances. The gimpbrushselectbutton.c file has the routine gimp_brush_select_button_set_brush() which is already set to obtain the current opacity, spacing, and paint mode settings when -1's are passed. Fixing the validation rather than changing the documentation seems the best and simplest solution.
If we decided to change this (but so far nobody has brought up an argument why we should do that), we should change it in the brush select button widget, not by changing the PDB.
Actually, the old brush select widget supported this, but I didn't carry this over properly in the new widget. The gfig and test-sphere changes should be reverted, and the new code fixed. This would not involve a PDB change.
I have reverted the change to the Gfig plug-in. The change to test-sphere.scm was fine nevertheless, it only changed opacity from 1.0 to 100.0. 2006-10-05 Sven Neumann <sven@gimp.org> * plug-ins/gfig/gfig-dialog.c: reverted change for bug #356901. * plug-ins/script-fu/scripts/test-sphere.scm: removed superfluous comment.
We still need to fix gimp_brush_select_button_new().
2007-01-03 Sven Neumann <sven@gimp.org> * libgimp/gimpbrushselectbutton.c (gimp_brush_select_button_new): initialize opacity, spacing and paint mode as described by the API docs. Fixes bug #356901.
*** Bug 393158 has been marked as a duplicate of this bug. ***