When the core sends a NULL resource, which would be the default for object args,
hence is also what you get for the first call of a plug-in with a resource
parameter, libgimp was creating a GimpResource with NULL id, which is invalid.
It is much better to return NULL (since we made it so that NULL is a valid
value) and let the plug-in handle the NULL value as it sees fit for a given
parameter (they could just set the contextual resource for this type, or keep
NULL to mean "no resource selected").
This fixes failing to run plug-ins the first time (before any "last" values are
set). E.g. I had the issue when testing palette-sort.
Also I'm improving the error message when trying to use a non-installed resource
(it will now also print the resource ID and the error message). And the GError
was leaking in this case, so I properly free it now.
… moved to the implementation file.
When declaring with G_DECLARE_FINAL_TYPE(), the whole concept is that the struct
is made private (which also allows the type to evolve without breaking ABI if we
some day decide to make the class derivable). For this to make sense, the struct
goes in the implementation file, not the header.
For the rest, it's mostly alignment bugs and the like.
Fixes:
> libgimp/gimpresourceselectbutton.c:510:9: error: a label can only be part of a statement and a declaration is not a statement
> 510 | GimpResource *specific_value;
As well as some coding style bug (space after '*').
This fixes the VAPI build. I am actually astonished the lib build seem to have
passed and that we didn't get double definition clashes.
The build error was:
[750/2424] Generating libgimp/gimp-ui-3.0.vapi with a custom command
FAILED: libgimp/gimp-ui-3.0.vapi
/usr/bin/vapigen --quiet --library=gimp-ui-3.0 --directory=/builds/GNOME/gimp/_build/libgimp --pkg=babl-0.1 --pkg=cairo-1.0 --pkg=gdk-pixbuf-2.0 --pkg=gegl-0.4 --pkg=gio-2.0 --pkg=glib-2.0 --pkg=gobject-2.0 --pkg=gtk+-3.0 --vapidir=/builds/GNOME/gimp/_build/libgimp --girdir=/builds/GNOME/gimp/_build/libgimp --pkg=gimp-3.0 --metadatadir=/builds/GNOME/gimp/libgimp /builds/GNOME/gimp/_build/libgimp/GimpUi-3.0.gir
GimpUi-3.0.gir:22111.7-22111.33: warning: Virtual method `GimpUi.ResourceSelectButton.draw_interior' conflicts with method of the same name
GimpUi-3.0.gir:26688.73-26688.73: error: The type name `ResourceSelectButtonClass' could not be found
GimpUi-3.0.gir:26695.73-26695.73: error: The type name `ResourceSelectButtonClass' could not be found
GimpUi-3.0.gir:26704.73-26704.73: error: The type name `ResourceSelectButtonClass' could not be found
GimpUi-3.0.gir:26712.73-26712.73: error: The type name `ResourceSelectButtonClass' could not be found
GimpUi-3.0.gir:26720.73-26720.73: error: The type name `ResourceSelectButtonClass' could not be found
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
Only libgimpui depends on GTK+, display servers and other GUI-related
dependencies. There was a problematic include added in commit 0b56aa0d13 for
macOS, but the needed code (testing the macro GDK_WINDOWING_QUARTZ to use some
[NSApp activateIgnoringOtherApps:YES] API) doesn't seem to be present anymore in
there, so I think that removing this include (replace by including GLib for
other calls) should work fine. Of course, we'll know it when the separate CI
will test a macOS build as we still don't have in-Gitlab macOS jobs. :-/
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.
Missing functions were:
* gimp_image_get_selected_channels()
* gimp_image_get_selected_vectors()
* gimp_image_list_selected_channels()
* gimp_image_list_selected_vectors()
* gimp_image_set_selected_channels()
* gimp_image_set_selected_vectors()
* gimp_image_take_selected_channels()
* gimp_image_take_selected_vectors()
There are discussions of renaming GimpVectors to GimpPath, which would
also be consistent with the GUI and make the always-plural less akward
in API. We'll see. For now keeping named like this.
Now text layers are proper types, which means that the binding API will also be
nicer (e.g. `txt_layer.set_text('hello world')` in Python).
This commit also adds the param specs allowing to create plug-in procedures with
text layer parameters.
Finally it fixes the few calls in file-pdf-save (apparently the only plug-in
using specific text layer API right now) with explicit type conversion.
After re-reading #534, I realized I missed the discussion about unsupported
markup by the tool. Then I tested and confirmed what Ian Munsie initially said
in a comment: unsupported markups are properly rendered in the text layer, yet
are simply dropped when editing with the text tool.
This is actually the ideal behavior as it means that with the API, you can even
go further than what is currently possible with the GUI. So it gives nice powers
to people who can script GIMP. We still need some warning in the function
documentation to tell developers about this weakness in the tool GUI.
This complements the existing text_layer_get_markup function and allows
scripts to create and modify complex text layers.
It adds the <markup> root tag if it was not supplied and will run the
markup through pango_parse_markup() to check for errors.
Reviewer's (Jehan) note: this is a mostly untouched patch contributed in #534,
except that code moved around. I also fixed the header set in the .pdb, a link
to pango markup docs and added the meson changes.
Fixes:
> /usr/bin/ld: ../libgimp/.libs/libgimpui-3.0.so: undefined reference to `gimp_check_custom_color2'
I am actually unsure this fix is fine. It doesn't look like it should
work. And worse, I can't reproduce the fix by reverting it after.
The only other person who reported it was akk, with exactly the same
symptoms.
As diagnosed in #8649, using a guint32 for windows identifier may have been
right long ago (was it?), but is definitely not anymore. I can see that a XID is
an unsigned long nowadays (usually 64-bit on 64-bit Linux).
As far as I can see, on Windows, it would be a void* behind (which also
corresponds to the error message in #8649 description):
> typedef void *PVOID;
> typedef PVOID HANDLE;
> typedef HANDLE HWND;
Cf. https://learn.microsoft.com/en-us/windows/win32/winprog/windows-data-types
I *think* that pointers would be 64-bit on Windows 64-bit, though I'm unsure
(after all, this is an OS with 32-bit long int on 64-bit arch!).
Anyway, it's just better to move to 64-bit window identifiers.
Since Clang 15.0.0:
> The -Wint-conversion warning diagnostic for implicit int <-> pointer
> conversions now defaults to an error in all C language modes. It may be
> downgraded to a warning with -Wno-error=int-conversion, or disabled entirely
> with -Wno-int-conversion.
… gimp_procedure_config_save_metadata().
If you use gimp_procedure_config_save_metadata() or
gimp_procedure_config_end_export(), you don't really control the flags
and let the GimpProcedure API make somes choices for you, based on
various assumptions. One of them is that the procedure has specific
properties (named "save-*", either created manually or with the various
gimp_save_procedure_set_support_*() functions). So if you don't have
them, we should assume this format doesn't handle a given metadata
format and deactivate it.
For plug-ins with a different/specific logic, they are expected not to
use these helper functions. They would likely call lower level functions
such as gimp_image_metadata_save_finish() or the newer
gimp_image_metadata_save_filter(), where you control the metadata flags.
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
Adds a simulation_bpc and simulation_intent to GimpImage to allow
plug-ins to access it
for CMYK import/export.
Four pdb functions were added to enable this access:
image_get_simulation_bpc (), image_set_simulation_bpc (),
image_get_simulation_intent (), and image_set_simulation_intent ().
Next, it updates menu options and code to support GimpImage's
internal simulation intent and bpc.
New 'simulation-intent-changed' and 'simulation-bpc-changed signal
are emitted via
GimpColorManagedInterface so that relevant tools
(such as the
CYMK color picker, GimpColorFrame, and future pop-overs)
are aware of these changes.
Ironically, it is a test for the Windows platform but it cannot run on
Windows. First, because it expects a .so (which could be easily fixed),
but even more because from web search, it looks like the nm tool may not
exist on Windows (though I haven't checked).
Anyway we only ever ran it from Linux machines and up to now, it worked
just fine and was useful anyway. So let's go with it.
Also clean a bit remnants from older attempts to run this script.
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
The check script now takes into account both the autotools and meson
file hierarchy (in autotools, built libs are in .libs/ subdirs).
Also it now properly fails on missing lib.
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.
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.
Adds a simulation_profile to GimpImage to allow plug-ins to access it
for CMYK import/export.
Two pdb functions were added to enable this access:
image_get_simulation_profile () and image_set_simulation_profile()
Next, it updates menu options and code to support GimpImage's
internal simulation profile. Menu items are moved from View to Image's
Color Management section.
New 'simulation-profile-changed' signal is emitted via
GimpColorManagedInterface so that relevant tools (such as the
CYMK color picker, GimpColorFrame, and future dockable
dialogue) are aware of these changes.
The get() API are sometimes nicer in C code because it's just simpler to
loop through C arrays, but they end up with similar API to the list()
variants for binding, or with a useless size return value (since most
higher level languages have length-aware array types, which is what
GList are transformed into).
So let's use the list() variants as the main ones and skip the get()
variants. I hesitated to rename the list() variants to get() with
`(rename-to)` annotations but since I am unsure if the get() bindings
are absolutely useless, I don't think it's the best idea. Maybe on some
other language usable as GI binding, the get() variant might be
different again and nicer to use. So if we shadowed these by renaming
list() ones, the day we change our mind, we'd have to rename get() ones
too (which would be very confusing), or else break bindings' API. To
avoid this, I just skip the get() ones altogether in bindings but leave
their name available in the bindings.
… 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 was saving the last changed/saved date to IPTC tag DateCreated,
which should only be used for the original creating date of the image
and thus should not be changed by GIMP.
After discussion in the cited issue, there is no tag in IPTC that we can
use, so we remove saving modified date from the IPTC metadata.
Instead we add two XMP tags, one for modified date and the other for the
date that metadata was changed. Since we do both when exporting, both are
saved with the same date/time in ISO 8601 format.
This also fixes another issue where we were not storing the timezone offset
for Xmp.tiff.DateTime. Since this has the same format as the other
XMP tags, we fix this together with this issue.
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.
We changed the logic of _gimp_plug_in_domain_register() which is now
only called when a domain is explicitly registered (which is not the
case by default anymore). Let's update the function documentation and
also make it clear that third-party developers in particular should not
play with it if they want their plug-ins to be properly localized.
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.
Clearly the old logic for localizing plug-ins was "core plug-ins first":
* In particular, we were defaulting to the "gimp*-std-plugins" domain,
asking plug-ins to override it with libgimp function
gimp_plug_in_set_translation_domain(). Obviously any third-party
plug-in would have to call this function since their strings are most
likely not the same as GIMP core plug-ins.
* Moreover this was only for the localization of menu items, which means
we had to duplicate work anyway (simplified for core C plug-ins only
with the INIT_I18N macro).
* Also plug-ins had to manage the location of the Gettext catalogs,
which is simpler for core plug-ins with gimp_locale_directory(), but
more annoying for third-party plug-ins which can be installed
basically anywhere (assuming their message catalogs are in their
directory, not centrally installed on the system, especially for
non-UNIX-like packages, with relocatable GIMP, no central package
system and so on).
* Finally in this logic of centrally installed catalogs, we were
requesting that the domain name is unique, which makes sense in a
Linux world with well maintained packages to avoid name clashes, not
in a world with people making plug-ins without knowing too much what's
done by their neighbour.
So now the new logic is:
- By default, GIMP will search for a folder called locale/ on the same
level as the plug-in executable. The Gettext domain will be the name
of the executable folder (and it doesn't matter too much if it's not
unique in such configuration).
- This can be disabled by overriding set_i18n() to NULL or
reimplementing it. All our core plug-ins will do this since they will
continue to use the centrally installed "gimp*-std-plugins" domain (or
"gimp*-python" for Python plug-ins).
- When not disabled while the folder is not available, warning messages
will be outputted to stderr, so that plug-in developers can easily
detect what is missing and how to handle it (and how to easily support
internationalization of their plug-in).
- gimp_plug_in_set_translation_domain() is removed and a future commit
is going to get rid of _gimp_plug_in_domain_register() because we are
going to get rid of the core-side localization of menu items (with
logic explained in the upcoming commit).
When overwriting the same file when exporting, we didn't check if the
image previously had a thumbnail. If the default setting in Preferences
is to add a thumbnail, then it would add one where it shouldn't.
Since thumbnails get saves as part of the EXIF metadata, we need to check
that to see if there was a thumbnail in the original image.
However, we were always removing the thumbnail from the metadata on import.
Let's delay removing this metadata until we need to, which has the
advantage that the metadata in our viewer is more complete.
When exporting starts, we add a check in gimp_image_metadata_save_prepare
to see if there was a thumbnail in the EXIF metadata. If not, then we
disable the thumbnail flag.
In gimp_image_metadata_save_filter we remove the thumbnail metadata when
the user doesn't want to save a thumbnail, or when the image format
does not support EXIF.
- Some coding style fixes (alignment, etc.).
- Adding missing "Since: 3.0" annotations. We are still wondering
whether this should go in 2.10, in which case, it would become
"Since: 2.10.32" annotations. See discussion in !274.
- Changing gimp_checks_get_colors() signature: merge the 4 color
arguments into 2 (inout) arguments which seems a bit nicer in C,
whereas binding handles such arguments correctly. The other
alternative would have been to at least change the order to have out
arguments in the end.
I also hesitated to get another API in libgimp, which would have been
config-aware (just returning the 2 check colors, depending on user-set
Preferences), then having GimpPreviewArea handling 2 colors (without a
GimpCheckType input). But actually, doing this, we'd remove the nice
menu popup where one could choose a generic check type (not everyone
wants to play with specific non-gray colors) in Gimp*Preview widgets.
So in the end, I left this whole thing as-is.
Instead I document the function with code sample to initialize
properly the GimpPreviewArea (since libgimpwidgets/ are independent
with no knowledge of the core config) in order to respect user
preferences.
- Hide the color properties in gimp_preview_area_menu_new() because
anyway gimp_preview_area_menu_new() does not support GimpRGB
properties right now (so all we get are warnings). It's still possible
to select custom colors on GimpPreviewArea, simply we are stuck at the
ones set in Preferences globally for now (unless a plug-in creates
custom GUI to set these).
Fixed Conflicts from !274:
libgimp/gimp.h
libgimpwidgets/gimppreviewarea.c
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
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.
Fixes:
> [33/48] Generating Gimp-3.0.gir with a custom command
> ../../../../../../../dev/src/gimp/libgimp/gimpimageprocedure.c:228: Warning: Gimp: missing ":" at column 41:
> * @run_func: (closure run_data) the run function for the new procedure.
> ^
On Windows our Lua goat extension crashed when run. After some digging
it appeared that incorrect annotations were the cause.
The destroy annotation was clearly an error when loking at the gir API
documentation. However, it also crashed on `(closure run_func)`, which
seemed to be as it should.
After comparing what other libraries do and testing, it seems that the
documentation for (closure CLOSURE) is incorrect. See the discussion
in the above mentioned issue.
The correct use here seems to be to use it as annotation of the run_func
where CLOSURE points to the user data. So, that's what we implement here.
Fix the dependency by making the stamp an actual (yet empty/no-op)
header file which is included by all generated source file. This way, we
ensure that meson rebuild .o files when the .pdb sources are changed.
This is the second solution proposed by eli-schwartz here:
https://github.com/mesonbuild/meson/issues/10196#issuecomment-1080053413
The build now successfully build the PDB files into the source folder
itself. Unfortunately it seems I can't get meson dependencies to work
properly, once more! I added a "sources" argument to the relevant
library() or static_library() but it still uses old versions to build
these. E.g. if I add an error on purpose to a pdb file, the next build
still passes, yet the second-next fails (as it should have before).
Note that I even tested a declare_dependency() with just the "sources"
arguments, because it says "sources to add to targets (or generated
header files that should be built before sources including them are
built)" (so I assume it means that it should be trigger a rebuild,
otherwise it's useless) but it's just not working. I'll investigate
more.
Still going with this for now, because at least generating the PDB
source was a big miss until now. But we should
The initial attempt of this commit was to remove the `GtkAction` usage,
but grew a bit wider than that. The following happened:
* The dialog became a proper GObject, rather than being a big chunk of
static variables that were hopefully initialized before you used them.
* The dialog now uses `GAction`s to implement actions, and converted
some signal handlers to actions as well.
* The plug-in run procedure now uses `GtkApplication`. This is one hand
necessary to be able to set accelerators for actions, but on the other
hand is more future-proof, as GTK4 removes `gtk_main()`
This new function is an alternative to existing
gimp_image_metadata_save_finish, when you want to save metadata
yourself and you need only filtering processing.
It returns filtered metadata allowing the caller
to save the finalized metadata via other means
(via native format’s library for example)
This API can be used to support metadata saving of image formats
not directly supported by gexiv2/exiv2.
… %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 the warning is:
WARNING: Invalid fragment for 'Gimp.Config': it should be struct
It implements the [iface@Config] interface and therefore has all its
^~~~~~~~~~~~~~
This warning feels wrong as we use GimpConfig for the name of the
interface, yet in the .gir file, I see `<record name="Config" …>` yet
`<interface name="ConfigInterface" …>`.
I suppose gi-docgen would want us to use [iface@ConfigInterface] if we
want to link the interface. But looking at the gir file, all interesting
interface methods are associated to the Config record. So let's just go
with the [struct@Config] proposed by the warning.
In any case, something feels wrong or broken here, but we need to fix
this for the CI. I hope Niels can look at it at some point.
Fixing these 2 warnings in the CI which end up fatal:
WARNING: Invalid fragment for 'Gimp.Parasite': it should be struct
Serializes the object properties of @config to a [class@Parasite].
^~~~~~~~~~~~~~~~
WARNING: Invalid fragment for 'GLib.MainLoop': it should be struct
it has a GUI and is hanging around in a [class@GLib.MainLoop], it must call
^~~~~~~~~~~~~~~~~~~~~
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.
As Massimo notes, the issue is not about the callback being broken in
bindings, but simply that bindings fail to handle random data without an
associated size. So let's just add the size. I confirmed testing API in
the Python binding that it now works fine.
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
While we do have quite a few gimp_pdb_run_procedure*() functions now, I
always felt that one based on a config file was missing, even more as we
are getting further and further into using config objects in plug-ins.
In C, the gimp_pdb_run_procedure() function is without a doubt the
easiest one. But such variable arg functions are not available on
bindings, and having to deal with GValue and GimpValueArray is a real
pain.
Also using a config file has the very great advantage that we don't need
to care about order. For instance, if I need to set the 10th argument of
a PDB call (and leave the rest to default values), I don't have to set
all 9 previous arguments. I can set only this one if I want. This
advantage is useful also for C code by the way.
For the record, here is how you could load then export an image with the
"file-png-*" PDB procedures in Python:
> c = Gimp.get_pdb().lookup_procedure('file-png-load').create_config()
> c.set_property('file', Gio.file_new_for_path('/path/sample.png'))
> r = Gimp.get_pdb().run_procedure_config('file-png-load', c)
> d = Gimp.Display.new(r.index(1)) # Give it a display to work on it.
Now exporting:
> img = r.index(1)
> c = Gimp.get_pdb().lookup_procedure('file-png-save').create_config()
> c.set_property('image', img)
> c.set_property('file', Gio.file_new_for_path('/path/exported.png'))
> layers = img.get_layers()
> c.set_property('drawables', Gimp.ObjectArray.new(Gimp.Drawable, layers, False))
> c.set_property('num-drawables', len(layers))
> r = Gimp.get_pdb().run_procedure_config('file-png-save', c)
… assert the existence of GError.
This is even worse as deserialize() method does not even take a GError
parameter anyway so this assert will always go off when a
deserialization failed (which happened in my case as I was working on a
plug-in API, hence gimp_procedure_config_load_last() actually failed to
load a previous version of the plug-in-settings when I changed a
procedure arg's type).
Just fail the deserialization normally and let the calling code handling
this case. Nevertheless it is kind of useful to bubble-up the error to
calling code, so I add a TODO in the interface header (hopefully to see
and improve this before we release GIMP 3.0).
Last deprecated usages in this file. Actually there are a few other
calls but deprecated on GExiv2 0.14.0, hence over our current
requirement. So we'll have to handle these later.
Replace functions gexiv2_metadata_set_xmp_tag_struct() and
gexiv2_metadata_get_tag_type() with their _try_ variants.
Note that I print to stderr rather than raising a warning here as I am
quite unsure where the list of XMP metadata we are gathering comes from.
Is it fully validated by GExiv2, and therefore no errors are ever
supposed to happen? (in such case, we should raise a warning if it does)
Or is it user-provided data (e.g. from a file loaded in GIMP which can
contain broken metadata)? In such a case, we should probably handle the
error slightly differently to warn for non-processable data (hence
possibly metadata loss at export time).
For the time being, then go with this weak handling and take care of
this better when we can look further into this.
… gexiv2_metadata_try_set_tag_string().
These are usage where we have full control over tag names and values so
no error is supposed to happen. This is why we use them as code warnings
and not returned error (because if an error did happen, this would be a
bug rather than a user error or a system error).
Here are the changes:
- Separate the check for comment contents and the one of whether or not
it starts with "charset=Ascii ". Indeed in my tests, we still want to
handle the empty string case or the "binary comment" case even without
the charset prefix (currently it was both or none, so I encountered
cases with a broken "binary comment" comment because the charset
prefix was absent).
- Add some #warning in order not to forget to remove the bogus "binary
comment" test. Indeed after some digging in Exiv2 code, I could
confirm this return value got removed in commit 9b510858 of Exiv2
repository, i.e. since Exiv2 0.27.4. Now in my test case where I had a
tag containing only 0s, Exiv2 returns an empty string, which is
perfectly fine (and it's up to us to keep or ignore it).
- Use gexiv2_metadata_try_get_tag_interpreted_string() instead of their
deprecate non-try counterparses. Right now, I am just outputting any
error message to stderr, as I'm not sure if this is the kind of errors
we want to warn people about. I guess it would depend on which type of
errors exactly are returned, so let's see if we encounter some case in
the future. For now stderr is fine to detect these.
yocto/oe is capable of building gobject introspection despite cross-compiling.
add an option to enable gir build even if cross-compiling
Signed-off-by: Markus Volk <f_l_k@t-online.de>
Reviewer note (Jehan): this whole stuff is a mess. Actually I'd like to
simply get rid of the whole no-gir-when-cross-compiling logics but I
still can't figure out how to cross-build generically with GIR.
Yet since some manage it with yocto, let's unblock them.
See #7208.
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
I cleaned many remaining places where the concept of linked item still
survived.
On loading an XCF file with linked items, we are now going to create a
named sets for the linked items, allowing people to easily select these
back if the relation was still needed.
We don't remove gimp_item_get_linked() yet and in particular, we don't
save stored items into XCF files. This will come in an upcoming change.
When running tests, the data are not meant to be necessarily installed.
Therefore icons won't be found when calling gimp_widgets_init().
Add some special-casing to find them relatively to the install
directory.
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().
Saving a thumbnail is closely related to the other metadata preferences,
but so far this was the only one that didn't have a preference for a
default user value.
This commit adds a preference in the metadata section where a user can
select whether thumbnail saving is enabled by default or not.
Since it appeared with GLib 2.68.0, we could not change this until we
bumped the dependency which has only become possible a few days ago
(since Debian testing is our baseline for dependency bumps). Cf.
previous commit.
As this is a drop-in replacement (just a guint parameter changed to
gsize to avoid integer overflow), search-and-replace with:
> sed -i 's/g_memdup\>/g_memdup2/g' `grep -rIl 'g_memdup\>' *`
… followed by a few manual alignment tweaks when necessary.
This gets rid of the many deprecation warnings which we had lately when
building with a recent GLib version.
Jehan