Eventually this is meant to fully replace GimpRGB (as well as GimpHSV, GimpHSL
and GimpCMYK), both in libgimp and in core code, as part of both the space
invasion and the API rework. For this first commit, I keep this new object side
by side to GimpRGB.
Due to GObject Introspection we can't have the last part of an
identifier start with a digit, since that part will be used in Python
as the identifier, and Python doesn't allow that to start with a digit.
e.g. GIMP_ROTATE_90 would be used in Python as
image.rotate(Gimp.RotationType.90)
To fix this we add DEGREES in front of the number, without a '_',
even though that looks ugly.
With the sample image that contains the XMP namespace Item with the
correct url included, it is not returning the url when we ask for it.
This seems to be an exiv2 issue, but needs more research.
What GIMP doesn't do, is report that the namespace url we were looking
for wasn't found, and make sure that this doesn't block us from
handling all XMP tags.
To improve this on our end, we check if there was an error getting the
url, and if yes:
- We generate a warning message
- We create a dummy namespace url, with a special check for the missing
Item namespace to give it the correct known namespace url.
… a GimpProcedureConfig for arguments.
This also factorizes the code to load metadata. By default, a GimpLoadProcedure
will try and load metadata from a file (if Exiv2 knows the format). The run()
function will be allowed to edit the GimpMetadata object but also the load flags
before it is actually attached to the image, allowing plug-ins to have custom
metadata handling code when needed.
This is used in the generated GUIs for GimpChoice arguments, but also for
validation of property setting.
New functions:
* gimp_choice_set_sensitive()
* gimp_string_combo_box_set_sensitivity()
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 will be used for creating limited lists of strings as argument types for
procedures.
Ideally enums are the best type for this, but it can only be used for generic
libgimp* enum types, not custom enums created only for a given plug-in. For
this, we currently just demote the args to ints which lose any semantic. A
limited list of string will give back some semantic and some better validation,
even though it's a tiny bit more annoying to work with strings than int types
(at least in C).
Some images have Exif.Photo.PixelXDimension and Exif.Photo.PixelYDimension
metadata tags in addition to Exif.Image.ImageWidth and
Exif.Image.ImageHeight (mainly tiff images). So far, we were not updating
these optional tags, meaning they could get out-of-sync with the actual
dimensions when resizing and then exporting the image.
Since these tags are non essential, we will only update them if they are
already present.
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
Our metadata library exiv2 only registers the most commonly used xmp
namespaces. Other namespaces need to be explicitly registered. We did not
read or try to store these namespaces, which caused a lot of warnings about
"No namespace info available for XMP prefix '...' and then we could not
process that metadata or save/export those tags.
We had to wait for gexiv2 support for registering/reading namespaces, but
that was added in gexiv2 version 0.12.2 (and the "try" version in 0.14.0).
When reading xmp metadata we process all namespaces and add them to our
metadata xml when we haven't seen them before in the same image.
A GHashTable is used to keep track of the prefixes we have seen before.
The new namespace xml tag is skipped in older GIMP versions, but will be
used now to add the namespaces when exporting images with xmp metadata.
We were using fixed size buffer with strcpy/strcat, which gave warnings in
coverity.
Even though in our case there was no chance of buffer overflow, let's
replace this by using g_strconcat, which allocates memory for the
string which we free after use.
Also get rid of a few unneeded memset's for strdata.
Photoshop can save metadata when exporting to tiff or jpeg, including
a thumbnail that we can't update. This can contain sensitive data, so we
should not export it. See issue #8383.
To do this, we add the two Photoshop specific tags that we know of to the
list of tags that should not be exported, so they won't be saved.
As per the changes in commit 005b3a05b8 and discussions in !800,
gimp_locale_directory() returns a string in the OS encoding for all but Windows
(UTF-8), i.e. the "filename" type in GIR annotations.
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
In other cases we need to free value, but in this case it was used and
trying to free it caused a GIMP to crash.
Let's duplicate value, so we don't need to special case our g_free.
In the original gimp-2-10 branch where this feature was originally
worked on, we had to keep gimp_checks_get_shades(). But on `master`
where we will reset and break API, let's get rid of this function which
should not be used anymore.
Since we changed it, the version must be incremented too. It means that
third-party developers will have to rebuild their plug-ins.
Fixed Conflicts from !274:
libgimpbase/gimpprotocol.h
- Slightly nicer layout for the checkboard color widgets.
- Also set the 2 color widgets insensitive when the check type is not
GIMP_CHECK_TYPE_CUSTOM_CHECKS.
- Default the custom colors to the same colors as the basic default
grayscale. I mean, red and blue defaults are fun and all, but that's
like radioactive colors burning the eyes! Ahahah!
- Unlike the gimp-2-10 commit (cf. !274), on the main dev branch, use
the new GimpLabelColor widget.
Fixed Conflicts from !274:
app/dialogs/preferences-dialog.c
- 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
with many Xmp.photoshop.DocumentAncestors tags
This is similar to #7464, but in this case the XMP metadata was already
included in an XCF image.
We check for the occurrence of Xmp.photoshop.DocumentAncestors and stop
handling values when there are more than a 1000.
It would be easier to just check length for all tags and always
ignore when there are more than a 1000 values.
But in that case we would need to be sure there are no valid reasons for
tags to occur more than a 1000 times. So let's just limit it to this
specific tag.
NULL is not a proper value for GStrv yet we cannot escape it in the PDB
since we generate default values for non-passed arguments (especially in
interactive case where most procedure arguments aren't set). And for
such boxed type, it will be NULL.
So when we see a NULL GStrv parameter, let's not ignore it (which will
just crash the plug-in). Simply transform it to a GStrv of size 0.
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
There are still deprecations going around but for GExiv2 0.14.0 so we
can't change these yet.
Note also that I try a slightly different approach as I don't set a
GError for well-known tags as there is no reason these fail. I only add
a GError when we construct tags or similar calls.
I was waiting for GExiv2 to merge a patch I submitted:
https://gitlab.gnome.org/GNOME/gexiv2/-/merge_requests/20
Then we waited for it to land in a released version then for this
version to be in Debian testing. It's all done now and we bumped the
GExiv2 dependency in the previous commit (which makes it a master-only
fix). So all good.
Some images have huge amounts of XMP tag Xmp.photoshop.DocumentAncestors.
In certain cases more than 100000 values. This is apparently due to a bug
in certain versions of PhotoShop.
This makes deserializing it in the way we currently do too slow, probably
because of continuous reallocations after adding each value. Until we can
change this let's remove everything but the first 1000 values when
serializing. I think it is very unlikely there are any real cases where
more than a 1000 ancestor documents are referenced in an image. Testing
shows that this amount doesn't cause any serious slowdowns.
In some conditions, and only with some installations, the called GDB
ends up hanging and never returning. Worse, even if you use non-blocking
functions such as poll() or select() with a timeout, in order to wait
for the debugger output, these block anyway and never return.
We are currently unsure what exactly causes such problem, but not using
the thread command in gdb avoids it for now.
This is a bit of a regression, though most of the time anyway the useful
traces are the main thread ones (I think ever since I implemented this,
there must not have been more than 2 or 3 cases where we actually needed
the secondary traces). So it's acceptable, at least for now.
Otherwise if we add a NUL byte after the last byte, we might right past
the allocated memory. Thanks to Massimo for reporting this error raised
by Address Sanitizer and valgrind (cf. #7539).
In cases where the whole EXIF MakerNote is invalid we still load that
MakerNote data and export it too, causing partial invalid EXIF metadata.
We don't need to explicitly save Exif.Photo.MakerNote at all, because
as soon as we try to save a brand specific tag exiv2 will create that
MakerNote tag itself.
So from now we don't save the MakerNote but only the tags that go in it.
In issues like #2159 where exiv2 doesn't parse all tags inside certain
brand specific MakerNotes correctly, we will still export invalid EXIF.
That is an exiv2 issue that we can't do much about unless we remove all
MakerNote metadata including those that we can read, which doesn't seem
like a good idea at all.
Jehan