… 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()`.
When dropping a color, we fill all the selected layers with this color.
When dropping a buffer, multi-layer is same as no layers selected (i.e.
we paste once as a new layer, not multiple times).
Logics stay the same except I replace gimp_image_get_active_drawable()
by gimp_image_get_selected_drawables() to show I looked at this code.
Basically it doesn't look like we can really handle multiple layer
selected here, unless we want overly-long window titles.
This fixes default constraint values when multiple layers are selected.
In particular fixed aspect ratio defaults in Crop or Rectangle Select
tools, in layer mode, when several layers are selected, the default
aspect ratio is the layer's ratio if all layers have the same
dimensions, otherwise it's the image dimension ratio.
Also fixes a bug when trying to create the tool rectangle while multiple
layers are selected (the rectangle was always of size 0 at point 0).
Finally fixes the auto-shrink feature with multiple layers selected. The
rectangle will now auto-shrink to the smallest rectangle which
encompasses the contents of all selected drawables inside the existing
rectangle.
In gimp.css, don't set a minimum height for GimpDisplayShell
statusbars. Instead, in GimpStatusbar, set the widget's minimum
height to the maximum of its children's natural heights. Note that
we have to do this manually, instead of using a size group, since
GtkSizeGroup::ignore-hidden is deprecated (and nonfunctional) in
GTK3.
Right now I don't change the logics of any of the tools, except that the
GimpTool class now stores a list of drawables instead of a single
drawable. This can be later used on a case-by-case basis to make various
tools actually work on multiple drawables.
Partially revert commit c73710e410,
avoiding updating tool widgets unconditionally on tool resume in
GimpDrawTool -- it's too expensive in general.
Instead, handle display-shell changes in GimpToolWidget, by adding
GimpToolWidget::update_on_{scale,scroll,rotate} flags, which
subclasses can use to request an update on any of these events.
Set the flags as necessary for the affected widgets.
Wherever we store arbitrary-format colors in an opaque buffer, use
double for the buffer, instead of char, so that it has a strict-
enough alignment to handle all our used pixel formats.
Color picking on a single layer still works as it used to. On multiple
layer, it will now pick on the composited color, similarly to sample
merged if only selected layers were made visible.
The PDB/libgimp function gimp_image_pick_color() is also updated to work
on multiple drawables too, giving the same ability to plug-ins (the only
call to this function in core plug-ins have been updated).
I am currently unsure of this one. Since GimpCanvasLayerBoundary is a
GimpCanvasRectangle, it now shows the minimum rectangular boundary
containing all selected layers.
Should we instead show all individual layers boundary? I didn't choose
this because it would make the whole canvas much more messy, and also I
figure that layer boundaries are mostly used to get an idea of your
current working space extent, for instance if we were to resize the
canvas. Matter to discuss, I guess.
These actions raise a GimpViewableDialog. For this to work, I made this
widget work with a list of GimpViewable, not a single viewable anymore
(so maybe the widget class name should change?).
When this list contains only a single GimpViewable, it will display
exactly like before, with a viewable preview. With several viewables,
the preview won't show.
This allows to add masks to all selected layers at once, with the same
basic options for all masks, as set in the dialog.
After much thought, tests and discussions with Aryeom, we decided adding
back an active item concept additionally to the selected items ones is a
bad idea as it makes only usage unecessarily complex.
We will just have selected layers. Some kind of operations will work
when more than one item (layers, channels, vectors) are selected while
others will require exacty one item.
In particular, let's replace instances of gimp_image_(s|g)et_active_*()
by corresponding gimp_image_(s|g)et_selected_*(). Also replace single
item in various undo classes by GList of items.
Also "active-*-changed" GimpImage signals are no more, fully replaced by
"selected-*s-changed".
This is still work-in-progress.
When using "show all" by default, gimp_display_flush() can be
called during GimpDisplayShell construction, before the newly-
constructed shell is assigned to the display. Use an ugly hack to
just ignore the flush when this happens.
Add a new GimpToolFocus tool widget, which defines a focus region,
consisting of an inner limit, an outer limit, and a transition
midpoint between them. The widget allows controlling the limits
and the midpoint, and moving, scaling, and rotating the region.
Add a new GimpCanvasLimit canvas item, which draws an area limit
for different shapes. It will be used by the following commits to
implement GimpToolFocus.
In GimpDrawTool, update the tool widget on GIMP_TOOL_ACTION_RESUME,
so that it can respond to changes in the display-shell scale/
offset. We'd previously done that for individual tools/widgets,
but let's just do it in one place.
... when rulers and scrollbars are hidden
In gimp_display_shell_fill(), make sure a size-allocate always
happens for the canvas, even when the rulers and scrollbars are
hidden, so that the pending size_allocate_center_image is handled,
and doesn't block canvas drawing.
* Don't generate our own marshallers if they are available in GLib
already
* Don't set the c_marshaller parameter in `g_signal_new()` if it's a
default marshaller provided by GLib. See commit message of commit
39e4aa3c57 on why this is the case.
Found via `codespell -q 3 -S ./ChangeLog*,*.po -L als,ang,ba,chello,daa,doubleclick,foto,hist,iff,inport,klass,mut,nd,ower,paeth,params,pard,pevent,sinc,thru,tim,uint`
In GimpCanvasBufferPreview, take a strong reference on the preview
buffer, fixing a segfault in the foreground-select tool when
switching engines while the grayscale preview is active.
Add a "clip" property to GimpCanvasTransformPreview, specifying the
transform's clipping mode, and clip the preview accordingly.
In GimpTransformGridTool, sync the tool's clipping mode with the
preview's clipping mode.
Add a "Unified interaction" option to the 3D Transform tool, and a
corresponding "unified" property to GimpToolTransform3DGrid. When
active, all three interaction modes of the grid (camera, move, and
rotate) are available simultaneously, regardless of the active
dialog page. In this mode, the inner and outer regions of the item
are used for moving and rotation, respectively, and the vanishing-
point is controlled through through a handle.
In GimpToolTransform3DGrid, fix the test for determining which side
of the plane is facing the viewer, affecting the direction of
rotation when rotating around the local Z axis.
Add a new GimpToolTransform3DGrid tool widget, subclassed from
GimpToolTransformGrid, which can be used to perform 3D
transformations.
The widget can be in one of three modes:
CAMERA - allows adjusting the primary vanishing point by moving a
handle.
MOVE - allows moving the object through dragging.
ROTATE - allows rotating the object through dragging.
By default, controlling the transformation through dragging applies
to the X and Y axes. Holding Shift (or setting the "constrain-
axis" property) restricts the motion to only one of the axes.
For the MOVE and ROTATE mode, holding Ctrl (or setting the "z-axis"
property) allows controlling the Z axis instead.
For the same modes, holding Alt (or setting the "local-frame"
property), applies the adjustments in the object's local frame of
reference, instead of the display's global frame of reference.
Add a boolean GimpTransformGridTool::dynamic-handle-size property,
which controls whether the handle sizes are adjustment dynamically
according to the grid's size, or remain fixed. This property is
TRUE by default, to maintain the current behavior.
In GimpToolTransformGrid, allow setting "inside-function" and
"outside-function" to a new NONE value, performing no
transformation when dragging the respective area.
More of the files were wrong, or at least not absolutely identical to
the files generated by the autotools. I am not doing any code change
other than trying to make both build systems produce identical files
(except for slight differences on 2 files not worth the effort) even
though maybe some things can be improved (especially on the include
list). Maybe to be improved later.
Also fixing 2 of the previously autotools-generated files because of
space typos which should have been committed earlier.
Finally it is to be noted that there is no logics to copy the generated
files back to the source directory in the meson rules. I am not sure
anyway this is really worth it and maybe we should just stop tracking
these generated files eventually.
See #3996.
This was a warning raised by the Coverity scan on one of the lines. I
fix also in the same time other arithmetics mixing int and double.
Better be thorough.
The specific warning was on:
> circ = 2.0 * G_PI * (private->width / 2)
where the division was integer, which was probably not intended. Of
course that error (display item only) was likely barely visible anyway,
still…
and remove a gazillion gtk_widget_show() all over the place, some
places need a gtk_widget_hide() now, and I'm pretty sure I broke at
least one thing in all those files...
... which is similar to gimp_display_shell_get_pickable(), however,
it returns the projection, rather than the image, only when
gimp_display_shell_get_infinite_canvas() is TRUE, i.e., when the
shell is in "show all" mode *and* canvas padding is disabled.
Jehan