g_list_find_custom() uses a GCompareFunc which has a return value
similar to strcmp(), i.e. with 0 for equality (and not a boolean, which
is basically the opposite).
Even though "Export Image as HEIF" is not technically wrong (AVIF is AV1
encoding inside HEIF container), it is better to be more accurate.
Moreover as Daniel Novomesky was explaining in a MR, nowadays when we
say "HEIF", we usually mean "HEIC", whereas you'd use the explicit
"AVIF" naming for HEIF/AV1 images. This seems confirmed by Wikipedia
which says that HEIC is the "implied default codec for HEIF".
So let's just make the AVIF vs HEIF distinction here (I could have used
AVIF vs. HEIC which is even more explicit but I decided to keep the
less-specific yet more used HEIF naming).
This should give a nice name to distribution archives so that they are
not all called `artifacts.zip`. Names will better describe their
contents (target OS or source and short commit hash, because for CI
builds, it's important to know which commit is being tested).
Also replace CI_COMMIT_REF_NAME in other artifact names by
CI_COMMIT_REF_SLUG. Otherwise if a branch has a slash (quite common in
branch names), only the part after the last slash is used for archive
naming.
Finally immediately exits from dependency build with error code (!= 0)
if `crossroad install` command failed.
… the CI.
There are 2 finale steps before finale binary distribution on Windows.
We must compile the GSettings XML schema files and register GdkPixbuf
loaders (for file format support in the GUI).
I used to provide a wrapper to be run inside Windows before first GIMP
run. Never did I realize that I can compile the distributed GSettings
schemas with the native `glib-compile-schemas` (works fine in my tests).
As for the GdkPixbuf loaders, we inspect DLL libraries, hence we do
require the target `gdk-pixbuf-query-loaders` which is unfortunately a
Windows executable. Yet it seems to work fine with Wine, so let's be
done with it in the CI instead of requiring manual steps from testers of
the CI builds. Then a few `sed` calls are enough to make the path in the
produced text file relative instead of absolute (which works fine, again
in my tests at least).
This means that I don't have to distribute the 2 binaries and the DLLs
they depend on anymore. Moreover let's remove the wrapper (but still
generate one which just calls GIMP so that we call it from the tree
root, where it's much less messy).
Note: I failed to install wine32 (32-bit Wine) on the Gitlab runner.
After following all instructions, I encountered weird errors. So
instead, I just make the win32-nightly job depend on win64-nightly and
copy `loaders.cache` from one to another, as it is a
platform-independent text file (as long as we provide the same GdkPixbuf
loaders on both of course, which we do).
The main purpose of these jobs is to only package the strict necessary
for a working GIMP under Windows, i.e. getting rid of all unnecessary
executables, and inspecting binary dependencies recursively to only
package used DLLs.
The dll_link.py script is taken from Siril codebase (see commit a86e82a8
on Siril repository, by FlorianBen). This was a very nice idea, and
makes for much smaller test archive (Siril is also GPLv3 so licensing is
ok for the reuse, also anyway it's just a small independent build
script).
Moreover having it as a separate job allows to have artifacts with only
the finale distribution (artifacts on the build job also have the build
directory and the whole prefix, which we want to keep in order to debug
when needed).
Hopefully I am not missing anything. Siril seems to package more, like
various gdk-pixbuf-*.exe, gspawn-*.exe and gdbus.exe. I am wondering if
these are actually necessary. I could run GIMP fine without these in
quick tests, but I guess I'll have to investigate a bit more to figure
this out. That's what nightly builds are for, after all, so hopefully
people will report if we miss some runtime dependencies.
… GimpIntComboBox "value" property.
For this, I connect to the "changed" signal, which is equivalent anyway.
Otherwise the link was not bidirectionnal, so selecting a new item in
the combo list was not actually changing the internal value, hence the
binding set by gimp_prop_int_combo_box_new() was not complete either.
Not sure how I missed that. Hopefully not missing anything else!
Existing default was requesting a window of size 1024×768 at position
(200,100). While it may seem a reasonable default on nowadays displays,
it was not on some intermediate size displays which are considered HiPPI
anyway.
Taking my personal example, my screen is 2560×1440, which is considered
HiPPI by GNOME 3 with a scale ratio of ×2. As a consequence, setting a
size of 1024×768 was actually creating a window of 2048×1536, which is
already higher than the screen. Worse, gtk_window_resize() resize the
window without taking into consideration the title bar, which in my case
added 74 pixels, so GIMP window started at 1610 pixels of height, much
bigger than my screen size, hence unusable (and for some reason, with
the title bar out of the screen so without knowing Super+click shortcut
to move or Super+Up to maximize, people would have a hard time to resize
or close GIMP).
This issue only happens at the first launch of GIMP, when no user
sessionrc exists yet. Once you resize yourself the main window, then
restart GIMP, it is fine (as next times, it will use the user's
sessionrc). Yet it is already a bad first impression.
For temporary workaround, let's use a smaller 800×600 defaults (which
will actually span on 1600×1200 pixels + decoration size on scale ratio
×2).
Still I don't like using arbitrary numbers for window size default.
As we see here, it can end up into all sort of weird result. Even more
with all the scale ratio business which didn't exist back in GTK+2.
Instead, the defaults should have no size, and our code should just
resize to whatever makes the most sense on the current display, which I
believe should likely be maximized. Unfortunately I have a hard time
with gtk_window_maximize() which doesn't seem to do anything at all
(does GNOME ignore _NET_WM_STATE_MAXIMIZE_* hints when requested by
applications maybe?). So until we find the right system, let's go with
this lower window size defaults at least.
As it involves some API changes and code moved from libgimp to core, I
just won't backport all the new rotation policy logics in gimp-2-10
branch. I could have cooked something up, at least to have new settings
in Preferences for updating the rotation policy (currently it's
implemented as a global parasite which is quite bad as it makes the
settings virtually non-updatable in a sane way) but really I don't want
to spend too much time on this. So let's just say it's GIMP 3
improvements.
... and also doesn't write out all the data, if it does a "goto out" for
any reason. A leaked file descriptor can prevent a file from being
renamed or deleted on Windows.
See also #3740.
Reviewer note (Jehan): this may not be the main issue as reporters were
not writing about export failure. So there is probably another case even
when the plug-in successfully wrote the TIFF image.
Apparently the docs changed so the anchor is broken (and contributors
are confused). Also the docs now says that Container Registry is enabled
by default. I assume this must be a recent change which is not available
yet in GNOME's Gitlab instance. So we must ask people to do the opposite
of what the docs says.
When loading a theme on Windows we always get an error like:
themes_theme_change_notify: error parsing [path including drive letter to:]\theme.css: theme.css:8:99Failed to import: Operation not supported
The location points to the end of the filename of the first @ import url string.
This is caused by the string not being an url.
Based on suggestions from Jehan and lillolollo we replace g_file_get_path
with g_file_get_uri since an url is what is expected here.
Since that function already escapes the string we also remove
g_str_escape here.
Since meson 0.43.0 (below our current requirement), 'symbol_prefix'
argument of gnome.generate_gir() allows an ordered list. If I prepend
'gimp_ui', it makes any gimp_ui_*() function to not start with 'ui_'.
In particular, GimpUi.ui_init() becomes GimpUi.init() which is much less
redundant.
In several GeglOperation filters, it is possible to pick a color. Up to
now, it was only possible to pick a color from the active layer (the one
you run the operation on). With this change, we can also pick in Sample
merged mode, same as Color Picker tool and other color tools.
The rational: advanced users won't really care about defaults (they know
to switch this option on/off depending on situation) but maybe beginners
would be less confused to pick "what they see" on first use, rather than
picking on the active layer? Now whatever is the default won't change
anyone's daily usage of GIMP. Clearly every image and use case is
different, so both with or without sample merged are useful one time or
another (this is why the option exists). It's really about the less
surprising option for beginners, based on usage statistics.
I ran a small poll on Twitter/Reddit/Patreon/Tipeee and ended up with
numbers of 131 for switching to "Sample merged" as default and 43
against, which is about 75% in favor. So let's just switch. It makes
sense anyway.
Make some of the bigger Preferences pages automatically scrollable if
needed. Based on my tests, this should be enough to fit on quite small
displays, at least with the default themes, even the 1366×768 reported
as too small. It should even fit in 1280×720 (in my tests, it did).
Targetting even smaller screens may be overdoing it for an image
manipulation software. We'll see if people still ask for a smaller
dialog.
Our Preferences exposes a concept of "Preferred color profile" (for RGB,
grayscale and CMYK), which is used in some places to be proposed as
default alternative to built-in profiles. But it was not used in the
import color profile dialog (only 2 choices were: keep the image profile
or convert to built-in RGB).
This commit now adds this third choice, which is even made default when
hitting the "Convert" button directly, without tweaking with the dialog.
Because we can assume that if someone made the explicit choice to label
such a profile as "Preferred", this is more likely the one to convert to
(if one even wants to convert from an embedded profile anyway).
As for the `Preferences > Image Import & Export > Color profile policy`,
they now propose 4 choices: Ask, Keep embedded profile, Convert to
built-in or preferred profiles.
… gimp_image_policy_color_profile().
These functions allow a plug-in to explicitly execute the Rotation and
Profile conversion policies on an image (which may be any of
Rotating/Discarding/Ask or Converting/Keeping/Ask respectively). These
policies are automatically executed when loading an image from GIMP
interfaces, but they won't be when loading an image from the PDB. Then
it is up to the calling code to decide what to do (which can be either
some arbitrary code or following the user policy).
Orientation is now handled by core code, just next to profile conversion
handling.
One of the first consequence is that we don't need to have a non-GUI
version gimp_image_metadata_load_finish_batch() in libgimp, next to a
GUI version of the gimp_image_metadata_load_finish() function in
libgimpui. This makes for simpler API.
Also a plug-in which wishes to get access to the rotation dialog
provided by GIMP without loading ligimpui/GTK+ (for whatever reason)
will still have the feature.
The main advantage is that the "Don't ask me again" feature is now
handled by a settings in `Preferences > Image Import & Export` as the
"Metadata rotation policy". Until now it was saved as a global parasite,
which made it virtually non-editable once you checked it once (no easy
way to edit parasites except by scripts). So say you refused the
rotation once while checking "Don't ask again", and GIMP will forever
discard the rotation metadata without giving you a sane way to change
your mind. Of course, I could have passed the settings to plug-ins
through the PDB, but I find it a lot better to simply handle such
settings core-side.
The dialog code is basically the same as an app/dialogs/ as it was in
libgimp, with the minor improvement that it now takes the scale ratio
into account (basically the maximum thumbnail size will be bigger on
higher density displays).
Only downside of the move to the core is that this rotation dialog is
raised only when you open an image from the core, not as a PDB call. So
a plug-in which makes say a "file-jpeg-load" PDB call, even in
INTERACTIVE run mode, won't have rotation processed. Note that this was
already the same for embedded color profile conversion. This can be
wanted or not. Anyway some additional libgimp calls might be of interest
to explicitly call the core dialogs.
A log error can have a NULL domain (apparently equivalent to "" default
domain, according to g_log_set_handler() docs and we even explicitly
list the NULL domain in the log_domains array in app/gimp-log.c.
Yet our log handler was not expecting such possibility and was running a
g_str_has_prefix() on NULL. Not sure why it aborted there. It might be
because outputting a new warning inside the warning handler did not go
well. Anyway this seems to fix our side of the bug #5358. The main fix
will likely be on GEGL side (UMFPACK_ERROR_out_of_memory error).
Negative int values were not correctly handled because value.v_int is unsigned
causing a conversion to a large positive value.
To fix this we cast it to gint64 first before making it negative.
It seems to be an old file move error I did 2 years ago (commit
1e5cf10585). I wonder how I only realize this now. Anyway let's remove
this lonely file.
It's done, all Python plug-ins have been either ported to the new API +
Python 3, or they have been discarded (and moved to gimp-data-extras for
whoever wants to salvage them).
Let's get rid of the outdated pygimp directory (whose code has not been
built in the master branch for years now anyway)! Woohoo!
shadow_bevel.py and sphere.py are unfinished and have been in the
"Unstable" only builds for years. This is probably not worth porting
them. Let's just delete them.
I have actually move these (as well clothify and whirlpinch removed
yesterday) to the gimp-data-extras repository so if anyone wants to
revive them, and port them to GIMP 3, they can start off from there.
Following Elad Shahar advice (cf. #4368), let's delete whirlpinch.py and
clothify.py.
The rational is that whirlpinch.py does the same thing as the existing
"plug-in-whirl-pinch" which is itself a compat plug-in to
"gegl:whirl-pinch" operation. Also the Python file has an explicit
command saying it is exactly the same algorithm, yet with no preview and
slower. Finally it was not even installed on stable build. It doesn't
look like there is any reason to keep it (it was probably a demo/test
Python plug-in).
As for clothify.py, a quick look at the short code shows it is exactly
the same algorithm as clothify.scm, with the same arguments and
installed on the same `Filters > Artistic` menu (except that the Python
version is not installed on stable builds).
So let's just keep the script-fu version as it has been the used version
until now, and there is no deprecation going on in one side or another.
So let's keep what already works.
Since the patch was initially contributed, some parts of the
introspection changed. First all GUI-related code is in a GimpUi module
now.
Also Gimp.get_pdb().run_procedure() is now using a list instead of a
GimpValueArray.
Note by reviewer (Jehan): merging this port as it was in GIMP 2.10
anyway, but is this even still needed code? This plug-in is not even
available on stable release, it looks like for-development only
benchmark, and I'm not sure if it's relevant anymore, especially in our
GEGL-fueled new world.
Please anyone who knows a bit more on the history of this plug-in and
the evolution of our gimp_drawable_foreground_extract() algorithm, feel
free to weigh in and tell us what this was for exactly and if it's still
relevant.
This is a basic port without any UI.
Invoking the plugin will just sort the entire palette based on
default parameters.
The original plugin had several broken options, which I tried to fix.
Plug-ins that work from different bindings probably want to use their
own list-type to specify arguments, rather than working with a more
cumbersome `GimpValueArray`.
This new API should make it less verbose. For example:
```
args = Gimp.ValueArray.new(5)
args.insert(0, GObject.Value(Gimp.RunMode, Gimp.RunMode.NONINTERACTIVE))
args.insert(1, GObject.Value(Gimp.Image, image))
args.insert(2, GObject.Value(Gimp.Drawable, mask))
args.insert(3, GObject.Value(GObject.TYPE_INT, int(time.time())))
args.insert(4, GObject.Value(GObject.TYPE_DOUBLE, turbulence))
Gimp.get_pdb().run_procedure('plug-in-plasma', args)
```
becomes
```
Gimp.get_pdb().run_procedure('plug-in-plasma', [
GObject.Value(Gimp.RunMode, Gimp.RunMode.NONINTERACTIVE),
GObject.Value(Gimp.Image, image),
GObject.Value(Gimp.Drawable, mask),
GObject.Value(GObject.TYPE_INT, int(time.time())),
GObject.Value(GObject.TYPE_DOUBLE, turbulence),
])
```
The script `create_test_env.sh` was registered in meson as a run target
(i.e. to be run manually by `ninja create_test_env`), which is really
not useful. So a `ninja test` was outputting various:
> You have a writable data folder configured (/gimp/build/dir/app/tests/gimpdir-output/gradients),
> but this folder does not exist. Please create the folder or fix your
> configuration in the Preferences dialog's 'Folders' section.
Unfortunately run target are only meant to be run from command lines and
cannot be used in 'depends' argument of test() or 'dependencies' of
executable() because "in Meson all dependencies are to output files, not
to concepts" (cf. https://github.com/mesonbuild/meson/issues/1793).
So instead a run_target() just directly use a run_command() and make
this script run during configuration step. Also make the shell script
executable as it was not.
See also #5666 as it was one of the errors outputted by the reporter's
log (though probably not the main issue).
The XPM export plugin was saving the color `None` (transparency) to the
exported image palette for all images with an alpha channel, even if the
color was not used on any pixel.
Since it's nice to have `None` as the first color on the palette, mapped
to character ' ', and it could be too wasteful to scan all pixels twice,
the approach taken was to just undo it's premature insertion.
(cherry picked from commit 0a9bb2839e)
Existing code was returning only 1 out of 2 expected procedure names.
See !241 for the originally proposed patch by Rafał Mikrut (@qarmin).
Unfortunately the proposed patch also had another bug and was always
returning an empty list instead, and the contributor is unresponsive. So
let's just redo the patch and be done with it.
Also using g_list_prepend() instead of g_list_append() as it's usually
more efficient (to be fair here, for a size-2 list, it doesn't matter
much but it's good practice anyway) and order doesn't matter.
Thanks Rafał Mikrut for noticing the original bug!
Dušan Kazik