On most keyboards the [ and ] keys are shared with { and }. Which means
that if you press Shift+[ you get {. We were using this key combination
to increase the tool's size by 10 and the other to decrease it by 10.
However, on all keyboards where these keys share the same physical spot
on the keyboard, this wasn't working.
So, let's change the actual keys to do this to { and }.
Somehow the test part of the flatpak job stopped working. The last
instance which worked was 3 weeks ago, but after a lot of debugging I
realized that it is not because of any code change on our side. The
exact same commit which worked 3 weeks ago won't anymore!
The standalone bundle is actually built and works fine when tested
independently. What fails is the `meson test` inside the flatpak
environment. Somehow when GIMP is rebuilt inside the test flatpak
environment, it doesn't build the plug-ins yet one of our tests
(save-and-export) requires plug-ins to open some file formats. Note that
I double-checked, the plug-ins were well built and loading any format
works fine in the standalone flatpak, just not in this specific step.
I am completely unsure what broke, yet it is apparently outside GIMP
code. So for now, I just copy-paste the whole flatpak job which we were
including from another repository and remove the `meson test` part.
Our plug-in was not correctly handling DICOM images that use big endian
transfer syntax, which is deprecated.
We add support for that here, add a few g_debug statements to make future
debugging easier, and also return an error when we encounter a transfer
syntax that we can't handle instead of blindly continuing.
Synced from the beta flatpak. The previous manifest rules were building
fine on Flathub build servers, but not on GNOME CI ones. It resulted in
a bunch of "ISO C++17" errors when building ilmbase.
Maybe just restricting to C++14 through build fine would have been fine,
but anyway let's also update the dependencies in the same time as we
were outdated.
As per state in the wip/release/2-99-8 branch of the beta flatpak.
In particular, we update some dependencies (poppler, ghostscript and
SuiteSparse).
It should also fix the master flatpak build which seemed to fail on
downloading SuiteSparse sources. Since their upstream moved their
tarballs to be downloaded from Github, it should take care of this issue
by side effect.
… source colors
Reviewer's (Jehan) note: a git blame shows this is a clear bug
introduced in commit 9d19bf2a3e, which was a cleanup patch, and the
BOUNDS() macro was not doing exactly the same thing as the code before
the change.
The MSYS2 package got recently bumped from 3.8 to 3.9.6.
At first I wanted to update our packaging and installer scripts to be
more generic using glob patterns (so that they should work now and
should continue to work even if bumping to a higher minor version in the
future). Unfortunately this would work for `package-gimp-msys2.sh` but
in `files.isi`, it would only work for `libpython3.*.dll`, not for the
python3.9/ folder. InnoSetup apparently doesn't support using a folder
as source (or maybe just a folder with glob like `python3.*`) as it
resulted in a "No files found matching" error.
So leave everything with the accurate version (because anyway it's much
better to get an early failure than only at the very last step).
Same as MSYS2, add a patch to fix keyboard input when using IMEs (which
should hopefully fix#1603). Note that this patch should be in the next
release.
Also remove the Windows Pointer Input Stack support as it is in 3.24.30.
Finally apply the patch from gtk!3661 for testing (instead of the patch
from gtk!3275), as it is supposed to fix#5475. This is the reason why
we still build our own GTK3.
Commit 8025962a20 was meant to make the icon code work on relocatable
builds, yet I realize that the gdk_pixbuf_new_from_file() calls were
still using the DATAROOTDIR build-time macro. I had forgotten to update
these.
Also update a bit the error handling by adding a GError (for more
debugging info) and a newline for pretty-printing.
We don't know when the next GTK3 release will be and this is cool enough
that we want to test it soonish, even more with MR !458 coming soon with
a Preferences option.
Unit testing consider warnings as criticals and doesn't like when it
cannot find the installed application icons. To fix the `make check`,
just print the missing icon information to stderr, but don't make it a
GLib warning.
Since GIMP looks for its icons at runtime and would output warnings if
it doesn't find them, it's better to install first. Not really sure it's
ideal though, but it will do for now. Maybe I should just g_printerr()
instead of g_warning().
While passing the DATADIR macro works fine natively on Linux, it somehow
failed with the mingw-w64 build with a very weird error:
> <command-line>: error: expected identifier or '(' before string constant
I could not understand what it means, as the '-DDATADIR="/some/path"'
syntax is completely fine as far as I can see.
Anyway since I see that DATAROOTDIR is already defined in meson config.h
(but not in the autotools build, just the meson one!), and using
datarootdir instead of datadir seems to be just fine (actually maybe
even more appropriate when it comes to looking up the hicolor
application icons), I just switched to using it.
In the same time, I realized that my code using build-time macros won't
work for relocatable builds anyway. So this commit also adds a bit of
code path variant using gimp_installation_directory() in the case of
ENABLE_RELOCATABLE_RESOURCES code path.
From years of discussions, it turns out that:
- The thumbnailed-Wilber icon replacing the generic icon of GIMP often
makes the application harder to spot in the icon list of running
processes.
- In single-window mode in particular, it makes even less sense as we
just show the one active image anyway.
- Even in multi-window mode, nowadays many OSes or desktop group windows
of a same application under one icon. So we end up with several image
windows under a thumbnail only showing the top image. This happens in
KDE, GNOME, Cinnamon and Windows at least apparently (as far as is
being reported).
- Some platforms would just use only the OS-declared icon and not care
about runtime-declared ones. This is apparently the case on macOS, and
also on GNOME when the desktop file is seen by the desktop
environment. So all our code about generating thumbnailed icon is
wasted on these platform anyway.
- When intensively testing the current behavior, I had cases when the
icon was not properly updated. We could of course investigate and fix
the issues, but considering all the previous points, it might make
more sense to simply drop the feature which is mostly useless, or
worse bothersome, hence simplify the code greatly.
- Finally API to set icons from GdkPixbuf data has been removed in GTK4.
So long term, trying to keep this whole machinery feels like just
making our life difficult for a feature which all OSes seem to
deprecate and which might not be possible anymore soon (or just get
harder and harder to implement).
Note that I don't use gtk_window_set_default_icon_name() because it
would use the icon from our theme, yet so far we are not sure it makes
sense for the application icon which we probably always want to be the
same, whatever the chosen theme. Finally I just list various common icon
sizes because GTK API doesn't seem to be clever enough yet. I can't just
give it 1 SVG image (e.g. with gtk_window_set_default_icon_from_file())
and hope it does the resizing at the last minute. It turns out it
doesn't and we get an extra-small icon. So instead, let's generate
common sizes ourselves from the same SVG.
The markdown triple-quote (```) has to be on its own at the start of a
newline. Schumaml was telling me that too many reporters would paste
just after some text, which would therefore break the markdown syntax.
Instead let's add 3 newlines before the triple-quote, so that even
people who would not hit the "Copy Bug Information" button (but instead
select and copy) have a hint that these newlines are made on purpose.
Also add a comment (which is discarded by Gitlab) to make this even more
obvious.
Then even when pasting just after some text on the same line, the
triple-quote will end up on its own line.
The goal of this function is to give the focus to the active image
display. This is implemented as a core GimpDisplay virtual function
(with the actual implementation in GimpDisplayImpl), allowing to be used
even in core code, without actual GUI code (this was not necessary right
now, but I think it will be useful in future use).
This function is now called from the toolbox code (cf. 2 commits
earlier), avoiding code duplication. I also added a usage at the end of
toolbox_paste_received() so that a newly opened image by middle-click
paste in the toolbox directly gains focus.
Testing this middle-click opening of image by their PATH/URI in the
toolbox, I realized there was a bug. The original author was obviously
intending to unref the toolbox which was ref-ed when calling
gtk_clipboard_request_text(), not freeing the toolbox context, which
could have dire consequences!
Fixes this CRITICAL:
> g_object_unref: assertion 'G_IS_OBJECT (object)' failed
Toolbox buttons don't require focus keeping (no further keyboard input
needed and navigating through/selecting tools can be done in various
other ways with a keyboard), which is why I removed focusability of
toolbox buttons years ago (cf. commit c83ee61c07).
Nevertheless this is not enough, and since toolbox is meant to work on
the canvas anyway, let's give back canvas focus as soon as any click
happens on dead areas of the toolbox, as well as on the Wilber (dnd)
drawing, but also each time a toolbox button is selected (though not on
the longer clicks to see tool groups' contents without selecting a tool
in the end).
This will allow to directly start working on the image without requiring
a click first (for instance panning with Space or similar interactions
which would not give focus to the image canvas).
This can also count as an alternative to the "Esc" shortcut to get
canvas focus back, with a mouse click instead of a keyboard key.
The issue is that on X11, the window manager draws the title bar hence
uses the window-close button from the system theme, though on Wayland,
GTK draws its own decorations. Since we provide a window-close, we end
up with a disparity between this button and the minimize/maximize
buttons.
For now, let's just get rid of the window-close icon as an easy
solution, thus GIMP will always use the window-close icon from system
theme.
Maybe we can restore our own window-close icon later, but we should make
sure to at least have also window-maximize and window-minimize icons so
that the 3 buttons next to each others won't look too mismatched. And
probably we should also have smaller-size design because it doesn't look
so nice when huge (though it might simply have been the size mismatch
which gave this feeling).
The dll_link script would overwrite the same dependencies over and over,
for instance when needed for several binaries, but worse when available
in several source directories. In our case, we look up 2 source
directories: our install prefix first, then the prefix for MSYS2
pre-built packages. So it turns out we would be overriding any
custom-built package also installed through MSYS2 (such as our patched
GLib, see my previous commits).
As a side effect, it should also make the packaging step faster as we
don't re-copy unecessarily the same DLLs over and over again. The first
one to go in stays in.
Since gio searches its modules based on the paths as advertized by the
pkg-config, let's just move the pre-compiled modules (by MSYS2
packages). We build the same version of glib2 with the same options, and
only one additional patch. So this should not be a problem to use the
pre-built modules rather than rebuilding glib-networking too.
The MSYS2 package for glib2 does not include the patch to reduce delays
of disconnected volumes (glib!2020).
Let's make our own build in our CI for our installer (hence also
experimenting overriding MSYS2 packages when necessary).
Except for this patch, this is the same build as MSYS2 and same option,
as described in:
https://github.com/msys2/MINGW-packages/tree/master/mingw-w64-glib2
Instead of a target, let's make it a test. Also I realize it already
existed as desktop_file test. But it's simpler to run it directly (no
need of an external script).
Note: I still leave the test-desktop.sh script as it is used for the
autotools test.
We were only comparing the po list with the language list in the
gimp3264.iss. Nevertheless since we also generate the <code>.setup.isl
files, we should also verify generated files corresponds exactly to the
same list of languages.
This commit does it for meson and autotools build.
This is also how I fixed the meson list (cf. previous commit).
… conversion when some characters are not convertible.
Currently failed conversion ended up in incomplete .isl files (autotools
build) or would break the build (meson build). Ideally we should use a
target encoding which contains all source characters, but we use the
encoding as set upstream in the LanguageCodePage for the given language
file.
I'm not sure if maybe we can mix the encoding by adding our own
LanguageCodePage at the top of our generated <code>.setup.isl, then we
could just use UTF-8 for any language. It will have to be tested.
For now let's just discard non-convertible characters, assuming there
shouldn't be too many of these. It's the lesser evil in this situation.
The authors.xml validation was also not run. This is nearly the last of
getting rid of run_target(). There is still the desktop file validation
but it doesn't have any output argument. We'll see how we update this
last one.
The only other usages of run_target() are proper usage (creating
'install-*' targets).
Similarly to the previous commit, the tips and tags validation was never
happening as it was implemented as a custom top-level meson target. It
was not run during a normal build. Now it is.
Basically the build was never running this target (unless maybe in some
edge cases as could be demonstrated by the Arch repository build, though
it doesn't look they did anything particular; this is how we discovered
the bug #6447 as this was not run on our own local or CI builds).
Reading the docs of run_target(), it may actually have been normal.
run_target() looks to be only about creating top-level targets (which
one could run with `ninja validate_menus` in our case for instance), not
actually running the command (i.e. badly named function).
For the command to be actually run on a normal build, let's use a
custom_target() as proposed by Paolo Bonzini. After testing, the xmllint
validation is properly run on a normal build and dependency works fine
with both the source XML and generated XML files (touching these files
trigger a rebuild).
The output of xmllint is stored in some dummy file, which is only useful
to prevent re-running the command at each build even though source XML
were unchanged (so it's more of a flag file).
See discussion in #6447.
Jacob Boerema