- Fix annotations for gimp_export_options_get_image() to make it
actually introspectable with the GimpImage being both input and
output. Even though the logic doesn't change much (the input image may
be overriden or not), it doesn't matter for introspection because
images are handled centrally by libgimp and therefore must not be
freed. Actually deleting the image from the central list of images
though remains a manual action depending on code logic, not some
automatic action to be handled by binding engines.
- Add G_GNUC_WARN_UNUSED_RESULT to gimp_export_options_get_image()
because ignoring the returned value is rarely a good idea (as you
usually want to delete the image).
- Remove gimp_export_options_new(): we don't need this constructor
because at this point, the best is to tell plug-in developers to just
pass NULL everywhere. This leaves us free to create a more useful
default constructor if needed, in the future. Main description for
GimpExportOptions has also been updated to say this.
- Add a data_destroy callback for the user data passed in
gimp_export_procedure_set_capabilities().
- Fixing annotations of 'export_options' object from pdb/pdb.pl: input
args would actually be (nullable) and would not transfer ownership
(calling code must still free the object). Return value's ownership on
the other hand is fully transfered.
- Add C and Python unit testing for GimpExportOptions and
gimp_export_options_get_image() in particular.
- Fix or improve various details.
Note that I have also considered for a long time changing the signature
of gimp_export_options_get_image() to return a boolean indicating
whether `image` had been replaced (hence needed deletion) or not. This
also meant getting rid of the GimpExportReturn enum. Right now it would
work because there are no third case, but I was considering the future
possibility that for instance we got some impossible conversion for some
future capability. I'm not sure it would ever happen; and for sure, this
is not desirable because it implies an export failure a bit late in the
workflow. But just in case, let's keep the enum return value. It does
not even make the using code that much more complicated (well just a
value comparison instead of a simple boolean test).
This patch creates a GimpExportOptions class in both
libgimpbase and in libgimp. Currently it is a mostly empty
object, but it will be added to after 3.0 to allow for
additional export options (like resizing on export while
leaving the original image intact)
libgimp/gimpexport.c was removed, and most of its content
was copied into libgimp/gimpexportoptions.c. gimp_export_image ()
was replaced with gimp_export_options_get_image () in all
export plug-ins.
GimpExportProcedure has a new function to set the default
image capabilities for each plug-in on creation. It also sets up
a new callback function, which allows the options to respond to
user setting changes (such as toggling 'Save as Animation' in the
GIF or WEBP Plug-in).
This fixes all our GObject Introspection issues with GimpUnit which was
both an enum and an int-derived type of user-defined units *completing*
the enum values. GIR clearly didn't like this!
Now GimpUnit is a proper class and units are unique objects, allowing to
compare them with an identity test (i.e. `unit == gimp_unit_pixel ()`
tells us if unit is the pixel unit or not), which makes it easy to use,
just like with int, yet adding also methods, making for nicer
introspected API.
As an aside, this also fixes#10738, by having all the built-in units
retrievable even if libgimpbase had not been properly initialized with
gimp_base_init().
I haven't checked in details how GIR works to introspect, but it looks
like it loads the library to inspect and runs functions, hence
triggering some CRITICALS because virtual methods (supposed to be
initialized with gimp_base_init() run by libgimp) are not set. This new
code won't trigger any critical because the vtable method are now not
necessary, at least for all built-in units.
Note that GimpUnit is still in libgimpbase. It could have been moved to
libgimp in order to avoid any virtual method table (since we need to
keep core and libgimp side's units in sync, PDB is required), but too
many libgimpwidgets widgets were already using GimpUnit. And technically
most of GimpUnit logic doesn't require PDB (only the creation/sync
part). This is one of the reasons why user-created GimpUnit list is
handled and stored differently from other types of objects.
Globally this simplifies the code a lot too and we don't need separate
implementations of various utils for core and libgimp, which means less
prone to errors.
This commit renames the GimpVectors
object to GimpPath in both app/core and
in libgimp. It also renames the files
to gimppath.[ch] and updates the relevant
build and translation files.
There are still outstanding gimp_vectors_* ()
functions on the app side that need to be renamed
in a subsequent commit.
...to paths.
Similar to d0bdbdfd, but covering the
app/core versions of the API instead of
libgimp. Changes the names of
gimp_image_*_vectors () API to
gimp_image_*_path[s] ().
Also renames some related functions such
as gimp_image_pick_path (). The next step
will be to rename the gimp_vectors_* () to
gimp_path_* ().
...to paths
The first step in converting GimpVectors
to GimpPath. The PDB API for any
gimp_image_*_vectors () is now
gimp_image_*_paths ().
This commit only covers libgimp, and
the app/core versions will be renamed in
a following commit.
If the type is not registered, g_type_from_name() is not able to find
the GType from the type name.
Fixes:
> gimp_gp_param_to_value: type name GimpGroupLayer is not registered
Also add a bit more type handling code.
While setting a plug-in as transient usually worked, it was failing in
cases the plug-in's progress bar was not initialized (i.e. if
progress_init() was not called before setting the dialog transient).
This commit stores the calling display, core side too (libgimp side, the
plug-in already had the calling display ID information), and we use this
when a GimpProgress has not been created yet.
Port all plug-ins to retrieve the layers
directly from the image rather than
having them passed in. This resolves some
issues with introspection and sets the
foundation for future API work.
It's still basic but will help to share code for support of various vector-able
formats, such as the logic for dimensioning them, but also the generated GUI.
Not only this, but we are paving the way for the link layers (though it'll be
after GIMP 3, we want plug-in procedures' API to stay stable) by giving a way
for a plug-in procedure to advertize a vector format support. This way, the core
will know when a source file is vector and can be directly reloaded at any
target size (right now, in my MR for link layers, the list of "vector" formats
is hardcoded, which is not reliable).
Our pluginrc code is already able to output NULL for a default NULL color, but
reading was failing with the following error:
> GIMP-Error: Error while parsing '/home/jehan/.config/GIMP/2.99/pluginrc' in line 289: unexpected identifier 'NULL', expected number (integer) - fatal parse error
Let's support reading such a value now, especially as for Python plug-ins, we
are not even able to add a default value to GeglColor arguments (cf. previous
commit).
There are no plug-ins which uses GimpRGB for procedure argument, nor is there
any base PDB procedure. We don't pass this type anymore through from/to
core/plug-ins. So let's clean the whole code out as a next step to get rid of
GimpRGB from our codebase!
Resolves#10932
Since GIMP distinguishes between saving
XCF and exporting image like PNG,
we should change the PDB to show
export rather than save in the function
calls.
This commit is in conjunction of gimp-data@d273a18 and finally makes Windows
build able to run GIMP main binary itself (or gimp-console) in order to create
various images, before the installation step.
- Use the new GIMP_TESTING_INTERPRETER_DIRS environment variable for uninstalled
binary to find the .interp files (in particular because our build scripts are
usually Python scripts these days).
- Use the new GIMP_TESTING_ENVIRON_DIRS environment variable to find .env files
(actually running uninstalled GIMP binaries did run without this one, but it's
not a bad idea to add support anyway.
The python.env file sets a ':' separator for GI_TYPELIB_PATH environment
variable, which breaks finding the correct typelib files in the CI, when also
manually setting a previous value for this environment variable.
I'm not entirely sure the rule of swapping ':' for ';' is universally true on
Windows, whichever the variable. Let's hope that it is.
Though I had already implemented passing GeglColor through the PDB, it was not
complete. In particular, the protocol was not able to pass GeglParamColor specs.
Fixes:
> LibGimp-WARNING **: 16:06:09.451: _gimp_gp_param_def_to_param_spec: GParamSpec type unsupported 'GeglParamColor'
This is part of the fix to issue #10811, though it's not complete yet.
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().
It was not possible anymore for a plug-in to add a menu item to the
local Layers, Classes and Paths dialogs using <Layers>, <Classes> and
<Vectors> at the start of the menu path.
For the first two this was due to an extra length parameter and the
drawables now being inside a GimpObjectArray. To fix that we check
inside the object array for the correct type names.
I'm not sure if Vectors ever worked, but testing shows it only adds
runmode and image parameters. To fix that we do the same as for
ColorMap.
Having windows ID as guint32 is a mistake. Different systems have
different protocols. In Wayland in particular, Windows handles are
exchanged as strings. What this commit does is the following:
In core:
- get_window_id() virtual function in core GimpProgress is changed to
return a GBytes, as a generic "data" to represent a window differently
on different systems.
- All implementations of get_window_id() in various classes implementing
this interface are updated accordingly:
* GimpSubProgress
* GimpDisplay returns the handle of its shell.
* GimpDisplayShell now creates its window handle at construction with
libgimpwidget's gimp_widget_set_native_handle() and simply return
this handle every time it's requested.
* GimpFileDialog also creates its window handle at construction with
gimp_widget_set_native_handle().
- gimp_window_set_transient_for() in core is changed to take a
GimpProgress as argument (instead of a guint32 ID), requests and
process the ID itself, according to the running platform. In
particular, the following were improved:
* Unlike old code, it will work even if the window is not visible yet.
In such a case, the function simply adds a signal handler to set
transient at mapping. It makes it easier to use it at construction
in a reliable way.
* It now works for Wayland too, additionally to X11.
- GimpPdbProgress now exchanges a GBytes too with the command
GIMP_PROGRESS_COMMAND_GET_WINDOW.
- display_get_window_id() in gimp-gui.h also returns a GBytes now.
PDB/libgimp:
- gimp_display_get_window_handle() and gimp_progress_get_window_handle()
now return a GBytes to represent a window handle in an opaque way
(depending on the running platform).
In libgimp:
- GimpProgress's get_window() virtual function changed to return a
GBytes and renamed get_window_handle().
- In particular GimpProgressBar is the only implementation of
get_window_handle(). It creates its handle at object construction with
libgimpwidget's gimp_widget_set_native_handle() and the virtual
method's implementation simply returns the GBytes.
In libgimpUi:
- gimp_ui_get_display_window() and gimp_ui_get_progress_window() were
removed. We should not assume anymore that it is possible to create a
GdkWindow to be used. For instance this is not possible with Wayland
which has its own way to set a window transient with a string handle.
- gimp_window_set_transient_for_display() and
gimp_window_set_transient() now use an internal implementation similar
to core gimp_window_set_transient_for(), with the same improvements
(works even at construction when the window is not visible yet + works
for Wayland too).
In libgimpwidgets:
- New gimp_widget_set_native_handle() is a helper function used both in
core and libgimp* libraries for widgets which we want to be usable as
possible parents. It takes care of getting the relevant window handle
(depending on the running platform) and stores it in a given pointer,
either immediately or after a callback once the widget is mapped. So
it can be used at construction. Also it sets a handle for X11 or
Wayland.
In plug-ins:
- Screenshot uses the new gimp_progress_get_window_handle() directly now
in its X11 code path and creates out of it a GdkWindows itself with
gdk_x11_window_foreign_new_for_display().
Our inter-process transient implementation only worked for X11, and with
this commit, it works for Wayland too.
There is code for Windows but it is currently disabled as it apparently
hangs (there is a comment in-code which links to this old report:
https://bugzilla.gnome.org/show_bug.cgi?id=359538). NikcDC tested
yesterday with re-enabling the code and said they experienced a freeze.
;-(
Finally there is no infrastructure yet to make this work on macOS and
apparently there is no implementation of window handle in GDK for macOS
that I could find. I'm not sure if macOS doesn't have this concept of
setting transient on another processus's window or GDK is simply lacking
the implementation.
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.
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.
It does absolutely nothing except sitting there, providing an is-a
relation (both ways because GimpData is its only subclass). This will
simplify having more libgimp API on GimpResource, without having to
add different PDB code for app and libgimp.
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
Since we now generate actions for GEGL ops, we might as well generate menu items
for these too.
What I did:
- Move the "GEGL Operation…" tool (generic dialog with a drop-down list of all
non-ignored GEGL ops) to Tools menu.
- Create a "GEGL Operations" submenu in Filters > Generic.
- Move "GEGL Graph" to the top of this new submenu.
- Generate a new menu item for each generated action tied to a GEGL plug-in,
alphabetically sorted.
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
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
Generated *enums.c now have an additional stamp no-op header include
(see last 2 commits). Sync this change into the autotools generation
scripts to prevent back and forth useless generation of these files each
time we switch from one build system to another.
They are nearly the same as initially, except that now they include an
intermediate stamp header which will be generated by the build system.
The only 2 enums which don't need these includes (and are not versioned)
are libgimp/gimpenums.c and libgimpthumb/gimpthumb-enums.c.
Our meson build system was not properly building the enums.c file,
because they are versionned.
I did a similar trick as what I did for the pdbgen, which is that I used
a wrapper script around the existing perl script, which sets proper
options and generate a stamp file in the end (which is considered by
meson as the actual custom target, not the C file since it is generated
in the source dir).
The most important part is that the stamp file is a generated header
source (not just a random text file) which is **included** by the
generated C file. This is what will force meson to regenerate the C file
if the header is updated, **then** build using this new version, not use
an outdated versionned version (which would make for hard to diagnose
bugs), through the indirection of the intermediate stamp header.
See #4201.
See also: https://github.com/mesonbuild/meson/issues/10196#issuecomment-1080742592
This reverts commit 82f9ae1ab7.
This is not needed anymore. Cf. previous commit which implements the
force-requery differently and much more widely based on actual language
being stored and compared between runs.
Since localization is fully handled plug-in side now (see #8124), we
need to make sure the query functions are run again for all plug-ins
when the UI language changes (otherwise we might end up with
localizations from the previously used languages).
We were already reloading plug-ins when explicitly changing the lang in
the Preferences, but this new implementation is much better as it's
generic. In particular, it will also handle the case when the system
language changes (or when you play with locale environment variables).
Plug-in localization was always partially plug-in side, especially for
things like custom GUI. But labels or blurb in GIMP (such as in menus or
action search) were localizing GIMP side.
It had many drawbacks:
- To get menu localization, a plug-in had to set up gettext, even though
they might want to use something else for their GUI (after all, giving
facilities for gettext is a good idea, but there is no reason to force
using this system).
- There was a complex internal system passing the localization domain
name, as well as the catalog file system path to core, then through
various classes which we can now get rid of.
- There could be domain name clashes, if 2 plug-ins were to use the same
i18n domain name. This was handled in now removed functions
gimp_plug_in_manager_get_locale_domains() by simply keeping a unique
one (and gimp_plug_in_manager_bind_text_domains() would just bind the
domain to the kept directory). In other words, one of the duplicate
plug-ins would use the wrong catalog. We could try to make the whole
thing more complicated or try to forbid plug-ins to use any random
name (in particular made easier with the new extension wrapper). But
anyway this whole issue doesn't happen anymore if localization is
fully made plug-in side, so why bother?
I tried to evaluate the advantages of the core-side localization of
plug-in labels/blurbs and could only find one theoretical: if we wanted
to keep access to the original English text. This could be useful
(theoretically) if we wanted to search (e.g. in the action search) in
both localized and English text; or if we wanted to be able to swap
easily en/l10n text in a UI without reload. But even if we were to ever
do this, it would only be possible for plug-ins (GEGL operations in
particular are localized GEGL-side), so it lacks consistency. And it's
unsure why special-casing English should really make sense for other
language natives who want text in their lang, and search in their lang.
They don't necessarily care about original.
So in the end, I decided to simplify the whole thing, make localization
of plug-ins a plug-in side thing. Core will only receive translated text
and that's it. It cuts a lot of code out of the core, simplify runtime
processing and make plug-in creation simpler to understand.
The only think I still want to look at is how exactly menu paths are
translated right now. Note that it still works, but it's possible that
some things may be worth improving/simplifying on this side too.
I hesitated a lot whether we should just drop the whole localization of
plug-ins' label and description (blurb) within the core. Actually the
commit messages I wrote a few days ago were moving towards this logic.
It really looks to me like plug-in localization can happen fully within
plug-in themselves. As far as I can see, the only advantage which the
current logic has theoretically is that if we needed, we have access to
both the original strings and their translations (e.g. it could be
useful for text search). Nevertheless I am not sure if we will ever make
use of this, and this is limited cases as all filters turned GEGL ops
don't have such ability anyway.
Nevertheless since previous contributors clearly put quite a lot of work
on this code of localizing the plug-in's label and description within
the main binary, I want to give myself a little more time to think and
study the whole thing because doing anything rash.
In the meantime, what changes is that by default now, a plug-in without
a local gettext catalog is simply not localized. In particular, the core
process doesn't try to localize it using the default catalog, a.k.a.
GETTEXT_PACKAGE"-std-plug-ins" ("gimp30-std-plug-ins"). It just doesn't
make sense and the worst which could happen would be to get unexpected
and wrong translations.
Now by default, plug-ins will try to find a catalog in their main
folder, named as this folder. If it fails to find it, a message is
printed to stderr and localization is disabled (rather than falling back
to a default catalog). It is up to plug-in developers to either install
a catalog, or implement set_i18n() to give the right catalog, folder, or
disable localization with gettext, as handled by libgimp.
Reviewer (Jehan) note: cherry picked from MR !274. Still deciding
whether this will be pushed to gimp-2-10 branch too.
Fixed Conflicts from !274:
app/dialogs/preferences-dialog.c
app/display/gimpdisplayshell-draw.c
app/plug-in/gimppluginmanager-call.c
libgimp/gimp.c
libgimp/gimp.h
libgimpwidgets/gimppreviewarea.c
libgimpwidgets/gimppreviewarea.h
libgimpwidgets/gimpscrolledpreview.c
Some strings might be generated during query() call, in particular all
strings related to procedure parameters. The core will only know the
localized version at query() time, so you could end up in situation with
the core showing you the wrong language. This can be reproduced in the
Procedure Browser when it shows localized procedure data: when you
change the language and restart GIMP, if the plug-ins aren't re-queried,
then it will still show the localization of query time.
Note that it doesn't matter on plug-in side, i.e. anything in the run()
call is properly localized, included the plug-in GUI using the same
procedure parameters for widget labels or tooltips. Only the "vision" of
the core is stuck at what it got when the plug-in was last queried, and
until the next update of the plug-in.
The CLI options now know which procedures are batch procedures or not.
First it means that it won't just randomly try any procedure name one
may pass and will properly output an error if you pass a non-existing
interpreter procedure.
Secondly, there is no default interpreter anymore (unless only one
interpreter exists). If you don't set an interpreter procedure with
--batch-interpreter or if you pass a wrong one, it will output the list
of available batch procedure, thus helping you understanding how to use
the --batch option.
Jehan