… for the case of data managed by the GimpContext.
Checking stable and dev builds, it looks this just never worked and used
to just show no selection. Now it is more annoying as it shows a
selected item, but not the right one. Just make sure it shows whatever
is currently selected.
The whole hack for types managed by context is not needed anymore. It
works fine with generic code. Also because of this hack, there was a bug
when clicking on some button raising a container popup (such as the
"Dynamics" button in tool options) would reset the context to default
value (e.g. reset to "Basic Dynamics") on the first click, without
raising the popup. Only the second click would raise a popup.
… GimpContainerComboBox and add a warning when the implementation is
missing.
Basically the default get_selected() implementation only works for
context properties, not for more generic usage of GimpContainerView. The
new warning will be a lot more informative and will help any future
cases where we might experience this bug.
This fixes the container entry never showing (e.g. brush/dynamics) names
because after commit ca3c480314, the class could not properly detect if
the selection changed (as default implementation for this virtual
function does not suit this subclass).
This prevents repeatitively running the same signals when it is useless.
In particular, I encountered a case of infinite loops between
"floating-selection-changed" and "select-items" ending up infinitely
calling each other (then crashing GIMP).
It's a bit ugly, but it's not like this is run many times (only once
when loading the icon theme, or changing it).
Fixes this error appearing in various unit tests' output:
> gimp_icons_sanity_check: Icon theme path has no 'hicolor' subdirectory: /builds/GNOME/gimp/_install/share/gimp/2.99/icons
(even though it was not a test-failure error, it's still better to limit
output for debugging)
This should fix these errors when running `ninja test` or `make check`:
> GIMP-Error: Failed to open file ?/builds/GNOME/gimp/_install/etc/gimp/2.99/templaterc?: open() failed: No such file or directory
> GIMP-Error: Failed to open file ?/builds/GNOME/gimp/_install/etc/gimp/2.99/controllerrc?: open() failed: No such file or directory
When running tests, the data are not meant to be necessarily installed.
Therefore icons won't be found when calling gimp_widgets_init().
Add some special-casing to find them relatively to the install
directory.
An item with no path is actually not always a bug. It can very well be
normal if the tree view has a filtering logics where some items which
were selected suddenly get hidden.
The 3 available formats are: simple text search, regular expressions and
glob patterns (cf. previous commit). I did a small step back from
previous commit by getting "is-pattern" property back in GimpItemList
instead of having this case as a value of GimpSelectMethod. The reason
is that it would render a useless value in the Preferences combo box.
Text search is the default.
… search syntaxes.
The layer tree view is only using regexp so far, but the core code is
updated to allow more.
Simple text search is actually a bit more than "simple". It implies
tokenization of the text, Unicode normalization and case-folding. It
will also search with ASCII alternatives when possible. This includes
things like non-accented ASCII characters matching accented variants
which is neat.
Now it's not perfect. For instance tokenization seems very limited to
writing systems with spaces or alike. In particular, I tested with
Japanese and since you would typically write without spaces, a whole
group of several words would be one token. Since the text search
algorithm only search from token start, this is quite a failure as you
can't search with intermediate words only.
Similarly to how I improved "Select Top|Bottom Channel", we should set
the actions sensitive not only when the selected layers are not the
top/bottom ones, but also when there are no selected layers at all.
I had a weird case where g_return_val_if_fail() ran but I could not
reproduce and am not sure how this happened. Adding the item name may
help diagnose the next time it happens.
Now that we have implementations for select_items everywhere and that
all the code is only wired to call or handle select_items, the single
item variant select_item is of no use anymore. Let's make a big cleanup.
… for the container tree view contextual menu.
A very annoying point of contextual menus is that they happen on button
press whereas menu item selection happens on button release. When the
menu corner is positionned on the click position, nothing bad happens;
yet when place is missing on screen, the menu might get positionned over
the pointer position. And worse, the mouse position might be just over
an activatable menu item. So we end up in this weird situation where a
click implies: press, menu opens, release, random item (whatever is
below the pointer) is selected and menu closes.
To get rid of this weird case, let's have our contextual menu happen on
button release. In reality, I don't think anyone cares that it happens
on press or release, you just "click". But what you certainly don't want
is to click random menu items!
… the container.
There was this weird case which we somehow could only reproduce on
Aryeom's computer/build, not mine, with the same code and reproduction
steps (reproducible at will on her build only). Basically when drag'n
dropping a duplicated layer inside a collapsed layer group, the
row-expanded handler would try to select the moved layer before it is
actually inserted. This would end up into crash-happy code.
I'm still unsure of why the order of operation is different here, but
anyway what is for sure is that the `inserting_item` boolean flag was
not protecting much. It's not like it's an actual mutex and anyway this
is not multi-threaded code either so this flag was mostly useless (which
is why we were crashing). Instead let's actually look if the item is in
the container or not.
… and use gimp_container_view_select_items() when the context changes.
Even though some types of containers still expect only a single
selected content, we should slowly move to multiple item code. The
reason is to avoid 2 code paths which makes the code more complicated
and bug-prone. When all child classes of GimpContainerView will have a
valid select_items() implementation, we can work on getting rid of the
select_item() in favor of the multi-item one.
The bin window coordinates must be converted to the widget coordinates.
This was not a problem before, but now with the column header, if I
don't do this, the position is wrong.
Similarly fix the lock popover position.
The header shows a lock icon, making the column more discoverable, as
well as where to click.
Also the title for the item column will now be used for the
multi-selection label (counting items) instead of using
GimpItemTreeView's options box.
Finally use the new gimp-multi-lock icon to show multiple locks.
This makes for much nicer and usable item tree.
… the selected items only.
This is the exact same algorithm as Shift-click, except that Shift-click
switch exclusivity within the whole level of items. Alt-click does the
same but only within selected items in the list.
Similar to exclusive visibility switch on layers, now for locks too, if
one wants to lock all layers within a same level for instance by
Shift-clicking the lock icon.
Also once again I factorized the exclusive switching code to ensure it
will always works the same for all similar features (visibility and all
locks).
First of all, let's use gtk_widget_show|hide() instead of
gtk_popover_popup|down() because these GtkPopover API sometimes "block"
when clicking very fast on the item list, sometimes up to 2 or 3 seconds
(at least it feels like seconds, I haven't actually timed it!). I'm sure
the ones who developed it thought it was nice to have slow popping
dialogs, but when working fast, this can only be frustrating).
The second responsiveness change is that when clicking out of useful
lock popover area, it should not only close, but the event should be
passed onto the item tree view when relevant. This allows for instance
to directly click on an eye cell to change a layer visibility without
having to click twice.
Use this for the alpha lock brought by GimpLayerTreeView.
Also use the new gimp_widget_blink_rect() to only blink the appropriate
cell when a lock is preventing you from doing something.
… and drop the link cell (the lock cell takes the space).
This is an experiment with the following logics:
* I am getting rid on the linked item logics, so the icon cell
disappears anyway.
* The lock buttons are not so visible above the Layers dockable and so
many have I seen people frustrated of not being able to do some action
until they realize they locked something in the layer (even sometimes
advanced users).
The icon next to the eye is much more visible. Also I will now display
different icons depending on the type of locks. If a single lock is in
effect, I show the corresponding icon. If 2 or more locks are in
effect, I show a generic lock icon.
* With multi-selection of items in particular, this top lock row was a
lot more weird and could show inconsistent state (some of the
selection is locked, other is not). Now the per-row lock icon allows a
much nicer granularity.
Sometimes one may want to lock visibility of a given layer. This is very
useful in particular when shift-clicking a layer visibility. In this
case, it won't be included in the list of layer to update. This can be
used for instance if you want some layers always visible (or always
hidden) while setting exclusive visibility of some other layers only.
Instead of just storing list of layers, I created a new simple type
GimpItemList (actually GimpItemSet would be better named, but
unfortunately we use this name for an enum type). So far, this new class
can handle 2 types of item sets: named fixed sets and pattern-generated
sets.
I am unsure if regular expression is the right choice as it may
obviously be a bit too technical. I hesitated with glob-type match for a
while. We'll see!
The eventual goal is to replace the "linked layers" concept, which is
why I am using similar vocabulary. The point is that linked layers are
mostly useless/redundant now with multiplie layer selection, except for
one thing: they kind of serve like a way to "save" a selection of layers
(to be moved/transformed together mostly). Apart from this, multiple
selection is more powerful on any way. You can do much more than
transforming the layers together (you can reorganize them together,
delete them, crop them and so on).
Therefore this new feature is the way to fill the only weakness of layer
selection: its ephemerality. Now we can save a given set of layers, not
even only one, but as many as we want, and under a meaningful name, for
later reuse.
Moreover it will make layer-handling core code much simpler as we
currently have 2 concepts of layer set: multiple selection and links.
The new stored links are only a way to recreate multiple selections.
More is to come, for instance right now, these are not stored in the XCF
format. Also it would be awesome to add logical operators (Shift for
union of layer sets, Ctrl for subtraction and Shift-Ctrl for
intersection). And finally I was thinking about a way to select by
pattern (regular expression? Shell-style glob patterns?) and even store
these patterns. So if you save a "Marmot .*" selection pattern, then
when you select it later, new layers matching this pattern will be
included too (instead of fixed-in-time list of layers).
This change allows gimp to run from the build
directory rather than having to wait for the
full packaging process to complete.
However, 'gi' modules are not loading properly
so that might be something to fix.
Rather than trying to fix up our own heuristics using a
`GtkMenuPositionFunc`, use whatever GTK provides to position given a
specific rectangle, which also has the benefit of nicely integrating
with GDK backends such as Wayland. Another advantage is that we can use
GdkGravity to center the popup.
Since GTK 3, GtkWidget also gained a "popup-menu" signal, which we
can/should use instead of rolling our own context signals.
Rather than trying to fix up our own heuristics using a
`GtkMenuPositionFunc`, use the API that GTK provides to have popup
nicely placed near the pointer, which also has the benefit of nicely
integrating with GDK backends such as Wayland.
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.
Fix as suggested by Massimo with formatting adjustments by me.
The use of gulong and glong is not cross platform safe: on Windows it is
32-bit and on most other platforms 64-bit.
Let's use guint64 and gint64 instead.
When we try to export a grayscale image with layers with negative offsets
to a GIH brush GIMP crashes without producing any crashlog.
Running in GDB showed us that there is heap corruption caused by incorrect
computation of buffer sizes because of the negative offsets.
In file_gih_image_to_pipe there is a comment that offsets are assumed
positive, but no checking is done whether that is correct.
Let's add some checks, set offset to 0 if negative and adjust width and
height accordingly.
… gimprc's manpage as a consequence.
When running `gimp-console-2.99 --dump-gimprc-manpage` to output a man,
a line was:
> .TH GIMPRC 5 "Version @GIMP_VERSION@" "GIMP Manual Pages"
This is clearly the autotools substitution syntax, which is not being
used here (this is not a .in file processed by the build system), maybe
from some older build logics.
… multiple drawables as parameter.
Previous commit 7bb892f3 was "making it work" by making the API
inconsistent and also only using the first drawable, which is making the
logics meaningless.
Instead accept multiple drawables, and export only the selected drawable
(when alone) or the merged-down image containing only the selected
drawables (when many).
Note that in current implementation, this is not useful from GUI calls
because the fully merged image is always exported when run interactively
or with last vals (i.e. from the GUI) because gimp_export_image()
flattens the image. So this change would only work when called
non-interactively from other plug-ins. In such a case, multi-layer
images do no longer return an error and whatever items are selected
would change the export result.
See also #7370 for a discussion about how to handle the selected items
during export (because currently the `drawables` parameter of
GimpSaveProcedure's run function is clearly a mostly bogus parameter).
In all 3 cases:
- xcf_read_string allocates a new string into name.
- If an error is detected, we return early - name remains unused.
Therefore we add g_free (name) to these early returns to avoid a leak.
Saving a thumbnail is closely related to the other metadata preferences,
but so far this was the only one that didn't have a preference for a
default user value.
This commit adds a preference in the metadata section where a user can
select whether thumbnail saving is enabled by default or not.
On Windows the --dump-gimprc, --dump-gimprc-system and --dump-gimprc-manpage
command line parameters do not produce any console output.
Apparently we need to request the handle for stdout instead of directly
using1 for stdout.
After this commit there still is a problem when redirecting output to a
file. It seems that the buffer where stdout is stored is not flushed or
the file pointer is reset to 0 every time causing overwrites instead of
appending to the file.
When running gimp-console-2.99.exe --dump-gimprc-system we get two
warnings:
(gimp-console-2.99.exe:23000): Gimp-Config-WARNING **: 16:08:29.604:
FIXME: Can't tell anything about a gint64.
Let's fix this by adding G_TYPE_INT64 as a known integer value.
This is untested on my side, because the bug only happens on native
builds with meson (our CI has cross-builds with meson and native builds
with autotools and I only do cross-builds locally) but I think/hope it
will work.
Basically we were using .full_path() because these rc files were also
used as input of some configure_file() calls which doesn't like custom
target objects as input (it wants strings or file objects). Yet a bug
in meson didn't like the colon used in native Windows full paths ('C:'
and such) when used in windows.compile_resources(). This has been fixed
by Luca Bacci in: https://github.com/mesonbuild/meson/pull/9368
Yet we just cannot depend on very early meson (or worse dev meson code).
On the other hand, if the input is a custom_tgt object, it uses the
object ID which we give as first parameter of custom_target() so we know
it's appropriately named without colons (such as 'gimp_plugins_rc').
Thus we should not bump into this issue again.
For the few usage in configure_file(), I just add a .full_path() only
when needed at call time.
Last but not least, I replace the bogus `meson --version` call by a
`python3 -c 'exit()'` as advised by Eli Schwartz:
2afa019c70 (note_1284951)
The reason is that it is apparently possible (or will be when some
reimplementation of meson will be done) that the `meson` executable
itself does not exist. On the other hand, `python3` should always be
there, as a mandatory dependency of the build tool.
In order to use an appropriate `python3`, I made the
pythonmod.find_installation() check required in our build (which should
not be a problem since it's a meson requirement as well), even when the
-Dpython option is false (this one depends on other requirements too
anyway, such as version and pygobject). This way I can call this meson
variable of discovered python in my bogus call, instead of calling a
(potentially different) python from PATH environment.
… 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.
Reported by Massimo, though this one is wrong, as far as I can see.
`ninja scan-build` apparently reports `result` as being returned after
being freed here. But actually since we are setting the `error` with
g_set_error() in the same time we free `result`, we would also enter the
`if (*error)` block a few lines later, which would return NULL and not
`result` anyway. I guess the static analyzer could not see that far.
Still, set the pointer to NULL with g_clear_pointer(), which should be
enough to make the static analyzer happy.
Commit 086ae77929 had broken dnd of colors from toolbox's color area on
Wayland. Clearly Wayland did not like we changed focus on a click,
breaking the drag.
To fix this, do not propagate button press and release events from the
GimpFgBgEditor editor anymore. Yet, since changing colors is usually to
be used (often immediately) on the canvas, giving back the focus to
canvas still makes sense. Therefore, instead of using press/release
events, add semantic signals to GimpFgBgEditor: color-dropped,
colors-swapped and colors-default (additionally to already existing
color-clicked). Then connect to these new signals to grab focus for
canvas when relevant.
Thanks to Massimo for raising the broken color dnd feature.
Commit 83b3d9e5 broke dragging color widgets into channels dockables or
other item dockables.
Thanks to Massimo for raising that these lines were needed for this.
The "Source" dropdown to choose an image or pattern, and to check
"Sample merged" seem important enough that I moved them up the source
tool options. I also added a label giving information about the image
source being currently set, i.e. in particular which image (when the
source is another image), how many composited layers (or all of them
with "Sample merged" checked), or if each layer is its own source.
For this to happen, I moved src-drawables property from GimpSourceCore
to GimpSourceOptions (though without making it a config property,
because we don't want this option to be saved in config files). It
actually makes sense, it is a kind of "option" of how the tool will
behave, and then it is also visible by the options GUI.
The "Registered" alignment is used to paint from one layer to another
(in same or different image) at exactly the same image coordinates. It
doesn't make much sense with the self-to-self painting when having
multiple drawables selected.
Note that it still works with the other new feature using multiple
layers as a composited source (limited "Sample merged"-like painting,
but only from specific layers).
Similar to the source core fix, but a bit simpler because we don't have
to deal with a source and a target offset, let's fix offset handling in:
- Blur / Sharpen tool.
- Dodge / Burn tool.
- Ink tool.
- MyPaint brush tool.
- Smudge tool.
As expected from early changes of code, painting was widely broken with
offsetted layers, because previous code used to process the drawable
offset earlier in the painting process, on paint tool level, whereas now
the tool gives coordinates in image space to the paint core (because it
gives a list of drawables which may have different offsets, hence image
space is the only valid coordinates space). This means the various paint
core algorithms must handle each drawable's offset at actual painting
time.
By doing this, I also add the ability to use a composited projection of
the selected drawables as source. This is similar to "Sample merged"
except that instead of using the whole visible image, we use what would
have been visible if only the selected layers existed.
Note that this doesn't work together with the previously added ability
of multi-cloning from each layer to itself. This ability works for
cloning from multiple layers to one.
- Make the various virtual methods of GimpPaintCore use a list of
drawables as argument instead of a single drawable.
- gimp_brush_core_eval_transform_dynamics() can work with an image as
argument rather than a drawable as it doesn't actually depends on
specific drawable data.
- New function gimp_paint_tool_enable_multi_paint() to be used in init()
method of paint tools to announce that this tool can work with
multiple layers selected.
- Use gimp_paint_tool_enable_multi_paint() in the GimpSourceTool base
class only for now.
This is a first step for multi-layer drawing, but we don't want it to be
possible in just any random cases, which is why I add a special function
to advertize this capability. We will use it for special-casing the
clone (as well as heal and perspective tools most likely) tool to work
on several layers at once. At this step, it is still very bugged and not
really working properly. In particular, since we don't process the
drawable offset early anymore (because it makes no sense when we pass a
list of drawables with different offsets), I suspect that all the
offset-related code will be very broken.
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.
- Migrate "view-rotate-reset" to "view-reset" (there is a
"view-rotate-reset" in GIMP 3, but it will be only for rotate; what
was really doing the same-named action in 2.10 is now what
"view-reset" does).
- Make sure we don't migrate "file-export" from a 2.10 config. From a
2.8 or below, we don't to migrate it (same as for 2.8 to 2.10), but in
a 2.10 config, it is already the same action as the one in GIMP 3.
Action "view-rotate-reset" renamed to "view-reset" (resets both flipping
and rotate). New "view-rotate-reset" and "view-flip-rotate" for
resetting rotation and flipping respectively.
Fix the search for previous folders, which was broken as it was
specifically expecting 1-digit numbers so far.
The differences of the GIMP 3 config import are:
- update sizes and positions in the sessionrc according to the scale
factor, because GTK2 doesn't have scale support. It means that, e.g.
with a 2× display, all sizes and positions in GIMP 2.x must be divided
by 2 (otherwise the first thing many people will get when testing GIMP
3 for the first time is an off-screen window).
Of course, I even wondered if it would not be nice to just drop the
sessionrc altogether and start with a nice blank slate, but then you
also lose the opened dock and their organization and some settings
(such as whether you chose single or multi window mode, etc.).
- scripts/ and plug-ins/ are not imported. Probably makes no sense so
far as they would end up broken (but maybe it's not true for all
script-fu scripts?).
Since the recent changes to add template ability, there are just too
many widgets stacked up vertically. Let's use a little better the
horizontal space.
To do this, I moved the preview on the right side of the "Canvas Size"
and "Offset" number fields, and the "Center" button just below it.
Let's move the g_date_time_format() to the inner block when it's
actually used, hence also make sure we allocate and free at the same
code level, which is a better practice to limit leaks.
Reported by Massimo.
As explained in GTK docs, size groups are referenced by every internal
widget hence can be immediately unref-ed after we added widgets in it.
In the end, when the widgets are destroyed (thus removed from the
group), the group will end up freed too.
Reported by Massimo.
I first sink these, because they are floating references. This is
actually quite unneeded here, because callbacks given to
gimp_action_group_add_procedure_actions() (this is what
filters_history_cmd_callback() is for) are not taking ownership of the
GVariant anyway. Yet just in case this ever changes, this is the proper
way to do it to avoid a double-free in the future. We take ownership in
the calling code, hence we free the variable there after using it.
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.
This function is not only used as "response" callback to the merge
dialog, but also called directly with a NULL dialog in
image_merge_layers_last_vals_cmd_callback().
This commit fixes:
> (gimp-2.99:130330): Gtk-CRITICAL **: 22:17:59.774: gtk_widget_destroy: assertion 'GTK_IS_WIDGET (widget)' failed
Reported by Massimo.
A few issues existed in the code:
- When the layers selection is changed, make sure we remove all
duplicates (not only the first one). This should not be a problem
anyway because we also do this duplication cleanup elsewhere now, but
still…
- Fix gimp_image_layer_stack_cmp(): we were comparing a GList element to
the data of another, so we were actually never finding duplicates.
- Add gimp_image_clean_layer_stack() for internal layers stack
management/cleanup. It takes care of recursively making sure we don't
leave duplicates, and remove all empty lists.
- Now use this new cleanup function inside
gimp_image_remove_from_layer_stack() instead of doing some incomplete
and broken element removal. This was especially broken as we were
removing a GSList element from a list we were iterating on (so we were
dereferencing a now freed element). This last issue was reported by
Massimo, and this is how I found the more general failure in this
layers stack management.
This fixes a memory leak as reported by Massimo.
But even more, we fix the code logics, as the cleanup items were never
added to the `item_cleanups` list.
As noted in issue #6032 the error message when trying to open a pattern
file with dimension larger than supported could be a little clearer.
We do this by adding the maximum allowed dimensions to the error message.
For GIMP patterns we have maximum allowed dimensions which we check when
loading a pattern. However, we did not check this when saving a pattern.
See issue #6032.
This commit adds a check when saving a pattern and adds a descriptive
error to make clear why saving fails.
I somewhat bisected GEGL commits between 0.4.20 and 0.4.24 and found
that the one that introduced the env var GEGL_OPERATION_INIT_OUTPUT is
the first showing the problem.
Reviewer (Jehan) note: so it would be commit 6e9610e65c on GEGL repo.
This fix makes sense as it means that since this commit, the output
buffer could have random values. It's not a problem for any operation
where we fill every value, but I guess it's not the case for
"gimp:cage-coef-calc" which was likely relying on the old behavior of
being 0-initialized.
Reviewer comment (Jehan): we have used this patch successfully on our
installers since start of 2021 (see commit b4d665d of our gtk-osx fork)
and it really improved the situation. I only fixed minor coding style
stuff in the patch.
Looking at what it does, I guess it is not ideal long-term if related to
10-bit display (as I understand from the comment), which a graphics app
would want to support properly. But for now, this is better than
extra-slow display until we get macOS developers able to look at this
more in depth in the future (I don't think that our dependencies are
really ready yet for 10-bit display support anyway, though I may be
wrong).
Some other forums seem to say it comes from macOS invalidating now more
than it should (i.e. the whole area instead of only the changed area)
and this NSViewUsesAutomaticLayerBackingStores flag would disable this
behavior. It might be one of these reasons, the other or both. This is
anyway a good first start for future contributors.
g_alloca() is unadvised. Even though it might be more efficient in some
specific cases, it is pretty subject to stack overflow when a lot of
memory is requested.
Let's allocate dynamic memory instead. To avoid doing it too much, let's
just reuse the same pointer especially since region of interest will
usually be the same size when iterating a buffer, except for border
ones (which would usually be smaller, so we can use the same allocated
buffer again). I still make size checks, just in case.
Thanks to Massimo Valentini for finding these.
Fixes:
> GLib-GObject-CRITICAL **: 13:21:53.256: Object 0x5485140 of type GimpLineArt not finalized correctly.
> GLib-GObject-CRITICAL **: 13:21:57.472: Object 0x231f520 of type GimpExtension not finalized correctly.
Outputted when glib is built with -Dglib_debug=enabled and GIMP is run
with GOBJECT_DEBUG=objects.
After updating a keyboard shortcut in the Configure Keyboard Shortcuts
Dialog, the section containing the changed shortcut disappeared.
Apparently a changed shortcut makes its parent invisible so we make the
parent visible again.
We also store a text version of the selected path in the tree to the
shortcut and then use that to restore the path after making the parent
visible again.
On most keyboards the [ and ] keys are shared with { and }. Which means
that if you press Shift+[ you get {. We were using this key combination
to increase the tool's size by 10 and the other to decrease it by 10.
However, on all keyboards where these keys share the same physical spot
on the keyboard, this wasn't working.
So, let's change the actual keys to do this to { and }.
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 markdown triple-quote (```) has to be on its own at the start of a
newline. Schumaml was telling me that too many reporters would paste
just after some text, which would therefore break the markdown syntax.
Instead let's add 3 newlines before the triple-quote, so that even
people who would not hit the "Copy Bug Information" button (but instead
select and copy) have a hint that these newlines are made on purpose.
Also add a comment (which is discarded by Gitlab) to make this even more
obvious.
Then even when pasting just after some text on the same line, the
triple-quote will end up on its own line.
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.
Testing this middle-click opening of image by their PATH/URI in the
toolbox, I realized there was a bug. The original author was obviously
intending to unref the toolbox which was ref-ed when calling
gtk_clipboard_request_text(), not freeing the toolbox context, which
could have dire consequences!
Fixes this CRITICAL:
> g_object_unref: assertion 'G_IS_OBJECT (object)' failed
Toolbox buttons don't require focus keeping (no further keyboard input
needed and navigating through/selecting tools can be done in various
other ways with a keyboard), which is why I removed focusability of
toolbox buttons years ago (cf. commit c83ee61c07).
Nevertheless this is not enough, and since toolbox is meant to work on
the canvas anyway, let's give back canvas focus as soon as any click
happens on dead areas of the toolbox, as well as on the Wilber (dnd)
drawing, but also each time a toolbox button is selected (though not on
the longer clicks to see tool groups' contents without selecting a tool
in the end).
This will allow to directly start working on the image without requiring
a click first (for instance panning with Space or similar interactions
which would not give focus to the image canvas).
This can also count as an alternative to the "Esc" shortcut to get
canvas focus back, with a mouse click instead of a keyboard key.
Don't enable conditionally based on the buildtype.
Further, don't use `add_project_arguments()` to enable the instructions:
this will lead to crashes within g-ir-scanner, which can't properly
parse these instructions.
https://gitlab.gnome.org/GNOME/gimp/-/issues/5053
In Linux, I never paid attention to it because it was already working as
expected (i.e. Ctrl-n followed by Enter for instance would directly
create the image), but when testing GIMP on Windows for the installer,
these last few days, I realized that hitting Enter when focus was on the
dimension entries was doing nothing. Having to move the pointer to click
a button on Windows was frustrating. So let's make OK the official
"default response" directive for this dialog.
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!
Simply it should free only GimpProgressDialog as these would be
dedicated dialog (with no meaning once progression is done), and leave
alone other GimpProgress widgets. In any case, it should not output
CRITICAL errors on these.
Fixing the following CRITICAL:
> GIMP-CRITICAL: gui_free_progress: assertion 'GIMP_IS_PROGRESS_DIALOG (progress)' failed
When dropping an image on the toolbox.
… waitpid().
My use case was when testing gimp_brush_select_new(). When doing tests,
if I let the brush dialog opened while letting the plug-in exit cleanly
otherwise, it somehow locked the process (I have stared so long at the
code and still don't understand why).
gimp_plug_in_close() would wait for it forever and the only way out was
to kill GIMP completely. I guess we could try to run waitpid() with
WNOHANG (and finally force-kill the child if it really won't exit) but
it made the whole logics extra complicated.
The logics with g_child_watch_add_full() is that we don't stop waiting
for the child and just set a callback to finalize what needs to be. Now
the worst case scenario would be to leave zombie processes dangling
around, but it's better than freezing GIMP.
Finally as a weird side effect, doing this change even unblocked the
process with an unfinished brush selector so we don't even have a zombie
anymore (at least for this specific case). All good in the end!
Last side effect: it can speed up a tiny bit the plug-in close as we
don't wait for processes anymore, which could be more visible at first
startups (when we reload all the plug-ins). Though there is nothing
scientific to my numbers, after multiple "first startups", it seems it
went down from about 3.5 to 3.2 seconds on my specific machine. Nothing
extra fancy, but we take what we can (and speeding up the startup was
never the goal of this change anyway). It doesn't affect Windows which
has its own logics to handle process termination and I preferred not to
touch it.
There are 2 parts for this fix:
- First expect the GimpPdbDialog to possibly disappear while
gimp_pdb_dialog_run_callback() is running. This can indeed happen as
this core dialog is tied to a PDB call. If the calling processus
crashes (which may happen, and has to be expected for third-party
plug-ins), then this dialog may just end up closing at anytime (signal
"plug-in-closed" from the plug-in manager which implies a
GTK_RESPONSE_CLOSE, hence dialog destruction).
To account for this, we check the dialog availability with a weak
pointer and returns the info to the caller as well.
- Don't connect to "value-changed" on the spacing adjustment because
when a crash happened, I had cases when the adjustment was finalized
while being set (crash in GTK code). This one is a bit harder to
explain (I had to look long at backtraces) but having a proper signal
"spacing-changed" on the GimpBrushFactoryView is actually much cleaner
code than relying on a public object anyway and it fixes this crash.
Note: this fix is related to my previous commit. When running
gimp_brush_select_new() from Python code, the plug-in crashed when
trying to run the callback, which also resulted into core crash (and
this part is obviously not acceptable at all).
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).
Removing useless condition, add a g_return_if_fail() assertion for the
only (impossible unless bug) case which we don't expect. Also set
default mindist to G_MAXLONG instead of a magic number (which was ok now
but might become a problem if some day colormap allowed more than 16-bit
per channel colors).
Finally break when we reach a distance of 0 since we won't get lower
anyway, so better stop early.
Thanks to Rafał Mikrut and Øyvind Kolås for code commenting.
We cannot just compare the drawable format with the model-type specs of
the color model. We need to include the space now.
In my case, some random screenshot converted to gray then indexed would
assert because the format is "Y' u8-space-gray-sRGB" (or for layers with
alpha: "Y'A u8-space-gray-sRGB"), hence indexed conversion failed and
ended up dark.
With my previous commit, I improved the search action display and search
algorithm (which was returning wrong results), but we had lost showing
the non-sensitive reason in menu item tooltips. This fixes it, by
actually appending the reason, but only in the end, on the GtkWidget
tooltip (not in the action's tooltip itself).
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).
… single drawable run() function.
All Scheme scripts use a single drawable and I am not so sure where to
change this (or rather I hope someone will handle this rather than I).
So let's not output a warning which would result into a stacktrace,
blocking the GUI for a second or 2 and displaying an annoying popup each
time. Let's just output to stderr for now until we get into a better
state.
which will be shown when selected template pixel density don't
match the image pixel density.
Then user can either keep current image ppi and scale the
template or set image ppi to match the template ppi.
Density selector displayed only when template and image ppi
differs and template unit is matches one of real world units
(e.g. inch, mm, etc).
Closes: GNOME/gimp#1140
This should not be necessary and I could not find the bug in GIMP. It
might be in GTK. In any case, after hiding, then showing back the export
dialog, the file name is right but not the output folder (apparently
revert to current working directory?).
With this, we force it to be the same folder as we left the dialog.
The gimp_drawable_type() is an issue though as gimp_drawable_get_type()
is already defined as a common GObject API.
Though I'm actually wondering if GimpImageType is well called. Rather
than Type, shouldn't we go with ColorModel?
sed -i 's/\<gimp_drawable_bpp\>/gimp_drawable_get_bpp/g' "$@"
sed -i 's/\<gimp_drawable_width\>/gimp_drawable_get_width/g' "$@"
sed -i 's/\<gimp_drawable_height\>/gimp_drawable_get_height/g' "$@"
sed -i 's/\<gimp_drawable_offsets\>/gimp_drawable_get_offsets/g' "$@"
s/gimp_image_base_type/gimp_image_get_base_type/
s/gimp_image_width/gimp_image_get_width/
s/gimp_image_height/gimp_image_get_height/
Sorry plug-in developers, more porting work! But really this seems like
the right thing to do in order not to get stuck with inconsistent naming
for many more years to come.
… and s/gimp_parasite_flags/gimp_parasite_get_flags/
It is better to have a consistent API and the fact is that most
getter/setter functions use the _get|set_ naming, but these didn't.
I wondered if this was such a great idea to rename these anyway because
even though we are breaking API in GIMP 3, is it the best idea to rename
when no functional change happened? After discussing with Wormnest, we
still agreed it was still better to go for consistency rather than
regret later (and be stuck with these names for many more years).
Also this fixes these warnings:
> [649/1205] Generating gimp-3.0.vapi with a custom command
> Gimp-3.0.gir:24162.7-24162.56: warning: Field `Gimp.Parasite.flags' conflicts with method of the same name
> Gimp-3.0.gir:24318.7-24318.52: warning: Field `Gimp.Parasite.name' conflicts with method of the same name
Removing this check makes the treatment of LittleCMS consistent with
all the other dependencies checked in the same file, which only check
that the runtime version is at least the required version.
As long as we were compiled against LittleCMS >= 2.8, and are now
running against a version that has at least the same symbols, it doesn't
necessarily matter whether the version we are running against is the
same one we were compiled against.
Distributions like Debian and Ubuntu track the versions in which
individual symbols were introduced, which allows runtime dependencies
to be weakened when no newer symbols are actually used; this is
practically necessary when working with very large numbers of packages,
to avoid a new version of a dependency library unnecessarily blocking
upgrade of dependent packages. However, this doesn't work if dependent
packages add their own checks that bypass this mechanism.
Signed-off-by: Simon McVittie <smcv@debian.org>
LittleCMS 2.12.0 defines LCMS_VERSION as 2120. We want to print that
as 2.12.0, not 2.2.0.
Resolves: https://gitlab.gnome.org/GNOME/gimp/-/issues/6505
Signed-off-by: Simon McVittie <smcv@debian.org>
I always found the docs misleading because when it says "Returns the
list of layers contained in the specified image", I really read "all the
layers, at any level", except it doesn't. It only returns the root
layers and it is up to the plug-in developer to loop through these if
one needs to go deeper.
So let's make the function docs clearer.
… drawable array instead of a single drawable.
Instead of expecting a single drawable, GimpImageProcedure's run()
function will now have an array of drawable as parameter.
As a consequence, all existing plug-ins are broken again. I am going to
fix them in the next commit so that this change can be easily reviewed
and examined if needed later.
I only fix the Vala demo plug-in now (or rather, I just use the first
layer in the array for now) because otherwise the build fails.
The new function gimp_procedure_set_sensitivity_mask() allows plug-ins
to tell when a procedure should be marked as sensitive or not.
gimp_procedure_get_sensitivity_mask() retrieves this information.
Currently plug-ins are automatically marked as sensitive when an image
is present and a single drawable is selected. Nowadays, we can have
multiple selected layers so we should allow plug-ins to tell us if they
support working on multiple drawables. Actually we could even imagine
new plug-ins which would be made to work only on multiple drawables.
Oppositely, there are a lot of plug-ins which don't care at all if any
drawable is selected at all (so we should allow no drawable selected).
Finally why not even imagine plug-ins which don't care if no image is
shown? E.g. plug-ins to create new images or whatnot. This new API
allows our core to know all this and show procedure sensitivity
accordingly. By default, when the function is not called, the 1 image
with 1 drawable selected case is the default, allowing existing plug-ins
easier update.
Note: this only handles the sensitivity part right now. A plug-in which
would advertize working on several layer would still not work, because
the core won't allow sending several layers. It's coming in further
commits.
See the comments in MR!424.
Basically realpath relies on false assumptions (probably ones which used
to be true when the API got created) on the max size of a path. Actually
nowadays paths can be much bigger than what the macro advertizes or can
even be unbounded.
The Linux version of realpath() allows the second parameter to be NULL,
in which case it would allocate the buffer, exactly for this reason
(written in the BUGS section on the man). Unfortunately this behavior is
not standardized in POSIX and the man from Apple I found does not
indicate it will do this.
So let's use g_canonicalize_filename() instead, which seems to do the
right thing. Similarly use g_strdup_printf instead of g_snprintf().
Crop tool used stale color / pattern values when performed crop with
'Allow growing' option. Its context was not notified when fg/bg/pattern
value was changed.
Closes: #4103
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
It isn't being used by any plug-in or any code in GIMP at all even.
Let's get rid of it while we can still break API, so we can cut down on
all the complexity of the gimp-param stuff a bit.
Allow guides with off-canvas position since we can now view and work
outside the canvas.
The guide deletion interaction does not change too much, except you
don't delete a guide because it's dropped off-canvas but off the display
viewport area instead.
The XCF format is technically unchanged as PROP_GUIDES was already
expecting an int32 coordinate (with max value at GIMP_MAX_IMAGE_SIZE,
way below uint32 max anyway). Yet our code and XCF docs was explicitly
asking to ignore negative coordinate guides, which makes a change in
behavior for the XCF parser, hence a new version XCF 15. Loading will
still ignore negative position guides (even also bigger than image
dimension guides now) for XCF 14 and below, but will now accept any
position for XCF 15 and above.
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.
Adds option for selecting predefined page sizes using the same
template selector as on "New Image" dialog.
Syncs up size and offset unit selectors to have the same value
when changing template and when resetting whole dialog.
See the discussion in #5339. Basically this is now technically a tool
(i.e. a child class of GimpTool) and tools can be activated anytime,
even when no images are opened, i.e. when they are useless (for instance
paint tools). Filters on the other hand were historically only
activatable with opened images. With time, tools and filters difference
got slimmer (until there are technically none nowadays where GEGL ops,
levels or curves are implemented as GimpTool too) if not only a
conceptual difference.
Since here GEGL Operation is really more on the "filter" side, let's
just move it to the "Filters > Generic" submenu, just next to "GEGL
Graph", also removing the mention of it being a "tool" from the help
message.
Then I will merge !402 for it to be (in)active depending on images, as
other filters do.
Since we now hide the file dialog when exporting, progression ends up
invisible, which is especially a problem with big files. Therefore use
the image display as a GimpProgress to show progression.
When running several times an export plug-in while one is still running,
the export file dialog may get destroyed and the second running plug-in
would try to call functions on a destroyed dialog, hence crashing core
GIMP.
XMP metadata saved by GIMP 2.8.x or earlier can have duplicate tags
making the XMP data invalid. There's not much we can do without a
whole lot of processing and complicated code and even then no
guarantee we would catch everything.
Instead let's just try to improve the message to the user so they
will be more likely to understand what's going on.
Using a `GtkListBox` allows us to make more complex widgets when trying
to showcase an icon theme. For example, we can make the example icon a
bit bigger so you don't have to squeeze your eyes to distinsguish them
one from another.
Other possibilities we can do with this widget, is for example making
the folder label clickable to open the file explorer, or add a flat
button at the end with the same purpose. Since it's now just a
`GtkLabel`, we can also make the path selectable (for copy-pasting).
Using a `GtkListBox` in the "Image Window Title & Statusbar formats"
preference pane allows us to make more complex rows, such as adding a
visual example of how the format string behaves on a given image title.
This commit just converts the `GtkTreeView` into a `GtkListBox`, so
nothing has functionally changed (yet), except that rows now give
feedback when the user hovers over them.
Although I haven't been able to reproduce it, it is apparently
possible to get a Stack Overflow when loading xcf files with
presumably very large dimensions on Windows. From what
I'm reading Windows normally has a smaller stack size than
Linux, probably why it hasn't surfaced there.
Instead of allocating on the stack let's do a g_malloc0
combined with g_free.
The `precision` parameter in particular had no min/max, which meant we
could provide a forbidden parameter (e.g. a negative precision) which
would cause a core CRITICAL. We must forbid illegal values from PDB side
(hence outputting a normal plug-in error message, not a core bug).
Also improving a bit the description of this parameter as I was
wondering what precision was needed exactly to get a stroke length. This
is the precision for determining whether a portion of the stroke is
"straight enough" or if we want to break it into smaller pieces until we
get a straight portion.
All it will mean is that we are looking for perfectly straight lines
when deciding if a bezier curve can be considered straight. It's a fair
condition and represents perfectly what a precision of 0.0 would mean.
… apply to GIMP GUI not text layer rendering in image.
Reviewer note: this is the theoretical fix, but it won't work right now
because Cairo explicitly bypasses grayscale antialiasing when system set
subpixel one. Still let's push this first patch, but the issue will be
actually fixed when Cairo will merge my MR too:
https://gitlab.freedesktop.org/cairo/cairo/-/merge_requests/114
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