… DBus calls.
In particular, Aryeom would start GIMP and directly double click some
image to be loaded in GIMP in the very short while when splash is
visible. Previous code would wait for the `restored` flag to be TRUE.
This was nearly it as we can actually start loading images as soon as
the 'restore' signal has passed. Yet the flag is set in the main
handler, but we actually also need the <Image> UI manager to exist,
which is created in gui_restore_after_callback() (so also a 'restore'
handler, yet after the main signal handler, i.e. after `restored` is set
to TRUE). Without this, gui_display_create() would fail with a CRITICAL,
hence file_open_with_proc_and_display() as well.
I could have tried to set the `restored` flag later, maybe with some
clever signal handling trick (and handle both the GUI and non-GUI cases,
i.e. I cannot set the flag inside gui_restore_after_callback() as it
would break the non-GUI cases). Instead I go for a simpler logics with a
new `initialized` flag which is only meant to be set once, once
everything has been loaded, i.e. once you can consider GIMP to be fully
running hence ready to process any common runtime command.
This is a continuation of #5888 as I realized that most layer modes were
fixed with my commit b3fc24268a (and follow-up f40dc40cbc) but at least
2 were still crashing GIMP: "Luma Lighten/Darken only" modes.
There were 2 bugs here:
* The first bug was that when gimp_operation_layer_mode_real_process()
ran, gimp_operation_layer_mode_prepare() had not been run yet.
prepare() is called before the process() of GeglOperation, but it
would seem the process() of GimpOperationLayerMode on the other end
happens before GeglOperation's prepare() is run. I am absolutely
unsure if this is expected or not and have a hard time figuring out
all the details of the C/C++ cohabitation.
As a solution, I am moving out the fish caching (the needed part
inside gimp_operation_layer_mode_real_process()) in its own function
so that I can easily call it separately before inspecting the fishes.
* The second issue was that some blend functions needed more than a
GeglOperation alone. E.g. blend_function() for luma lighten
gimp_operation_layer_mode_blend_luma_lighten_only() would call
gegl_operation_get_source_space() which requires the node to exist.
Similarly for the Luma darken only mode. So I keep both the node and
operation around, and when finalizing, I free the node (which in turn
frees the operation).
Ell > if you are reading our commits, I would really appreciate your
review (or fixes) of my code here! :)
My previous commit broke the autotools build. Apparently when using
g_object_unref(), some C++ symbol leaked into libapppaint.a archive
library, hence the main binaries (e.g. gimp-2.99) could not be linked
without adding -lstdc++ flag:
> /usr/bin/ld: paint/libapppaint.a(gimppaintcore-loops.o): undefined reference to symbol '__gxx_personality_v0@@CXXABI_1.3'
> /usr/bin/ld: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: error adding symbols: DSO missing from command line
Not exactly sure why using this GLib function in particular caused this,
but let's just try another approach in order not to link the main binary
with C++ standard lib.
Instead let's manage all GeglOperation allocation in gimp-layer-modes.c
by adding a gimp_layer_modes_exit() function and some static array for
storing operation object of each layer mode.
The GimpOperationLayerMode variable member in DoLayerBlend was not
properly constructed. C++ class constructor can be called by creating
object variables, but with GObject, we have to do it with pointers.
Otherwise here we were only allocating the memory for the struct, but
not actually calling any initialization functions.
Also it would seem that the struct was not initialized at zero, as the
space_fish variable was not NULL when it should (i.e. even with same
composite and blend space), hence composite_to_blend_fish was not NULL
and since the operation was not a valid GeglOperation when entering
gimp_operation_layer_mode_real_process(), we crashed.
Not sure how it went unseen for so long!
So instead let's make the layer_mode class member into a pointer. As
such, I have to properly allocate and free it. This is also why I am
adding a copy constructor which will ref the pointer (otherwise we unref
more than we ref as the default copy constructor would just copy the
pointer).
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.
Renaming the temporary function gimp_scale_entry_new2() into
gimp_scale_entry_new() now that the original code is entirely gone. This
is now a fully-fledged widget with a nice and proper introspectable API.
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.
* Add a gimp_scale_entry_get_value() because if we don't do a property
widget, getting the value of the widget easily is a main point.
* Move gimp_scale_entry_(set|get)_logarithmic() to the new class API.
* Internally edit the GtkSpinButton width depending on min/max values,
place digits, and possible value sign.
* Emit a "value-changed" signal (similarly to other widgets with a
value), for cases when just binding the "value" property is not
enough.
* Finally use the new API in palette-import-dialog.
It is more accurate to say it returns a list of parasite names rather
than a list of parasites (as we could take it as meaning a list of
GimpParasite). Of course, we would soon see the actual element contents
(if not for the introspection metadata (element-type gchar*)), but
better being accurate in textual docs too.
… instead of gimp_pdb_is_canonical_procedure().
The later would set an error saying "Procedure name '%s' is not a
canonical identifier". Yet the data label is not a procedure name. It is
a random name. I'm not sure why we need it to be canonical too, but why
not. In any case, let's use the right function.
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.
There were still a few references to functions which have been removed
from GIMP 3 (because they were deprecated in previous versions), which I
found as I was doing an inventory of removed functions.
This is not a fix, only an extra-ugly workaround so that at the very
least we don't end up with a splash screen taking the whole display on
Wayland.
Basically by setting 1/3 as the max splash size, a Wayland desktop with
no scale ratio will have a splash taking a third of the screen while it
would take 2/3 of the screen with a scale ratio of ×2 (of course, it
will still be very broken with a scale ratio of ×3 but are there
displays needing such high scaling?). The real fix will be when GTK/GDK
fix their API so that it returns what the docs says it should (i.e. a
size in "application pixels" not "device pixels"), as it does on X11,
Windows, macOS… Then we won't create random max size and we will be able
to properly control our splash size.
Note that this neither fixes nor works around the position issue on
Wayland (in my case, the splash was just always on top-left of the
display).
These small glitches have bothered me for a while now, so I finally
fixed these before the dev release!
Basically there were 2 fixes:
1. use the ink extents to compute any drawn area as this is what will be
actually drawn.
2. Not only expose the drawn area of the new text, but also the one of
the previous text in order to be sure all text pixels are correctly
reset (in case the new text is smaller than previous one). I.e. we
must expose the smallest rectangle containing both previous and new
area of text.
As AppStream docs says, <description> "tag should be translated
by-paragraph" in upstream metadata, which is what we have always done
(i.e. <_p> tags which becomes <p xml:lang="xy">).
Unfortunately as_app_get_description() is optimized to work for
Collection Metadata where the 'tag is translated "as a whole"' (again
cf. specs) for faster parsing. So we were ending up with a text mixing
the original and all localized texts.
I have opened a bug report to appstream-glib:
https://github.com/hughsie/appstream-glib/issues/381
While waiting for this to be fixed (i.e. when the function will handle
both cases accordingly to the metadata source), this code makes my own
locale extraction (defaulting to original text which is assumed to be
the previous same level tag with no xml:lang if no tag with the exact
lang attribute was found).
Stable versions (i.e. minor version number even, e.g. 2.10.22) will only
look for higher stable releases. But for unstable versions, we will want
to look up the development releases too. So for instance GIMP 2.99.2
will warn if the development version 2.99.4 has been released, but also
if the stable version 3.0.0 has been released (whatever is the highest,
which is the stable version in this example).
I had recently created gimp_item_is_ancestor() but realize it duplicates
gimp_viewable_is_ancestor() (which works on GimpItem since it's a parent
class). No need for duplicate code.
Thanks to Wormnest for pushing me to look further. Since gimp-file-save
is actually redirecting the call to another procedure (save proc for the
specific format) which might have more arguments, including string
arguments. When it finds any, it sets it to an empty string "" (which I
guess is ok as "default value when we don't know what to put there").
The previous code would not hurt. Starting at the fourth argument
(GFile), it would just do nothing, then continue with the firth and
further. Still, starting directly at the fifth arg is the proper code
for this.
The About dialog refreshes the release information relatively to
currently running version before being displayed. No check of the remote
JSON file is done, it only verifies whether any stored released
version/revision is not same (or even lower) than the actually running
version. Indeed existence of a stored release means that a newer version
exists at check time only. On later run, this stored information may
have become deprecated.
First the deserialize data of extensionrc was wrong. I need to edit the
pointer to the GList (and dereference it when I need).
Also when inserting an extension into the `running_extensions` hash
table, I could not reuse the same string as in the `processed` list
(because this string is going to be freed at end of initialization).
Just always use the GimpObject name of the extension, since it will stay
alive for as long as the object is alive.
g_list_find_custom() uses a GCompareFunc which has a return value
similar to strcmp(), i.e. with 0 for equality (and not a boolean, which
is basically the opposite).
When loading a theme on Windows we always get an error like:
themes_theme_change_notify: error parsing [path including drive letter to:]\theme.css: theme.css:8:99Failed to import: Operation not supported
The location points to the end of the filename of the first @ import url string.
This is caused by the string not being an url.
Based on suggestions from Jehan and lillolollo we replace g_file_get_path
with g_file_get_uri since an url is what is expected here.
Since that function already escapes the string we also remove
g_str_escape here.
In several GeglOperation filters, it is possible to pick a color. Up to
now, it was only possible to pick a color from the active layer (the one
you run the operation on). With this change, we can also pick in Sample
merged mode, same as Color Picker tool and other color tools.
The rational: advanced users won't really care about defaults (they know
to switch this option on/off depending on situation) but maybe beginners
would be less confused to pick "what they see" on first use, rather than
picking on the active layer? Now whatever is the default won't change
anyone's daily usage of GIMP. Clearly every image and use case is
different, so both with or without sample merged are useful one time or
another (this is why the option exists). It's really about the less
surprising option for beginners, based on usage statistics.
I ran a small poll on Twitter/Reddit/Patreon/Tipeee and ended up with
numbers of 131 for switching to "Sample merged" as default and 43
against, which is about 75% in favor. So let's just switch. It makes
sense anyway.
Make some of the bigger Preferences pages automatically scrollable if
needed. Based on my tests, this should be enough to fit on quite small
displays, at least with the default themes, even the 1366×768 reported
as too small. It should even fit in 1280×720 (in my tests, it did).
Targetting even smaller screens may be overdoing it for an image
manipulation software. We'll see if people still ask for a smaller
dialog.
Our Preferences exposes a concept of "Preferred color profile" (for RGB,
grayscale and CMYK), which is used in some places to be proposed as
default alternative to built-in profiles. But it was not used in the
import color profile dialog (only 2 choices were: keep the image profile
or convert to built-in RGB).
This commit now adds this third choice, which is even made default when
hitting the "Convert" button directly, without tweaking with the dialog.
Because we can assume that if someone made the explicit choice to label
such a profile as "Preferred", this is more likely the one to convert to
(if one even wants to convert from an embedded profile anyway).
As for the `Preferences > Image Import & Export > Color profile policy`,
they now propose 4 choices: Ask, Keep embedded profile, Convert to
built-in or preferred profiles.
… gimp_image_policy_color_profile().
These functions allow a plug-in to explicitly execute the Rotation and
Profile conversion policies on an image (which may be any of
Rotating/Discarding/Ask or Converting/Keeping/Ask respectively). These
policies are automatically executed when loading an image from GIMP
interfaces, but they won't be when loading an image from the PDB. Then
it is up to the calling code to decide what to do (which can be either
some arbitrary code or following the user policy).
Orientation is now handled by core code, just next to profile conversion
handling.
One of the first consequence is that we don't need to have a non-GUI
version gimp_image_metadata_load_finish_batch() in libgimp, next to a
GUI version of the gimp_image_metadata_load_finish() function in
libgimpui. This makes for simpler API.
Also a plug-in which wishes to get access to the rotation dialog
provided by GIMP without loading ligimpui/GTK+ (for whatever reason)
will still have the feature.
The main advantage is that the "Don't ask me again" feature is now
handled by a settings in `Preferences > Image Import & Export` as the
"Metadata rotation policy". Until now it was saved as a global parasite,
which made it virtually non-editable once you checked it once (no easy
way to edit parasites except by scripts). So say you refused the
rotation once while checking "Don't ask again", and GIMP will forever
discard the rotation metadata without giving you a sane way to change
your mind. Of course, I could have passed the settings to plug-ins
through the PDB, but I find it a lot better to simply handle such
settings core-side.
The dialog code is basically the same as an app/dialogs/ as it was in
libgimp, with the minor improvement that it now takes the scale ratio
into account (basically the maximum thumbnail size will be bigger on
higher density displays).
Only downside of the move to the core is that this rotation dialog is
raised only when you open an image from the core, not as a PDB call. So
a plug-in which makes say a "file-jpeg-load" PDB call, even in
INTERACTIVE run mode, won't have rotation processed. Note that this was
already the same for embedded color profile conversion. This can be
wanted or not. Anyway some additional libgimp calls might be of interest
to explicitly call the core dialogs.
A log error can have a NULL domain (apparently equivalent to "" default
domain, according to g_log_set_handler() docs and we even explicitly
list the NULL domain in the log_domains array in app/gimp-log.c.
Yet our log handler was not expecting such possibility and was running a
g_str_has_prefix() on NULL. Not sure why it aborted there. It might be
because outputting a new warning inside the warning handler did not go
well. Anyway this seems to fix our side of the bug #5358. The main fix
will likely be on GEGL side (UMFPACK_ERROR_out_of_memory error).
The script `create_test_env.sh` was registered in meson as a run target
(i.e. to be run manually by `ninja create_test_env`), which is really
not useful. So a `ninja test` was outputting various:
> You have a writable data folder configured (/gimp/build/dir/app/tests/gimpdir-output/gradients),
> but this folder does not exist. Please create the folder or fix your
> configuration in the Preferences dialog's 'Folders' section.
Unfortunately run target are only meant to be run from command lines and
cannot be used in 'depends' argument of test() or 'dependencies' of
executable() because "in Meson all dependencies are to output files, not
to concepts" (cf. https://github.com/mesonbuild/meson/issues/1793).
So instead a run_target() just directly use a run_command() and make
this script run during configuration step. Also make the shell script
executable as it was not.
See also #5666 as it was one of the errors outputted by the reporter's
log (though probably not the main issue).
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.
Additionally to loading the default fontconfig configuration file, GIMP
also looks up /etc/gimp/<version>/fonts.conf and
$XDG_CONFIG_HOME/GIMP/<version>/fonts.conf (or equivalent in other
OSes). If these don't exist (which is the most common case), this is not
considered a bug. Fontconfig had a regression bug of
FcConfigParseAndLoad() in 2.13.92, which was fixed in a later commit:
fcada52291
As a consequence of this bug, font loading failed in Windows when these
non-mandatory files were absent.
The current commit, originally proposed by Jacob Boerema (@Wormnest) and
slightly reviewed works around the issue, because anyway there is never
any reason why failing to load these files should break font loading as
a general rule. Even if these files exist and are broken (wrong syntax
or whatnot), we should just output some warning on stderr and continue
loading without these additional confs.
With fontconfig 2.13.92, warnings will be also outputted (wrongly), but
at least it won't block loading anymore.
Also let's unref() the `config` object even when the whole font loading
succeeds. Man of FcConfigSetCurrent() clearly says that the reference
count of config is incremented since 2.12.0 (our current minimum
fontconfig is 2.12.4) so let's not leak one reference.
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.
... which set/remove a GLIB log handler for all domains used by
GIMP.
Use the new functions in errors.c, instead of using
g_log_set_handler() directly.
Similar logics as Fuzzy Select (see my previous commit), which is that
if multiple layers are selected (and sample merged unchecked), it
selects as though through a composited image containing only the
selected layers.
When several layers are selected, and using the Fuzzy select tool
without sample merged (with sample merged, nothing changes, it just uses
the whole image), the selection will be based off a composited render of
the image with only selected layers.
This is equivalent to only making selected layers visible, running fuzzy
select with sample merged, then making other layers visible again.
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 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.
Making these actions multi-layer aware here means checking only exactly
1 layer is selected as these actions have GUI which shows dimensions and
preview (for the layer dimension action, i.e. layers-resize).
The call to "gimp-xcf-save" was not updated to the new API using a
drawable array and a GFile.
Also for the drawable array, do not actually create it as anyway this is
a dummy argument (so far XCF saving does not even look at this argument
and just re-check the selected drawables).
This parameter is not even used because we check again the selected
drawables during the save code. Allowing to set to 0 is simpler, and
also it will allow to not have to allocate an object array for layers
when we want to avoid memory management (during crash handling).
… argument validation.
I had a case where argument validation was failing on range and no error
was propagated (during a crash handling). Let's not leave these errors
totally silent as it makes such usually easy issues harder to debug. In
the specific case of no GError passed, just print the error to stderr.
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.
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_edit_selection_tool_translate(), perform the active-item
checks even when the effective translation offset is 0, and only
bail afterwards. This avoids erroneously calling
gimp_tool_message_literal() with a NULL message in this case.
In file_open_image(), mount non-native files *before* looking up a
file-proc. Previously, we'd only mount the file after the initial
lookup, and fail to perform a second lookup if the mount succeeded,
leaving us with a NULL file-proc and a subsequent segfault.
Additionally, simplify the rest of the remote-file code-path.
Basically if you enabled OpenCL or any of the experimental tools, it
will show the Playground in Preferences. Otherwise, say you enabled some
experimental feature months ago (e.g. with the CLI option) and you now
experience crashes or whatnot. And you forgot how to change it, and only
remembered that there was something in Preferences. It would make you
crazy to not find the tab again to disable the option.
This is even more important as OpenCL is moving from a normal option to
a playground option. So you might not even have ever seen the Playground
tab in Preferences and would not know how to disable OpenCL after you
enabled it originally in "System Resources" tab.
So now Playground is visible with any of these 3 conditions:
* If you use an unstable version.
* If you run GIMP with --show-playground option.
* If you previously enabled one of the playground options.
After discussions on IRC, it was decided that our current level of
support of OpenCL was not good enough. As a normal settings, people just
see it as a normal acceleration checkbox, even despite the warning text
and emoticone saying the opposite (i.e. it may even slow things down in
some cases).
Basically this feature needs more love to be back into mainstream
Preferences.
This file is available in a flatpak sandbox and will contain various
info such as the build commit, very useful info as we can have several
builds for a same version. For instance if we have exactly the right
commit, we can load exactly the same binary as a bug reporter very
easily, hence are able to get source correspondance without necessarily
asking reporters to install debug symbols (though it stays easier if
they can do it).
Other interesting info contained in this file are the exact runtime
used, the installed application or runtime extensions, the permissions
(people may override our flatpak permissions so it's useful to be able
to check when they did) and environment variables…
Even though this engine is optional, we already have the code to detect
its absence at runtime, and to fallback to "gegl:matting-global". So it
won't be a problem even then.
When the operation is present though, it definitely makes a lot more
sense than matting global as default, because it performs a lot better
in most cases (as far as I could see as well as others).
If the next visible layer below a selected layer is itself selected, we
want to create bigger merge list with all 3 layers merged at once (or
even more if the next-next is also selected, and so on).
When several layers are selected, each layer will merge down with the
layer below it. This is similar to running Merge Down several times, one
for each selected layer.
This seems to have been broken since much longer, but it only made a
problem with recent changes. Since we were duplicating layer groups and
contents layers at once, the current code could not keep layer selection
other than at root level in a duplicated image.
Use the layer paths to make sure we select exactly the right copied
layers, since the path should not change in a fully duplicated image.
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.
So it seems that pango_attr_iterator_range() could return G_MAXINT for
a Pango attribute when it is at the end of the preedit string. Looking
at Pango code, I see they initialize the attribute end property to
PANGO_ATTR_INDEX_TO_TEXT_END (G_MAXUINT), later clamped to G_MAXINT by
pango_attr_iterator_range(). So I guess for the specific case where we
are at the text end, it is normal. Only weird thing is that this didn't
happen at all on X11, only in Wayland.
So let's do our own pre-check. Also double the check by adding a UTF-8
validation.
This fixes preedit text not being displayed and the following warning:
> Gtk-CRITICAL **: 12:31:25.118: gtk_text_buffer_emit_insert: assertion 'g_utf8_validate (text, len, NULL)' failed
Even worse, this was potentially an out-of-range reading, though
fortunately checked early enough.
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.
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.
Fix horizontal downscaling of brush mipmap levels with odd width.
We'd previously fail to skip the last pixel of each input row,
which isn't included in the output when the width is odd, causing
subsequent output rows to be shifted to the right.
In GimpDrawableFilter and GimpFilterTool, use an absolute offset
for the split-preview guide position, instead of storing it as a
fraction of the drawable's width/height. The latter could
introduce rounding error, which would result in wrong coordinates
when converted back to an absolute offset.
In GimpGuideTool, when a custom guide (whose position is updated
directly on motion) is moved past the display bounds, keep updating
the corresponding GimpGuide's position, even though the guide will
be removed on release, to avoid leaving the guide at its old
position.
This affects the filter tool's split-preview guide.
Rather than the "Save" and "Close" buttons which were very weird, if not
misleading. When Aryeom was giving a course to students, several thought
the buttons were broken because "nothing happened" when clicking "Save".
So instead, "OK" will just save and exit (equivalent to click "Save"
then "Close" on old GUI) as it is the common usage and should be doable
in a single click.
"Cancel" closes while resetting to how the settings were before opening
the dialog.
Finally "Reset" just reset the settings to how they were before opening,
without closing the dialog.
This also makes the buttons look/behave like the ones on Preferences,
which is nice consistency-wise too.
The `mask` test was broken as it is not multi-layer aware (also it
should be negated).
The "layers-mask-edit" test was actually already fine because current
implementation only allow mask editing when single-selected. Still
improve the test to use multi-layer variables.
Thanks to Cyril Richard for the original patch proposition from which I
derivated the present one.
... by grouping the geometry options, which can be controlled
through the on-canvas UI, in an expandable frame, which is
collapsed by default. The shape option is not part of the group,
and is moved to the top.
In _gimp_prop_gui_new_generic(), when a pair of consecutive
properties have "range-start" and "range-end" roles, respectively,
and "luminance" units, use a range prop-widget for both properties,
as per the previous commits.
The range is sorted by default, unless the first property has a
"range-sorted" key of "false".
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.
... which takes a pair of GtkAdjustments, and binds the value of
the first to the lower-limit of the second, and the value of the
second to the upper-limit of the first.
In tool-group GimpToolButton tooltips, in addition to showing the
description of the currently-active tool, list the other tools in
the group as well, to improve discoverability.
Add a new GimpAccelLabel widget, which shows an accelerator label
for a given GimpAction. Unlike GtkAccelLabel, GimpAccelLabel
doesn't show a user-provided label in addition to that.
Note that the size request of GtkAccelLabel doesn't include the
accelerator part, which is desirable in some contexts.
GimpAccelLabel doesn't suffer from that.
This implied a lot of other core changes, which also pushed me into
improving some of the edit actions and PDB calls to be multi-layer aware
in the same time.
Note that it is still work-in-progress, but I just had to commit
something in an acceptable intermediate state otherwise I was just going
crazy.
In particular now the various transform tools are multi-layer aware and
work simultaneously on all selected layers (and the linked layers if any
of the selected layers is linked too). Both preview and final transform
processing works.
In the limitations, preview doesn't work well (only one layer in the
preview) when there is a selection (though the actual transform works).
Also I am left to wonder how we should process this case of canvas
selection+transform on multi-layers. Indeed currently I am just creating
a floating selection (like we used to for the selection+transform case)
containing a transform result of the composited version of all selected
layers. This is a possible expected result, but another could be to get
several transformed layers (without composition). But then should the
"Floating Selection" concept allow for multiple Floating Selections?
Sooo many questions left to answer.
The PDB creates the array of drawables as a `const GimpItem *` and the
compiler does not like when we drop the const qualifier. So force this
const dropping with explicit type casting.
This function returns a new list of items from an input list. The output
list will optionally contains linked items if any of the input item is
linked so we don't have to rewrite the same duplicated code for every
feature where item links matter.
Moreover it also filters descendants if any of the input items is an
item group, hence avoiding to apply a transformation twice to a
descendant item.
Use this new function already in 2 places, hence skimming quite a bit of
redundant code.
Proposed by Aryeom to make it more obvious of a possible issue when
running "Alpha to Selection" and ending up with an empty selection
(which is useless hence may means there might have been a problem in
one's workflow).
This warning will also occur for similar actions (i.e. the
Add|Substract|Intersect Alpha to|from|with Selection actions).
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.
This can be used in various places where we want to check whether a
previously saved list of drawables is still the same list of selected
drawables. It used to be easily done with an equality test with a single
active drawable, but not anymore with a list of selected drawables.
Override GimpTool::get_popup() in GimpDrawTool, forwarding the
request to the tool widget, if one exists.
Remove the same code in GimpVectorTool -- this now works for all
tools/widgets.
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.
In GimpImageProxy, implement GimpColorManaged by forwarding the
functions to the underlying GimpImage, and forwarding the signals
in the other direction. This fixes color-managed view in the
Navigation dockable.
Also layers-flatten-image does not care about the layer selection and
layers-anchor works anyway only when there is a floating selection,
which means only one layer selected.
All selected layers' masks are combined first as union, then the result
is combined with the image selection according to requested boolean
operation.
Also use new gimp_channel_combine_items() function.
Don't just name all alpha-selection actions the same "Alpha to
Selection". Have some more accurate naming like "Substract Alpha from
Selection", etc.
Also improve the action names to make them more accurate as these names
are not only in menus anymore, but also in search actions (and we want
to avoid duplicate naming).
When converting an image to indexed mode, zero-out transparent
pixels instead of leaving junk in their indices, which might well
be out of range of the palette.
I created a new function gimp_channel_combine_items() which combines a
list of items with a channel. The list of items is first combined
together as an union set, then only combined with the channel with the
desired operation (this is important for operations such as intersect
which was broken in my previous commit because all items would be
intersected with each other and the selection, whereas we actually want
the union of all items to be intersected with the selection). This new
function is now used for "Alpha to Selection".
Also similarly to copy or color-pick on multi-layers, alpha to selection
will now use the composited result of the multi-layers as visible. In
particular it means that opacity, modes and visible properties on the
layers are taken into account. Alpha to selection on a single layer
though still works as previously, only taking the non-composited layer
data into account.
I am actually struggling if alpha to selection on uncomposited layers
(just an union on alpha to selection result for each layer) would not
make sense to on some workflows. To be experimented.
Finally it is to be noted that this function should also work on
channels and vectors (both single or multiple; and of course in such
cases, compositing does not matter) though I haven't tested yet. It
could even work with a source mix of layers, channels and vectors,
though our GUI does not allow such action currently.
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.
It means forbidding gradient tool to work (with appropriate error
message) on multiple selection. Applying gradient on several layers at
once is not supported at this point.
This fixes:
> GEGL-WARNING: (../../src/gegl/gegl/buffer/gegl-tile.c:127):gegl_tile_dup: runtime check failed: (! src->damage)
Which happened when a GEGL operation was running and you canceled it in
the middle, say with the ESC key (easy to reproduce with long operations
such as "Color to Gray"). In such case, obviously don't try to copy the
unfinished operation result into the dest buffer.
This fixes bugs introduced in commit a7c59277fb where I obviously didn't
properly checked all the places where gimp_selection_float() was used
after its parameters changed.
Multi-drawable selection for text tool only matters for positionning the
newly created text layer, if relevant. If a single layer is selected, it
will be same as before.
With multiple selection, the new layer just ends up to the top in case
of layers from different containers.
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).
When creating a palette out of an image without checking "Sample
Merged", it will now extract the colors out of each individual selected
layers separately. This allows to create palettes even out of all layers
of an image but still considering these individually.
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.
Multi selection actually only really matter when "Merge within active
groups only" option is checked, in which case we are able to merge
layers within several layer groups simultaneously, and end up with
multi-selected merged layers.
Also not sure why both layers-merge-layers and image-merge-layers exist,
as they are exactly the same (exact same callback called when
activated).
When several layers are selected, select their render, similar to how
"edit-copy-visible" would have copied an image with only these layers
made visible.
Also apply the same logics to PDB function gimp_edit_copy() which can
now be used on several drawables at once.
Similarly to shift-click on visibility or link toggles, shift-click on
expanders allow to expand/collapse all layer groups at a given level,
but the one initially clicked.
- Don't trigger selection when toggling visibility/links.
- Fix some weird selection jump when clicking while editing a layer
title.
- Return FALSE in GDK_BUTTON_RELEASE so that viewable larger preview
popup gets closed on mouse release.
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.
Both with the various action layers-lock-alpha, layers-opacity-* and
layers-mode-*, as well as through the layer tree view GUI (alpha lock
icon, opacity slider and layer mode combo box).
Not fully happy of the action sensitivity tests as they check for
content-lock, layer groups and such, but independently. So we could have
a case with 2 layers selected: a group + a content-locked layer. Yet the
layers-mask-apply action would be sensitive.
But instead of make more tests, I am nearly wondering if we should not
make action sensitivity more lax as the actual action will make the
tests anyway and just do nothing.
… multi-selected items.
The idea is that with multi-selection now enabled, you may always lose
track and forget you have several items/layers selected. This is
especially true when you have a lot of layers (say you selected one at
the top of the list, another at the bottom; without scrolling, you may
only see one of them). And this can become bad when doing some
destructive action which is allowed on several layers at once (say
deleting several layers while you thought you were deleting only one!).
I got the idea from Thunderbird GUI which also displays the number of
selected conversations in the email list. Same as in Thunderbird, I also
wanted to theme the label similarly to a selected item in the item list
below. This was hard because there is no way to reference the parent
theme colors from within GIMP one. Instead I made so that the label is
always shown as a fully selected text (which is a bit ugly semantically,
but I could not find better). Why I wanted this styling is to give *a
bit* of focus to this info so that it is not too invisible. Otherwise
purpose is defeated. Yet this label is still more subtle than
Thunderbird one (don't want to take all attention toward it). Hopefully
this got the right in-between style.
Jehan