The invasion extended to some core widgets too, in particular GimpColorPanel (a
subclass of GimpColorButton). There was quite a lot of code depending on these
widgets.
We pass 2 GeglColor through the wire now. Since it is passed very early
(when sharing the configuration), I had some issues with initialization
order of GEGL, and in particular when calling gegl_init() before
gegl_config() inside _gimp_config(), I had a bunch of such criticals:
> Plugin script-fu: GLib-GObject: CRITICAL: Two different plugins tried to register 'GeglOpPlugIn-transform-core'
Anyway in the end, I store the passed colors as raw bytes and strings in
the GPConfig object, and re-construct the GeglColor last minute in
_gimp_config().
- app: gimp_context_get_(foreground|background)() are now returning a GeglColor.
- libgimp: PDB functions named similarly in libgimp are returning a newly
allocated GeglColor too.
- A few other PDB functions (the ones using these functions) were updated and
their signature changed to use GeglColor too, when relevant. Plug-ins which
use any of the changed libgimp functions were fixed.
- GimpContext: signals "(foreground|background)-changed" are now passing a
GeglColor.
- libgimpconfig: new macro GIMP_CONFIG_PROP_COLOR using gegl_param_spec_color().
- GimpContext: properties "foreground" and "background" are now GeglParamColor
properties.
- app: All code interacting with GimpContext objects were updated to receive a
GeglColor (that they may still convert, or no, to GimpRGB for now).
- app: gimp_prop_gegl_color_button_new() was added as an alternative to
gimp_prop_color_button_new() when the property is a GeglParamColor. Eventually
the former should replace completely the latter.
- libgimpwidgets: gimp_prop_color_area_new() now works on GeglParamColor
properties only.
- libgimp: gimp_procedure_dialog_get_widget() will generate a GimpColorArea for
GeglTypeParamColor arguments.
This partially revert some of the changes in commit 652a1b4388 because the
Windows CI suddenly failed because of this (my local build on Linux didn't have
any problem though) with:
> /usr/bin/x86_64-w64-mingw32-ld: libgimp/libgimpui-3.0-0.dll.p/gimpproceduredialog.c.obj: in function `gimp_procedure_dialog_save_defaults':
> /builds/GNOME/gimp/_build/../libgimp/gimpproceduredialog.c:2570:(.text+0x633): undefined reference to `_gimp_procedure_config_save_default'
> /usr/bin/x86_64-w64-mingw32-ld: /builds/GNOME/gimp/_build/../libgimp/gimpproceduredialog.c:2576:(.text+0x644): undefined reference to `_gimp_procedure_config_has_default'
> /usr/bin/x86_64-w64-mingw32-ld: libgimp/libgimpui-3.0-0.dll.p/gimpproceduredialog.c.obj: in function `gimp_procedure_dialog_load_defaults':
> /builds/GNOME/gimp/_build/../libgimp/gimpproceduredialog.c:2549:(.text+0xa2f): undefined reference to `_gimp_procedure_config_load_default'
> /usr/bin/x86_64-w64-mingw32-ld: libgimp/libgimpui-3.0-0.dll.p/gimpproceduredialog.c.obj: in function `gimp_procedure_dialog_constructed':
> /builds/GNOME/gimp/_build/../libgimp/gimpproceduredialog.c:368:(.text+0x11b1): undefined reference to `_gimp_procedure_config_has_default'
This is because these functions are used not only inside libgimp but also
across inside libgimpui. As a consequence, the build fails when linking
libgimpui.
Some of these should not even be visible by libgimp and were just fine as static
as well! For the rest, I make them really private (not only with a private
header).
I am using the same GimpDrawableChooser with an additional drawable_type
argument to only show the appropriate tab if we want to limit what can be
chosen.
None of our plug-ins actually use a GimpLayer or GimpChannel only arg so far,
but if we have some day, or if some third-party plug-ins want to have such arg,
now they quite easily can!
I believe it should not happen in normal GUI case (which is when you create a
GimpProcedureDialog). I had the issue while moving around some plug-in code and
moved dialog creation before gimp_ui_init() by mistake. The issue was not
obvious until I followed the trace inside libgimp. This would be even more
frustrating for plug-in developers so let's have a clear warning message giving
the possible plug-in crash reason.
This name was really irking me because it's not a button (anymore? Maybe it used
to be just a button). Depending on the specific widget, it will have several
sub-widgets, including a label. And it can theoretically even be something else
than a button.
So let's just rename these widgets with the more generic "chooser" name.
- Move the property widget functions for GimpResource properties into a new
libgimp/gimppropwidgets.[ch] file. This mirrors the files
libgimpwidgets/gimppropwidgets.[ch] which are for more generic property types.
- Rename the functions gimp_prop_chooser_*_new() to gimp_prop_*_chooser_new().
- gimp_prop_chooser_factory() doesn't need to be public.
- Add a label to GimpResourceSelectButton, make so that the
gimp_prop_chooser_*_new() functions set the property nick to this label and
add this label to the size group in GimpProcedureDialog.
These will replace the int arguments used in place of enums. The problem of int
arguments used as list of choices is that it makes calling PDB functions very
opaque. This is especially bad when a list is long, so you constantly have to
refer to the documentation to understand what a series of numbers mean in
argument lists.
And the second issue is that plug-in developers have to manually maintain a list
of values both in the GUI and in the documentation string. This help text may
get out-of-sync, may end up with missing values or whatnot. Also if it is used
as tooltips, it makes for very weird tooltips in the graphical interface, with
an overlong technical list of int-values mapping which should ideally only be
made visible in the PDB procedure browser listing.
Now gimp_procedure_dialog_get_label() can work both with an existing property ID
or a new property ID. In the former case, it will simply sync the label with the
procedure argument, which will make it easy to update the label contents. In the
latter case, it just initialize with the provided text.
I was clearly confused when I wrote this. The sinking part matters to take
ownership of a reference in the widgets table, but we don't need to ref widgets
again before inserting them in containers. We were leaking widgets and as a
consequence the config object (and as a second consequence, some objects such as
resources for resource-selection widgets).
This function allows to change the sensitivity of a widget depending on the
value of another property.
We already had gimp_procedure_dialog_set_sensitive() except it was only syncing
with a boolean property, whereas the new function can compare with any property
type.
The GimpProcedureDialog API allows int and double SpinScales. However,
it calls gimp_prop_widget_set_factor () which requires doubles.
A conditional check for a double property was added to this call.
A check was also added to ensure int properties have a factor of 1.0.
… don't include it from public gimpui.h.
As reviewed during !786, if this file is private, the name should show it
clearly. And of course, we must not include it from another public header, since
it won't be installed.
This also fixes building plug-ins with gimptool as reported by tmanni:
e00f2d7f50 (note_1650791)
Simplifies chooser widgets (e.g. GimpBrushSelect) by eliminating attributes (e.g. opacity) of chosen resource.
See #8745, but this commit fixes that by first refactoring the code.
Refactors GUI widgets (e.g. GimpBrushSelectButton and GimpBrushSelect etc.)
Refactor by "Extract class" GimpResourceSelectButton from GimpBrushSelectButton etc.
This moves common code into an inherited class (formerly called GimpSelectButton)
but the subclasses still exist.
The subclasses mainly just do drawing now.
Refactor by "Extract module" GimpResourceSelect from GimpBrushSelect etc.
Moves common code into one file, generic at runtime on type of GimpResource,
that is, the new code dispatches on type i.e. switch statements.
In the future, when core is changed some of that can be deleted.
The files gimpbrushselect.[c,h] etc. are deleted.
The module adapts the API from core to the API of callbacks to libgimp.
Note that core is running the resource chooser (select) widgets remotely.
Core is still calling back over the wire via PDB with more attributes
than necessary.
The new design gets the attributes from the resource themselves,
instead of receiving them from core callback.
The libgimp side adapts by discarding unneeded attributes.
In the future, core (running choosers for plugins) can be simplified also.
Fix gimp_prop_chooser_brush_new same as other resources.
Finish changes, and clean style.
Annotations
So procedures can declare args and GimpProcedureDialog show chooser
widgets
Fix so is no error dialog on id_is_valid for resources
Palette.pdb changes and testing
Memory mgt changes
Gradient pdb
Font and Pattern tests
Test brush, palette
Cleanup, remove generator
Rebase, edit docs, install test-dialog.py
Whitespace, and fix failed distcheck
Fix some clang-format, fix fail distcheck
Fix distcheck
Cleanup from review Jehan
Continuing the changes in #8124, let's have properties labels and blurbs
both localized on plug-in code, i.e. with gettext calls directly in
GIMP_PROC_ARG_*() calls.
Note that it was already the case for blurbs (longer description,
tooltip) as I couldn't find code where we'd localize it further down the
line. But we were running gettext on nicks (shorter description, label)
inside GimpProcedureDialog code. Let's not do this anymore.
This will make the whole localization much more clear and obvious. There
is no "later localized" case anymore. Now let's localize everything
directly when the arguments are created.
… GimpIntStore for value filling.
GimpIntComboBox was not taking ownership of the value store whereas the
newer GimpIntRadioFrame was taking ownership. As a more common practice,
I decided to leave ownership to the caller (which will therefore have
the responsibility to free the data) in the main class and property
widget APIs.
On the other hand, let's steal ownership of the store objects in the
gimp_procedure_dialog_get_int_*() functions as these are really used for
very quick and easy creation of dialogs by script writers. It would even
allow to create a GimpIntStore inline within the widget creation
function, if one wanted to.
… GimpProcedureDialog.
- gimp_prop_file_chooser_button_new() now works also with properties
G_PARAM_SPEC_OBJECT having a value_type == G_TYPE_FILE (additionally
to GIMP_PARAM_SPEC_CONFIG_PATH properties).
- gimp_procedure_dialog_get_widget() will now create a
GtkFileChooserButton in open mode for such a GFile property.
- New gimp_procedure_dialog_get_file_chooser() API to create
GtkFileChooserButton for GFile properties in other modes.
Current limitation: GtkFileChooserButton doesn't have a label. This
should be fixed, probably by creating another custom widget with would
be a labelized file chooser button.
… %GIMP_TYPE_SPIN_SCALE in gimp_procedure_dialog_get_widget().
The dedicated function is for when a plug-in wants to use a scale range
multiplied by a factor. Otherwise using the generic function is fine.
Now using the new GimpLabelColor as new default for RGB properties. It
makes more sense that the default is editable widgets. Also it has a
label, which is better default widget.
Also gimp_procedure_dialog_get_color_widget() now only returns
GimpLabelColor widgets.
It is now discouraged to create constructor-type _new() functions with
custom code, which would make them behave differently that simply
calling g_object_new() with the GType and relevant properties. The main
reason is that some bindings would create objects with g_object_new() so
it should create expected code too.
See: https://gi.readthedocs.io/en/latest/writingbindableapis.html#custom-code-in-constructors
Here it was complicated for a few reasons:
- We hack "title" property to use the procedure's menu label by default
(without the mnemonic). For this, I overrode the GtkWindow property.
- We want "use-header-bar" to follow the application general settings by
default. Since it is a CONSTRUCT_ONLY property, g_object_set() is not
possible in _init() and _constructed(). Instead I had to override the
constructor() method to set this at construction time (yet still allow
API users to override this with the property at creation).
- Similarly, "help-func" is a CONSTRUCT_ONLY property, so I used the
same trick.
- As for "help-id", I now just set it in _constructed(). This was the
easy one.
All objects of subclass GtkWindow are special as they are actually owned
by GTK which keeps a list of all top-level windows. So we cannot
actually give ownership to the caller. With GObject Introspection in
particular, it will mean a double-free of the dialog object (by GTK,
then by the GI layer).
See also gobject-introspection#394.
As explained in GtkSizeGroup docs, all objects inside a size group holds
a reference to it. So once we destroy the last object inside these, it
will be freed too and we should drop the initial reference after adding
the objects.
Only the main size group reference is kept until the end, because we are
adding and removing objects from it regularly, so it is possible that it
is empty again at some intermediary states. Yet we don't want to free it
when this happens.
All the widgets with label inside GimpProcedureDialog have same
GtkSizeGroup (dialog->priv->label_group), which result in wrong sizes of
widget if any of the label is long. In this commit, a new GtkSizeGroup
is made for each of the container, so that labels are aligned but size
of widget in one container do not affect size of widgets in other
containers.
For the widget not belonging to any of the container, default
GtkSizeGroup (dialog->priv->label_group) is used.
Added an option for exporting thumbnail in WebP Export dialogbox.
Additionally, introduced a function gimp_procedure_dialog_fill_expander.
The function is similar to gimp_procedure_dialog_fill_frame but allows
adding GtkExpander instead of GtkFrame.
Though the previous implementation worked fine on C plug-ins, I realized
it was problematic on bindings. In particular, the Python binding at
least was somehow freeing returned floating objects, unless assigned to
a variable.
For instance, the widget returned by the following code:
> dialog.get_color_widget('color', True, GimpUi.ColorAreaType.FLAT)
… was freed by the PyGObject binding when it was floating, even though
(transfer none) was set (hence telling the binding it should not free
the returned object). The workaround was to assign it to some variable,
even though I was not planning to use it.
Making sure all references are full fixes it.
GObject docs also notes:
> **Note**: Floating references are a C convenience API and should not
> be used in modern GObject code. Language bindings in particular find
> the concept highly problematic, as floating references are not
> identifiable through annotations, and neither are deviations from the
> floating reference behavior, like types that inherit from
> GInitiallyUnowned and still return a full reference from
> g_object_new().
… GimpProcedureDialog.
Technically I added:
- New gimp_prop_color_select_new() property widget to create a
GimpColorButton for a given GimpRGB property.
- gimp_procedure_dialog_get_widget() now supports a GimpRGB property and
will create a GimpColorArea by default.
- When the default doesn't suit you, a new function
gimp_procedure_dialog_get_color_widget() allows to create either a
GimpColorArea or a GimpColorButton (editable = TRUE), as well as
choose the area type (small or large checks, as well as flat area,
i.e. no alpha support).
New function to set some procedure dialog's widget sensitive, either
with a constant value, or by binding it to a boolean property (or its
inverse) of a config object.
As the docs says, this was always allowed and would just imply we want a
dialog with all properties in order of declaration.
Yet this usage would output this warning:
> plug-ins/file-fli/fli-gimp.c:976:3: warning: not enough variable arguments to fit a sentinel [-Wformat=]
This commit take care of this warning.
… but only when a menu label was set with
gimp_procedure_set_menu_label(). In such case, this menu label is used
as dialog title (with mnemonic underscore removed).
Jehan