Until now, we were following a similar concept of color schemes as what most OS
are doing. For instance, Freedesktop recently introduced a tri-state color
scheme of "Prefer Light", "Prefer Dark" and "Default", the latter being either
whatever the software prefers (e.g. we prefer either Dark or Gray for graphics
software usually) or what the system prefers. See #8675.
Until now, with GTK, we only had a boolean "prefer dark" setting through the
"gtk-application-prefer-dark-theme" settings. There is not even a "prefer
light".
Nevertheless for graphics application, there is clearly a third case (fourth if
we added a "follow system color preferences" which we don't implement for now):
gray mode and in particular middle gray. Having a middle gray UI is often
considered a necessity when working on colors in order to protect our perception
of color from being influenced by surrounding UI.
To fill this need, we were proposing a Default vs. a Gray theme in GIMP, but
this was a bit confusing and felt illogical, as discussed on IRC some time ago.
Also depending on whether you chose "prefer dark" or not for the gray theme,
this one was itself 2 themes, which made things odd and harder to work on.
Instead this commit:
- adds a color scheme concept in GIMP with 3 variants so far: light, gray and
dark. A possible fourth (future) variant might be to follow the system
preference (do all OS provide such a queriable option?).
- Our Gray theme is merged into Default (as the gray color scheme variant).
- Custom themes can add the following CSS files: gimp-light.css, gimp-gray.css,
gimp-dark.css which are the base file for their respective scheme. gimp.css is
still used as a fallback though it is not necessary (our own Default theme
does not provide a gimp.css anymore). Custom themes don't have to provide all
3 variants. A theme can just provide one or 2 variants if it only wants to
support 1 or 2 use cases.
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.
After testing, setting a window as transient to another from another process is
still broken on Windows and it's hard to diagnose without using Windows
directly. Since it's not just broken, but it even hangs the whole process, which
is quite a blocker issue, let's disable again the whole code on Windows.
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.
Instead of passing a guint32, pass the proper type, since our the HANDLE type
can be 64-bit on Windows (according to links I found).
I was hoping it might be the reason for the breakage under Windows, though I
also found Microsoft documentation saying that the 64-bit handle can be safely
truncated: https://learn.microsoft.com/en-us/windows/win32/winprog64/interprocess-communication?redirectedfrom=MSDN
Nevertheless I'd appreciate testing again from NikcDC or anyone else, as I
reactivated setting transient between processes on Windows.
Note that I also pass the proper types on X11 now (Window), even though guint32
worked fine. Better be thorough.
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.
When building using MSYS2 CLANG64 profile using clang 16, we get the
following error (besides other required patches):
../../gimp/app/widgets/gimpwidgets-utils.c:931:12: error: incompatible
pointer to integer conversion returning 'HGDIOBJ' (aka 'void *') from a
function with result type 'guint32' (aka 'unsigned int')
[-Wint-conversion]
For now, let's use the same GPOINTER_TO_INT macro we use in
gimpprogressbar.c. In the end, we need a 64-bit type available to
plug-ins, but that needs more work.
There were some mixups between a few representations of menus:
* "_Some Menu" and "Some Menu" should both map to an existing submenu "Some
Menu" (in the GimpMenuModel, the stored path doesn't contain mnemonic).
* The menu item on the other hand should contain the mnemonic and not lose it.
* Not only this, but "Some _Menu" should still map to the same menu, even if it
was already created with a different mnemonic. It means that the first
registered menmonic "wins", as we don't want duplicate submenus with same
title (even if they have different mnemonics).
So this new code is better handling the canonical menu path (no mnemonic, no
section name, no double slashes, no trailing slashes, etc.) vs. the
canonical-with-mnemonic menu path to avoid weird duplicates.
Some additional bugs are fixed where we were creating weird empty submenus
containing the same items as the parent menu as well, or when a submenu title
was a perfect prefix of another submenu title at the same level.
This will replace the "placeholder" concept where I was using an invisible item
with a label and no action, making it invisible. Instead let's just name
(internally) our sections. This has the following advantages:
* Conceptually more correct: basically we just want to place items among the
same category of actions.
* Easier to search for plug-in developers who'll want to place their plug-in
procedures in menus, because it uses an actually searchable attribute
("section-name").
This is used by the core by allowing a special syntax in menu paths: if finished
by "/[Section]" then the item will be place in the specific section named
"Section". In case one actually wanted to create a submenu called "[Section]",
they can use a double bracket: "[[Section]" and "[[Section]]" will both map to a
normal submenu (not a section name) titled "[Section]".
All other usage of square brackets will not be processed in a particular way.
E.g. "Hello [World]" will end a submenu titled "Hello [World]" or "Bye]" will be
a submenu and so on.
Finally this system is currently limited to the position of the item itself,
i.e. must be placed as last element in the path. In particular, you currently
cannot use it to position a new submenu inside a section. E.g. say that I want
to create a submenu "From Platforms" under the "Open" section of the File/ menu.
This is currently impossible. With this syntax, we can create new items directly
in the "Open" section, or create a "From Platforms" submenu in the end of the
File/ menu, containing our new procedures.
This could be a good improvement to come.
Note that it will only work for a model attached to a GimpMenu. In particular,
it won't work for the menu bar set to a GtkApplication with gtk_application_set_menubar().
In other words, it won't work on macOS where we let the OS handle the menu.
As a further optimization (even though the big slowness situation is
mostly resolved anyway by a previous commit), let's not rebuild whole
submenus when only a few items were added or removed.
1. First make so that the action "visible" property is only notified
when the visibility actually changes. This way, we can trust that the
visibility of the item and of the related action are properly synced.
2. Now when an action which is part of a model changes its visibility,
we trigger the proper "items-changed" only for the added or removed
item. This replaces the old procedure of removing then adding
everything (i.e. a full reset).
3. Add some utilities in gimpwidgets-utils (out of gimpmenushell.c).
This cleans up some code and also fixes some bugs which were
reimplementing (badly) some code I already implemented (right) in a
static function.
4. Fix adding new actions in sub-sub… menus. Previous code was only
showing the direct parent, not up to the root menu (this could happen
for plug-in procedure items in particular).
Also getting rid of gimp_action_get_proxies() API which is now unneeded, I
think. Furthermore GimpMenu does not have to connect to label changes. Making
the items proxies of a GimpAction is enough to have them updated.
All the remaining pieces of code where the deprecated concept of accelerator
closure was still used have now been replaced by proper GimpAction API using the
newer GAction API.
Depending on the OS, the display name may contain characters which are not valid
for action names. In particular, on X11, the display name could be ":0.0" and
therefore we get actions named "app.dock-move-to-screen-:0.0" or
"app.view-move-to-screen-:0.0". The ':' is invalid, which will make calling
gtk_application_get_accels_for_action() (or more simply our new function
gimp_action_get_accels()) fail.
See docs of g_action_parse_detailed_name().
The new utils function gimp_make_valid_action_name() will help us get rid of
invalid characters for actions whose name was generated from strings we don't
totally control.
Furthermore gdk_screen_make_display_name() is now deprecated API, and more
generally gdk_display_get_n_screens() is deprecated too and now always returns 1
since GTK+ 3.10 (i.e. now each display only contains a single screen). Instead
we just use gdk_display_get_default_screen() for every GdkDisplay. So this
commit doubles as GTK/GDK deprecation cleanup.
The changes include:
* A popover will be displayed on tool buttons and relevant property
widgets.
* Now the welcome dialog gives focus up to its parent window because
really the whole focus styling issue in GTK is a bit maddening. The
colors are all faded, the popover widgets are barely visible, and so
on.
* Timing is tweaked a bit to give more time reading the popover tips.
… blinking it.
This will be necessary to demo new features available only in some
situations. E.g. a new option in line art detection mode of bucket fill
would require said mode to be enabled.
First I strip now every pieces of text. What it allows it to span the
script elements for instance on several lines, indent them and all that.
The second thing is that since all the dockable start with "gimp-", we
may as well allow use shorter names (both are allowed). Same for tool
(which we special-case), all the tool buttons ID start with "tools-"
since we reused the action names. So let's just allow the shorter
versions.
Finally I create a new gimp_blink_toolbox() which is a specialized
gimp_blink_dockable() for the toolbox in particular. I also move there
the whole code about selecting the right tool.
… a given order, not at the same time.
This will be used for the release item notifications to show people
where new or changed features are, but in an ordered blink scenario. For
instance: select a tool first, then blink some tool icon, open the tool
options and finally blink the specific new/changed option. I am hoping
it would help awareness of changes (just words on a web news may make
some features a bit foreign when not used to look in details in advanced
options).
Since it appeared with GLib 2.68.0, we could not change this until we
bumped the dependency which has only become possible a few days ago
(since Debian testing is our baseline for dependency bumps). Cf.
previous commit.
As this is a drop-in replacement (just a guint parameter changed to
gsize to avoid integer overflow), search-and-replace with:
> sed -i 's/g_memdup\>/g_memdup2/g' `grep -rIl 'g_memdup\>' *`
… followed by a few manual alignment tweaks when necessary.
This gets rid of the many deprecation warnings which we had lately when
building with a recent GLib version.
Most important of all, we shouldn't assume that if a given GDK backend
is enabled at compile time, that this is also the one that is being
used. For example, on Linux, both `GDK_WINDOWING_X11` and
`GDK_WINDOWING_WAYLAND` can be set, but you still need to do a runtime
check if you're running under one WM or the the other.
A small cleanup is that we immediately check if a widget is realized by
checking if it's `GdkWindow` is NULL or not and return immediately
(since we need to check its type later on anyway).
Finally, we can remove `GDK_NATIVE_WINDOW_POINTER` as that is a GTK+ 2.0
construct, so it's dead code anyway.
The GDK_WINDOWING_X11 build-time macro check is not enough as GDK can be
built with both X11 and Wayland backends. We need to add a runtime check
of the type of display.
... which takes a pair of GtkAdjustments, and binds the value of
the first to the lower-limit of the second, and the value of the
second to the upper-limit of the first.
Similar to commit 9b328167ab. Apparently the same function is duplicated
into app/.
Also add some comments to detail a bit more where this Win32 different
comes from (i.e. bug 359538 on Bugzilla, commit eac61e1e12).
Moreover I tested the same steps as comment 4 in bug 359538 on master
code. The same hanging issue still exists with GTK+ 3.24.1 (in a Windows
VM at least). I have not investigated further, just wanted to check if
this ifndef could not simply be removed by now. Apparently not.
Step one: get rid of all those deprecation warnings that make
it hard to see any other warnings:
- add a lot of dummy API to GimpAction, GimpActionGroup, GimpUIManager
etc. which simply forwards to the deprecated GTK functions, they
will all go away again later
- rename GimpAction to GimpActionImpl
- add interface GimpAction that is implemented by all action classes,
creates a common interface and allows to remove some duplicated
logic from GimpToggleAction and GimpRadioAction, and at the same
time adds more features
gimp_widget_flush_expose() has been removed since commit 3089a20167.
I now reimplemented it by simply checking if event sources are waiting
to be processed.
This was heavily needed as the statusbar was not showing any progress
(at least on highly demanding process, such as saving or loading files),
and therefore we were stuck with a seamingly frozen GUI.
Despite the name, this does not apply to a widget in particular anymore,
but to the whole program events.
Since commit fe139e5662, when
blinking a widget, we cancel blinking for all its ancestors. Avoid
redrawing all the ancestors as a result, unless they're actually
blinking. This prevents some noticeable lag when blinking a
widget.
We currently construct the tool-options GUI for all the tools at
startup, which takes a significant amount of time. Instead,
only register the GUI construction function with the tool-options
object, using the new gimp_tools_set_tool_options_gui_func()
function, and use the registered function to construct the GUI when
actually needed.
It was using deprecated functions regarding how icons used to work, such
as gtk_icon_set_get_sizes() or gtk_style_context_lookup_icon_set().
The last uses of this function disappeared in last commit.
All babl formats now have a space equivalent to a color profile,
determining the format's primaries and TRCs. This commit makes GIMP
aware of this.
libgimp:
- enum GimpPrecision: rename GAMMA values to NON_LINEAR and keep GAMMA
as deprecated aliases, add PERCEPTUAL values so we now have LINEAR,
NON_LINEAR and PERCPTUAL for each encoding, matching the babl
encoding variants RGB, R'G'B' and R~G~B~.
- gimp_color_transform_can_gegl_copy() now returns TRUE if both
profiles can return a babl space, increasing the amount of fast babl
color conversions significantly.
- TODO: no solution yet for getting libgimp drawable proxy buffers in
the right format with space.
plug-ins:
- follow the GimpPrecision change.
- TODO: everything else unchanged and partly broken or sub-optimal,
like setting a new image's color profile too late.
app:
- add enum GimpTRCType { LINEAR, NON_LINEAR, PERCEPTUAL } as
replacement for all "linear" booleans.
- change gimp-babl functions to take babl spaces and GimpTRCType
parameters and support all sorts of new perceptual ~ formats.
- a lot of places changed in the early days of goat invasion didn't
take advantage of gimp-babl utility functions and constructed
formats manually. They all needed revisiting and many now use much
simpler code calling gimp-babl API.
- change gimp_babl_format_get_color_profile() to really extract a
newly allocated color profile from the format, and add
gimp_babl_get_builtin_color_profile() which does the same as
gimp_babl_format_get_color_profile() did before. Visited all callers
to decide whether they are looking for the format's actual profile,
or for one of the builtin profiles, simplifying code that only needs
builtin profiles.
- drawables have a new get_space_api(), get_linear() is now get_trc().
- images now have a "layer space" and an API to get it,
gimp_image_get_layer_format() returns formats in that space.
- an image's layer space is created from the image's color profile,
change gimpimage-color-profile to deal with that correctly
- change many babl_format() calls to babl_format_with_space() and take
the space from passed formats or drawables
- add function gimp_layer_fix_format_space() which replaces the
layer's buffer with one that has the image's layer format, but
doesn't change pixel values
- use gimp_layer_fix_format_space() to make sure layers loaded from
XCF and created by plug-ins have the right space when added to the
image, because it's impossible to always assign the right space upon
layer creation
- "assign color profile" and "discard color profile" now require use
of gimp_layer_fix_format_space() too because the profile is now
embedded in all formats via the space. Add
gimp_image_assign_color_profile() which does all that and call it
instead of a simple gimp_image_set_color_profile(), also from the
PDB set-color-profile functions, which are essentially "assign" and
"discard" calls.
- generally, make sure a new image's color profile is set before
adding layers to it, gimp_image_set_color_profile() is more than
before considered know-what-you-are-doing API.
- take special precaution in all places that call
gimp_drawable_convert_type(), we now must pass a new_profile from
all callers that convert layers within the same image (such as
image_convert_type, image_convert_precision), because the layer's
new space can't be determined from the image's layer format during
the call.
- change all "linear" properties to "trc", in all config objects like
for levels and curves, in the histogram, in the widgets. This results
in some GUI that now has three choices instead of two.
TODO: we might want to reduce that back to two later.
- keep "linear" boolean properties around as compat if needed for file
pasring, but always convert the parsed parsed boolean to
GimpTRCType.
- TODO: the image's "enable color management" switch is currently
broken, will fix that in another commit.
- remove gimp_widget_flush_expose()
- remove the "now" argument to gimp_display_shell_flush() and make it
only update widget states
- rename gimp_display_flush_whenever() to gimp_display_flush_update_region()
and call gimp_display_shell_flush() separately in the only case we
passed FALSE to flush_whenever()
- remove th flush_now interval logic from GimpDisplay, as soon as we
have exposed the canvas, we are in the loop for the next frame clock
tick anyway, so delaying a useless and removed process_updates serves
no purpose
- in gimptool-progress.c, create the invisible grab widget also for
non-cencelable cases, so we can always safely run the main loop
manually to make the progress updates visible
- in gimp-gegl-apply-operation.c, always run the main loop manually
to make the progress updates visible
- in gimpstatusbar.c, leave some FIXME comments as reminder that
we might need the same logic as in gimptool-progress.c
Actually since we make this function so that it should never return
NULL, we may as well return a test at the end. If pixbuf is NULL, then
it is an implementation bug somewhere and we should fix it.
Massimo spotted some warning with clang in #1608 about pixbuf being used
initialized. Rather than just initializing it, I am actually
reorganizing a bit more the function because there was a bit of a logics
bug. In some weird case, it would have still been possible for this
function to return NULL instead of a magenta square (the case was: the
icon was not present in the icon theme; then wilber-eek was either not
present or failed to load).
This new code organization is more clearer, as a step by step, should
better identify the various failure cases and always return an allocated
GdkPixbuf.
When the "gimp-wilber-eek" fallback will fail to load, we just create an
ugly magenta square instead.
See Mitch's review at #1608.
Master adaptation for commit 32931c4606.
We were already doing so when an icon was simply absent from the icon
theme. But we may still end up in cases where the icon is seemingly
present, yet it fails to load (for instance the image file is
corrupted). When this happens, let's also try to load the wilber-eek
fallback.
Note that it doesn't completely stops gimp_widget_load_icon() from
possibly returning NULL (in the case where "gimp-wilber-eek" is also
missing/corrupted for instance), so calling code must still account for
possible NULL return value.
Basically the same as commit 9c6237b182
except that I just redid it instead of cherry-picking because the code
was a bit too different.
