User can select color subsampling/pixel format
(YUV444, YUV420, RGB)
User can select encoder speed (Slow, Balanced, Fast)
Lossless option delivers visually lossless output
For Python, Lua and Javascript, make the option boolean (with 'yes'
being the default). No need of a warning when not installing the
plug-ins as this would have been disabled explicitly anyway. When
installing the plug-ins, only make interpreter checks as precautionnary
verifications which don't actually change anything (except outputting
some warnings if interpreters are not found). Basically for these 3
bindings, the interpreters are only runtime dependencies anyway. So it
doesn't matter if they are not available at build time. In particular,
we get rid of the 'force' option.
Vala rules do not change as the vala compiler is indeed needed at build
time and current checks work correctly. I just add a "Vala plug-ins"
line in the summary message of the meson configuration, as it was
missing.
The metadata format being appdata, I simply use standard intltool
merging on the XML templates.
For Automake, I use a custom rule instead of INTLTOOL_XML_RULE because
the m4 macro hardcoded $(top_srcdir)/po as translation source directory
(yet we want to use po-plug-ins/ for these). Meson on the other side
already used a custom command.
First the deserialize data of extensionrc was wrong. I need to edit the
pointer to the GList (and dereference it when I need).
Also when inserting an extension into the `running_extensions` hash
table, I could not reuse the same string as in the `processed` list
(because this string is going to be freed at end of initialization).
Just always use the GimpObject name of the extension, since it will stay
alive for as long as the object is alive.
This is an extension containing a few demo plug-ins. This is good to
demonstrate the extension format. It will also allow to disable these
plug-ins (if at some point, one doesn't want to show these demo
plug-ins anymore).
And finally it deals with the fact that our plug-in code is stupid, as
it just tries to find the first executable with the same name (minus
extension) as the plug-in folder. This doesn't go well on Windows, where
the permission system is non-existent. So our code just ends up trying
to run the first file with a similar name in a plug-in folder. As the C
goat-exercise contains both an exe and the C source (and the system
probably returns files in alphabetic order), GIMP under Windows tries to
run the C source instead of the executable (this obviously doesn't go
well).
We could try to do more complex logics, like not aborting if the first
file run fails and try the next one in the plug-in folder. Or maybe just
rename the C file to another name. But any of these is just in the end
the proof that our plug-in discovery right now is just bogus. The
extension system is explicit, not based on randomly trying out files.
Plug-ins entry points are explicitly listed in the metadata manifest.
Also taking the opportunity to change the minimum size since I
apparently counted it wrong. The size always seems to be 46
starting from psp file-version 4 up to and including the current
version 13.
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).