It is theoretically possible for an id to be reused. Normally here this
should not happen, since the previous item would have been removed from
the hash table anyway, when it got destroyed. Let's still be thorough.
This means that images' ownership is not given to caller in particular.
libgimp will now keep a reference of all GimpImage-s it creates and
return this same reference if called again. It also means that you can
now compare images by pointer comparison (as 2 GimpImage objects
representing the same image ID will be equal).
Obviously as a side effect, gimp_image_list() is changed to (transfer
container) as you must only free the container now, not the elements.
Also various other functions creating new images are now (transfer none)
too.
Long-time plug-ins will have to be taken in consideration in a further
step (we currently never free GimpImage for destroyed images in
particular).
I.e.: gimp_image_get_(layers|channels|vectors)(), gimp_image_list() and
gimp_item_get_children().
Instead of returning an array of IDs, these will now return a GList with
the right objects ready to use.
No need of is_id_arg() anymore in pdb/lib.pl. Let's reuse the {id}
value. Also I had to add an additional trick for GimpDisplay which we
will now generate as such in libgimp PDB files, but still need to show
as GimpObject on app/pdb/.
As previously, only the new classes and the PDB generation for a first
step.
I did the same trick with GIMP_DEPRECATED_REPLACE_NEW_API macro, apart
for some minor widget API to preview drawable, which I will fix right
away in our plug-ins.
Though it is still possible to use an image ID as procedure parameter,
it is now possible to pass a GimpImage GParamSpecObject.
Over the wire, this will transform back and forth into a GimpImageID,
totally transparently for the plug-in which will only always get a
GimpImage.
Same as previous commit: by default the new API will be used. But if a
plug-in builds with GIMP_DEPRECATED_REPLACE_NEW_API macro, then the same
function names will call the old API with ids.
By default the new API will be used. But if we build with
GIMP_DEPRECATED_REPLACE_NEW_API macro, then the same function names will
call the old API with ids.
This way, we don't have to update all our plug-ins at once (which I
tried and is very tedious work).
Note that bindings won't have access to the deprecated API at all.
This means that all functions which were returning or taking as
parameter an image id (as gint32) are now taking a GimpImage object
instead.
The PDB is still passing around an id only over the wire. But we create
an object for plug-ins to work on.
This is quite a huge API break, but is probably the best bet for the
future quality. It will make nicer API instrospection (and nicer API in
binding), will fix the issues with pspec on GimpImageID in Python
bindings (which makes the current Python API unusable as soon as we need
to work on images, which is most of our plug-ins!), etc.
Also it will allow to use signals on images, which will be a great asset
when we will finally have bi-directionnal communications (i.e. plug-ins
would be able to connect to image changes, destructions, and whatnot).
that make dealing with value arrays easier and shorter, e.g.
g_value_get_boolean (gimp_value_array_index (args, n)):
becomes:
GIMP_VALUES_GET_BOOLEAN (args, n);
It's just too weird to be public. Remove its properties from the wire
protocol and from pluginrc. Instead, have all GParamSpecs' flags on
the wire and in pluginrc, so we can use stuff like
GIMP_PARAM_NO_VALIDATE.
Port the remaining few places to GIMP_PROC_ARG_STRING().
I'm sure something is broken now wrt UTF-8 validation,
will add tighter checks in the next commit.
And always pass URIs to all file procedures, the ones what didn't
register as "handles remove" will only ever get local file:// URIs.
Change all file plug-ins (also legacy ones) to expect URIs instead
of filenames, and convert to local paths in the plug-in.
The wire protocol should now be almost 100% clean of non-UTF-8 strings.
which looks much like gimpconfig-params.h and contains macros
(e.g. GIMP_PROC_ARG_BOOLEAN() and GIMP_PROC_VAL_BOOLEAN()) for all
GimpProcedure argument and return value types supported by the
protocol, and makes the boilerplate of setting up a procedure more
readable and much less indented.
This file is C-only and not introspected.
Adding menu paths must be possible even after the procedure has been
installed, script-fu registers all menu paths afer installing its
procedures so they are properly sorted.
Implementing a shiny new GimpFileProcedure doesn't mean the libraries
we use to load/save files are automatically capable of handling remote
files, so we still need this flag.
which are GimpProcedure subclasses with API to register as load/save
handlers and their own kind of run functions that get their standard
arguments passed directly instead of packed into a GimpValueArray.
They also register their standard arguments themselves, which removes
quite some boilerplate from load/save plug-ins.
Remove gimpprocedure-private.[ch] because install() and uninstall()
are now virtual functions of GimpProcedure.
While doing so, better break the various source categories in libgimp/.
Otherwise we were duplicating some of the header list, hence we forgot
to install some of the headers.
The dialog was still calling the old API gimp_pdb_query() everywhere,
which made it fail to run when called with the new API (e.g. from the
Python console or directly from an action) whereas it worked well when
called from the old API (e.g. from the Script-fu console). By testing
the existence of the GimpPDB singleton, we can have this dialog work in
both cases.
because they are deprecated.
Change GIMP_ICON_TYPE_INLINE_PIXBUF to GIMP_ICON_TYPE_PIXBUF and the
libgimp API to (icon-name, GdkPixbuf, GFile). Use the file's uri and a
PNG blob of the pixbuf to pass around on the wire and for storage in
pluginrc.
gimp_procedure_set_icon() and gimp_procedure_get_icon() are not very
nice functions for bindings. They are still usable, but in most
bindings, the data parameter/returned value would end up like a uint
list which you'd want to convert to a string in the icon name or file
path case. It's still possible but very cumbersome.
Instead, I skip both functions for bindings and create specific
gimp_procedure_set_icon_*() and gimp_procedure_get_icon_*() functions,
which are much more binding-friendly.
Autotools were not able to recognize that libgimp-@GIMP_API_VERSION@.la
and ../libgimp/libgimp-@GIMP_API_VERSION@.la were the same dependency.
Hence with multi-thread builds, GI scanner was not waiting for the lib
to be totally built then failed randomly. Make the path recognizable,
and do the same for libgimpui.
For other libgimp*, I still need to keep a more complicated relative
path, but since libgimp/ is the last processed folder, AFAIK it won't
make problems.
libgimp is anyway processed at the very end after all other libgimp*
were built. This way, it also fixes#3746, by removing the $(top_srcdir)
everywhere from introspected files, hence making the build work again
with older automake.
So a value array can now we created like this:
array = gimp_value_array_new_from_types (&error_msg,
G_TYPE_STRING, "foo",
G_TYPE_INT, 23,
G_TYPE_NONE);
Change PDB generation to use this, which makes for much nicer code in
the libgimp wrappers, and only set arrays separately instead of all
values.
Pass the help-id specified by the procedure to the core, and use it in
the core if set instead of always using the procedure's name (which
was probably good enough for all eternity, but it's still more
consistent this way).
They only contain private functions and don't need to be installed or
included by gimp_pdb_headers.h.
The PDB generation part is done by adding a "lib_private" variable
that can be set on PDB groups which should not be public API; the rest
is manual Makefile fiddling.
gpointer is not introspectable, and we should always prefer boxed types
over generic pointer types (thanks nielsdg!).
This also fixes gjs (JavaScript) binding of our API which was dropping
our properties even though they didn't need to be handled in JS.
See https://gitlab.gnome.org/GNOME/gjs/issues/266
for procedure arguments. This implies creating a new value array in
gimp_procedure_run() if the passed array is too short, instead of
just appending to the passed array, which was ugly anyway.
Add gimplegacy-private.h to keep it separate from gimp-private.h.
The legacy code could now be removed and GimpPlugIn-ported plug-ins
would not notice.
Move all old wire code to gimplegacy.c and add wire code to
GimpPlugIn, which now talks with the GIMP core all by itself.
Add some more ASSERT_NO_PLUG_IN_EXISTS assertions to gimplegacy.c and
fix new code that was still using legacy API.
Especially as an example of setting a (rename-to) since variable
arguments are not introspectable, hence gimp_pdb_run_procedure() won't
be available for non-C plug-ins.
The idea is that we already have a GimpProcedure object in libgimp
which has name, help, blurb, arguments, return values and everything,
so we really don't need a parallel API to query PDB procedures for
their properties.
- make run() a virtual function of GimpProcedure
- move GIMP_PDB_ERROR to GimpPDB
- GimpPDBProcedure is a trivial subblass which populates
GimpProcedure's members by querying the PDB.
- make "plug-in", "procedure-type" and "name" construct-only
properties of GimpProcedure.
This is all work in progress.
Otherwise, if run in gimp_proc_browser_dialog_new(), this initial search
does not happen when constructing with g_object_new(). This is
especially bad for non-C plug-ins as gimp_proc_browser_dialog_new() is
not introspectable because of variable arguments.
Which return proper GParamSpecs. Incuding some useless testing code in
gimp_procedural_db_proc_info(), to make sure things work, will go away
again soon.
Also generate comments like "Must be freed with g_free()" for all
return values instead of manually and inconsistently having them on
some return values only.
gimp_procedure_run(): fill the value array with default values so a
procedure's run callback always gets a complete value array. No more
number-of-arguments checking in any new-style plug-in.
and make the places who need it dispatch between it and
_gimp_read_expect_msg() depending on whether there is a GimpPlugIn
instance. Also clean up some includes.
Basically this commit makes sure that all return values that are marked
as "Returns:" also have a `(nullable)` annotation if it is mentioned on
the same line that NULL can also be returned.
This will prevent a few problems in GObject-introspection.
Documentation-wise in C, this doesn't matter a lot, but it allows
GObject-Introspection based bindings to use their built-in versions when
they want to render any kind of documentation (for example, docs for
Python plugins can render `%NULL` as `None`).
Also remove several functions which never made it to a GIMP release.
They were really meant as temporary anyway from the start, waiting for
the new API to be usable.
And GimpParam is not boxed anymore. This is made useless by the fact it
is not an introspected type now.
This will be used at least for the Python plug-ins. There is currently a
bug preventing to set a GParamSpec in a binded API.
See pygobject#227.
Please don't revert this. At least it allows to use the new GimpPlugIn
API for Python plug-ins. Anyway even with the old API, I had to add ugly
temporary API for the introspection (which I will now remove as they
never made it to release). So we are trading an ugly situation for a
less ugly one.
We can always remove these 2 new functions before release if we find or
implement better solutions before.
Instead of calling _gimp_close() and returning from both the main
plug-in loop variants, simply only return and call gimp_close() from
where both loops return to, at the end of gimp_main_internal().
Move the legacy loop code to gimplegacy.c. Also remove some more stuff
from gimp.c, old and new are now completely separate apart from one
glitch which wll go next...
The way currently implemented plug-ins are, they are already
NULL-terminating the returned arrays. Since a procedure name cannot be
NULL itself by definition, defining the array length by a terminal NULL
is enough. There is no need to also add a n_procedures parameters which
is just one more possible bug source, even more as we were already
expecting the NULL termination by using g_strfreev() to free the memory.
Anyway a length parameter does not bring any advantage since a plug-in
can still "lie" about its array size (just as it can forget to
NULL-terminate it) and when this happens, the plug-in will segfault.
That's it, it's just a plug-in programming error.
Last but not least, some binding seem to have issues with returned array
setting an (out) parameter as the length. In pygobject at least, the
length parameter doesn't disappear and we end up with this ugly
signature:
> In [3]: Gimp.PlugIn.do_query_procedures.__doc__
> Out[3]: 'query_procedures(self) -> list, n_procedures:int'
See bug report pygobject#352.
To avoid this, we should either set both the array and the length as
(out) parameters or just set the returned array as NULL-terminated
(which is the solution I chose).
Start copying all the actual wire communication to GimpPlugIn, and
move the legacy versions to gimplegacy.c.
This implies having the entire protocol code twice, but without any
if(PLUG_IN) { plug_in_stuff(); } else { legacy_stuff(); }
At the moment it is a wild mixture of old and new, but when finished
the wire code in gimplegacy.c will be entirely separate from the wire
code in GimpPlugIn, which will make it easy to g_assert() that only
one API is used by a plug-in.
which must be called by GIMP_EXTENSION procedures when they are ready
to run their temporary procedures. Move gimp_extension_ack() to
gimpobsolete.[ch].
... gimp_procedure_new_return_values().
When we see how this has been used in help or goat-exercise plug-ins so
far, we clearly see that we expect this function to be used as last call
in a run callback. Well we could save the result, free the error, then
return the result, but it's cumbersome.
So instead let's officially expect gimp_procedure_new_return_values() to
take ownership of the GError (i.e. it will free it in the end).
For this reason, remove the `const`.
The GimpPlugInClass:: prefix is important, otherwise GObject
Introspection tools don't take these docs/annotations into account (and
most methods ended up not introspected at all with annotations).
Also fix some typos/sillinesses (missing colon; useless (out) on Returns
value and useless (element-type) for easily scannable type).
- We were leaking the result of query_procedures(). At least as far as
the example port (help plug-in) shows, it returns a newly allocated
array of strings, NULL-terminated. This needs to be freed by the
calling code.
- Add documentations on GimpPlugIn virtual methods so that people know
what to do with them, and what kind of data to return (like
NULL-terminated array of procedure names for simple freeing).
- Fix _gimp_plug_in_init() which was registering the query procedures
instead of the init ones. For this, I added a `init` boolean parameter
to gimp_plug_in_register().
- Finally check the return value of gimp_plug_in_create_procedure(). It
is possible that a plug-in not properly implemented returns NULL and
we want to avoid the CRITICAL. Still output a warning as this is
likely a plug-in development bug.
Recent commits broke the Python plug-ins again because the old API is
not introspected anymore. Of course, by release of GIMP 3, we should
probably remove this deprecated API from introspection. But first, we
have to figure out how and if the new API can be used in bindings.
This reverts commit 833666d462.
The _pdb files are an implementation detail and we do not want
separate doc sections for them, the conflicts need so be resolved in
another way.
Otherwise we get a few duplicate sections since some of the non-PDB
files are named similarly.
Fix this GObject introspection warning and other similar warnings:
> libgimp/gimp_pdb.c:28: Warning: Gimp: multiple comment blocks
> documenting 'SECTION:gimp:' identifier (already seen at gimp.c:129).
This fixes such warning:
> Warning: gvalue.c:188: cannot initialize GValue with type 'GEnum',
> this type is abstract with regards to GValue use, use a more specific
> (derived) type
And obviously all subsequent errors because we failed to initialize and
set enum values. Such as:
> Warning: g_value_set_enum: assertion 'G_VALUE_HOLDS_ENUM (value)' failed
I have only fixed this in gimp_image_pdb. If enum types are used in
other places with GimpValueArray, we must fix them the same way.
All foo_pdb.c functions in libgimp regenerated. I have reviewed this a
dozen times, but please have a look, there might well be glitches and
our public API is sortof important...
Validation is not the job of a GValue setter, we have the GParamSpec's
value_validate for that. Also, we will use the new GType-based API in
the libgimp PDB wrappers soon, and it's really a bad idea to
implicitly call e.g. gimp_image_is_valid() from the generated
gimp_image_is_valid() wrapper.
which takes and returns GimpValueArrays. This or something similar is
the new central function for running core procedures. Use the new
function from gimp_run_procedure2().
- libgimpbase: change GPParam to transfer all information about the
GValues we use, in the same way done for GPParamDef. GPParam is now
different from GimpParam from libgimp, pointers can't be casted any
longer. The protocol is now completely GimpPDBArgType-free. Remove
gp_params_destroy() from the public API.
- libgimp: add API to convert between an array of GPParams and
GimpValueArray, the latter is now the new official API for dealing
with procedure arguments and return values, GimpParam is cruft (the
wire now talks with GimpPlugIn more directly than with the members
of GimpPlugInInfo, which need additional compat conversions).
- libgimp, app: rename gimpgpparamspecs.[ch] to simply
gimpgpparams.[ch] which is also more accurate because they now
contain GValue functions too. The code that used to live in
app/plug-in/plug-in-params.h is now completely in libgimp.
- app: contains no protocol compat code any longer, the only place
that uses GimpPDBArgType is the PDB query procedure implementation,
which also needs to change.
- app: change some forgotten int32 run-modes to enums.
The `data` property of a GimpParam is a union. Unfortunately setting a
union is not supported by GObject Introspection yet. So I create some
APIs to create GimpParam-s from values. Note that this is temporary API
(i.e. it may be removed before GIMP 3 release) since we likely won't use
this GimpParam type anymore with the new plug-in API. But for now, this
is necessary, at least for testing and porting Python plug-ins.
Also for GimpParam to be actually introspectable, I had to make it a
boxed type, but since no length information is available for various
variants of the type (arrays, whose length information is a separate
parameter), the copy and free functions are basically broken or leaking
respectively for all types requiring a length.
Bottom line: this is ugly and we really need a new introspectable
parameter type. But for now, it allows to start porting some of our
Python plug-ins.
- Change the wire protocol's GPProcInstall to transmit the entire
information needed for constructing all GParamSpecs we use, don't
use GimpPDBArgType in GPProcInstall but an enum private to the wire
protocol plus the GParamSpec's GType name. Bump the wire protocol
version.
- Add gimpgpparamspecs.[ch] in both app/plug-in/ and libgimp/ which
take care of converting between GPParamDef and GParamSpec. They
share code as far as possible.
- Change pluginrc writing and parsing to re-use GPParamDef and the
utility functions from gimpgpparamspecs.
- Remove gimp_pdb_compat_param_spec() from app/pdb/gimp-pdb-compat.[ch],
the entire core uses proper GParamSpecs from the wire protocol now,
the whole file will follow down the drain once we use a GValue
representation on the wire too.
- In gimp_plug_in_handle_proc_install(), change the "run-mode"
parameter to a GParamSpecEnum(GIMP_TYPE_RUN_MODE) (if it is not
already an enum). and change all places in app/ to treat it as an
enum value.
- plug-ins: fix cml-explorer to register correctly, a typo in
"run-mode" was never noticed until now.
- Add gimpgpcompat.[ch] in libgimp to deal with all the transforms
between old-style wire communication and using GParamSpec and
GValue, it contains some functions that are subject to change or
even removal in the next steps.
- Change the libgimp GimpProcedure and GimpPlugIn in many ways to be
able to actually install procedures the new way.
- plug-ins: change goat-exercise to completely use the new GimpPlugIn
and GimpProcedure API, look here to see how plug-ins will look in
the future, of course subject to change until this is finished.
- Next: changing GPParam to transmit all information about a GValue.
Mostly the same code as GimpProcedure in app/pdb/.
Move the "run" function to GimpProcedure. Add API to GimpPlugIn to
list and create procedures, and always keep a list of the plug-ins
procedures around. Still only using the old params and return_vals.
The new way of doing plug-ins:
- subclass GimpPlugIn in your plug-in
- implement its query() and run() methods, run() will move to a
new GimpProcedure class soon
- instead of MAIN(), say GIMP_MAIN(YOUR_PLUG_IN_TYPE)
Instead of keeping around a GimpPlugInInfo struct, libgimp will
create an instance of your plug-in class, keep it around during
the plug-in's lifetime, and call its virtual functions.
... GimpRunProc function type.
In gimp_install_procedure(), make sure that @params and @return_vals are
processed as arrays, otherwise they are unusable.
As for the run procedure, make so that @return_vals and @n_return_vals
are considered as returned values. In Python binding for instance, that
makes these not parameters anymore, but actually returnable by the run
function.
With these changes, I made the first fully functional GI Python plug-in,
which just creates a new image and a display for this image. Still a lot
to improve clearly, but we are on the right track. :-)
This is an alternative way to set up a plug-in callbacks, apart from
setting directly the PlugInInfo struct properties.
The reason is that setting directly the Gimp*Proc properties crashes the
plug-in, when done through the Python GI binding.
It is most likely a bug in Pygobject, unless we need the proper
annotation (which I haven't found yet). See:
https://gitlab.gnome.org/GNOME/pygobject/issues/24#note_564968
Setting these callbacks from the C code works fine though, hence this
new API. It is to be noted that the '(scope async)' is the most
important part in this function annotations. Without these annotations,
the function pointers become invalid at the end of the set_callbacks()
call, hence the plug-in crashes when they are actually called.
Unfortunally I am also notified by ebassi that using (scope) at all (any
of the 3 possible values) is just wrong. An API change will be
necessary. For the time being, I leave this like this, for the sake of
testing further, but we'll need to improve things.
It doesn't really help yet with the problem I encountered allowing to
set and run these Python callbacks from the C code (cf. pygobject#347),
but at least let's improve a bit the documentation.
With GObject introspection, this allows to properly use this function,
otherwise it sees the argv argument as a string (and not an array of
string), which cannot be used properly.
For instance, with Python binding, you can just call it like this:
> Gimp.main (info, sys.argv)
At first I thought these could be different namespaces, but actually
GObject Introspection parses files and can only use (AFAICS) the
namespace actually used in our C function, which is always `gimp_` (and
not `gimpbase_` or whatever.
So make the introspection at the root level, and it will include all
libgimp* libraries in one namespace, same as the C lib anyway. For now
only libgimp and libgimpbase as I am still testing.
Also I move the introspectable sources in their own file in order to
share the list between the library building Makefile and the GI
makefile because I can't find how to pass over variables otherwise.
Jehan