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.
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.
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.
… 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.
… 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.
- 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.
… in Image properties window.
Basically using non-translatable "%s (%d %s)" and filling it with
"Indexed color" base type name and translatable _("colors") is very bad
practice (localization-wise):
- First it assumes everyone uses round brackets.
- It also assumes the order of words (number before word it determines
but even the image type before detail brackets).
- And finally it doesn't handle plurals. Of course, we could say that
the 1 color case is a very edge impractical case, but plural is not
only about 1 vs other numbers. Some languages have more cases, and
using ngettext allows translators to handle these.
The color space label may be a bit long (depends on profile title which
may just be anything and we don't control it), so I allow it to wrap.
The file path on the other hand would not work well with wrapping. It
already has ellipsis in center, but GTK always gives the max size it can
as a default. So if the file is even just slightly deep in the file
tree, we end up with extra-wide Image Properties dialog.
My trick is to give a sensible max size at dialog creation (25
characters max) but to disable this max size as soon as the window gets
realized, hence allowing the label to actually grow up to the contents
actual max size, were one to manually resize the window.
"RGB color" is not a color space, only the model. To get full color
space information, we want to display the model and associated profile
name.
Of course, the "Image Properties" dialog also has a tab displaying
details about the color profile. Still it's better if the general info
displays not too wrong label contents.
Note: when the used color profile has no name, it will show "(unnamed
profile)" which is still more informative than just the color model.
By just redirecting to parent signal handler, it was not properly
grabbing focus. In particular for a text entry with number input, we
want the text to be fully selected (because when we go for inputting
numbers, it is more often than not because we want to enter free numbers
very different from existing value).
So let's grab the entry focus, hence fully select current number
contents.
Instead of setting always manually the step and page increments when
creating a GimpScaleEntry, let's just generate some common cases
automatically. Indeed the increments are rarely something you want to
care about. The algorithm used is:
- For a range under 1.0, use a hundredth and a tenth (typically a [0,
1.0] range will step-increment of 0.01 and page-increment of 0.1).
- For small ranges (under 40), step-increment by 1, page-increment by 2.
- For bigger ranges, step-increment by 1, page-increment by 10.
For use cases when you absolutely want specific increment values, I add
the gimp_scale_entry_set_increments() function. It is much better to
have a small and understandable constructor call followed by
configuration calls (only when needed) rather than a constructor with a
crazy amount of parameters. Hence gimp_scale_entry_new() went from 17
arguments (absolutely unreadable calls) to now 5.
Some people had been complaining that they couldn't find some actions in
some case, which was only because they were in states where the actions
were non-sensitive. So it was "normal" (i.e. not a bug), yet I can see
how it can be disturbing especially when we don't realize that an action
is meant to be inactive in some given case.
Of course the option to show all actions already existed in the
Preferences. But as most options in Preferences, this is hardly
discoverable and many people only use default settings. Moreover showing
hidden action made the action search cluttered with non-sensitive
actions in the middle of sensitive ones.
This change gets rid of the "Show unavailable actions" settings and
always show all matching actions. In order not to clutter the list with
useless results, I simply updated the display logics to always show
non-sensitive action after sensitive ones. Note that even non-sensitive
actions will still be ordered in a better-match-on-top logics, yet they
will be after sensitive actions. So the top results will be the best
matches among sensitive actions (action in history), followed by various
levels of matches (actions with matching labels, tooltips, different
order matches, etc.); then they will be followed by best matches among
non-sensitive actions, followed by the same levels of matches as
sensitive ones.
This way, we still keep a very relevant result and there is no need to
have a settings for this.
When clicking on the selection mask (in the dockable view) or when
dropping a color on this same view, we can now select by color based on
the selected layer composition (not only one single layer, nor the whole
image as sample merged, but also a specific list of composited layers).
gimp_channel_select_by_color() is made multi-drawable aware as a
consequence of this.
I discover that the selection editor has these hidden features that (1)
you can click on the selection mask (in the editor dockable) and it
behaves like the Select by Color tool (not sure exactly how useful this
feature is as you can't really see where you click though) and (2) you
can drop a color and it will select this color also as though clicked by
the Select by Color tool (which looks quite useful!).
These features use the option values as set in the Select by Color tool.
Unfortunately both these features were broken when a non-zero threshold
was set because it expects a [0-1] range whereas threshold is [0-255].
Fix the parameters in gimp_channel_select_by_color() call.
Add a new "Messages" boolean parameter to performance logs, which,
when set, records GLIB log messages in the performance log as
markers, with an accompanying sample capturing their backtrace.
This option is enabled by default.
This got *again* broken because of some obviously impossible code path
(a same variable tested against 2 different values!). Anyway I tested
again and it seems that all possible interactions in the item tree views
are now correctly working though the whole button press code is a very
complicated mess of possible variations and events.
Add an option to record progressive performance logs. Progressive
logs contain complete information after each recorded sample, by
writing partial address maps at each sample, containing all new
addresses introduced by the sample. Furthermore, when recording a
progressive log, the output stream is flushed after each sample.
This allows recording complete logs even in cases where they can't
be properly terminated, such as when GIMP crashes or freezes in the
middle of the log.
Progressive logs are disabled by default, since they potentially
increase the sampling cost. They can be enabled through a toggle
in the log file-dialog, or through the
GIMP_PERFORMANCE_LOG_PROGRESSIVE environment varaible.
When recording a performance log, allow setting the log parametrs
through the file dialog. Currently, this includes the sample
frequency, and the option to include backtraces.
These options are still controllable through the
GIMP_PERFORMANCE_LOG_SAMPLE_FREQUENCY and
GIMP_PERFORMANCE_LOG_BACKTRACE environment variables. When set,
the variables override the values entered through the UI.
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.
Align GimpSpinScale with gimp-2-10, by modifying its appearance and
behavior to match the 2.10 compact style, fixing interaction along
the way. Unlike 2.10, there is no option to revert to the old
style.
After discussion with Carlos Garnacho, we came to the conclusion this
list is a useless feature. Basically what we call "input device" here is
pretty much "pointer device" only. We are indeed explicitly ignoring any
device identified as GDK_SOURCE_KEYBOARD, leaving us only with various
types of pointer devices (and pads actually, which maybe we should also
ignore in fact).
Such devices don't usually come with "keys", only "buttons". And in rare
cases of very weird devices coming with both buttons and keys, they will
usually identify as 2 separate devices (a pointer device and a keyboard
one) anyway, in Carlos experience, so we would still wouldn't have
access to the real keys anyway.
Moreover these keys were not only useless, but also sometimes confusing,
because some pointer devices would actually list keys, but then if you
tried to map some key event, it would not do anything (as they are not
real keys). The tablets I was testing with were such, reporting hundreds
of keys which do nothing and only confused the hell out of me.
Carlos says it probably means that the tablet drivers send bogus data of
key descriptions (so a bug in the driver, harmless yet still confusing).
So let's just get rid of this key list as our tablet expert says these
are bogus and let's see if anyone ever reports feature loss of some
extra weird pointing device which one would have used in GIMP while
mapping keys. Note that I leave the concept of keys inside
GimpDeviceInfo for now (only removing the widget part listing these)
just in case we realize we have to bring these back (less chance of code
conflict in the future when reverting the small GUI commit). But chances
are high that we will have to clean GimpDeviceInfo too and just get rid
of key code there.
In other dialogs, it is not a revert to how it was before opening the
dialog, but a reset to default settings.
To just revert to dialog opening values, we can just use "Cancel" and
reopen the dialog (a bit cumbersome, but not something done often
anyway).
Currently what "Reset" does is to set back the device mode and any
customized axe curve. It doesn't touch customized keys, but these will
disappear anyway in a further commit.
In GimpHandleBar, add gimp_handle_bar_{set,unset,get}_limits(), to
allow settings the handle-bar limits explicitly, rather than
inheriting the adjustment limits.
... which creates a widget controlling a pair of lower/upper range-
limit properties, comprised of a handle-bar and two spin-buttons.
If the "sorted" parameter is TRUE, the "lower" property is bounded
above by the "upper" property, and vice versa.
