Our historical heuristic looking for the main monitor based on the
position of the center point in a widget just won't work on Wayland,
where reported positions are always (0, 0). This is the security policy
for this protocol.
Instead we must now use gdk_display_get_monitor_at_window() which is
meant to do the approximation of determining the monitor a surface is
in, for us.
This is one step of implementing color management for Wayland (#15827),
except we still need the code to get the ICC profile of a monitor (but
now at least, we know which monitor we are on).
This is a followup of commit 0ff960c45b which was the right fix, except
that we must make sure that gimp_widget_free_native_handle() is called
**before** chaining up with the widget's parent dispose().
Failing to do so, the first parent's dispose() was destroying the
associated GdkWindow, which made it impossible to call
gdk_wayland_window_unexport_handle() on it. And therefore we were still
getting handle callbacks possibly run after the window was destroyed, if
we were very fast enough to destroy a window immediately when it is
being shown.
I was still experiencing a crash when closing the Export file dialog
very fast with Esc while it was reappearing after canceling a plug-in's
export dialog.
This followup commit reorders the one case where we still had the crash
because of this order issue, and adds some docs comment to tell
developers how to use the freeing function.
It is a tentative fix as I had this crash once but could not reproduce
it, even redoing dozens of times the same thing I had done.
It is mostly based on something which GDK docs of
gdk_wayland_window_export_handle() says:
> To unexport the window, gdk_wayland_window_unexport_handle() must be
> called the same number of times as gdk_wayland_window_export_handle()
> was called. Any 'exported' callback may still be invoked until the
> window is unexported or destroyed.
So when the GdkWindow of the widget still exists, let's make sure we
call gdk_wayland_window_unexport_handle() as many times as necessary (no
less, but also no more!), and also that we disconnect all handlers which
could possibly call more export_handle().
- Added some link annotations to other types or functions.
- Rename argument @handle to @window_handle in
gimp_widget_set_native_handle() docs to be similar to the same
argument of gimp_widget_free_native_handle().
- Fix annotation of the handle to (inout) though I do wonder if this can
really be usable in bindings anyway because of the asynchronous logic.
Maybe these functions should be skipped for bindings?
- Better docs globally, and in particular remove implementation details
in gimp_widget_free_native_handle() docs. The point is that the handle
is to be considered opaque data and you have to run this when you ran
gimp_widget_set_native_handle(). No need to talk about Wayland or
explain that it's a GBytes or meanings of "handle".
- Also clean the free() implementation with far too many comments, and
in particular we don't need to check if the handle is NULL.
g_clear_pointer() steps out by itself if the pointed data is NULL.
This patch fixes a few (mostly Windows-specific) warnings on build.
* Hides functions like gimp_get_foreign_window () and variables
like transient_set that aren't used in Windows.
* Changes hollow_g_shell_quote () to not return a const gchar *,
since the value it returns is actually not const.
* Cast update_interval to double to remove warning about mixing
enums and doubles in division.
Previous code was right and equivalent because
gdk_wayland_window_export_handle() works asynchronously in the same
thread (so phandle can't get overridden by mistake). Nevertheless a
quick code scan felt surprising, so to avoid any such future surprise,
let's just switch the order of statements.
Refactor: extract method gimp_widget_free_native_handle.
This reduces duplication of code and encapsulates Wayland specific code.
Call the new function in more places.
This is expected to fix#11613 but it is hard to be sure
since the exact sequence of events in 11613 was never determined
and only reproduceable in some flatpak builds.
Calling the new function in more places also should eliminate leaks.
But I did not test there was a leak prior to this fix.
Ports the animation code started in e13cc635
to an independent gimp_widget_animation_enabled()
function. This allows plug-in authors to
also conditionally turn off animations if
the user's system settings say to do so.
The function is applied to the About
Dialogue animation as well as two Easter
Egg animations:
* Wilber's eyes blinking after 23 minutes
on an empty canvas
* Wilber's eyes following the mouse after
a certain sequence of tools is clicked
Some minor code style issues remained
that I missed in 8adcc0cd.
After further reflection, I also converted
the code to be a private/internal function.
gimp_prop_check_button_new () should
cover the majority of GtkCheckButtons,
and the function is currently only used
to fix the size of those widgets. We could
revisit it as a public function in the
future if more use cases are found.
Resolves#10026
Selecting a checkbutton makes the label bold. This increases the
width of its label, and if it's the longest item in a box, this
causes minor "twitching" and resizing of the dialogue.
This patch calculates the size of the label when bold and then
requests the label width be set accordingly to resolve the issue.
Resolves#11246
On Windows, a CRITICAL was being repeatedly thrown if you
didn't have a monitor profile set. This is because we were trying to
pass a NULL value to g_utf16_to_utf8 () whenever we needed to
convert a GeglColor.
This patch fixes it by checking if the monitor profile filename is NULL
before trying to convert it.
The invasion extended to some core widgets too, in particular GimpColorPanel (a
subclass of GimpColorButton). There was quite a lot of code depending on these
widgets.
This includes improvements on the out-of-gamut colored corner being shown for
unbounded component types out of the [0; 1] range (with some small margin of
error to avoid e.g. a -0.0000001 value to show as out-of-gamut).
There are still improvements to be made on the color rendering. In particular,
it still draws as CAIRO_FORMAT_RGB24 cairo surface. We should probably move to
draw as CAIRO_FORMAT_RGBA128F eventually (more precision and even allowing to
draw unbounded colors with a possible option, instead of always clipping).
Also adding the libgimpwidgets API gimp_widget_get_render_space().
- New function gimp_cairo_set_source_color() which is meant to replace
gimp_cairo_set_source_rgb(a?)() eventually. This new function sets the Cairo
source color, using the target monitor's profile of the widget where the Cairo
surface is meant to be drawn on. It also uses the color management settings
(such as whether a custom profile was set, instead of using system profile, or
also simply whether color management was disabled at all). It doesn't
soft-proof the color yet.
- Padding and out-of-gamut colors drawing now use the new
gimp_cairo_set_source_color(). These don't need any soft-proofing anyway.
- Out-of-gamut color property in GimpColorConfig is now a GeglColor property.
MINGW64
- uses 0x601 as value for _WIN32_WINNT. No need for us to define
it to that value or even lower values in some places.
This also gets rid of: warning: "_WIN32_WINNT" redefined
- has 0x0502 for WINVER, so get rid of us setting it to 0x0500 in
gimp-app-test-utils.h. It also seems that the need to use G_OS_WIN32
has disappeared here.
- DIRECTINPUT_VERSION is 0x0800, no need for us to set it to that value.
- AI_ADDRCONFIG was apparently missing from the MINGW headers in the
past, but not anymore.
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.
In commit 48c27770 some unicode related changes were made. As a result of
that on Windows display_device, which was previously a duplicated string,
is now referenced directly.
However, the g_free was not removed, causing a crash.
We resolve this by removing the obsolete g_free.
Adds a simulation_bpc and simulation_intent to GimpImage to allow
plug-ins to access it
for CMYK import/export.
Four pdb functions were added to enable this access:
image_get_simulation_bpc (), image_set_simulation_bpc (),
image_get_simulation_intent (), and image_set_simulation_intent ().
Next, it updates menu options and code to support GimpImage's
internal simulation intent and bpc.
New 'simulation-intent-changed' and 'simulation-bpc-changed signal
are emitted via
GimpColorManagedInterface so that relevant tools
(such as the
CYMK color picker, GimpColorFrame, and future pop-overs)
are aware of these changes.
gimp_color_config_get_simulation_color_profile() is returning a new
object, so we had 2 code paths giving either allocated data or not.
Therefore simply ref the passed softproof profile in the second code
path, and don't ref it anymore when caching it (especially as it might
also be NULL at that point).
Adds a simulation_profile to GimpImage to allow plug-ins to access it
for CMYK import/export.
Two pdb functions were added to enable this access:
image_get_simulation_profile () and image_set_simulation_profile()
Next, it updates menu options and code to support GimpImage's
internal simulation profile. Menu items are moved from View to Image's
Color Management section.
New 'simulation-profile-changed' signal is emitted via
GimpColorManagedInterface so that relevant tools (such as the
CYMK color picker, GimpColorFrame, and future dockable
dialogue) are aware of these changes.
… for the container tree view contextual menu.
A very annoying point of contextual menus is that they happen on button
press whereas menu item selection happens on button release. When the
menu corner is positionned on the click position, nothing bad happens;
yet when place is missing on screen, the menu might get positionned over
the pointer position. And worse, the mouse position might be just over
an activatable menu item. So we end up in this weird situation where a
click implies: press, menu opens, release, random item (whatever is
below the pointer) is selected and menu closes.
To get rid of this weird case, let's have our contextual menu happen on
button release. In reality, I don't think anyone cares that it happens
on press or release, you just "click". But what you certainly don't want
is to click random menu items!
The variables `screen` and `display` are only for the GDK_WINDOWING_X11
code path.
Fixing the following warnings:
libgimpwidgets/gimpwidgetsutils.c: In function 'gimp_monitor_get_color_profile':
libgimpwidgets/gimpwidgetsutils.c:502:21: warning: unused variable 'screen' [-Wunused-variable]
502 | GdkScreen *screen;
| ^~~~~~
libgimpwidgets/gimpwidgetsutils.c:501:21: warning: variable 'display' set but not used [-Wunused-but-set-variable]
501 | GdkDisplay *display;
| ^~~~~~~
Basically this commit makes sure that all return values that are marked
as "Returns:" also have a `(nullable)` annotation if it is mentioned on
the same line that NULL can also be returned.
This will prevent a few problems in GObject-introspection.
