We were already considering the plugins under /common but
not the plug-ins that have their own subdirectories. So,
plugin_executables did not match custom_plugin_targets and
the build started to fail on macOS where install_name_tool
neeeds to process the plugins setting the right LC_RPATH.
Our build files were relying 'sysroot' to find gexiv2.h but this is
not possible with Apple Clang om which sysroot points to macOS SDK.
So, exotic environments like Homebrew were failing. Let's fix this.
The whole multi-threading changes in the help plug-in seem to badly
break on macOS. See discussion in reopened#12898.
We decided to get rid of it for now and see later if we need to
reimplement this (after understanding what is going on).
Revert "plug-ins: fix#13049 Calling help on unknown help-id causes..."
This reverts commit 7d153bcc6d.
Revert "plug-ins/help: fix thread unnecessarily waiting when locale_parse failed"
This reverts commit fd0ccfa16c.
Revert "plug-ins/help: fix crash when locale is NULL"
This reverts commit 4075add5b4.
Revert "plug-ins: fix failing to access help from within GIMP"
This reverts commit 38f0527ebc.
Revert "plug-ins: add some better error handling when the docs XML request/parsing fails."
This reverts commit 543bb374a8.
Revert "plug-ins: try to load the gimp-help.xml file in a thread."
This reverts commit f2d47e910b.
progress on statusbar to not finish
When we returned early we did not call `_gimp_help_progress_finish`.
We fix this by adding those calls in relevant places.
If `gimp_help_locale_parse` failed due to a locale not having a
manual available, we always waited for a 10 seconds timeout because
we depended on the value of success to decide if we should exit
the thread.
When the call finishes we should always stop, so add a `done` parameter
and depend on that to decide if we can exit instead of using success.
Due to recent changes locale could become NULL when no manual for a
certain locale was found.
We fix this by always checking locale first.
While working at this, I realized that nowadays we don't need a second
loop when the specified help-id was not found. Even if a certain
help-id is not translated, an untranslated page is always available
when an English page is present.
So just reference the help_missing page for the locale we found instead
of doing another loop over the locales.
Since commit 543bb374a8 when
`gimp_help_domain_lookup_locale` returned NULL, we returned with
an error condition.
However, when a user with locale en_CA, or sv_SE tries to access our
help, it should not return, but try again with en and sv respectively.
Instead of returning, only call `gimp_help_locale_map` when locale is
not NULL. Then after that we continue with second pass processing
which will handle the above cases.
I had a case where the GIO API ended just stuck and never returning.
This API is made to work in a thread so that you can cancel loading URIs
from the main thread. Let's make use of that.
This is a followup of previous commit. We must set the win_subsystem
option on executable() so that the result binary is compiled as a GUI
application (and doesn't output a console every time).
The previous commit is still needed and is what allows us to control
when to actually display a console.
… to GIMP_PDB_PROC_TYPE_PERSISTENT, let's rename some procedures.
s/gimp_plug_in_extension_enable/gimp_plug_in_persistent_enable/
s/gimp_plug_in_extension_process/gimp_plug_in_persistent_process/
s/gimp_procedure_extension_ready/gimp_procedure_persistent_ready/
Even though it's not public yet (and won't really be for GIMP 3.0), I
created a new concept of "GIMP Extension" (.gex files) which bundles
various types of data for GIMP, such as plug-ins but also brushes and
other resources, themes, icons, etc.
Having 2 different concepts named the same is confusing, especially
since one of them is not really self-explaining IMO (why are "always-ON"
plug-ins called "extensions"?). So even though this is the older
concept, and since we are anyway massively breaking the API for GIMP
3.0, let's rename this older concept. "Persistent Plug-Ins" is much more
self-defining.
With the new API introduced int d1c4457f,
we next need to port all plug-ins using
the argument macros to functions.
This will allow us to remove the macros
as part of the 3.0 API clean-up.
If we leave a space between the macro name and opening parenthese for argument
lists, the args are not considered macro args (which will be discovered when
using it). I experienced this issue while testing code on some plug-in
yesterday, so thought I might as well fix all these broken macros for casting to
the specific GimpPlugIn subclass, so that we won't have a next time.
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.
Passing (name, type, value) triplets is actually useless because we can get the
type information from the procedure/config anyway. That only adds one more
verification to do. Let's just change the function so that we pass (name, value)
couples instead, pretty much like in `g_object_set()`.
As far as plug-in API is concerned, at least the calling API, order of arguments
when calling PDB procedures doesn't matter anymore.
Order still matters for creating procedures with standard arguments (for
instance, "run-mode" is first, then image, or file, drawables or whatnot,
depending on the subtype of procedure), but not for calling with libgimp.
Concretely in this commit:
- gimp_pdb_run_procedure_argv() was removed as it's intrinsically order-based.
- gimp_pdb_run_procedure() and gimp_pdb_run_procedure_valist() stay but their
semantic changes. Instead of an ordered list of (type, value) couple, it's now
an unordered list of (name, type, value) triplets. This way, you can also
ignore as many args as you want if you intend to keep them default. For
instance, say you have a procedure with 20 args and you only want to change
the last one and keep the 19 first with default values: while you used to have
to write down all 20 args annoyingly, now you can just list the only arg you
care about.
There are 2 important consequences here:
1. Calling PDB procedures becomes much more semantic, which means scripts with
PDB calls are simpler (smaller list of arguments) and easier to read (when
you had 5 int arguments in a row, you couldn't know what they refer to,
except by always checking the PDB source; now you'll have associated names,
such as "width", "height" and so on) hence maintain.
2. We will have the ability to add arguments and even order the new arguments in
middle of existing arguments without breaking compatibility. The only thing
which will matter will be that default values of new arguments will have to
behave like when the arg didn't exist. This way, existing scripts will not be
broken. This will avoid us having to always create variants of PDB procedure
(like original "file-bla-save", then variant "file-bla-save-2" and so on)
each time we add arguments.
Note: gimp_pdb_run_procedure_array() was not removed yet because it's currently
used by the PDB. To be followed.
This is not the main reason for the specific output in #9994. These ones are
more probably because of similar usage in GTK (which updated its own calls to
g_file_info_get_is_hidden|backup() in version 3.24.38). But we should likely
also update the various calls we have to use the generic
g_file_info_get_attribute_*() variants.
To be fair, it is unclear to me when we can be sure that an attribute is set.
For instance, when we call g_file_enumerate_children() or g_file_query_info()
with specific attributes, docs say that it is still possible for these
attributes to not be set. So I assume it means we should never use direct
accessor functions.
The only exception is that I didn't remove usage of g_file_info_get_name(),
since its docs says:
> * Gets a display name for a file. This is guaranteed to always be set.
Even though it also says just after:
> * It is an error to call this if the #GFileInfo does not contain
> * %G_FILE_ATTRIBUTE_STANDARD_DISPLAY_NAME.
Which is very contradictory. But assuming that this error warning was
over-zealous documentation, I kept the direct accessors since they are supposed
to be slightly more optimized (still according to in-code documentation) so
let's priorize them when we know they are set for sure.
Now that we bumped our meson requirement, meson is complaining about
several features now deprecated even in the minimum required meson
version:
s/meson.source_root/meson.project_source_root/ to fix:
> WARNING: Project targets '>=0.56.0' but uses feature deprecated since '0.56.0': meson.source_root. use meson.project_source_root() or meson.global_source_root() instead.
s/meson.build_root/meson.project_build_root/ to fix:
> WARNING: Project targets '>=0.56.0' but uses feature deprecated since '0.56.0': meson.build_root. use meson.project_build_root() or meson.global_build_root() instead.
Fixing using path() on xdg_email and python ExternalProgram variables:
> WARNING: Project targets '>=0.56.0' but uses feature deprecated since '0.55.0': ExternalProgram.path. use ExternalProgram.full_path() instead
s/get_pkgconfig_variable *(\([^)]*\))/get_variable(pkgconfig: \1)/ to
fix:
> WARNING: Project targets '>=0.56.0' but uses feature deprecated since '0.56.0': dependency.get_pkgconfig_variable. use dependency.get_variable(pkgconfig : ...) instead
Hence avoiding the stderr messages. These are going to be localized with
centrally installed catalogs "gimp*-std-plugins", "gimp*-script-fu" and
"gimp*-python".
We now handle core plug-in localizations differently and in particular,
with kind of a reverse logic:
- We don't consider "gimp*-std-plugins" to be the default catalog
anymore. It made sense in the old world where we would consider the
core plug-ins to be the most important and numerous ones. But we want
to push a world where people are even more encouraged to develop their
own plug-ins. These won't use the standard catalog anymore (because
there are nearly no reasons that the strings are the same, it's only a
confusing logic). So let's explicitly set the standard catalogs with
DEFINE_STD_SET_I18N macro (which maps to a different catalog for
script-fu plug-ins).
- Doing something similar for Python plug-ins which have again their own
catalog.
- Getting rid of the INIT_I18N macro since now all the locale domain
binding is done automatically by libgimp when using the set_i18n()
method infrastructure.
Recently it was not possibly in master to open online help (F1).
This is also mentioned in issue #7915. However, on macOS there are likely
also other problems, which is why I'm hesitant to close that issue with
this fix.
(help.exe:57964): LibGimpBase-CRITICAL **: 15:24:38.792:
gimp_value_array_index: assertion 'index < value_array->n_values' failed
(help.exe:57964): GLib-GObject-CRITICAL **: 15:24:38.792: g_value_get_boxed:
assertion 'G_VALUE_HOLDS_BOXED (value)' failed
This was most likely caused by 8eb7f6df9e
Changing this to use args 0 and 1 instead of 1 and 2 fixes the problem.
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
This gives a big cleanup in the meson.build files of the plug-ins.
It's also quite a bit more maintainable, since anything that changes in
libgimp's dependencies, linkage, ... doesn't have to be copy-pasted into
each plug-in.
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.
