… new action_pixel_size of GimpMyBrushTool.
MyPaint brush tool clearly shows the limits of my trick to have some
enum actions work with absolute values whereas others work with
per-mille values between the property min and max.
Indeed firstly MyBrush's "radius" value is logarithmic and can be
negative. Since the enum trick relies on the fact that negative values
are the semantic enumerated constants, it's broken in such case. The
second issue is that while it was acceptable to use int size for most
paint tools (even though they were double), here radius only goes from
-2.0 to 6.0; so using int values only would leave us with jumping brush
sizes.
So now I create a proper double action which simply takes pixel size and
use this from the on-canvas brush size changing. No weird trick, no int
or sign limitations.
I also add a new optional action_pixel_size in GimpToolControl.
Note: I'm also updating a bit the logic for the MyPaint brush outline
function gimp_mybrush_tool_get_outline(). Indeed after skimming a bit
through mypaint-brush.c code in libmypaint, I am not sure at all we need
to use the offset_by_random like this. And really this shown outline
seems more indicative than anything else when I see the actual size
printed by the various brushes. Finally here it was counter-productive
as I needed to get easily the logarithmic radius from the pointer
interaction on canvas.
Note that the other paint tool which is not as common as others is the
MyPaint brush tool. At first I thought the circle outline didn't work.
It does actually work, but the radius concept is simply very weird in
this tool so we have to move a lot. To be investigated.
Generated *enums.c now have an additional stamp no-op header include
(see last 2 commits). Sync this change into the autotools generation
scripts to prevent back and forth useless generation of these files each
time we switch from one build system to another.
They are nearly the same as initially, except that now they include an
intermediate stamp header which will be generated by the build system.
The only 2 enums which don't need these includes (and are not versioned)
are libgimp/gimpenums.c and libgimpthumb/gimpthumb-enums.c.
Our meson build system was not properly building the enums.c file,
because they are versionned.
I did a similar trick as what I did for the pdbgen, which is that I used
a wrapper script around the existing perl script, which sets proper
options and generate a stamp file in the end (which is considered by
meson as the actual custom target, not the C file since it is generated
in the source dir).
The most important part is that the stamp file is a generated header
source (not just a random text file) which is **included** by the
generated C file. This is what will force meson to regenerate the C file
if the header is updated, **then** build using this new version, not use
an outdated versionned version (which would make for hard to diagnose
bugs), through the indirection of the intermediate stamp header.
See #4201.
See also: https://github.com/mesonbuild/meson/issues/10196#issuecomment-1080742592
Similar to the previous commit, use the new
gimp_tools_show_tool_options() before blinking various tool options
widgets since these are good hints for wrongly set options and it's nice
to be able to point at these.
Create a new utils function gimp_tools_show_tool_options() because
showing the tool options is likely a common action. And use this when we
want to blink the selection mode box, for this particular case.
Note that I could not reproduce the CRITICAL here (i.e. even if the
dockable was not showing, it seems the widget did already exist anyway).
Anyway it's a nice idea to raise the tool options as the goal is to
explicitly show how to fix the issue here. And it should fix the bug for
people encountering it.
So it turns out that the "notify::src-drawables" property signal in
particular can happen during gimp_paint_tool_paint_interpolate() called
from gimp_paint_tool_paint_thread(). Though the function
gimp_clone_options_gui_update_src_label() was run in the main thread in
simple cases, being called this way through a paint thread happens when
very quickly changing the source while painting, which is what Aryeom
does (when I see her using the clone tool, she would sometimes change
the source very quickly several times in barely a few seconds).
Yet GTK and GDK calls must not happen in non-main threads. It used to be
acceptable when protected with gdk_threads_enter|leave() calls but doing
this is deprecated. The now sanctioned method is to call the GTK code in
an idle function since these are guaranteed to run in the main thread.
This was most likely explaining why crashes could quite randomly happen,
though I'm not sure why I never had those (even though I could reproduce
the GTK calls happening in non-main threads, but without crashing GIMP)
and why Aryeom gets these much more often suddenly. Maybe some recent
dependency change which would trigger these more easily or other
context-dependant changes (as most non thread-safe code, bugs and crash
often happen in race conditions, so are harder to reproduce reliably)?
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.
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).
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.
… 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).
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.
… 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.
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.
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).
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.
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.
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.
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.
Jehan