There is really no need to make back and forth between a string and
int/enum representations, and it actually cause problems at times.
It's also a problem for the button representation where a modifier will
be represented as a key.
For now GimpModifiersManager returns the same modifiers as what was
previously hardcoded and we have no GUI yet to change the settings. But
the core of the new feature is in place.
This object's goal will be to manage customized modifiers per input
device button, which is why I add it to GimpDisplayConfig. It is in its
own new config file (`modifiersrc` in config dir) because it requires
GDK types access (well I could have done without, but it would have been
less semantic, hence not as good of an API). Anyway it is only useful
when running GIMP as GUI.
The GUI widget and the usage code to make this actually useful will come
in upcoming commits.
When no accelerator was set yet, display relevant information, and when
the button is toggled (waiting for shortcut input), display also
relevant info.
A widget to grab a shortcut provided interactively. It can also grab
modifier-only shortcuts so we will be able to use it for the new
on-canvas interaction customizability.
Maybe we'll use this later for the shortcuts dialog which should really
be improved some day and has a lot of known issues. We'll see.
Moving the "Space Bar", the "Snapping" and the new "Zoom" (MR !570)
settings there. The idea is that it will also be where we'll customize
more on-canvas related abilities, such as the new zooming behavior
preferences, but also possibility to customizing or disabling the
various canvas action, and finally customizing the new contextual
settings such as brush sizing or other.
… 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.
These actions can be activated with a double value. These will be useful
to create new types of size action, which are based on accurate pixel
values instead of an enum hacked to set per-mille values between a
min/max.
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.
Until now, it was only stopiing when releasing right click, but it's
actually more accurate when releasing the Alt key first as the button is
on the mouse/stylus (so releasing it can provoke small hand moves,
especially visible with stylus, I think). Now it stops whatever is
released first.
I started from mitch's patch (though code changed so it was not working,
yet I ended up with quite a different direction).
Modified from original proposition in #498:
* Do not mix opacity and brush size in a same action, one on horizontal
movement, the other on vertical. The problem is that it's hard to stay
perfectly horizontal or vertical, so you nearly necessarily change one
while changing the other and this would be frustrating.
* Do not just use modifiers, but modifiers + right click. The logics is
that left (or whatever is the first button) click is for the tool
actions. The middle click for navigation (panning, rotation, and even
layer navigation now). Right click for now is only for menu. With this
change, let's use right click for various settings-related changes
too. Also we already have people complaining with things like canvas
rotation happening unexpectedly even though it requires button clicks.
Imagine an action just made of modifiers! Many people would hit these
by mistake all the time!
* Focus on brush size only for now. Instead of just calling the action
repetitively with the "SElECT_NEXT" action as proposed in the original
patch by mitch, let's compute the actual size between the press and
release. This would allow to have a real visual hint and also would
make it a lot more useful and meaningful to be an on-canvas change.
Say you want to reproduce a stroke size on canvas. You can click the
center and expand to retrieve approximately the size without computing
it in pixels.
Limitations and future work:
* This is a first draft and I still want to test if it works well with
the "lock brush to view" and with scale factor > 1.
* I want to associate this with work done for #7034 so that visual hint
still appear even when we have no visual hint set.
* I am not so fond of with the way we use enum actions which doesn't
really make satisfying logics (I hacked a bit over it, but it's
getting ugly). I'm considering creating int/double actions to really
set some values with exact numbers through actions.
* Right now we need to stop the right click first. I want to be able to
stop the brush sizing with releasing Alt too.
* It would be nice to make this all more customizable, which is why I
called internal variable "mod1_setting". The goal will be to have
other types of actions possibly. Also it could be deactivatable for
people really not liking these or hitting these by mistake (while not
needing these). Same for the navigation shorcuts.
* Similarly the right-click menu could be deactivatable or switched to
other actions conditionally (through Preferences). It is doubtful how
useful it is (compared to using the same menus on top of the GUI)
though I don't want to just delete the option because some people
would clearly be used to it.
* I think we should start breaking down the whole tool events code a bit
more, in particular the function gimp_display_shell_canvas_tool_events().
* For more settings, a small on-canvas GUI could be of interest where
you could customize various values through sliders and buttons, and
also where you could put your favorite brushes or dynamics or whatnot.
It's not replacing the more complete dockable but could be a nice
quick version for fast editing.
Uses the simulation rendering intent from GimpImage instead of
from GimpColorConfig. This affects the SamplePoint and Pointer dialogues
as well as the Color Info Window.
Adds a dropdown for Simulation Profile, Intent, and BPC
to the Create a New Image dialog.
This allows users to assign a soft-proofing profile when the image is
first created. It defaults to "None", however. Users can also set the
default simulation rendering intent and BPC status per image.
These options are also removed from the Preferences dialog.
Adds a simulation_bpc and simulation_intent to GimpImage to allow
plug-ins to access it
for CMYK import/export.
Four pdb functions were added to enable this access:
image_get_simulation_bpc (), image_set_simulation_bpc (),
image_get_simulation_intent (), and image_set_simulation_intent ().
Next, it updates menu options and code to support GimpImage's
internal simulation intent and bpc.
New 'simulation-intent-changed' and 'simulation-bpc-changed signal
are emitted via
GimpColorManagedInterface so that relevant tools
(such as the
CYMK color picker, GimpColorFrame, and future pop-overs)
are aware of these changes.
When zooming using scroll events it is possible to zoom out of gradient
bounds because the check for out of bounds zoom happens with the zoom
value before the delta is applied. The correct way is to limit the zoom
value after the delta is applied.
Zoom focus centers the point that we're zooming into at the location of
the mouse pointer.
Default zoom focus value is 0.5 which results in previous behavior.
Smooth scrolling requires that we keep the scroll (or zoom, as is in
this case) state as a floating-point number. The scroll deltas are often
less than one, thus storing scroll state as an integer will effectively
ignore most of the scroll events.
Adds a soft-proof toggle button to GimpStatusBar. It's only active when
a simulation profile is set on the current image. It stays in sync with
the View menu soft-proofing option as well.
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
Adds a parasite to .xcf that stores the soft-proofing profile.
Existing color profile saving/loading functions now take in a
parasite name parameter so they can be used for either profile.
Some of our calls to run_command() would have failed with future
versions of meson if we didn't set the "check" parameter. In particular,
in various calls, we don't want to fail the whole build configuration
when the command does (as it's an optional feature check). In such a
case, it is important to be explicit as future will default to fail
then.
Fixes:
> WARNING: You should add the boolean check kwarg to the run_command call.
> It currently defaults to false,
> but it will default to true in future releases of meson.
> See also: https://github.com/mesonbuild/meson/issues/9300
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.
These changes make it possible to build against homebrew for mac
libraries. Homebrew stores it's libraries in seperate folders and so
blanket includes were not working.
PYTHONPATH is not needed and interferes with homebrew build.
langinfo.h is not on all systems apparently and/or the locale item we
test for may not be available everywhere. Actually even on Linux, after
testing more deeply, I could create cases where nl_langinfo() would not
return a result (if the locale is broken through environment variable
for instance). setlocale() seems to always return usable value so far,
so I fallback on it. As a last resort, I look at environment variables
(even though these may contain invalid contents.
As for Windows and macOS, I try to use more platform-specific methods.
In macOS in particular, as I understood from reports, GIMP follows
correctly the language preference order, which means we should not look
at a single (top) lang, but at the whole list of prefered languages as a
single settings to determine whether the language was changed or not.
Should fix on Windows:
> fatal error: langinfo.h: No such file or directory
and on macOS:
> error: use of undeclared identifier '_NL_IDENTIFICATION_LANGUAGE'
This reverts commit 82f9ae1ab7.
This is not needed anymore. Cf. previous commit which implements the
force-requery differently and much more widely based on actual language
being stored and compared between runs.
Since localization is fully handled plug-in side now (see #8124), we
need to make sure the query functions are run again for all plug-ins
when the UI language changes (otherwise we might end up with
localizations from the previously used languages).
We were already reloading plug-ins when explicitly changing the lang in
the Preferences, but this new implementation is much better as it's
generic. In particular, it will also handle the case when the system
language changes (or when you play with locale environment variables).
This removes rate limiting of ruler and statusbar updates which were
required to get acceptable performance on mac for drawing on the canvas.
This requires the latest changes on in Gtk 3.24 to be incorporated.
Plug-in localization was always partially plug-in side, especially for
things like custom GUI. But labels or blurb in GIMP (such as in menus or
action search) were localizing GIMP side.
It had many drawbacks:
- To get menu localization, a plug-in had to set up gettext, even though
they might want to use something else for their GUI (after all, giving
facilities for gettext is a good idea, but there is no reason to force
using this system).
- There was a complex internal system passing the localization domain
name, as well as the catalog file system path to core, then through
various classes which we can now get rid of.
- There could be domain name clashes, if 2 plug-ins were to use the same
i18n domain name. This was handled in now removed functions
gimp_plug_in_manager_get_locale_domains() by simply keeping a unique
one (and gimp_plug_in_manager_bind_text_domains() would just bind the
domain to the kept directory). In other words, one of the duplicate
plug-ins would use the wrong catalog. We could try to make the whole
thing more complicated or try to forbid plug-ins to use any random
name (in particular made easier with the new extension wrapper). But
anyway this whole issue doesn't happen anymore if localization is
fully made plug-in side, so why bother?
I tried to evaluate the advantages of the core-side localization of
plug-in labels/blurbs and could only find one theoretical: if we wanted
to keep access to the original English text. This could be useful
(theoretically) if we wanted to search (e.g. in the action search) in
both localized and English text; or if we wanted to be able to swap
easily en/l10n text in a UI without reload. But even if we were to ever
do this, it would only be possible for plug-ins (GEGL operations in
particular are localized GEGL-side), so it lacks consistency. And it's
unsure why special-casing English should really make sense for other
language natives who want text in their lang, and search in their lang.
They don't necessarily care about original.
So in the end, I decided to simplify the whole thing, make localization
of plug-ins a plug-in side thing. Core will only receive translated text
and that's it. It cuts a lot of code out of the core, simplify runtime
processing and make plug-in creation simpler to understand.
The only think I still want to look at is how exactly menu paths are
translated right now. Note that it still works, but it's possible that
some things may be worth improving/simplifying on this side too.
g_warning() (as well as g_critical() and g_return_*()) are reserved for
core code bugs, and therefore triggers a debug dialog with a backtrace
to report.
Here I encountered such duplicate because ts-helloworld.scm was moved
around from scripts/ to plug-ins/ since commit d5a83429b4 and I hadn't
done a clean uninstall (so of course someone with package installation
should not have such a debug dialog). Yet it could actually happen for
various reasons, such as third-party plug-ins actually registering
identically named actions. Such reasons are not core code bugs and we
don't want to trigger a debug dialog (and have people report bugs to us
which are not actual bugs and which we have no power to fix) each time a
plug-in developer uses a too generic action name.
So instead let's just print to stderr for now. I also add the
information on which plug-in was discarded (otherwise if you actually
have 2 different plug-ins doing different things, you wouldn't know
which one is the visible one and which one can't be called).
Note that I hesitated with a g_message() which would pop-up a
user-facing error and would help them better handle their plug-in
conflict. But I'm not sure it's ideal in current state of things either.
It might be much better handled when we will have moved to recommending
extensions wrapping plug-ins.
Adds a simulation_profile to GimpImage to allow plug-ins to access it
for CMYK import/export.
Two pdb functions were added to enable this access:
image_get_simulation_profile () and image_set_simulation_profile()
Next, it updates menu options and code to support GimpImage's
internal simulation profile. Menu items are moved from View to Image's
Color Management section.
New 'simulation-profile-changed' signal is emitted via
GimpColorManagedInterface so that relevant tools (such as the
CYMK color picker, GimpColorFrame, and future dockable
dialogue) are aware of these changes.
The "revision" field in the json file is supposed to be int. But I just
realized that it was set as string a few times, e.g. in Windows
installer revision 1 of GIMP 2.10.32. As a consequence, the revision 0
build was not able to detect the new revision.
I have now fixed the data in gimp-web repo (commit e7690e36), yet just
in case this ever happens again, I make the parsing code more robust by
accepting string revision, then properly converted to int.
… in the gimp-release file.
This will be useful to disable the update check for the Windows Store
even though we use the same build as the installer. All it will take
will be to append the line `check-update=false` to the file
`share/gimp/2.10/gimp-release` and it will behave as though the update
check is disabled (even though it is actually built-in).
More safety checks for detecting broken xcf files, also based on examining
issue #8230.
After reading an offset where layer, channel, etc. data is stored, we
add a check to make sure that offset is not before where we read the
offset value. Because the data is always written after the offset that
points to it.
Improvements in loading broken xcf files, based on examining issue #8230.
Besides checking for a minimum width and height, GIMP also has a maximum
size we can and should check.
In the case of the image itself, we change invalid dimensions to a size of
1 in hope that the individual layers etc will have the correct size.
For layer, we will also try to go on, but for channel and layer mask, we
will give up.
gimp_channel_is_empty returns FALSE if channel is NULL. This causes
gimp_layer_invalidate_boundary to crash if the mask channel is NULL.
With a NULL channel gimp_channel_is_empty should return TRUE, just like
the similar gimp_image_is_empty does, because returning FALSE here
suggests we have a non empty channel.
I hesitated a lot whether we should just drop the whole localization of
plug-ins' label and description (blurb) within the core. Actually the
commit messages I wrote a few days ago were moving towards this logic.
It really looks to me like plug-in localization can happen fully within
plug-in themselves. As far as I can see, the only advantage which the
current logic has theoretically is that if we needed, we have access to
both the original strings and their translations (e.g. it could be
useful for text search). Nevertheless I am not sure if we will ever make
use of this, and this is limited cases as all filters turned GEGL ops
don't have such ability anyway.
Nevertheless since previous contributors clearly put quite a lot of work
on this code of localizing the plug-in's label and description within
the main binary, I want to give myself a little more time to think and
study the whole thing because doing anything rash.
In the meantime, what changes is that by default now, a plug-in without
a local gettext catalog is simply not localized. In particular, the core
process doesn't try to localize it using the default catalog, a.k.a.
GETTEXT_PACKAGE"-std-plug-ins" ("gimp30-std-plug-ins"). It just doesn't
make sense and the worst which could happen would be to get unexpected
and wrong translations.
Now by default, plug-ins will try to find a catalog in their main
folder, named as this folder. If it fails to find it, a message is
printed to stderr and localization is disabled (rather than falling back
to a default catalog). It is up to plug-in developers to either install
a catalog, or implement set_i18n() to give the right catalog, folder, or
disable localization with gettext, as handled by libgimp.
gimp_procedure_set_static_help().
The indentation was wrong, probably because of changes to function
names. Fix the generation scripts and regenerate the PDB C files.
We changed the logic of _gimp_plug_in_domain_register() which is now
only called when a domain is explicitly registered (which is not the
case by default anymore). Let's update the function documentation and
also make it clear that third-party developers in particular should not
play with it if they want their plug-ins to be properly localized.
There was one case where horizontally and vertically were mixed up in a
call to gimp_display_shell_scale_image_stops_to_fit.
The more usual order of parameters is first horizontal and then vertical.
So, let's fix the actual functions that have the illogical order
of vertically, horizontally instead of fixing the one call.
This brings it in line with the order in other functions and makes it less
likely we mistakenly mess up the parameters.
Besides that gimp_display_shell_scale_image_stops_to_fit also was not
declared as a local function, so we add that too.
With large image sizes a 32-bit int is not enough for the intermediate
computations, which byte per pixel, width and height are.
So, just like the function below it does: gimp_gegl_pyramid_get_memsize,
we will cast these to gint64.
Thanks to Massimo Valentini for finding the cause.
Bugs introduced in commit 8eb7f6df9e9:
- The type test was wrong.
- The UTF-8 validation test was also wrong since it was still working on
a GimpArray even though the data was now a GStrv.
Also I stop at the first invalid UTF-8 string element, otherwise later
valid strings may hide previous invalid ones.
This bug was mostly invisible since we don't have any core PDB API with
GStrv parameter so far, only GStrv return value.
Even if the container is empty, then we return a GStrv of length 1 (i.e.
an array of length 1, terminating with NULL).
In particular, it improves gimp_container_get_filtered_name_array() as
well, and in turn various list-returning functions in libgimp. This
makes the API more consistent after changes from commit 8eb7f6df9e.
Note that a NULL return can be acceptable for error cases, but an empty
list because this is the expected result for the request should be an
empty GStrv, not NULL.
In particular, since commit 8eb7f6df9e, even if it returns an empty
array of procedure, it still means an array of size 1 (with the unique
value being NULL).
This also fixes the public API gimp_pdb_query_procedures() which returns
a GStrv using gimp_pdb_query() internally.
Finally don't keep track of the array size, just recompute it with
g_strv_length(), which will be much less bug-prone.
Since GStrv are NULL-terminated arrays, there is always one additional
pointer (NULL).
This is a detail, but still, wrong since commit 8eb7f6df9e which
replaced GimpStringArray by GStrv usage.
- Slightly nicer layout for the checkboard color widgets.
- Also set the 2 color widgets insensitive when the check type is not
GIMP_CHECK_TYPE_CUSTOM_CHECKS.
- Default the custom colors to the same colors as the basic default
grayscale. I mean, red and blue defaults are fun and all, but that's
like radioactive colors burning the eyes! Ahahah!
- Unlike the gimp-2-10 commit (cf. !274), on the main dev branch, use
the new GimpLabelColor widget.
Fixed Conflicts from !274:
app/dialogs/preferences-dialog.c
- Some coding style fixes (alignment, etc.).
- Adding missing "Since: 3.0" annotations. We are still wondering
whether this should go in 2.10, in which case, it would become
"Since: 2.10.32" annotations. See discussion in !274.
- Changing gimp_checks_get_colors() signature: merge the 4 color
arguments into 2 (inout) arguments which seems a bit nicer in C,
whereas binding handles such arguments correctly. The other
alternative would have been to at least change the order to have out
arguments in the end.
I also hesitated to get another API in libgimp, which would have been
config-aware (just returning the 2 check colors, depending on user-set
Preferences), then having GimpPreviewArea handling 2 colors (without a
GimpCheckType input). But actually, doing this, we'd remove the nice
menu popup where one could choose a generic check type (not everyone
wants to play with specific non-gray colors) in Gimp*Preview widgets.
So in the end, I left this whole thing as-is.
Instead I document the function with code sample to initialize
properly the GimpPreviewArea (since libgimpwidgets/ are independent
with no knowledge of the core config) in order to respect user
preferences.
- Hide the color properties in gimp_preview_area_menu_new() because
anyway gimp_preview_area_menu_new() does not support GimpRGB
properties right now (so all we get are warnings). It's still possible
to select custom colors on GimpPreviewArea, simply we are stuck at the
ones set in Preferences globally for now (unless a plug-in creates
custom GUI to set these).
Fixed Conflicts from !274:
libgimp/gimp.h
libgimpwidgets/gimppreviewarea.c
Reviewer (Jehan) note: cherry picked from MR !274. Still deciding
whether this will be pushed to gimp-2-10 branch too.
Fixed Conflicts from !274:
app/dialogs/preferences-dialog.c
app/display/gimpdisplayshell-draw.c
app/plug-in/gimppluginmanager-call.c
libgimp/gimp.c
libgimp/gimp.h
libgimpwidgets/gimppreviewarea.c
libgimpwidgets/gimppreviewarea.h
libgimpwidgets/gimpscrolledpreview.c
Remove `gimp_display_shell_keyboard_grab()` and ...ungrab(), as there
doesn't seem to be any good reason to grab the keyboard away from the
compositor. More annoyingly, in some systems it will pop up a dialog
on each first click whether the user is okay with inhibiting shortcuts.
This was missing ever since commit 505a78e715 but we weren't seeing this
by using meson. I opened the report #8145 for us to handle this
regression of the build system.
GIMP could crash if the information regarding old path properties read
from XCF was incorrect. It did not check if xcf_old_path succeeded and
kept trying to load more paths even if the last one failed to load.
Instead we now stop loading paths as soon as that function fails.
In case we have a failure here we also try to skip to the next property
based on the size of the path property, in hopes that the only problem
was this property.
When you change a settings in Preferences which requires a restart of
GIMP, the current list was pretty ugly. I am still not fond of the popup
but at least now the list formatting is slightly better:
- Use a unicode bullet point character in UTF-8 encoding (since
gtk_message_dialog_new() expects UTF-8 input) on each list element,
instead of just writing them without any kind of formatting.
- Use the "nick" of the changed config, not the name. The "name" is an
internal string which is really not meant to be viewed by people,
while the "nick" is more like a short human-readable name or title,
exactly what we want to show here.
Some strings might be generated during query() call, in particular all
strings related to procedure parameters. The core will only know the
localized version at query() time, so you could end up in situation with
the core showing you the wrong language. This can be reproduced in the
Procedure Browser when it shows localized procedure data: when you
change the language and restart GIMP, if the plug-ins aren't re-queried,
then it will still show the localization of query time.
Note that it doesn't matter on plug-in side, i.e. anything in the run()
call is properly localized, included the plug-in GUI using the same
procedure parameters for widget labels or tooltips. Only the "vision" of
the core is stuck at what it got when the plug-in was last queried, and
until the next update of the plug-in.
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)?
… plug-in code.
In particular, we should not hardcode this in core code anymore. The
behavior is otherwise exactly the same, except that we made the core
code generic as it should be.
The CLI options now know which procedures are batch procedures or not.
First it means that it won't just randomly try any procedure name one
may pass and will properly output an error if you pass a non-existing
interpreter procedure.
Secondly, there is no default interpreter anymore (unless only one
interpreter exists). If you don't set an interpreter procedure with
--batch-interpreter or if you pass a wrong one, it will output the list
of available batch procedure, thus helping you understanding how to use
the --batch option.
Call gimp_exit() rather than emitting the signal ourselves. This way in
particular, the still-opened images are properly freed and we avoid
GeglBuffer leaks from images we might have created in a batch script, or
the images loaded in command lines.
If you call GIMP with batch commands and ask it to quit immediately, you
are likely interested by failure information. For this reason, let's now
report exit code other than success, but only in such case. In
particular, even if the batch commands fail, but GIMP is not set to exit
immediately, we continue reporting SUCCESS run at the very end (when
exiting interactively).
Note that I hardcode a few exit values, as standardly found on Linux
(but not using headers which may not be found on all platforms; in
particular, Windows apparently doesn't use any standard code other than
0 for success, from what my searches return).
Additionally, when several commands are requested, GIMP will now stop at
the first failing and will return its error code, and print a message on
stderr to easily report the failed command for easier debugging.
Instead of asking for the script to call gimp_quit() which is not too
proper (because this is actually meant to exit the script, not GIMP
itself. Scripts should not have the ability to basically kill the main
GIMP process), let's add a `--quit` option which does what it says: it
quits GIMP immediately (after having processed any task on command line,
such as running batch scripts).
See discussions in issue #7376.
This should fix various hard-to-reproduce cases of:
> (gimp-2.99:50596): Gimp-Core-WARNING **: 00:42:45.324: gimp_finalize: list of contexts not empty upon exit (2 contexts left)
>
> stale context: (null)
> stale context: PDB Context
We had these warnings happening randomly at exit and not always
reproduceable with the same steps. I had it reproduced much more easily
when working with CLI scripts exiting GIMP immediately. The context were
actually properly unref-ed but simply we had to wait a bit for events to
be processed before destroying the main GIMP object.
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.
Jehan