Resolves#11890.
The Selection Editor's "vectors" group was not renamed
to "paths" during the GimpPath API update. This caused it
to be registered to a non-existent group, thus crashing.
Though these are not as user-facing as other strings, the action names
still are somewhat user-facing. Let's rename them consistently with the
GUI and the API.
This commit also handles user config migration so that custom shortcuts
are not lost.
- First do not write the protocol-version (and therefore don't read it either
nor compare it to the current one). This file does not depend on the protocol
version.
- Secondly when reading an unknown identifier, simply print a message on stderr
for debugging/reference, but ignore it up to the next closing parenthese.
- Finally do not use the file-version as a terminal error as well. When we read
a shortcutsrc with the wrong version, we still try to read it as best as we
can. There might be issues, but it's better than fully dropping the whole list
of shortcuts as it's a pretty important file (people might have spent a lot of
time tweaking their shortcuts!).
GTK docs is pretty clear:
> It is a programmer error to pass an invalid accelerator string. If you are
> unsure, check it with gtk_accelerator_parse() first.
So now we verify validity of accelerator strings before using shortcuts from
config files (which can be consider "human input" as the file can be modified).
Instead of unconditionally add all the non-special-cased GEGL operations into
the menu, only insert the ones which explicitly use the new "gimp:menu-path"
GEGL key.
See new operation "gegl:adaptive-threshold" added in commit 7c2a70eee for such
an example.
Note that the other operations will still get their own generated action (which
means for instance that you can still search them through the action search and
that you can create custom shortcuts for your favorite actions). But now you
won't get an overlong generic list of non-organized actions in a single submenu.
Some people were finding this messy when they had a lot of custom GEGL ops.
Also it means that GIMP doesn't assume that any custom GEGL op is usable in
GIMP. Indeed some ops are really not meant to be used as filters, which is why
we have the gimp_gegl_op_blacklisted() internal function; but this cannot apply
to custom operations. Therefore from now on, instead of GIMP guessing, operation
developers will have the say on whether their op should show in menus and in
which menu path exactly!
Moreover the "gimp:menu-label" key will be used as short label, i.e. the label
which is used in menus in particular, where the menu path is a helpful context
(hence allowing to use shorter menu labels).
The main "title" will still be used as long label, for places where there is no
such context, for instance the action search.
I don't see a reason why the quick mask menu was sharing the same
manager as the image menu (the main top menu and on-canvas). This was
why it was receiving all notifications to create menu items on <Image>,
as registered by plug-ins.
Giving it its dedicated manager makes sure this doesn't happen.
Since we now generate actions for GEGL ops, we might as well generate menu items
for these too.
What I did:
- Move the "GEGL Operation…" tool (generic dialog with a drop-down list of all
non-ignored GEGL ops) to Tools menu.
- Create a "GEGL Operations" submenu in Filters > Generic.
- Move "GEGL Graph" to the top of this new submenu.
- Generate a new menu item for each generated action tied to a GEGL plug-in,
alphabetically sorted.
… Default Values" buttons in Preferences.
I considered just deleting the "Remove all shortcuts" action, because really I
am wondering how useful (and often used) it is, and if we are not over-featuring
the shortcut code.
I could find the origin commit 6da388be93, from 2006. Basically the use case is
for people who want such a different mapping overall that it's much better to
start from a blank slate (e.g. DVORAK users, like in the original report).
See: https://bugzilla.gnome.org/show_bug.cgi?id=331839
So in the end, I just reimplemented this with newer code.
Also this commit fixes the "Reset Keyboard Shortcuts to Default Values" code.
This uses a new concept of "default shortcuts" which must be called only once at
most per action. Any subsequent shortcuts setting are custom settings.
Commenting these lines out is mostly informational as it allows to see in a
quick glimpse in shortcutsrc file which are custom shortcuts or not.
gtk_accel_map_load()/gtk_accel_map_save() are not working with the new
GAction-based code. GtkApplication does not seem to have helper functions to
simply load and save accelerators in a file, so we just implement it ourselves.
A few things are missing right now, namely:
- On parsing, it doesn't handle any kind of duplicate accelerators (possible
especially if someone edited the new shortcutsrc manually).
- On reading, maybe we should only write down the changed (from defaults)
actions, while keeping the old ones commented-out, as menurc used to be. This
is actually useful info both for debugging or even for users who want to look
at this file and see what they changed.
- We should add import code to transform the menurc into shortcutsrc when
updating GIMP, otherwise all custom shortcuts would get lost.
There is still the question on whether we should add the group name too. I think
we should, expecially for plug-in's procedure actions, though right now these
are just added in the GtkApplication's main action group anyway. I'll see later
to refine this.
In particular on macOS, we want to show some default common actions (see
comments in !837) but gtk_application_prefers_app_menu() docs says that it will
always return FALSE on this OS. So we ignore this call on macOS.
It was actually working fine except that I wrongly named the submenu somehow,
but even more particularly I thought it was about single dockables (tabs)
whereas it was about full detached docks (a window containing several
dockables/tabs).
Pre-GIMP-3.0 code logics would re-allocate several GimpMenuFactory or
GimpUIManager for no good reason. While it was still working with old GtkAction
code, with our new GAction-based code, we were ending up overriding an action
with a new version of the same action, while keeping reference to old actions.
This made for discrepancies of the enabled or visible state of actions.
The new code keeps singleton of some objects and references to already
registered GimpUIManager or GimpActionGroups objects and make sure no actions
with the same name are created twice.
Only remaining API are action group-related.
While doing this, I also remove 2 debug actions which don't look like they were
outputting any information anyway (at least on the main dev branch):
"debug-dump-items" and "debug-dump-managers".
My last commit message was slightly wrong. I had one last special case to
handle: the ability to only create sub-GimpMenuShell (i.e. at a specific path
other than the root).
We are using this ability in particular for tool options submenus.
These are the menu items such as the recently opened images, or recently used
filters, etc.
Some notes:
- I added back a "placeholder" concept in the GimpMenu logic. This will allow to
place items at specific positions in the menu (either under the placeholder,
which will make the last item be on top or above the placeholder to have the
last item be in the bottom, depending on needs). Technically placeholders are
just menu items with a label (used as placeholder key) and no associated
action, which I will leave invisible.
- I add a logic for submenus so that they are invisible by default and are only
made visible when we add a menu item with an action in there.
- I removed filling the "/Filters/Recently Used/Plug-ins" placeholder. As far as
I could see, it was never filled (neither with old or new code) and the
"/Filters/Recently Used/Filters" actually already takes care of filling the
"Recently Used" submenu with both GEGL operations and plug-in calls.
- The old gimp_ui_manager_add_ui() API is for all types of menus, e.g. including
the ones created by dockables or elsewhere whereas my new API is (for now)
still specific to the top menu. This will have to be further implemented
later. I left a bunch of "TODO GMenu"-s for the time being.
- I see 2 dock-related generated items which seem to never be added, for
recently added and closed docks. It doesn't seem to work in the old API as
well. I'll want to have a closer look too.
Depending on the OS, the display name may contain characters which are not valid
for action names. In particular, on X11, the display name could be ":0.0" and
therefore we get actions named "app.dock-move-to-screen-:0.0" or
"app.view-move-to-screen-:0.0". The ':' is invalid, which will make calling
gtk_application_get_accels_for_action() (or more simply our new function
gimp_action_get_accels()) fail.
See docs of g_action_parse_detailed_name().
The new utils function gimp_make_valid_action_name() will help us get rid of
invalid characters for actions whose name was generated from strings we don't
totally control.
Furthermore gdk_screen_make_display_name() is now deprecated API, and more
generally gdk_display_get_n_screens() is deprecated too and now always returns 1
since GTK+ 3.10 (i.e. now each display only contains a single screen). Instead
we just use gdk_display_get_default_screen() for every GdkDisplay. So this
commit doubles as GTK/GDK deprecation cleanup.
We had this feature "can-change-accels" where people could change their shortcut
directly by browsing the menu. It was on the property "gtk-can-change-accels" in
GtkSettings. Unfortunately this got deprecated in GTK+ 3.10 (more accurately in
dev version 3.9.8, according to GTK's NEWS file), and is therefore ignored since
then. In other words, whether we check the box or not in our Preferences, it
doesn't do a difference.
If we want to get the feature back some day, we'll have to reimplement it with
custom code. In the meantime, there is no need to leave this visible in
Preferences.
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.
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.
Step one: get rid of all those deprecation warnings that make
it hard to see any other warnings:
- add a lot of dummy API to GimpAction, GimpActionGroup, GimpUIManager
etc. which simply forwards to the deprecated GTK functions, they
will all go away again later
- rename GimpAction to GimpActionImpl
- add interface GimpAction that is implemented by all action classes,
creates a common interface and allows to remove some duplicated
logic from GimpToggleAction and GimpRadioAction, and at the same
time adds more features
windows_menu_display_query_tooltip(): bail out if "image" is
NULL. Can't happen currently but did happen temporarily while hacking
on related code. Better safe than sorry.
Registering a full menu path as a procedure's menu label is now
forbidden and causes the procedure to be rejected.
Bump the plug-in protocol version so a pluginrc containing such cruft
is not used.
The dashboard dockable shows the current GEGL cache and swap sizes,
and their recent history. It has options to control the update
rate and history duration of the data, and an option to warn (by
raising/blinking the dialog) when the swap size approaches its
limit.
Allow propgui constructors to specify an (optional) callback function
when creating pickers, to be called when a color/coordinate is picked,
similarly to controller callbacks.
Implement picker callback support in GimpFilterTool. When the active
picker has an associated callback function, call it instead of the
class's color_picked() function.
Add lots of "#include <gegl.h>" to .c files that miss it, which is
now necessary, since this commit adds a Babl* parameter in
propgui-types.h.
Alx Sa