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.
