gimp_procedure_new_return_values() takes ownership of the passed GError (it
allows, among other things, to call it directly as return value). So we must not
try and free it afterwards.
The gimp_procedure_run() already existed, though it was with an ordered
GimpValueArray array of arguments. Its usage feels redundant to the series of
gimp_pdb_run_procedure*() functions (which is confusing), but
gimp_procedure_run() was actually a bit more generic, because it does not
necessarily calls GimpProcedure-s through the PDB! For instance, it can runs a
local GimpProcedure, such as the case of one procedure which would want to call
another procedure in the same plug-in, but without having to go through PDB. Of
course, for local code, you may as well run relevant functions directly, yet it
makes sense that if one of the redundant-looking function is removed, it should
be the more specific one. Also gimp_procedure_run() feels a lot simpler and
logical, API wise.
A main difference in usage is that now, plug-in developers have to first
explicitly look up the GimpPdbProcedure with gimp_pdb_lookup_procedure() when
they wish to call PDB procedures on the wire. This was done anyway in the
gimp_pdb_run_procedure*() code, now it's explicit (rather than calling by name
directly).
Concretely:
* gimp_pdb_run_procedure(), gimp_pdb_run_procedure_config() and
gimp_pdb_run_procedure_valist() are removed.
* gimp_procedure_run() API is modified to use a variable args list instead of a
GimpValueArray.
* gimp_procedure_run_config() and gimp_procedure_run_valist() are added.
* gimp_procedure_run_config() in particular will be the one used in bindings
which don't have variable args support through a (rename-to
gimp_procedure_run) annotation.
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).
We cannot be 100% sure generically (i.e. for all possible bindings available
with GObject Introspection) if bindings add their own reference to objects or
not. Clearly we have cases when they always do (Lua, Javascript), cases when
they do only in certain conditions (global Python variables) and cases when they
don't (Vala). What we know for sure is that in these script languages,
developers don't manually manage memory anyway. So the additional reference is
not their fact.
So let's just maintain a list of automatic memory managed binding languages,
among the few we officially support (i.e. the ones for which we have working
test plug-ins) and verify by executable extension if the plug-in is written in
one of these.
Both keeping a manually-updated list and verifying by extension are not so
pretty solution, but for now it will do.
This will be useful for plug-in developers but also for us. Seeing we leak the
config object is often a good indication that something is wrong in our handling
of internal references (since everything relies on the config object in plug-ins
now, in particular all the GUI).
Otherwise we will always try to reuse previous values or use the default,
bypassing the actual passed values.
I encountered this issue while porting file-glob and realizing that the
"pattern" argument was always passed to NULL, ignoring the explicitly set
pattern.
When a procedure has no run-mode argument, we should simply not assume anything
and use the passed arguments (which is what the non-interactive mode does).
This new function is meant to replace gimp_procedure_new() when all plug-in
usage will have been switched.
This function creates the GimpProcedureConfig object on behalf of the plug-in
and calls gimp_procedure_config_begin_run() and gimp_procedure_config_end_run().
This way we ensure that all plug-in calls with successful result are properly
stored without asking the developer not to forget to call these (if a "good
practice" is in fact something we request to do every time, especially for good
user experience, we might as well make it rather a core process).
Advantages:
* Better interactive experience: using any plug-in will result in saved
previously used settings.
* for developers, working on config objects is also much more comfortable than
working on GValueArray;
* step forward for the future macro infrastructure: if we can ensure that all
plug-in calls are properly logged, then we can replay plug-in actions, in
NON_INTERACTIVE with the same settings.
Looking further, the @help is only used in gimp_proc_view_new() so far (for the
Procedure Browser) where the blurb and argument descriptions are already
localized. It makes no sense to only keep this in English. So let's ask to have
both arguments translated.
Now clearly we should not ask for @help to be mandatory. Very often, it makes no
sense to have a longer help string (the small blurb and the few arguments may be
very self-explanatory). So I make this argument nullable.
There is only the @help_id which I wonder if we could not have a simpler
function gimp_procedure_set_documentation_uri(). Indeed while having a unified
infrastructure with a XML summary and help IDs and whatelse makes sense for GIMP
as a whole, I think that many third-party plug-ins would work much better with a
very simple direct URL. Or it could even be a GFile to a local file (for
plug-ins which want to embed their documentation in the plug-in folder for
instance). To be continued…
Much like for images and items. Change the PDB to transmit IDs
instead of names for brush, pattern etc. and refactor a whole
lot of libgimp code to deal with it.
modified: libgimp/gimpplugin-private.h
GLib has a specific type for byte arrays: `GBytes` (and it's underlying
GType `G_TYPE_BYTES`).
By using this type, we can avoid having a `GimpUint8Array` which is a
bit cumbersome to use for both the C API, as well as bindings. By using
`GBytes`, we allow other languages to pass on byte arrays as they are
used to, while the bindings will make sure to do the right thing.
In the end, it makes the API a little bit simpler for everyone, and
reduces confusion for people who are used to working with byte arrays
in other C/GLib based code (and not having 2 different types to denote
the same thing).
Related: https://gitlab.gnome.org/GNOME/gimp/-/issues/5919
This was the last remaining bit in #8124. Basically I needed to check how
localization of menu paths worked. I was thinking of maybe have 2 arguments to
gimp_procedure_add_menu_path(), one non-localized (for default menu paths) and
one localized by the plug-in (for custom menus). That would break all plug-ins,
but also looking at our code, it's complicated to do right.
Instead let's just keep current API and add an example in function docs. We'll
see how we can improve the API if the very hypothetical problem I am foreseeing
actually happens some day: say a word in English translates to e.g. "Filters" in
some other language, whereas English "Filters" translates to yet another term;
in such case, this new menu would still merge with the default /Filters/ menu
when localized in this language, so we'd have the weird situation where the
custom menu label would have passed through 2 translations somehow.
But let's see how it goes. If we really need, in the future, we can deprecate
gimp_procedure_add_menu_path() and add a gimp_procedure_add_menu_paths() with a
base_path and a custom_path, while the custom_path would be expected to be
already translated.
GLib has a specific type of NULL-terminated string arrays:
`G_TYPE_STRV`, which is the `GType` of `char**` aka `GStrv`.
By using this type, we can avoid having a `GimpStringArray` which is a
bit cumbersome to use for both the C API, as well as bindings. By using
`GStrv`, we allow other languages to pass on string lists as they are
used to, while the bindings will make sure to do the right thing.
In the end, it makes the API a little bit simpler for everyone, and
reduces confusion for people who are used to working with string arrays
in other C/GLib based code (and not having 2 different types to denote
the same thing).
Related: https://gitlab.gnome.org/GNOME/gimp/-/issues/5919
gtk-doc has been slowly dying for the past few years; with gi-docgen we
have a nice successor.
This also makes sure the C documentation also uses the GIR file, which
in turn means faster build times (since all the C code doesn't have to
be parsed and recompiled again), and has a clear dependency graph.
See the [gi-docgen tutorial] for more info on how the system works.
[gi-docgen tutorial]: https://gnome.pages.gitlab.gnome.org/gi-docgen/tutorial.html
Fixes the patch from !470 which is mostly right, except that
g_param_spec_sink() may possibly lead to finalizing the GParamSpec
(typically if it was a just-created floating spec). We don't want to
return pointer to freed data. Let's return NULL instead.
Also looking closer at the memory handling here, it looks the right
annotation for @pspec is (transfer floating). Basically we are sinking a
floating object into a full object and taking ownership of this sunk
object. But if the object was already sunk, we are reffing it and
keeping this additional reference, not the passed argument's. Hopefully
it's right since the annotation and handling of floating object with
GObject Introspection seems very unclear to me (even in core GObject
code, I see what looks like contradictory annotations).
In the normal flow, pspec is persisted in the arguments array, and is
g_param_spec_ref_sink()'d in order to sink a possible floating ref. To
avoid a leak in the error case, we need to add some g_param_spec_sink().
When GIMP_PROCEDURE_SENSITIVE_NO_IMAGE is set on a GimpImageProcedure,
add GIMP_PARAM_NO_VALIDATE to the param spec flags, allowing to pass a
NULL image.
The new function gimp_procedure_set_sensitivity_mask() allows plug-ins
to tell when a procedure should be marked as sensitive or not.
gimp_procedure_get_sensitivity_mask() retrieves this information.
Currently plug-ins are automatically marked as sensitive when an image
is present and a single drawable is selected. Nowadays, we can have
multiple selected layers so we should allow plug-ins to tell us if they
support working on multiple drawables. Actually we could even imagine
new plug-ins which would be made to work only on multiple drawables.
Oppositely, there are a lot of plug-ins which don't care at all if any
drawable is selected at all (so we should allow no drawable selected).
Finally why not even imagine plug-ins which don't care if no image is
shown? E.g. plug-ins to create new images or whatnot. This new API
allows our core to know all this and show procedure sensitivity
accordingly. By default, when the function is not called, the 1 image
with 1 drawable selected case is the default, allowing existing plug-ins
easier update.
Note: this only handles the sensitivity part right now. A plug-in which
would advertize working on several layer would still not work, because
the core won't allow sending several layers. It's coming in further
commits.
which can be set to GIMP_ARGUMENT_SYNC_NONE (the default) or
GIMP_ARGUMENT_SYNC_PARASITE, which indicates that the argument should
be synced with an image parasite of the same name.
Add internal GimpProcedureConfig API to load/save "default values"
which are to be treated as if they were the hardcoded GParamSpec
defaults, but user-configurable. Also make all other load/save
functions available to other libgimp files.
In gimp_procedure_run(), if incomplete arguments are passed, don't
just complete them with the GParamSpec defaults, but look up the
user-saved defaults and use them if they exist. This happens before
everything else and brings back the PNG export feature of using
user-saved defaults also in non-interactive mode (but for all
procedures not just PNG export).
In GimpProcedureDialog, add "Load Defaults" and "Save Defaults"
buttons, they are the only way of managing the user-configurable
procedure defaults.
When clicking "Reset", show a popover with the reset options "Initial
Values" and "Factory Defaults".
Aux arguments are arbitrary values the procedure wants managed and
remembered across invocations in GimpProcedureConfig. They are not
passed to run() and are not known to the PDB, they only exist in the
procedure's config object.
and call it gimp_pdb_set_proc_icon(). Change icon registration code in
libgimp/ and app/ so it's now possible to register icons for temporary
procedures.
Turn all ID param specs into object param specs (e.g. GimpParamImageID
becomes GimpParamImage) and convert between IDs and objects in
gimpgpparams.c directly above the the wire protocol, so all of app/,
libgimp/ and plug-ins/ can deal directly with objects down to the
lowest level and not care about IDs.
Use the actual object param specs for procedure arguments and return
values again instead of a plain g_param_spec_object() and bring back
the none_ok parameter.
This implies changing the PDB type checking functions to work on pure
integers instead of IDs (one can't check whether object creation is
possible if performing that check requires the object to already
exist).
For example gimp_foo_is_valid() becomes gimp_foo_id_is_valid() and is
not involved in automatic object creation magic at the protocol
level. Added wrappers which still say gimp_foo_is_valid() and take the
respective objects.
Adapted all code, and it all becomes nicer and less convoluted, even
the generated PDB wrappers in app/ and libgimp/.
Turn GimpPlugIn into the main factory for all proxies and keep the
main hash tables there. The hash tables keep the initial reference.
For each GimpProcedure::run(), have s "sub-factory" which hands out
proxies to the actual procedure code. Each run() has hash tables of
its own which hold additional references. When run() is done, get rid
of its hash tables and their references, *and* drop the main plug-in
reference counts from the global hashes if the proxies' refcount has
dropped to one.
Jehan