The tool now takes care of the portion of the layer effectively displayed
on the screen. This allows faster expansion/shrink of the selection since
an area smaller than the whole layer is used.
It will also limit the impact of the incoming multilevels processing, which
tends to decrease the segmentation accuracy on thin structures, since users
often zoom-in to work on such thin details.
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()`.
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.
Setting the `"transient-for`" property for popups is important to
certain window managers (such as Wayland), so that they know what the
parent surface is and can position the popup accordingly.
This fixes the `GimpTagPopup` in wayland giving a critical message, and
getting a random position somewhere on the screen.
`g_object_notify()` actually takes a global lock to look up the property
by its name, which means there is a performance hit (albeit tiny) every
time this function is called. For this reason, we should try to use
`g_object_notify_by_pspec()` instead.
When run with GIMP_DEBUG=Gtk we get a lot of debug warnings for GimpRuler and once in a while for GtkScrolledWindow that State 0
doesn't match state 128 set via gtk_style_context_set_state (). This happens because we didn't enter the current state flags of
the widget but 0 and apparently Gtk isn't happy about that.
Let's fix this by using the actual state flags by calling gtk_widget_get_state_flags.
Based on other tools wording, as well as the algorithm description. In
any case, it looks a bit better than the previous description (with a
typo even).
In gimp_dialog_factory_register_entry(), strings were passed to gettext
without checking for a NULL pointer. The gettext documentation [1]
points out that that's undefined behavior, and musl libc's
implementation of gettext segfaults in that case.
[1] https://www.gnu.org/software/gettext/manual/gettext.html#Interface-to-gettext
If setting a RGB or Grayscale "preferred" profile, it usually means
these are profiles we are commonly working with, and it is to be
expected that images one might load may already have said profile. In
this case, don't ask what to do with this profile (keep, convert…). The
graphist kind of already answered this by explicitly listing this
profile as a "preferred" one.
Note that I also check for the built-in profile equality, though it
doesn't look like it is useful (gimp_image_get_color_profile() will
return NULL for an imported image which uses the built-in profile).
This is mostly to be thorough.
In some cases, error dialogs may end up below other windows and stay
unnoticed because of how dialogs are raised and hidden on various
platforms. This is not very constructive. It's much better to make sure
one sees an error when it happens (in some cases, it may mean possible
data loss, so it should be at least acknowledged; also seeing it later
may mean you can't know anymore which action triggered this error,
making the whole process kind of meaningless and hard to debug).
So anyway, let's make error messages prominent by having them always
above.
We are piling up the export dialog, then the specific plug-in dialog,
then possibly more dialog (cf. upcoming commits for metadata handling).
It's just too much and we end up in a mess of dialogs.
Of course, if the export fails or it is canceled, we are back to the
file dialog, which gets shown again, whereas if it succeeds, the file
dialog is destroyed (just as it already used to be).
Also anyway the file dialog is completely deactivated while the export
dialog is displayed, so it's just useless. Finally some export support
have features where looking at the canvas is useful, for instance JPEG
preview feature. With all these dialogs, it's nearly impossible to look
at the canvas (with a single display at least).
If you hover over the GimpSpinScale while clicking Shift, we want to be
able to advertize the fact that it will produce a different action than
the simple click. It makes the widget behavioral rules actually
discoverable.
Aryeom noted that current top arrow cursor comes from a time when the
interaction with this widget depended on a lower versus upper region and
doesn't gives anymore any idea of what the click would do.
So let's replace the top-arrow cursor with a "grab" cursor, which even
becomes a "grabbing" cursor when a grab is in progress.
Also let's now use gdk_cursor_new_from_name() with standard names from
the CSS specification, because it is the recommended way. According to
GDK docs, GDK cursor list is taken from the X cursor font and apparently
not all cursors are available on all platforms. So let's use the
standard CSS list, whose cursors are said to be available accross
platforms (at least very very likely, I guess).
Finally I renamed the target enum for the widget to be more semantic
than upper/lower which don't mean anything anymore in our new
interaction rules.
Currently to edit the value with the keyboard (i.e. get a focus on the
text entry part of the widget), we can click on the scale which gives us
edit focus, but it implies to change the value (which may not be
wanted).
An interaction method exists to do just this, which is the secondary
button (middle click). Unfortunately, though powerful, it is totally
"hidden feature". People expects to be able to click on the numbers they
see and start typing.
This change allows just this. Now when clicking on the number part, it
selects the whole text input contents (same as with middle-click ever
since commit 3449652fe8), without changing the value. You just enter in
text input mode.
To properly advertize the behavior change, the cursor will also change
when hovering, showing a text cursor over the existing number text.
As many of my changes, this change was designed together with Aryeom and
triggered originally by her usability feedbacks and inputs.
We were also reading "gtk-show-unicode-menu" which has also been
deprecated since GTK+3.10, and we were not even using the read value.
Just remove this piece of unused code.
The input menu added with gtk_im_multicontext_append_menuitems() mostly
provides redundant (e.g. the system defaults) or useless options
(e.g. "None" or "Simple" which basically seems to mean the keyboard
mapping straight keys, and I don't see why anyone would want this if one
set a system-wide input system). Worse it can provide unstable options
(e.g. "Wayland" which crashes GIMP when I tried it on my X11 desktop! It
crashes on _gtk_immodule_wayland_init which is probably normal as we are
not on a Wayland client).
Anyway gtk_im_multicontext_append_menuitems() is deprecated since
GTK+3.10, as well as the "gtk-show-input-method-menu" property which we
read from GTK+ settings (and is FALSE anyway nowadays by default, so we
usually never show this submenu) to determine if we should show this
additional text tool submenu or not. Furthermore there is just no reason
not to leave the defaults system input method engine does its job (which
works well as far as I can see, at the very least on my GNU/Linux X11
desktop and on Wayland also, last I tested).
Check at runtime for the operation availability and set the "Show Image
Graph" action active depending on this check.
This goes with discussions to make this operation optional with a
runtime check for the tool `dot`.
See: https://gitlab.gnome.org/GNOME/gegl/-/merge_requests/84
gimp_channel_select_alpha() was creating the same undo label "Alpha to
Selection" whatever the actual operation (add, remove, replace,
intersect). Select the right label depending on the actual operation.
… Selection.
I first wanted to switch the modifiers to Alt+shift, Alt+Ctrl clicks and
even implemented the changes, but realized the existing "Alpha to
selection" on Alt thumbnail clicks already mapped these for
Add/Remove/Intersect Alpha to Selection (though I broke this with commit
14b4c08881 but re-allows properly with exact modifier recognition in
order not to catch other types of combinations).
In the end, I see no other solutions than actually removing the mask
add/remove modifiers (#if-ed 0 for now, we'll see) in favor of the most
ancient feature.
Another sensible default change proposed by Aryeom. Until now, all tools
which could make use of dynamics started with "Dynamics Off". This makes
it very hard for beginners to start using GIMP with a tablet directly
without actually knowing the technical settings of the software.
This is even more anticlimactic in GIMP 3 as tablet are supposed to work
from scratch (and they do), but without a proper dynamics, you can't see
it. And same as it used to in GIMP 2.x, at first launch, it looks like
your tablet is not recognized.
We hesitated between "Pressure Size" and "Pressure Opacity". Both are
sensible choices and would directly show your tablet works. Also they
rely entirely on tablet-specific input (pressure), unlike e.g. speed or
direction, which is much better for the purpose of showing the tablet
works and is properly supported.
… multi-selection modifiers. Also they will work from now on on the
clicked mask only.
Shift and Ctrl clicks are reserved for multi-selection. We do not want
these to do other actions if you shift|ctrl-click on a mask (even more
as these alternative actions are hardly discoverable so it would make
for bug-like behavior to the person not knowing these alternative
actions).
So Alt-click is still used for showing/hiding the mask, but
enabling/disabling changes from Ctrl-click to Alt-Ctrl-click.
The second change is that these actions will not run the actions on
selected layers' mask, but on the clicked layer mask. This will at least
make these interaction different from the actions (e.g. with contextual
menu) and therefore it's not even redundant anymore. We will now have a
way to work on all selected layers vs a way to work on a clicked layer
(without changing the existing selection).
When Alt-clicking on an item thumbnail (for instance a layer thumbnail),
we can trigger an "Alpha to Selection" action on the clicked item. Yet
the check was not accurate and would work on Alt+any modifier(s)+click.
Let's catch exactly Alt-click only in order to allow for more actions on
other combinations.
In particular, we should not check if the extend/modify (shift/ctrl)
modifiers are ON, but also that no other modifier is ON. For instance an
Alt-Shift-click should not trigger multi-selection actions, allowing it
to be useful for other actions without also changing the selection in
the same time.
Long press on a viewable cell (such as the small layer or mask preview)
pops up a slightly bigger preview. We don't want this feature to be
triggered when any modifier is active, such as ctrl/shift (common with
multi selection in a tree view) or alt combinations (for various
alternative actions).
A UI unit test (paintbrush_is_standard_tool()) was expecting the
paintbrush tool to be the default tool. This actually does not fit with
my tests as the crop tool was always the default tool if I were to
delete my config folder and start anew. Yet it was working for the unit
testing at the very least since `make check` used to work until now, and
was broken by my change.
Anyway with my new code (commit 31e38fe869) and this additional one, now
it should work both for unit tests and real usage. So this commit fixes
both the CI and the originally expected default tool.
For the record, Aryeom also agrees that paintbrush is a good default
tool for first usage/discovery of GIMP.
This code is the result of discussions with and propositions by Aryeom
who thinks (and I agree obviously) better defaults should be set for
beginners and first time users. This is only a first change, more
similar defaults updates will likely come in the next few months on
various parts of GIMP.
Existing code was only making a differenciation for eraser-type devices,
setting them to the eraser tool (which is fine of course).
So far, I only added 2 types of device source, which we should
definitely special-case as well: pen tools (tablet styluses) should
definitely map to the Paintbrush (pencil could be a reasonable defaults
too, though brush seems much more common); as for touchscreen, I chose
to map to the Smudge tool (though to be fair, if we had gesture
recognition, this should not map to any tool). I also set a generic
default now because it seems that right now, we don't specify anything
(it ends up defaulting to the Crop tool for first uses, at least in my
case). I set the rectangle select tool. I'm not sure it actually makes
particular sense, but at least it's probably better to default to
something not destructive for first time users.
Virtual devices are only reflection on the various physical devices
attached to it (mouse, touchpads, styluses, etc.). Keeping settings when
the device is updated just makes no sense. Worse it can actually cause
issues by setting axis uses from one device where an axis use is 'none'
(and it's normal) to another where 'none' ends up disabling the axis.
Earlier code was assuming it should not happen. Actually it can happen,
in particular with virtual devices (on which several physical devices
can be attached and switching to one or the other would update the
virtual device to mimick its features).
It's just better practice to not leave all these variables public,
implicitly allowing various code to use these directly (also making
debugging harder).
Axis names can now be returned with gimp_device_info_get_axis_name() and
I added a dedicated function gimp_device_info_ignore_axis() to handle
the case of an axis to ignore.
I also improved the code with less redundancy to handle the axes data
(both the use and name arrays will always exist and be created and will
be n_axes size, hence avoiding bad out-of-bound reads). This includes
some code refactorization avoiding near code duplication to make sure we
always have the same logics, by creating the gimp_device_info_updated()
static function.
The issue has also been fixed on GEGL side (see commit 5ac40e3c3 in
GEGL), but since the fact that "gegl:introspect" relies on a runtime
tool (which is not the best thing), let's make an additional check
within GIMP to make sure we have a proper buffer as a result to
"gegl:introspect". Otherwise we might crash when trying to use this
NULL buffer.
- First only show the axes returned by GDK (which means the axes
returned by the driver if I understand correctly), and even within
these, ignore the ones set as "ignore" because they are likely bogus
axes (Carlos said drivers sometimes add a bunch of axis; I am guessing
this is because many are generic drivers for various models of tablets
so instead of have variable length of axes, they just set some to be
ignored).
- Also use the names returned by GDK instead of our fixed set of names.
The main advantage is that these are more accurate. For instance
rather than showing just "X" for the firxt axis, the GDK names would
be "Abs X" for a tablet and "Rel X" for a mouse. The drawbacks is that
it doesn't look like GDK is actually translating these, and since we
don't have the strings in our code, we don't either. This will have to
be figured out.
Note that we still need to use the fixed set of names
(axis_use_strings array) when a device is disconnected.
- If some device didn't have any axes at all, don't show an empty list.
Don't show the curve widget either.
- In the Axes list, select by default the first axis with curve (which
would be only pressure so far if a device has this axis), because this
is one of the main feature still doable with this dialog, so it's a
bit of a time-waste if we don't show this directly. In no axes has a
curve, just show the first axis in the list.
… gimp_label_spin_set_digits() and deleted gimp_prop_opacity_entry_new()
- The "digits" argument for the number of decimal places in
gimp_prop_scale_entry_new() is now mostly useless since GimpLabelSpin
(hence GimpScaleEntry too) got a nice estimation algorithm of sensible
values.
- Add gimp_label_spin_set_digits() function to manually set the digits
property when we don't like the estimated value.
- Also add a new "factor" argument to gimp_prop_scale_entry_new(). Its
role is to allow a GimpScaleEntry showing a factored range, typically
a [0, 100] range for some types of [0, 1] properties.
- Remove gimp_prop_opacity_entry_new() which was basically a
special-case of gimp_prop_scale_entry_new() and which can now be
easily reproduced by simply set factor=100.0.
- Update all usage of gimp_prop_scale_entry_new() in app/ and plug-ins/
with updated arguments. It is interesting to note that all existing
usage were either integers (digits=1) or when double, the estimated
decimal places are the same as the ones which were manually set (hence
showing the generic estimation is not too bad). So the new function
gimp_label_spin_set_digits() was not even needed once in current code.
A -quick done- first step towards the addition of a smart selection tool.
Require the gegl:paint-select workshop operation.
Still LOT of work to do (wip):
- fluctuations removal (GEGL side)
- multilevels pyramid approach + banded graphcut for instant result on large
image (GEGL ? GIMP ?)
- Gaussian Mixtures for color models (GEGL side)
- drawable offsets (GIMP side)
- undo / redo (GIMP side)
- scribbles edition mode (GIMP side)
- dedicated icons
- ...
Thomas Manni