On Windows, the title bar can be set to light or dark mode via DwmSetWindowAttribute ().
This adds code to update the main title bar and dialogue title bars based on the current theme.
The main title bar uses "prefer-dark-theme", while the dialogue title bars
uses the color of the widget background to assume the correct color.
Resolves the second half of #300.
This adds conditional code to the gtk_window_present () call in gui.c
to prevent it from running if the user requested it stay minimized in the shortcut
or commandline call on Windows.
It also keeps the splashscreen minimized in that case.
… with another default shortcut.
This won't happen with the standard US layout, but among all the layouts which
exist (or will exist), there is no say that the characters behing <shift>2-5
keys are not another one of our default shortcuts for other actions. We don't
want to print this case, because it is special enough that it's really not a
bug. In this case, we just ignore the transformed shortcut on the zoom action
and be done with it.
I am using the same GimpDrawableChooser with an additional drawable_type
argument to only show the appropriate tab if we want to limit what can be
chosen.
None of our plug-ins actually use a GimpLayer or GimpChannel only arg so far,
but if we have some day, or if some third-party plug-ins want to have such arg,
now they quite easily can!
Similarly to the various GimpResource select PDB calls, this allows to call a
core dialog in order to choose a drawable which will be returned back to the
calling plug-in.
This new GimpPickableSelect dialog is a subclass of GimpPdbDialog and uses the
same GimpPickableChooser widget as GimpPickablePopup, except that since it's
inter-process window management, it is harder to make a popup positioned
accurately relatively to a parent (especially on Wayland). This is why it's a
separate widget as a simpler dialog (which we will still try to make transient
as much as possible across platforms).
Brush, font, gradient, palette and pattern choices are currently chosen through
a dialog created by the core, which then returns the user choice to the calling
plug-in. This has the unfortunate consequence of having a pile of likely at
least 3 windows (main GIMP window by core process, plug-in window by plug-in
process, then the choice popup by the core process) shared in 2 processes, which
often end up under each other and that's messy. Even more as the choice popup is
kinda expected to be like a sub-part of the plug-in dialog.
So anyway, now the plug-in can send its window handle to the core so that the
resource choice dialog ends up always above the plug-in dialog.
Of course, it will always work only on platforms where we have working
inter-process transient support.
Having windows ID as guint32 is a mistake. Different systems have
different protocols. In Wayland in particular, Windows handles are
exchanged as strings. What this commit does is the following:
In core:
- get_window_id() virtual function in core GimpProgress is changed to
return a GBytes, as a generic "data" to represent a window differently
on different systems.
- All implementations of get_window_id() in various classes implementing
this interface are updated accordingly:
* GimpSubProgress
* GimpDisplay returns the handle of its shell.
* GimpDisplayShell now creates its window handle at construction with
libgimpwidget's gimp_widget_set_native_handle() and simply return
this handle every time it's requested.
* GimpFileDialog also creates its window handle at construction with
gimp_widget_set_native_handle().
- gimp_window_set_transient_for() in core is changed to take a
GimpProgress as argument (instead of a guint32 ID), requests and
process the ID itself, according to the running platform. In
particular, the following were improved:
* Unlike old code, it will work even if the window is not visible yet.
In such a case, the function simply adds a signal handler to set
transient at mapping. It makes it easier to use it at construction
in a reliable way.
* It now works for Wayland too, additionally to X11.
- GimpPdbProgress now exchanges a GBytes too with the command
GIMP_PROGRESS_COMMAND_GET_WINDOW.
- display_get_window_id() in gimp-gui.h also returns a GBytes now.
PDB/libgimp:
- gimp_display_get_window_handle() and gimp_progress_get_window_handle()
now return a GBytes to represent a window handle in an opaque way
(depending on the running platform).
In libgimp:
- GimpProgress's get_window() virtual function changed to return a
GBytes and renamed get_window_handle().
- In particular GimpProgressBar is the only implementation of
get_window_handle(). It creates its handle at object construction with
libgimpwidget's gimp_widget_set_native_handle() and the virtual
method's implementation simply returns the GBytes.
In libgimpUi:
- gimp_ui_get_display_window() and gimp_ui_get_progress_window() were
removed. We should not assume anymore that it is possible to create a
GdkWindow to be used. For instance this is not possible with Wayland
which has its own way to set a window transient with a string handle.
- gimp_window_set_transient_for_display() and
gimp_window_set_transient() now use an internal implementation similar
to core gimp_window_set_transient_for(), with the same improvements
(works even at construction when the window is not visible yet + works
for Wayland too).
In libgimpwidgets:
- New gimp_widget_set_native_handle() is a helper function used both in
core and libgimp* libraries for widgets which we want to be usable as
possible parents. It takes care of getting the relevant window handle
(depending on the running platform) and stores it in a given pointer,
either immediately or after a callback once the widget is mapped. So
it can be used at construction. Also it sets a handle for X11 or
Wayland.
In plug-ins:
- Screenshot uses the new gimp_progress_get_window_handle() directly now
in its X11 code path and creates out of it a GdkWindows itself with
gdk_x11_window_foreign_new_for_display().
Our inter-process transient implementation only worked for X11, and with
this commit, it works for Wayland too.
There is code for Windows but it is currently disabled as it apparently
hangs (there is a comment in-code which links to this old report:
https://bugzilla.gnome.org/show_bug.cgi?id=359538). NikcDC tested
yesterday with re-enabling the code and said they experienced a freeze.
;-(
Finally there is no infrastructure yet to make this work on macOS and
apparently there is no implementation of window handle in GDK for macOS
that I could find. I'm not sure if macOS doesn't have this concept of
setting transient on another processus's window or GDK is simply lacking
the implementation.
Though GTK+3 is supposed to take care of scaling fonts with high density
displays, it turns out it is not enough for many, for various reasons (taste,
eyesight, distance to the display…). So we add this additional settings to tweak
further the font size.
With Aryeom, we experimented/discussed both a percentage UI vs. an absolute font
size field (e.g. as they provide in GNOME Tweaks). In the end, we went for a
percentage UI because we realize that we don't necessarily know what is the
current size at all. Mostly you just want bigger or smaller, and don't
necessarily care so much at which value is the font size.
This settings only has a single limitation (that we could find), which is when
used on a theme with widget rules using absolute font-size rules (px, or
keywords such as small/medium/large). As long as the CSS rules are relative
though (either to the parent widget, or to the root size), then it works fine.
Basically a theme hard-coding font sizes won't fare well with this settings, but
since we can consider this bad practice, it's an acceptable limitation.
This is not the main reason for the specific output in #9994. These ones are
more probably because of similar usage in GTK (which updated its own calls to
g_file_info_get_is_hidden|backup() in version 3.24.38). But we should likely
also update the various calls we have to use the generic
g_file_info_get_attribute_*() variants.
To be fair, it is unclear to me when we can be sure that an attribute is set.
For instance, when we call g_file_enumerate_children() or g_file_query_info()
with specific attributes, docs say that it is still possible for these
attributes to not be set. So I assume it means we should never use direct
accessor functions.
The only exception is that I didn't remove usage of g_file_info_get_name(),
since its docs says:
> * Gets a display name for a file. This is guaranteed to always be set.
Even though it also says just after:
> * It is an error to call this if the #GFileInfo does not contain
> * %G_FILE_ATTRIBUTE_STANDARD_DISPLAY_NAME.
Which is very contradictory. But assuming that this error warning was
over-zealous documentation, I kept the direct accessors since they are supposed
to be slightly more optimized (still according to in-code documentation) so
let's priorize them when we know they are set for sure.
My previous commit was improving the case where we just get many "dirty" signals
and successful inhibition. But if the inhibition request was failing, we would
just retry again and again.
This new version will hold off on on requesting the inhibition for the next 10
minutes. Then it will try again, in case the failure reason might have been
temporary.
… the reason message changes (i.e. when number of dirty image changes).
Some actions in particular would trigger several "dirty" signals, there is no
reason to uninhibit then re-inhibit repeatedly. And as a more general rule,
there is no reason to do so even for different actions while we are not planning
to update the inhibition reason.
It may not be the main reason for #9446 (because if it hangs for several
minutes, there is likely something going on, deeper at the dbus call level), yet
it would definitely alleviate the issue (dividing the wait by as many times as
the dirty signal was emitted!).
When passing an application ID (which is necessary for application inhibition to
work, i.e. logoff/reboot/shutdown inhibition), GApplication will try to ensure
process uniqueness, which will trigger a new activation to an already running
process. Since our current code assumes that the application can be activated a
single time only, this was what was triggering a whole lot of errors (on the
first running process) in #9598 because there was all the initialization code
which ran again, whereas it was not supposed to.
This doubly-running initialization code was also what completely messed up the
session files, hence broke the GUI after a restart (#9599).
Therefore passing G_APPLICATION_NON_UNIQUE advertizes we don't want GIO to
handle process uniqueness for us.
Note that this is actually a very interesting feature which we have had in GIMP
codebase forever. It would be interesting to kill all our own uniqueness code in
favor of GIO code (and let them handle/maintain passing command line arguments
from one process to another, for all possible platforms). So I added a TODO for
this (for now, we just ignore this feature as it doesn't work well with current
codebase).
Resolves#5742.
From fa0a0212, the splash's upper text was scaled to PANGO_SCALE_MEDIUM
both when the image width was 2x or less than 2x. This was likely a
copy/paste issue, as the lower text code scaling does not duplicate.
This patch fixes it by changing the else condition to scale to
PANGO_SCALE_SMALL.
Note that this change is unlikely to be seen as it requires a very small
splash screen image to reach the else condition.
gtk_accel_map_load()/gtk_accel_map_save() are not working with the new
GAction-based code. GtkApplication does not seem to have helper functions to
simply load and save accelerators in a file, so we just implement it ourselves.
A few things are missing right now, namely:
- On parsing, it doesn't handle any kind of duplicate accelerators (possible
especially if someone edited the new shortcutsrc manually).
- On reading, maybe we should only write down the changed (from defaults)
actions, while keeping the old ones commented-out, as menurc used to be. This
is actually useful info both for debugging or even for users who want to look
at this file and see what they changed.
- We should add import code to transform the menurc into shortcutsrc when
updating GIMP, otherwise all custom shortcuts would get lost.
There is still the question on whether we should add the group name too. I think
we should, expecially for plug-in's procedure actions, though right now these
are just added in the GtkApplication's main action group anyway. I'll see later
to refine this.
Pre-GIMP-3.0 code logics would re-allocate several GimpMenuFactory or
GimpUIManager for no good reason. While it was still working with old GtkAction
code, with our new GAction-based code, we were ending up overriding an action
with a new version of the same action, while keeping reference to old actions.
This made for discrepancies of the enabled or visible state of actions.
The new code keeps singleton of some objects and references to already
registered GimpUIManager or GimpActionGroups objects and make sure no actions
with the same name are created twice.
Unless mistaken, this is not needed anymore. New GtkApplication code should take
care of creating the native macOS application menu for us.
Cf. also contents of MR !558.
If we want to reuse this in subclasses, we'd need to overwrite finalize() in the
subclasses, call gimp_core_app_finalize() and chain up with the parent class.
Not doing this until now was leading us not to call GApplication or
GtkApplication finalize() function, which in turn, in the later case, was
leaking all the GAction-s which were added to the GActionMap. I realized this as
a leaking GimpContext-s warning was printing when ending GIMP.
I realize that UI_TEST is only used in our meson scripts and is therefore not a
reliable environment variable to check for whether we are running a unit test or
not. GIMP_TESTING_ABS_TOP_SRCDIR should be present for all unit testing, on both
build systems.
The real problem was not actuall logout-inhibition, but availability of the
GtkApplication. Anyway this feature is not really to block the system while
unit-testing but for saving people's actual work. Let's just disable running it
in test case.
This will fix the CI.
- app_activate_callback() moved with other private functions.
- Removing the `if (app)` test in app_activate_callback() as we don't
set it to NULL anymore. The app variable is always set.
- As a consequence of the previous point, change signature of
app_exit_after_callback() which doesn't have to change the value of
app anymore.
- Don't emit direcly the "exit" signal from app_activate_callback(). We
must call `gimp_exit (gimp, TRUE);` instead, which does more than just
emitting this signal. It also takes care of cleaning any remaining
images without a display. If we don't do this, we are leaking
GeglBuffer when opening images from command lines while quitting
immediately with --quit.
- Get rid of gimp_core_app_set_values() which was completely bypassing
the fact that all the properties of a GimpCoreApp were construct-only.
Instead make these proper properties. I use a trick used in other
interface, creating a gimp_container_view_install_properties() which
is called from child classes.
The point of GimpCoreApp is not just to share a common interface, it's
rather to weakly simulate some kind of multi-inheritance in GObject.
Since we want both GimpApp and GimpConsoleApp to inherit from a same
parent class while we also want them to inherit either from
GtkApplication and GApplication respectively (yet without linking to
GTK in this second case), we are stuck as far as normal GObject
inheritance goes. This is why we use an interface to which we add a
private struct through a GQuark trick. We want the property settings
and function implementations to also be part of this shared code.
- Get rid of all the abstract methods of GimpCoreApp of the form
get_*(). These are useless as we don't expect these to have different
implementation depending on the actual child class. Once again, our
main goal was to simulate multiple inheritance rather than actually
have an interface with various implementations.
- Make "no-splash" a property of GimpApp, because it's cleaner this way.
- Fix gimp_core_app_private_finalize().
- don't use #pragma once, it's not standard. Just use include guards.
- Fix includes: order was wrong, include from the source, not other
headers, etc.
- Clean various other details, coding styles, fix several more bugs and
more…
Reviewer's message (Jehan): This was a work-in-progress by Niels, which
we only keep in this state because Lukas worked over it. I have rebased
and fix-amended many broken part of this commit, because various things
had been changed in these areas of code since this commit was initially
written.
This kinda reverts commit 6aebd30de1 ("app: remove
icon sizing preferences"), except that the code base is different enough since
this old commit was mainly for GIMP 2.10.x.
In any case, after initially thinking that GTK+3 handling for high density
display would be enough, we finally decide that adding back a Preferences-wide
setting for overriding the theme-set icon size is a good idea (additionally to
GTK+3 automatic support).
The base idea for removing the feature was that GTK+3 has high density display
support, through the "scale factor". Typically a high density display will
normally be set as using a ×2 scale factor so all icons will be double size.
Unfortunately it turns out it's not enough.
For instance, on very small screen estate, even with a scale factor of 1, if the
theme sets 24px toolbox icons, it may still take too much space.
Oppositely on huge screens, even with ×2 factor scale detected by the OS, the
icons may still feel too small (this is possibly what happens with #7023).
Furthermore there is also a matter of taste. Some people like small icons even
when they have the space. Others may want bigger icons, easy to click on.
Finally you can like a theme for its color scheme for instance, but it may not
have the icon size you want. Right now, we'd need to duplicate every theme in
small or bigger size. Instead of doing so, let's just have this global setting
overriding the theme rules.
Comparison with the 2.10 implementation:
- We still provide 4 sizes: small, medium, large and huge.
- We don't have the "Guess ideal size" setting anymore. Instead this is now a
mix of the GTK+3 scale factor logic and the theme-set or custom size. I.e.
that on a high density display with ×2 scale factor, we could have toolbox
icons up to 96 pixels (48×2)!
- We now try to have less custom code in widgets as we append the CSS rules to
the theme (similar to what we were already doing for dark theme or icon
variants). What happens in widget code is mostly to connect to changes in
themes and redraw the widgets which need to be.
- The custom size will now affect: toolbox icons, the FG/BG editor widget (in
both the toolbox and the color dockable), dockable tab icons, the main
dockable buttons, eye and lock header icons in item tree views, eye and lock
cell icons in the item lists.
There are still a bunch of areas where it is not taken into account, such as
plug-ins, and various dialogs, but even in custom-made interface in dockables.
Ultimately it might be interesting to have a way to sync more buttons and
widgets to a global size settings.
Lastly, I fixed a bunch of existing bugs where we were updating icon sizes with
gtk_image_set_from_icon_name() using the const icon name taken from
gtk_image_get_icon_name(). As this was reusing the same string pointer, we were
ending with freeing the icon name.
This way, what will happen is that:
- We can have a single "Default" theme which will have both the light
and dark versions.
- With our Default theme, when "Use dark theme variant if available" is
unchecked, we just follow the system-wide dark settings. (though I'm
unsure we actually do with current code; we do load our theme over the
system theme, which may be dark, but I don't think we'd load a dark
theme variant then)
- If the option is checked, we will load the specific dark variant,
bypassing system settings specifically for GIMP.
Technically for theme designers, all it takes to have a dark variant is
to add a gimp-dark.css next to gimp.css. `gimp-dark.css` is loaded
instead of `gimp.css` when the settings is checked.
Note: there is apparently a new freedesktop portal for setting the
prefered variant (and now it's apparently either light, dark or
default), which is now implemented by GNOME, KDE and Elementary at
least. It would be nice if we could grab this settings and use it if
available. The below link has code sample showing how to do it with
DBus:
https://gitlab.gnome.org/GNOME/Initiatives/-/wikis/Dark-Style-Preference
This object's goal will be to manage customized modifiers per input
device button, which is why I add it to GimpDisplayConfig. It is in its
own new config file (`modifiersrc` in config dir) because it requires
GDK types access (well I could have done without, but it would have been
less semantic, hence not as good of an API). Anyway it is only useful
when running GIMP as GUI.
The GUI widget and the usage code to make this actually useful will come
in upcoming commits.
Basically disabling commit 4f9b7373e6.
After some new patches for GTK3 I wrote, and removing the settings on
NSViewUsesAutomaticLayerBackingStores, Lukas reported that it works like
never before, faster than 2.10 even. Note that this could only be tested
on Monterey, nobody tested on Big Sur with this specific combination
yet.
In any case, we decided to remove this settings and add the new GTK3
patches.
See: https://gitlab.gnome.org/Infrastructure/gimp-macos-build/-/merge_requests/86#note_1384727
It's a bit ugly, but it's not like this is run many times (only once
when loading the icon theme, or changing it).
Fixes this error appearing in various unit tests' output:
> gimp_icons_sanity_check: Icon theme path has no 'hicolor' subdirectory: /builds/GNOME/gimp/_install/share/gimp/2.99/icons
(even though it was not a test-failure error, it's still better to limit
output for debugging)
When running tests, the data are not meant to be necessarily installed.
Therefore icons won't be found when calling gimp_widgets_init().
Add some special-casing to find them relatively to the install
directory.
Reviewer comment (Jehan): we have used this patch successfully on our
installers since start of 2021 (see commit b4d665d of our gtk-osx fork)
and it really improved the situation. I only fixed minor coding style
stuff in the patch.
Looking at what it does, I guess it is not ideal long-term if related to
10-bit display (as I understand from the comment), which a graphics app
would want to support properly. But for now, this is better than
extra-slow display until we get macOS developers able to look at this
more in depth in the future (I don't think that our dependencies are
really ready yet for 10-bit display support anyway, though I may be
wrong).
Some other forums seem to say it comes from macOS invalidating now more
than it should (i.e. the whole area instead of only the changed area)
and this NSViewUsesAutomaticLayerBackingStores flag would disable this
behavior. It might be one of these reasons, the other or both. This is
anyway a good first start for future contributors.
