The PDB context name was overwritten when syncing with the parent
context, e.g. the "User" context. Therefore "stale context" warnings,
for instance, were misleadingly telling the "User" context was leaking.
Performing zoom and rotation at the same time is inconvenient because
most of the time the user will want either zoom or rotation. This can be
solved by recognizing a "significant enough" zoom or rotation change
initiated by the gesture recognizer and then ignoring the other gesture.
GIMP stopped trying to read the XCF as soon as an invalid parasite was
encountered. However, in this specific case only the parasite data is
invalid, while the rest of the image is not corrupt.
Instead of terminating when we see a corrupt parasite, we skip to the
offset after the parasite. This may still be corrupt, but we can handle
that correctly, see e.g. the XCF in bugzilla issue 685086, which was
the reason of some of the previous changes.
Additionally:
- We add some logging to make it easier to handle future issues in this
area.
- We add tests for a NULL parasite name, and for reading a different
amount of parasite data than we expected. In both cases we return
NULL instead of a parasite.
Adds the new configuration option "drag-zoom-speed" to adjust the rate
at which mouse movement can zoom the canvas, ranging from 25% to 300%
of the base rate and applying to both drag-to-zoom modes.
This option can be found in the preferences dialog as:
Image Windows -> Zoom & Resize Behavior -> Drag-to-zoom speed
Adds a new configuration option "drag-zoom-mode" to choose whether to
zoom by distance of movement (newly added) or by duration of movement
(previous behavior) when zooming via dragging the mouse, defaulting to
distance.
This option can be found in the preferences dialog as:
Image Windows -> Zoom & Resize Behavior -> Drag-to-zoom behavior
Changes the behavior of gimp_display_shell_scale_drag() to factor in
the distance dragged, rather than just scaling a flat +/- 10% for each
detected movement event. The factor by which to change the scaling is
also altered from 10% at each movement event, to 1% compounded for
each pixel of distance dragged.
This makes zooming via Ctrl + Middle Click or Ctrl + Spacebar behave
more consistently and less jittery versus the previous method, while
offering more fine grained control.
This fixes the following warnings:
```
[2153/2321] Compiling C object app/widgets/libappwidgets.a.p/gimppanedbox.c.o
../app/widgets/gimppanedbox.c: In function ‘gimp_paned_box_dispose’:
../app/widgets/gimppanedbox.c:171:17: warning: comparison of integer expressions of different signedness: ‘gint’ {aka ‘int’} and ‘long unsigned int’ [-Wsign-compare]
171 | for (i = 0; i < G_N_ELEMENTS (paned_box->p->dnd_highlights); i++)
| ^
../app/widgets/gimppanedbox.c: In function ‘gimp_paned_box_drop_indicator_draw’:
../app/widgets/gimppanedbox.c:370:17: warning: comparison of integer expressions of different signedness: ‘gint’ {aka ‘int’} and ‘long unsigned int’ [-Wsign-compare]
370 | for (i = 0; i < G_N_ELEMENTS (paned_box->p->dnd_highlights); i++)
| ^
```
Fix the dependency by making the stamp an actual (yet empty/no-op)
header file which is included by all generated source file. This way, we
ensure that meson rebuild .o files when the .pdb sources are changed.
This is the second solution proposed by eli-schwartz here:
https://github.com/mesonbuild/meson/issues/10196#issuecomment-1080053413
The build now successfully build the PDB files into the source folder
itself. Unfortunately it seems I can't get meson dependencies to work
properly, once more! I added a "sources" argument to the relevant
library() or static_library() but it still uses old versions to build
these. E.g. if I add an error on purpose to a pdb file, the next build
still passes, yet the second-next fails (as it should have before).
Note that I even tested a declare_dependency() with just the "sources"
arguments, because it says "sources to add to targets (or generated
header files that should be built before sources including them are
built)" (so I assume it means that it should be trigger a rebuild,
otherwise it's useless) but it's just not working. I'll investigate
more.
Still going with this for now, because at least generating the PDB
source was a big miss until now. But we should
After some investigation, I am a bit unsure of why this happens exactly,
but I have a case that the device's axis number listed is not right and
this only becomes apparent after the stylus gets close then further from
the tablet-display. This fixes it, though I think we should look more
closely and reorganize a bit this whole part of our code, which seems
unecessarily complicated and duplicating some data already in GTK/GDK,
like the list of axis, etc. (though maybe it was necessary back when
this was first implemented before more logics got moved to GDK?)
Fixing broken coding style and factorize the code to avoid redundant
calls, in a separate commit as the contributor hasn't responded for
nearly 2 years. Also no need to get the character count as -1 offset is
equivalent to point after last character (or alternatively we could have
used gtk_text_buffer_get_end_iter()).
I had raised some more questions on a possible better implementation (or
maybe not), but since the discussion never happened, let's just push
as-is. It still improves things and the whole text editor should be
revamped some day anyway.
The MR commit (i.e. previous commit) fixes the specific case when
changing font size with no text selected (changing the size of the whole
text).
Since commit 4cf38d784f, when loading an indexed image, we would first
initialize a palette with 0 colors, then set it to the right colors.
Babl outputs the following message when initializing to 0 colors:
> ../../src/babl/babl/babl-internal.h:214 babl_log()
> attempt to create a palette with 0 colors. using default palette instead.
Let's only set the palette to Babl when it has colors.
We take a step back from the original MR which was proposing the "single
dot" cursor as a new "Pointer mode" option. I was really unsure this was
the best solution, especially reading again the whole original report.
It means that now nearly all of the original patch has been rewritten
another way, but let's leave the contributor commit as a start point to
get to where we are, and as acknowledgement of the contribution.
The reporter was annoyed by the crosshair when none were requested and
probably mostly for painting tools only (at least examples were about
brush or pencil, etc.) while showing outline. It looks to me like the
real issue was maybe when we were showing the big crosshair when using
the 4-arc fallback outline, for instance when using a dynamics changing
size. If so, this main issue is already fixed by my commit 64dc26064b.
No need of a new option for this, especially if the option can be as
confusing as a barely visible dot-cursor (I can already imagine the bug
reports of people tweaking random preferences and unhappy because the
pointer became invisible, while they don't know how they did it).
Instead I would say that when people specifically uncheck both "Show
brush outline" and "Show pointer" options, showing a huge crosshair
feels quite counter-productive. This is where I think that our small
unobtrusive cursor (probably a better name than "Single dot" by the way,
as it's not a single dot anymore) might be of use, the ultimate case
when someone really want a cursor as inconspicuous as possible, while
still having a visible feedback of the pointer position (even with
display-tablets, parallax issues make such a visual feedback important
to target where one paints).
So let's try this first and see how it goes.
- Rename "Mouse Pointers" frame to just "Pointers". It's not just about
mice but all input devices.
- Move the "Pointers" options from "Image Windows" to "Input Devices"
tab in Preferences. It looks to me it's more about how you configure
the device pointers than anything really image window-related.
- Move the paint tool options in their own frame "Paint Tools" to really
differentiate the generic pointer options from the ones specific to
paint tools. This frame is added side by side to the "Pointers" frame
to hopefully underline they are related.
I'm still annoyed by the fact that if you uncheck both the "Show brush
outline" and the "Show pointers for paint tools", it will actually show
a big crosshair in such case. I can understand the logic (not to end up
in a case with absolutely no indication of your pointer position). But
then shouldn't we rather forbid unchecking them both at once? It would
be clearer IMO. On the other hand, are they people who like this big
crosshair and use this?
Other thing I was annoyed about was whether you would not want to have
the "Single dot" only on paint tools. But as Aryeom told me, maybe let's
just try it like this and see if people ask for a double option (pointer
on paint tools vs. pointer on other tools) as it is already very
complicated settings as it is. I mean, at some point, I even thought
about a per-device pointer. I also studied the possibility of detecting
whether you are on a display-tablet (when you are more likely to want an
unobtrusive pointer) or not (then you may be very annoyed each time you
can't even spot easily your pointer on screen). But GTK doesn't give
this later information, and as Carlos reminded me, even a display-tablet
can be mapped to another display, so even if we could detect a
display-tablet, it would not mean it is reliable information for making
a decision on whether we want a light pointer.
… in previous commit.
Also fix some coding style bugs.
Finally change s/"Single Dot"/"Single dot"/ to have the same label
syntax (only capitalize the first letter of the label) as other labels.
Cf. #7034 and !466.
If someone explicitly asked not to get a pointer, yet to have the
outline, we should not force a crosshair on them while the fallback
outline is a perfectly visible mark (4 arcs around the pointer's
position, well delimiting the brush size).
… whether the program is really ready to accept various commands.
In particular, on macOS, one could drop an image to the app icon in the
dock while starting up (visible splash). Then if the profile convert
dialog appears *behind* the splash, it would block loading until an
action is taken (unfortunately as it's hidden, people may miss it and
would think GIMP froze), as reported by cyril and reproduced by lukaso.
I can't test myself, but I'm hoping this will fix the issue (similar to
commit a86ed68870 where we had a similar issue with dbus file opening
on Linux).
Since the "Dynamics Off" is set when the brushcore is started, when we
were running gimp_brush_core_eval_transform_dynamics(), we were using a
previous dynamics output.
Also it's better to shortcut the computation anyway when we know the
dynamics is not for use.
… cursor position label.
This should now resolve any weird jumping around of the cursor label,
hence the GUI. Of course, it could actually still happen when going
really off-canvas (though the max computation takes into account
reasonable off-canvas moving, which is totally possible, especially now
that we can view off-canvas so people can just work there and the GUI
should definitely not do weird stuff there as well).
This issue was not confirmed to happen on Linux and Windows, but was
making ugly label resizes on macOS by just moving the cursor on canvas
normally. See video in:
https://gitlab.gnome.org/GNOME/gimp/-/merge_requests/572#note_1389445
Ironically enough though, it started to happen even on Linux/X11, at
least, after commit 1baeffc913, which was the commit meant to fix this
on macOS!
This max size computation algorithm should work well enough for any
normal on-canvas usage, and even some off-canvas (yet close enough)
usage.
Note: this code only takes into account the pixel position case, for a
first implementation, which is why it's not pushed immediately in this
form.
Since we are pre-processing anyway the AppStream metadata file (because
appstream-glib doesn't pass unknown XML attributes, cf. a previous
commit), it does feel silly now to continue loading the file at runtime
too. Let's just pre-process more data into the constructed C files, i.e.
get the introduction paragraphs and the change items too.
The only other remaining advantage of appstream-glib was that it was
handling the localization but since we also have these localized strings
in our gettext files, we can as well translate with gettext as any other
strings and it works just fine.
It will also get rid of any packaging issue, forgetting to package the
metadata file (as we had on the Windows installer, and still have on the
macOS package). Now it will just always work because data is internal.
The changes include:
* A popover will be displayed on tool buttons and relevant property
widgets.
* Now the welcome dialog gives focus up to its parent window because
really the whole focus styling issue in GTK is a bit maddening. The
colors are all faded, the popover widgets are barely visible, and so
on.
* Timing is tweaked a bit to give more time reading the popover tips.
It's still the same visually but it will be useful for 2 reasons: first,
it makes nicer code to show/hide only this one box instead of 3 frames;
second it will be used for the release note demo feature so that we can
blink the full line art settings box.
… blinking it.
This will be necessary to demo new features available only in some
situations. E.g. a new option in line art detection mode of bucket fill
would require said mode to be enabled.
First I strip now every pieces of text. What it allows it to span the
script elements for instance on several lines, indent them and all that.
The second thing is that since all the dockable start with "gimp-", we
may as well allow use shorter names (both are allowed). Same for tool
(which we special-case), all the tool buttons ID start with "tools-"
since we reused the action names. So let's just allow the shorter
versions.
Finally I create a new gimp_blink_toolbox() which is a specialized
gimp_blink_dockable() for the toolbox in particular. I also move there
the whole code about selecting the right tool.
Make a different bullet points for items with demos and items without,
and add an info text explaining you can click on items with the right
bullet point to get a demo.
Hopefully it will fix the following CI errors:
> make[4]: *** No rule to make target '../../tools/generate-welcome-dialog-data.py', needed by 'welcome-dialog-data.h'. Stop.
I thought using relative paths was fine too here, but I might have been
wrong (even though it worked locally).
Even when using a GtkListBox, it's just prettier and clearer when having
a text list with dots IMO, especially since some text might wrap on
several lines. Let's just go with that.
The idea is to add some "demo" attribute to a list item inside the
<release> tag, since we already decided that (for now at least) we'd
keep a strict "intro + list" logics, as we did until now.
This demo attribute uses an internal format to specify successive
widgets to blink (like a demo path towards a feature). For now, what it
allows is:
* raise the toolbox, select a tool and blink the tool button.
* raise a dockable, blink any widgets in there.
Now it is still limited and needs to evolve. In particular:
* What happens if the blinked tool button was explicitly removed from
Preferences? Should we re-add it for the demo? And once done, should
we remove it again? Then should we select back the tool previously
selected?
* What happens if the dockable widget is not visible? Should we allow
changing the settings to be able to demo correctly the new/changed
settings? Should it be temporary? If temporary, it can be annoying as
you'd still have to look attentively the demo to find back the path to
the new settings. If not temporary, some people may dislike we touch
their settings.
* What if docks are hidden? Should we unhide them, then hide them back
after demo time?
Also regarding the implementation: originally I wanted to just grab the
demo attribute directly from the AppStream metadata file, but I realized
that appstream-glib cleans out unknown attribute from the XML. I could
then simply parse the file with a generic XML parser, but I found
simpler to pre-parse it into a header built within GIMP. I still use
appstream-glib at runtime as it takes care of localization for us
(though in the same time, we also have the localization in the main po
files, so maybe we could just embed the release note strings as well).
See appstream-glib report: https://github.com/hughsie/appstream-glib/issues/431
I add it in libgimpwidgets because we need to also use it on propwidgets
created from there, but it's actually only for core GUI usage, so it's
actually in a private part of the library.
Though it's actually doing quite a basic thing, it is nicer and more
foolproof than a manual g_object_set*() everywhere. Moreover it will be
nicer to grep.
… a given order, not at the same time.
This will be used for the release item notifications to show people
where new or changed features are, but in an ordered blink scenario. For
instance: select a tool first, then blink some tool icon, open the tool
options and finally blink the specific new/changed option. I am hoping
it would help awareness of changes (just words on a web news may make
some features a bit foreign when not used to look in details in advanced
options).
… in specific case when there is already a toolbox.
Even if there is already a toolbox in single window mode (hence it needs
neither to be created nor raised, as there is a single window), we still
want to return the dockable widget which can be used for other things.
What it means is that we will be a bit strict over our <release>
formatting which will have to always be a <p> introduction followed by a
list of items. This is what gimp_appstream_to_pango_markups() expects.
Since so far, this is how all our <release> tags were formatted anyway,
this is not too much of a problem.
Note that I keep the less strict gimp_appstream_to_pango_markup() and
use it for extension's appstream description as we will have no control
over these.
The main reason for this new rule and new display of our release notes
is that I am going to add the ability to click independent release items
so that people can get "blinking" indications of what changed when
relevant.
It should help with scrolling the dockable to make the widget visible,
as I don't see a generic and simple scrolling API for GTK scrollable
containers.
In my own testing, it did work if the dockable was already opened. But
if it was just created by the blink API, then giving focus immediately
doesn't properly scroll. I am not really going to investigate now, but
this should be fixed eventually.
This function will help us raise attention to various widgets in
dockables by blinking them similarly to how we blink locks or visibility
icons.
I associate this with the fact that property widgets identifier will be
the property name, so we get identifiers for free when creating widgets
through the propwidgets API.
… on the line art source.
Also now the default tooltip itself will be more appropriate rather than
reusing the "fill-transparent" property blurb (maybe I should have made
this its own property in fact).
This is the same thing as setting the max gap length to 0, except that
it would mean constantly having to play with the gap length and possibly
losing settings you carefully tweaked. Instead with this additional
settings, we hide the gap length settings when automatic closure is
disabled.
Also it makes kind of a nice parallel with the "Manual closure" settings
which can also be enabled/disabled similarly.
Since we can't select a source, the source tools would fail stroking
with an error. It was usable when stroking a path or selection as we
can have the source tool started while running these. Since only one
tool can be started at once, this is not possible when running bucket
fill tool (it would require some logics change on tools).
This change has 2 parts: add an "insensitive" column to
GimpContainerComboBox (allowing to mark some cells as insensitive), then
use this new attribute in the GimpBucketFillOptions to show all source
tools as insensitive.
There are clearly 3 types of settings:
1. How the line art is detected.
2. How do we additionally close line art.
3. How to handle fill borders.
Additionally I am rewording some options:
- "Allow closing lines in selected layer" -> "Manual closure in fill
layer"
- Adding the "Maximum gap length" into an "Automatic closure" frame to
hopefully make a parallel with the "manual" closure settings.
- "Allow completely transparent regions to be filled" -> "Detect opacity
rather than grayscale" (only within the context of the line art bucket
fill) because the main idea of this option is really here that we
detect lines as being opaque pixels, rather than detecting lines are
being dark pixels.
Finally don't hide the manual closure frame when in a configuration
where it won't be taken into account. Only make it insensitive. Having
options disappear is not so nice. Usually you want to gray them out to
have people realize they are simply not always usable.
Currently the option is quite simple. What should happen to make it more
usable:
* Right now, it uses the last stroke options (e.g. as used in a
previously run "Stroke Selection" or "Stroke Path"). We should have
some dedicated GUI in the bucket fill options.
* The bucket fill options GUI should really be redesigned. The more we
add options, the less understandable it is.
* There is a question whether we want to just use whatever brush size is
being set or if we want to have it vary and follow the line art width
(since we have proper distance map, we could use this to tweak the
stroke per-coords).
As usual, this feature was suggested by Aryeom who was still very
saddened that despite all the fancy features in this tool, it is not
able to produce nice rendering. So she proposed that the tool could
stroke the fill region borders.
Many features can only be run on items belonging to an image, and in
particular attached. It makes sense, but in the same time, there are
often some types of processing you'd like to do in background on
temporary items, not visible in GUI, and without any undo steps.
You could work on buffers directly, but sometimes it's also nice to
attach the semantics of the various items, and reuse logics (in
particular class method implementations) already existing. So I am
adding a concept of "hidden items" which is where you'd put items in
some temporary processing state when you don't want them to go anywhere
visible publicly.
For some reason, our flatpak is moving the appstream file elsewhere,
after installation (since all our scripts explicitly install it under
metainfo/).
So in flatpak, it is in /app/share/appdata/ (i.e. $DATADIR/appdata/).
This is not right and should not be done by the flatpak system, but
let's still verify this other location as it is legacy so it would not
be totally impossible that some distributions do something similar when
packaging GIMP.
See also: https://github.com/flatpak/flatpak/issues/4775
As Jacob reports in #6445, on a big-size monitor, the dialog is just too
big, this can be seen in particular with all the space left between info
text. Trying a new logic where I simply leave the dialog to be
allocated, then once I get its size, I generate an image roughly this
same size. This should avoid overly big images.
Thanks to Liam for the fix/feedback on IRC. There was a typo (s/if/it)
and some better wording proposition.
s/You can call if again/You can show it again/
Thinking again, this is simply the version of the config files, mapping
to the application version. Even though it's not really a user-visible
string (except in config files themselves), I find this a much more
elegant name than the ugly "last-run-version" (which is not even true
anymore once startup passed; it's only the last-run version info at very
beginning of the startup process).
This will be used by the update check to verify you are running a new
version since last time. In the future, it might even allow to handle
some types of config migrations if ever we update some things in-between
micro releases. Until now, we could only detect minor updates through
the config folder name (and even this was limited as the config folder
can be specified, in which case we would not even know what version the
files were for).
Maybe we could start also warning in cases of downgrading too, which can
break some configuration files (though there is not much we can do about
it other than warn as there is no time machine!).
Last point: if the new config value "last-run-version" doesn't exist, it
simply means the config folder was run for a GIMP before this point in
time/commit. So we just show the welcome dialog.
MacOS and Wayland need the hotspot in surface coordinates
* X11 needs the hotspot in pixel coordinates (not scaled)
* Windows doesn't handle scaled cursors at all
* Broadway does not appear to support surface cursors at all,
let alone scaled surface cursors.
https://gitlab.gnome.org/GNOME/gimp/-/merge_requests/545#note_1388777
I'm going to reuse this code in other parts of the file so make it a
utils function.
While doing so, I'm also improving a tiny bit the formatting of lists.
I realized the "recent-contributor" template was checking
`last-active >=2 and minor version >= 8`. Because of the second part of
this test, anyone with a last a last-active value of 3.0 was ironically
not included as recent contributors!
Meanwhile, I am bumping the definition of recent as 2.10 and over
(rather than 2.8 and over), for GIMP 3.0 release. Last thing, I am now
sorting by descending last-active (so contributors in GIMP 3.0 are shown
first).
Gets drawing in the canvas speed on retina displays up to the speed
of FullHD displays on macOS, making 2.99 usable on macOS.
Generic change:
Changes the cursor_label to delay the drawing phase to the idle
queue, from immediate draw on all platforms.
Before the fix in 32049afd (using a deprecated function in Gtk3)
any draws on this label forced a full canvas redraw. This is due
to a quirk in how GtkLabel functions.
The redraw occurred because GtkLabels resize themselves and everything
around them by sending a resize message any time they receive new
text. These resizes then trigger the full canvas resize which triggers
a full canvas redraw that cannot be optimized by either Gtk or Big Sur.
MacOS changes:
Only redraws the cursor position label and each of the horizontal and
vertical rules (cursor tracking widgets) 3 times a second max for a
total of 9 redraws a second (ideally out of 60, though I don't believe
under any circumstances that GIMP achieves a 60fps).
Each of the cursor tracking widgets gets its own timeslice, and so
will not redraw when the other cursor tracking widgets are drawing.
This is required because Big Sur is merging all draw rects into
one large rect, dramatically slowing down draws.
This timeslicing ensures that draw rects are maintained at the smallest
possible size. So the typical redraw is a small rect around the
brush. However, 9 times a second, the rect will include one of the
3 cursor tracking widgets (rulers and cursor label).
Additionally, the code tries to minimize resizing the width of the
cursor label by checking if the widget is too small for the text,
then setting the char_width to a greater size so that resizes won't
be that common.
This improves the appearance of the widget as it no longer
constantly jumps about in size on each cursor move.
Here is a discussion of the issue:
https://gitlab.gnome.org/GNOME/gimp/-/merge_requests/572#note_1389445
Reviewer's (Jehan) notes:
* The whole issue about GtkLabel resizing is no more after 32049afd. It
is normal for a widget to request a resize when needed. We just don't
want the statusbar to resize and triggering canvas redraws.
* Changing cursor position text into an idle function generally makes
sense.
Also it reverts commit 6de9ea7022 which had a bug I hadn't realized
when I accepted it: when we test for time, we don't know yet if it
will be the last position change, hence we could "refuse" the last
update. Therefore displayed cursor position would end up outdated
on macOS. This new implementation doesn't have the problem (the last
idle update always happens after the last move).
* The change about giving 1/3 timeslices to side canvas components
(rulers and statusbar) is a complete hack to work around the fact that
macOs doesn't report properly each damaged rectangle. Instead it
returns a huge bounding box. The workaround here is to expose these
area separately.
We have not been able to find a proper solution yet. This is the only
reason why I accept this code, for macOS only, to at least have
something usable there.
See discussions in MRs gimp!572 and gimp-macos-build!86. With these 2
MRs, Lukas reported GIMP 2.99 to perform even better than GIMP 2.10 on
Monterey, though it could not be tested on Big Sur unfortunately.
* Lastly the set_width_chars() thing is also an ugly hack which I will
try later to revisit (see !581). I only accepted it (with mandatory
macOS-only macro) to have an acceptable state for release after seeing
a screencast where the label size indeed "jumps around" on macOS.
… gimp_prop_widget_set_factor() to libgimpwidgets.
Now that GimpSpinScale is in libgimpwidgets, it's time to move the
associated prop too, to make it a prop widget with such a widget easily
creatable by plug-ins.
While doing so, I update both these functions logic, binding properties
together with the g_object_bind_property*() APIs (as we do already in
some other recent prop functions) rather than connecting to signals
ourselves. It makes for much simpler code.
Basically disabling commit 4f9b7373e6.
After some new patches for GTK3 I wrote, and removing the settings on
NSViewUsesAutomaticLayerBackingStores, Lukas reported that it works like
never before, faster than 2.10 even. Note that this could only be tested
on Monterey, nobody tested on Big Sur with this specific combination
yet.
In any case, we decided to remove this settings and add the new GTK3
patches.
See: https://gitlab.gnome.org/Infrastructure/gimp-macos-build/-/merge_requests/86#note_1384727
There is really nothing specific to the core application, it is quite a
generic widget, so it would be nice for plug-ins to be able to use this
widget.
XCF 17 includes the new visibility locks and the ability to add position
and alpha locks on layer groups.
I am going to push the various commits implementing these different
features together which is why we gather them as a single XCF version.
We want it to work whatever the level in the item tree. We only care
about whether the items are selected or not.
Also fixing the AppStream release tag for the description of this
feature.
Unlike other locks, visibility lock is less useful for locking a whole
hierarchy of item and their children. On the other hand, being able to
lock a group visibility while editing the children visibility is quite
useful.
It can be argued that layer groups can't be painted on, and that's
probably the original reason, but it's really just the same as "Lock
pixels". It is interesting to be able to lock alpha channels on a layer
group to simply lock all its contents alpha channels.
Since we are now allowed to move groups (which is the same thing as
multi-selecting all its children and moving them), it makes no sense
that this lock is disabled.
This works the same way as "Lock pixels" in that a locked grouped also
forbid moving children. And there was already some logics so that you
can't move a layer group if one of it's children is locked. So this lock
really works both ways and is a bit special.
Finally I cleaned up a bit the multi-layer selection logics and
messaging, as well as which lock to blink (similar to the previous
commit) for the "Lock position" case.
In particular, if painting on a layer whose parent's pixels are locked,
we were blinking an empty lock spot, which is confusing. Now
gimp_item_is_content_locked() will also return the proper item (when
relevant, i.e. when returning TRUE) which is locked. It may or may not
the same item as passed in (it may also be a parent item in particular).
See discussion in !572, #7840 and #7690. Note that this was reported on
macOS where the consequences were pretty dire, but it actually also
happens on other platforms, at least on Linux too (as confirmed in X11
with the GTK Inspector set to show graphics updates; on Wayland this
debug option doesn't work, but I assume it is the same).
I am not perfectly happy with this change either, because it is based on
part of the API which has various deprecated parts (hence doesn't exist
anymore on the main dev tree, i.e. it might have to be reviewed in GTK4;
of course, it's unsure, maybe the whole resize propagation to parent
containers is just better handled there and the problem won't exist
anymore).
In any case, it is cleaner than the proposition for this part of the
problem in !572 which is problematic (patching GtkLabel with a new API
which won't trigger resize even when actually needed, hence which likely
won't ever get accepted upstream because it's not right).
GLib has a specific type of NULL-terminated string arrays:
`G_TYPE_STRV`, which is the `GType` of `char**` aka `GStrv`.
By using this type, we can avoid having a `GimpStringArray` which is a
bit cumbersome to use for both the C API, as well as bindings. By using
`GStrv`, we allow other languages to pass on string lists as they are
used to, while the bindings will make sure to do the right thing.
In the end, it makes the API a little bit simpler for everyone, and
reduces confusion for people who are used to working with string arrays
in other C/GLib based code (and not having 2 different types to denote
the same thing).
Related: https://gitlab.gnome.org/GNOME/gimp/-/issues/5919
There were some comments claiming we don't care about thumbnail creation
errors, only thumbnail saving. It's not true. Thumbnail creation errors
are probably even more important (in this specific usage). For instance,
trying to debug a thumbnail load procedure, I had no clue on the error
even though our system was able to report the issue. It's not cool for
plug-in developers.
Now we will check the thumbnail creation error, write down the error
message on stderr and clear the error for the fallback round (using the
normal file load proc). Then if it errors out again, we will keep this
error as the finale one because it's probably more important if we can't
even open images than the unrelated thumbnail saving error. We will
still return the saving error if the load proc failed without error.
We were getting a critical when selecting multiple drawables (either
changing layer selection while the tool is ON, or starting with multiple
selection). We should not have assert code here, just handle the case
gracefully with a normal error message when trying to fill on several
layers at once.
The line art algorithm is useful but not always accurate enough and
sometimes it can even be counter-productive to fast painters.
A technique of advanced painters which Aryeom uses and teaches when she
wants to close an area without actually closing the line art (i.e. the
non-closed line is a stylistic choice) is to close with a brush the area
on the color layer. It has also a great advantage over the line art
"smart" closing algorithm: you control the brush style and the exact
shape of the closure (therefore something you'd usually have to redo
with a closure made by an algorithm as you would likely not find it
pretty enough).
This new feature takes this technique into account. Basically rather
than relying on the closure algorithm, you would close yourself and the
tool is able to recognize closure pixels by the color proximity with the
fill color.
Final point is that this additional step is made after line art
computation i.e. in particular the target drawable is not added to the
sources for line art logics. This allows to stay fast otherwise the line
art would have to recompute itself after each fill.
This also shows why the previous commit of moving the line art object
was necessary, because a painter would likely want to move regularly
from bucket fill to a brush tool to create area closures and we want to
avoid recomputation every time.
… fill tool finalizes.
The idea is that you may want to quickly switch tool to do something and
back to the bucket fill for line art selection. If the input drawable(s)
did not change, the previously computed data is still valid, so let's
hang on the line art object a little longer.
Since we are resetting the input when we get back, we would still
recompute anyway *if needed*, and the line art object does follow
changes on the input pickable so we would not get any deprecated data
anyway. Still we move around ownership a tiny bit to optimize for
common case of tool switching.
In order not to keep forever this data (buffer and distmap in
particular) forever just because one tried the line art once, I add a
timer to free it after a few minutes.
Moreover it will be useful for other cases, such as being able to share
the same line art object with the fuzzy select tool (if we end up adding
the line art option there). In a coming commit, the usage will be even
more obvious for use case where you want to edit the filled area with
other paint tools, then back to bucket fill while not touching the line
art source layer.
This change reduces to 3.33 times a second, the updates to the
status bar coordinates widget.
This dramatically improves frame rate on macOS retina displays
because it reduces the frequency of full screen updates which
are triggered by this widget updating and are very slow.
This makes the statusbar refresh changes mac only where the
benefit will be felt keenly, rather than saddling all
platforms with the change.
This was added in commit 88f97aedef and only expected to last until
fontconfig had a fix **and** it got into a released version.
This is now done.
I could verify in the git repo that fontconfig's commit 55eb1ef is
included since their tagged release 2.13.95 which is now on MSYS2
(2.13.96 there even), so we will use it for our next release.
Thanks to frogonia (long time no see! \O) for following up on this!
See also fontconfig report:
https://gitlab.freedesktop.org/fontconfig/fontconfig/issues/144
Actually we had half of the fix already with my recent change to source
tools giving the ability to clone from multiple source drawables where I
moved the source drawables from GimpSourceCore to GimpSourceOptions (see
commit 6ad00cdbba). The problem is that gimp_vectors_stroke() is using
the context paint options, but it is creating a brand new paint core
just to stroke the path. Hence with my recent changes, the drawable
sources were finally available to the path stroke, yet not the source
position. So stroking with clone was always starting on position (0, 0)
on the source.
This is now fixed by moving also the position properties "src-x" and
"src-y" on the GimpSourceOptions. This makes this info finally
accessible to the core when running the source tools this way.
Thanks to Eneko Castresana Vara for their initial contribution.
Unfortunately the code had diverged too much for it to be usable because
of our much too late review so a different fix had to be done.
As the name implies, this is an action to toggle the paint dynamics ON
and OFF which can be a huge time saver when painting and needing to
switch between dynamics and non-dynamics painting.
I have not been adding this action in the "dynamics-action" group
because these are only used for the dynamics dockable menu. Also it
looks like the context actions are not visible in the action search (I
can't recall why, but I think there was a reason), yet you can still set
a shortcut.
Should we create a new "paint-action" group maybe?
Fixes#4333
If the checkbox is unchecked: dynamics falls back to "Dynamics Off",
the current dynamics name and its options are hidden in the UI.
If the checkbox is checked: dynamics is set to previously used one
or the default one, all dynamics options are seen in the UI.
Relative to the MR !553 where I could verify that the function
gimp_tool_push_status() does not just push new messages, it also removes
any other message from the same context (and place the new one on
front, unlike gimp_tool_replace_status()).
Therefore calling gimp_tool_pop_status() before a push or replace is
simply wrong with undesirable effect (e.g. too many useless redraws,
which can be pretty bad on some platforms like macOS, but are not ideal
anyway as a general rule).
This change reduces redraws due to spurious status updates.
Before this change, the status would be unset, then set again to the
same value it was previously. Each turn around setting and unsetting
causes a redraw.
On macOS this redraw causes a full screen redraw. With this
optimization, when the status message is unchanged, no redraw occurs.
This is because `gimp_tool_push_status` checks to see if the message
has changed.
This is a better fix for the previously reverted commit, avoiding
"floating-selection-changed" and "select-items" to recursively calling
each other.
The other fix on context update is similar and was also triggering
crashes because of recursive signal calling after the previous revert.
Using the "list-add" icon instead of "document-save" indeed makes more
sense for the button to store a layer set.
Also renaming "Named Selection" to "New layer set's name", which should
hopefully be less confusing. In particular, we keep consistency with the
"No layer set stored" label (naming the concept "layer set" both times).
And it should be clearer that it is a field to write down a name for
creating a new layer set.
My commit ca28934dfc was very wrong. We absolutely need to set context
in list view editors too. In particular, we could not loop through fonts
in the Fonts dockable very quickly with up/down arrow keys anymore
(since the GimpFontFactoryView is a GimpContainerEditor).
When doing this though, we could have some weird crash in the
GimpContainerPopup watching for context change through button
press/release. Indeed when doing this, simply opening the popup (for
instance clicking on the Fonts icon in text tool options) would trigger
a context change as a button click consequence.
The solution is obviously to check which widget the button event belongs
to and ignore it if it happened on any other widget than the popup.
Now the source images are in the build dirs.
Also:
- clean the EXTRA_DIST contents on autotools;
- add dependencies rules in meson gresources to make sure icons are
built before resource build;
- finally remove a duplicate build rule in Color Makefile.
This moves to standard fullscreen behavior for Gimp.
Added benefit is that it no longer requires gdkquartz-cocoa-access.h
which the Gtk team wish to stop supporting https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/4303.
Bug 756178 also no longer manifests, cdc7542d46
so it is now safe to do.
Finally, removes dependency on objective c in the app/display directory.
This change makes sure that plugins can load when gimp is built with
meson. This is because .typelibs and .gir files do not have full library
paths when build using meson (this is different from autotools).
Due to recent changes on master the linked layers concept doesn't exist
anymore. The conversion code checks whether all linked layers are valid.
However, text layers need special handling.
In the past updating the linked_layers wasn't needed when the layer pointer
of a text layer was changed. Now, that has changed because we parse that
list for valid layers. To fix this we update linked_layers in the same
way as already was done for selected_layers.
Sometimes, you just want to quickly select layers, not necessarily save
the pattern. So you'd just type a few words, followed by Enter. Don't
save the search pattern everytime.
Only save it when you hit the "Save" button or on Shift-Enter.
We now save and load layer and channel item sets. Only missing set types
are path ones, but the whole path item is just its own exception in the
XCF format, and adding support for it, while keeping compatibility with
older XCF seem like a small headache. I could do it, but I actually
wonder if it is worth it. Would people really need to store sets of
paths?
Also this commit finally gets rid of any remnant of the old item "link"
concept (I think), so we are getting close to merging the branch.
I cleaned many remaining places where the concept of linked item still
survived.
On loading an XCF file with linked items, we are now going to create a
named sets for the linked items, allowing people to easily select these
back if the relation was still needed.
We don't remove gimp_item_get_linked() yet and in particular, we don't
save stored items into XCF files. This will come in an upcoming change.
- removing the GIMP_ITEM_SET_LINKED enum value.
- removing gimp_image_item_list_linked(). Now we should directly use
gimp_image_item_list_filter() instead.
- "preview-linked" option for transform tools is no more.
When double-clicking the non-selected layer, the function
gimp_view_renderer_set_border_color() was being called, which in turn
called gimp_view_renderer_update() after some idle time.
Unfortunately when this happened, the "update" handler
gimp_container_tree_store_renderer_update() would call
gtk_tree_model_row_changed() which triggers a focus out through GTK.
One way to handle this would be to call gimp_view_renderer_update()
directly when changing border color (no idle time), though it's actually
more efficient to just cancel the idle function when we start editing
the item name. It's just not necessary.
This got broken in commit 28f6b1b268 because you want to always throw
the signal. Yet the context must only be updated with a grid view in the
GimpContainerEditor. Now it should work correctly (hopefully!).
We must make sure to care about slight discrepancies between context
object and actually selected icon representing an object, which might be
slightly different at selection change time (just before everything
syncs up).
… 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.
Jehan