There was one case where horizontally and vertically were mixed up in a
call to gimp_display_shell_scale_image_stops_to_fit.
The more usual order of parameters is first horizontal and then vertical.
So, let's fix the actual functions that have the illogical order
of vertically, horizontally instead of fixing the one call.
This brings it in line with the order in other functions and makes it less
likely we mistakenly mess up the parameters.
Besides that gimp_display_shell_scale_image_stops_to_fit also was not
declared as a local function, so we add that too.
Reviewer (Jehan) note: cherry picked from MR !274. Still deciding
whether this will be pushed to gimp-2-10 branch too.
Fixed Conflicts from !274:
app/dialogs/preferences-dialog.c
app/display/gimpdisplayshell-draw.c
app/plug-in/gimppluginmanager-call.c
libgimp/gimp.c
libgimp/gimp.h
libgimpwidgets/gimppreviewarea.c
libgimpwidgets/gimppreviewarea.h
libgimpwidgets/gimpscrolledpreview.c
Remove `gimp_display_shell_keyboard_grab()` and ...ungrab(), as there
doesn't seem to be any good reason to grab the keyboard away from the
compositor. More annoyingly, in some systems it will pop up a dialog
on each first click whether the user is okay with inhibiting shortcuts.
Performing zoom and rotation at the same time is inconvenient because
most of the time the user will want either zoom or rotation. This can be
solved by recognizing a "significant enough" zoom or rotation change
initiated by the gesture recognizer and then ignoring the other gesture.
Adds the new configuration option "drag-zoom-speed" to adjust the rate
at which mouse movement can zoom the canvas, ranging from 25% to 300%
of the base rate and applying to both drag-to-zoom modes.
This option can be found in the preferences dialog as:
Image Windows -> Zoom & Resize Behavior -> Drag-to-zoom speed
Adds a new configuration option "drag-zoom-mode" to choose whether to
zoom by distance of movement (newly added) or by duration of movement
(previous behavior) when zooming via dragging the mouse, defaulting to
distance.
This option can be found in the preferences dialog as:
Image Windows -> Zoom & Resize Behavior -> Drag-to-zoom behavior
Changes the behavior of gimp_display_shell_scale_drag() to factor in
the distance dragged, rather than just scaling a flat +/- 10% for each
detected movement event. The factor by which to change the scaling is
also altered from 10% at each movement event, to 1% compounded for
each pixel of distance dragged.
This makes zooming via Ctrl + Middle Click or Ctrl + Spacebar behave
more consistently and less jittery versus the previous method, while
offering more fine grained control.
We take a step back from the original MR which was proposing the "single
dot" cursor as a new "Pointer mode" option. I was really unsure this was
the best solution, especially reading again the whole original report.
It means that now nearly all of the original patch has been rewritten
another way, but let's leave the contributor commit as a start point to
get to where we are, and as acknowledgement of the contribution.
The reporter was annoyed by the crosshair when none were requested and
probably mostly for painting tools only (at least examples were about
brush or pencil, etc.) while showing outline. It looks to me like the
real issue was maybe when we were showing the big crosshair when using
the 4-arc fallback outline, for instance when using a dynamics changing
size. If so, this main issue is already fixed by my commit 64dc26064b.
No need of a new option for this, especially if the option can be as
confusing as a barely visible dot-cursor (I can already imagine the bug
reports of people tweaking random preferences and unhappy because the
pointer became invisible, while they don't know how they did it).
Instead I would say that when people specifically uncheck both "Show
brush outline" and "Show pointer" options, showing a huge crosshair
feels quite counter-productive. This is where I think that our small
unobtrusive cursor (probably a better name than "Single dot" by the way,
as it's not a single dot anymore) might be of use, the ultimate case
when someone really want a cursor as inconspicuous as possible, while
still having a visible feedback of the pointer position (even with
display-tablets, parallax issues make such a visual feedback important
to target where one paints).
So let's try this first and see how it goes.
… cursor position label.
This should now resolve any weird jumping around of the cursor label,
hence the GUI. Of course, it could actually still happen when going
really off-canvas (though the max computation takes into account
reasonable off-canvas moving, which is totally possible, especially now
that we can view off-canvas so people can just work there and the GUI
should definitely not do weird stuff there as well).
This issue was not confirmed to happen on Linux and Windows, but was
making ugly label resizes on macOS by just moving the cursor on canvas
normally. See video in:
https://gitlab.gnome.org/GNOME/gimp/-/merge_requests/572#note_1389445
Ironically enough though, it started to happen even on Linux/X11, at
least, after commit 1baeffc913, which was the commit meant to fix this
on macOS!
This max size computation algorithm should work well enough for any
normal on-canvas usage, and even some off-canvas (yet close enough)
usage.
Note: this code only takes into account the pixel position case, for a
first implementation, which is why it's not pushed immediately in this
form.
… in specific case when there is already a toolbox.
Even if there is already a toolbox in single window mode (hence it needs
neither to be created nor raised, as there is a single window), we still
want to return the dockable widget which can be used for other things.
Gets drawing in the canvas speed on retina displays up to the speed
of FullHD displays on macOS, making 2.99 usable on macOS.
Generic change:
Changes the cursor_label to delay the drawing phase to the idle
queue, from immediate draw on all platforms.
Before the fix in 32049afd (using a deprecated function in Gtk3)
any draws on this label forced a full canvas redraw. This is due
to a quirk in how GtkLabel functions.
The redraw occurred because GtkLabels resize themselves and everything
around them by sending a resize message any time they receive new
text. These resizes then trigger the full canvas resize which triggers
a full canvas redraw that cannot be optimized by either Gtk or Big Sur.
MacOS changes:
Only redraws the cursor position label and each of the horizontal and
vertical rules (cursor tracking widgets) 3 times a second max for a
total of 9 redraws a second (ideally out of 60, though I don't believe
under any circumstances that GIMP achieves a 60fps).
Each of the cursor tracking widgets gets its own timeslice, and so
will not redraw when the other cursor tracking widgets are drawing.
This is required because Big Sur is merging all draw rects into
one large rect, dramatically slowing down draws.
This timeslicing ensures that draw rects are maintained at the smallest
possible size. So the typical redraw is a small rect around the
brush. However, 9 times a second, the rect will include one of the
3 cursor tracking widgets (rulers and cursor label).
Additionally, the code tries to minimize resizing the width of the
cursor label by checking if the widget is too small for the text,
then setting the char_width to a greater size so that resizes won't
be that common.
This improves the appearance of the widget as it no longer
constantly jumps about in size on each cursor move.
Here is a discussion of the issue:
https://gitlab.gnome.org/GNOME/gimp/-/merge_requests/572#note_1389445
Reviewer's (Jehan) notes:
* The whole issue about GtkLabel resizing is no more after 32049afd. It
is normal for a widget to request a resize when needed. We just don't
want the statusbar to resize and triggering canvas redraws.
* Changing cursor position text into an idle function generally makes
sense.
Also it reverts commit 6de9ea7022 which had a bug I hadn't realized
when I accepted it: when we test for time, we don't know yet if it
will be the last position change, hence we could "refuse" the last
update. Therefore displayed cursor position would end up outdated
on macOS. This new implementation doesn't have the problem (the last
idle update always happens after the last move).
* The change about giving 1/3 timeslices to side canvas components
(rulers and statusbar) is a complete hack to work around the fact that
macOs doesn't report properly each damaged rectangle. Instead it
returns a huge bounding box. The workaround here is to expose these
area separately.
We have not been able to find a proper solution yet. This is the only
reason why I accept this code, for macOS only, to at least have
something usable there.
See discussions in MRs gimp!572 and gimp-macos-build!86. With these 2
MRs, Lukas reported GIMP 2.99 to perform even better than GIMP 2.10 on
Monterey, though it could not be tested on Big Sur unfortunately.
* Lastly the set_width_chars() thing is also an ugly hack which I will
try later to revisit (see !581). I only accepted it (with mandatory
macOS-only macro) to have an acceptable state for release after seeing
a screencast where the label size indeed "jumps around" on macOS.
Since we are now allowed to move groups (which is the same thing as
multi-selecting all its children and moving them), it makes no sense
that this lock is disabled.
This works the same way as "Lock pixels" in that a locked grouped also
forbid moving children. And there was already some logics so that you
can't move a layer group if one of it's children is locked. So this lock
really works both ways and is a bit special.
Finally I cleaned up a bit the multi-layer selection logics and
messaging, as well as which lock to blink (similar to the previous
commit) for the "Lock position" case.
In particular, if painting on a layer whose parent's pixels are locked,
we were blinking an empty lock spot, which is confusing. Now
gimp_item_is_content_locked() will also return the proper item (when
relevant, i.e. when returning TRUE) which is locked. It may or may not
the same item as passed in (it may also be a parent item in particular).
See discussion in !572, #7840 and #7690. Note that this was reported on
macOS where the consequences were pretty dire, but it actually also
happens on other platforms, at least on Linux too (as confirmed in X11
with the GTK Inspector set to show graphics updates; on Wayland this
debug option doesn't work, but I assume it is the same).
I am not perfectly happy with this change either, because it is based on
part of the API which has various deprecated parts (hence doesn't exist
anymore on the main dev tree, i.e. it might have to be reviewed in GTK4;
of course, it's unsure, maybe the whole resize propagation to parent
containers is just better handled there and the problem won't exist
anymore).
In any case, it is cleaner than the proposition for this part of the
problem in !572 which is problematic (patching GtkLabel with a new API
which won't trigger resize even when actually needed, hence which likely
won't ever get accepted upstream because it's not right).
This change reduces to 3.33 times a second, the updates to the
status bar coordinates widget.
This dramatically improves frame rate on macOS retina displays
because it reduces the frequency of full screen updates which
are triggered by this widget updating and are very slow.
This makes the statusbar refresh changes mac only where the
benefit will be felt keenly, rather than saddling all
platforms with the change.
This moves to standard fullscreen behavior for Gimp.
Added benefit is that it no longer requires gdkquartz-cocoa-access.h
which the Gtk team wish to stop supporting https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/4303.
Bug 756178 also no longer manifests, cdc7542d46
so it is now safe to do.
Finally, removes dependency on objective c in the app/display directory.
I cleaned many remaining places where the concept of linked item still
survived.
On loading an XCF file with linked items, we are now going to create a
named sets for the linked items, allowing people to easily select these
back if the relation was still needed.
We don't remove gimp_item_get_linked() yet and in particular, we don't
save stored items into XCF files. This will come in an upcoming change.
Not sure why but adding a handler to the "expose-event" signal of
GimpDisplayShell (similarly to how we do it in master branch on "draw")
just didn't work. But it works on the already existing signal handling
on the canvas instead (which actually is not a bad deal, as we also
remove the coordinates translation so maybe we should test this on
`master` too).
Note: why we are backporting all this logics to gimp-2-10 is because
changes in macOS BigSur broke the selection's marching ants the same way
they broke on Wayland and it was confirmed this fix worked for BigSur as
well, at least on the dev builds.
It is unnecessary to backport for Wayland (because GIMP 2.10 is based on
GTK2 which anyway works only through XWayland, hence doesn't have the
issue), we do it only for macOS BigSur (and further). Well at least the
fix will hopefully work on the stable branch, because I cannot test
myself.
See issue #5952.
(cherry picked from commit 6be014fc59)
Cherry-pick note: it was initially only for gimp-2-10 but actually also
works fine on the GTK3 branch and fixes some selection coordinates issue
when rotating the canvas.
… an API that went private in GTK 3.
Note from reviewer (Jehan): this API became public again in commit
gtk@242b76a7, available since GTK 3.24.29. Before this, the function
(gdk_quartz_window_get_nswindow()) was actually still in the ABI, so we
could declare the function locally before using it. This is an ugly
workaround, but it works.
What we do is providing both solutions depending on GTK version, leaving
build warnings as constant reminders so that we remember to get rid of
the workaround when we bump minimum GTK requirement.
Cf. discussions in gtk#2452 and gimp!483.
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.
This started as yet another report of leak by Massimo. But really the
leak of the GdkPoint created by the function
gimp_display_shell_push_zoom_focus_pointer_pos() is not only when
delta_y is 0. There are a few code paths in gimp_display_shell_scale()
when we would not pop this point. One of them is for instance when
window resizing in multi-window mode is allowed. There might be more
(but the code is convoluted enough not to be 100% sure if these are
possible with our specific case).
This specific function was initially created only to be used for unit
testing code (commit 7e3898da09), but it ended up being also used
internally (commit 792cd581a2). Since I see that the test for which
this code was initially created even seem broken right now (the assert
part for position check is commented out!), I even wonder if we should
keep it. We could indeed instead just add optional start_x|y arguments
to gimp_display_shell_scale(), which would be much cleaner. But I leave
it for now.
Instead I just make sure we clean the created GdkPoint after calling
gimp_display_shell_scale(). Also I get rid of the GQueue. It is clear in
the code that we are not expecting queuing interaction of several
positions. Worse right now, we could end up in weird cases where the
pushed points are not used when they should, then could end up being
used later in totally unrelated interactions (this would make the shell
position jump here and there). So let's just make it a single point.
Finally adding some appropriate comments in parts which are still a bit
wrong.
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 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.
I'm still very unclear why exactly but it would seem that just queuing
the redraw with an idle function is not enough. At least on Windows,
Jacob was having cases where opening an image would get stuck unless the
mouse was moved (causing draw events most likely).
So let's use a timeout function instead. Probably no need to queue the
idle followed by the timeout function as we had before commit
4fee04b839. Instead just directly queue a draw if relevant, then run the
timeout at regular interval (marching ants speed).
This way, we would queue a lot less canvas region unnecessary redraws.
We still remake the time-before-last-draw check in the draw() signal
handling before we want to update the marching ants index for draw
events coming for other reasons (canvas updates, moving/zooming on
canvas, exposition changes, etc.).
… displayed.
We should use the dimensions from the GimpDisplayShell not the the
GimpCanvas. Indeed the canvas is shorter when rulers are visible, hence
the selection next to the extreme sides (bottom and right sides of the
canvas) was not drawn.
As suggested in a comment (itself coming from an IRC discussion), we
should not use gdk_window_(begin|end)_draw_frame() functions as this
works on X, but not on Wayland anymore. Instead draw directly during
draw() call of the shell widget, and force it to happen regularly, to
update the marching ants, via gtk_widget_queue_draw_region().
This is tested and works on Wayland. Please everyone, test thoroughly to
make sure it works well in all situations, and also that we don't get
any unexpected slowdowns.
Since the symptoms are very similar, it is highly possible that it also
fixes the issue #5952 too, for selection not showing on macOS since Big
Sur 11 (maybe they changed the same way as Wayland did). Unfortunately I
can't check this myself. Please test, whoever has access to a macOS Big
Sur and can build GIMP!
When a selection exists, we are copying then pasting the selection
contents. In particular, with multi-layer selection, it means pasting a
merged result of the selected layers (like a sample merged but limited
to selected layers).
Yet when no selection exists, with a single layer selected, a cut in
particular would remove the layer fully, then a paste would copy it
elsewhere (in the same image or even on a different image). This was
still working, but not with multiple layers. This is now fixed and we
can now copy/cut then paste several layers (without merge), which is
sometimes a very practical way to move layers (sometimes simpler than
drag'n drop, especially between images).
As a consequence, the PDB function gimp_edit_paste() now also returns an
array of layers (not a single layer).
In some cases, in particular for actions generated from plug-in
procedure right now, we were displaying the reason of the insensitivity
(typically right now, only the drawable type is cited). This was done by
appending the reason to the tooltip, separated by 2 newlines, which
resulted in extra ugly design, no nice way to style this info directly
(with pango for instance if the widget display allows it, or on a
separate info widget in a possible future, or whatnot).
Also it would mean that the action search could match a disabled action
by mistake if a search word happens to be in the reason message.
This improves the situation with the following changes:
* gimp_action_set_sensitive() now takes an optional reason string to set
the reason message.
* Same for gimp_action_group_set_action_sensitive().
* gimp_action_get_sensitive() returns an optional reason string.
* gimp_procedure_get_sensitive()'s tooltip return value now becomes a
reason (it won't contain anymore the tooltip and the reason
concatenated, only the reason for separate processing).
Was caused by widget tool fixed_center_x and fixed_center_y coordinates set to
coordinates of mouse click instead of rectangle center after converting channel
selection bbox to rectangle.
Now rectangle fixed_center_x and fixed_center_y coordinates are always updated
when tool widget x1, x2, y1, or y2 coordinates are updated.
Closes#6487
This commit also makes snap to grid and snap to vectors work off-canvas.
Since we now have off-canvas viewing, it just makes sense that snapping
would work there too.
Note that I disable snap to grid when "Show All" is OFF. I am actually
unsure this is right (as "Show All" is a view action, and we usually
don't change behavior based on view actions; for instance snap to guides
are not disabled if guides are hidden). Yet I noticed we do this in
various other features when off-canvas. We kind of use this view flag as
a switch for features working off-canvas (for instance, color picking
works off-canvas only when "Show All" is ON). So let's keep the same
logics for now at least.
Snap to guide or snap to vectors will always work though, because guides
and vectors are always visible off-canvas (even when "Show All" is OFF).
They always have been (visible, not snappable off-canvas; now they are
both).
Our error/message handling code was checking if the status bar label was
big enough to hold the message by checking the GtkLabel allocation. This
means that error message ended up on status bar only if a status text
bigger than the error message was previously displayed.
Even setting gtk_widget_set_hexpand() or the "expand" container child
property on the label, I could not find a way for GTK to actually give
it as much space as possible on the status bar.
Instead, I am computing the full container box size, starting from the
label x coordinate (assuming the label is the last shown widget on the
status bar, as usually the progress bar and the cancel buttons are not
shown in the same time as the message label). This gives me a much more
appropriate result of the maximum size which this label can hold without
ellipsizing.
By default, GTK keeps on showing client-side decorations when going
fullscreen, as it might contain buttons or other functionality that
would be lost. As such, we have to make sure we hide the titlebar.
Getting to that titlebar to update its "visible" property needs a bit of
effort, since it's an internal child of the GtkWindow that isn't exposed
in any way, so we take a little detour using `gtk_container_forall()`.
Jacob Boerema