GMenu for main menubar

[bhennion]

As discussed in #2722, the use of GtkMenuItem and derived classes have been deprecated in GTK3 and removed in GTK4.
So we need to get rid of them before porting.
They are replaced by the menu structure tree-type GMenu/GMenuItem triggering the activation/toggle of GAction's.

This PR is very early WIP, as a proof-of-concept and a way to discuss the pros and cons of various solutions. For now, all the static menus and submenus are there (but the entries are not activatable...), and the only dynamically populated submenu is the Recent Documents submenu. (also the toolbar are not generated anymore)

You can clic on gtk-new (or press Ctrl+N) or on some recently-opened file and see stuff in the terminal, so actions are triggered (not the right ones though).

You can already spot some of the issues we have to fix:

• Figure out how to have a check box/radio button displayed in the left column -- I AM NOT SURE THIS IS ACTUALLY POSSIBLE Edit: Yes it is!
• Create a GAction for every possible action. Provide an interface so that controllers can easily declare them "enabled/disabled" or set/get their value (for checkbox or radio choices).
• Figure out which actions should be "Application actions" and which should be "Window actions" and add them to the suitable GActionMap (G_ACTION_MAP(GtkApplication) or G_ACTION_MAP(GtkApplicationWindow))
• Replace gtk placeholder strings (e.g. gtk-new)
• NOT ADVISABLE (see next item) Add icons to the relevant entries
• NOT POSSIBLE: Have the icons in the left column (see gtk-save in screenshot above - not sure it's possible) Edit: icon won't get shown in GTK4 anymore anyway -- cf this post. We should probably drop them altogether already.
• Pöpulate other submenus
• Restore toolbars
• NOT POSSIBLE: Figure out if submenus can be disabled: no. this post seems to say it is not possible
• Fix: GMenu are not InitiallyUnowned!!
• Fix: gtk_popover_menu never shows icons: no icon in Pen toolbar linestyle selection dropdown menu

Some doc:

https://wiki.gnome.org/HowDoI/ApplicationMenu
https://wiki.gnome.org/HowDoI/GAction
https://docs.gtk.org/gio/class.MenuItem.html
https://docs.gtk.org/gio/class.Menu.html
https://docs.gtk.org/gio/class.Action.html
https://docs.gtk.org/gio/class.SimpleAction.html
https://docs.gtk.org/gio/class.ActionMap.html

https://gitlab.gnome.org/GNOME/Initiatives/-/wikis/App-Menu-Retirement

[bhennion]

The last commit introduces a database for GAction's.
The way menubars work in GTK4, each entry (sourced from a menu.xml file -- here ui/mainmenu.xml) is linked to a GAction.

The way GAction's work mean we have to rethink the way we handle actions as well. Please look at this doc to understand better what follows.

A GAction can have a state (e.g. a boolean for checkboxes, or an integer for radiobuttons), and/or a parameter (e.g. the path of the file for "Recent files" entries).
As expected, the parameter (if any) is passed to the callback function when the action is activated.
The state (if any) determines how entries (or for instance GtkToggleButton's) should be displayed.

Let me explain: to each menu entry is associated a GAction, and possibly a target value.

• If the action has a boolean state and no target value is given for the menu entry, it is displayed with a checkbox (reflecting the state of the action, of course).
• If the action has a state and the menu entry has a target value, then the entry is displayed with a radio button. The radio button is "on" if the action's state matches the entry's target value. This allows radio lists.

So for instance, all the entries in a radio list are linked to the same GAction (but different target values).

In particular, the enum in core/enums/Actions.enum.h is inadequate to this setting: we do not want one ActionType per menu entry (as it is in master), but one per "categories" of actions (e.g. just SET_COLUMNS instead of SET_COLUMNS_1, ... , SET_COLUMNS_8).

Now comes the issue: I need to filll the action database. For this, I need:

• Suitable callback functions: can be easily generated by splitting up Control::actionPerformed (see control/actions/ActionDatabase.cpp).
• More problematic: matching action state and parameter types between the action database and the ui/mainmenu.xml file. E.g. an entry in the .xml file

   <item>
     <attribute name="label" translatable="yes">_2 Columns</attribute>
     <attribute name="action">win.set-columns</attribute>
     <attribute name="target" type="i">2</attribute>
   </item>
  

  must correspond to an action with parameter of type int ("i" is the GVariantType for 32-bits integers). Otherwise, nothing works and you get no indication as to what is wrong.

@xournalpp/core @Febbe If you could have a look at the content of control/actions/ActionDatabase.cpp, and share

1. feedback on the best way to fill to this database
2. any idea you'd have on how to ensure (preferably at compile time or so) that the parameter types match (between ActionDatabase.cpp and ui/mainmenu.xml).

[rolandlo]

2. any idea you'd have on how to ensure (preferably at compile time or so) that the parameter types match (between ActionDatabase.cpp and ui/mainmenu.xml).

The only idea I have checking this at compile time is to read the contents of mainmenubar.xml into a constexpr char* (using std::embed from https://github.com/MKlimenko/embed) and adding some static_asserts that check the parameter_type matches with what you can read from that constexpr char*.

[bhennion]

The only idea I have checking this at compile time is to read the contents of mainmenubar.xml into a constexpr char* (using std::embed from https://github.com/MKlimenko/embed) and adding some static_asserts that check the parameter_type matches with what you can read from that constexpr char*.

I guess making a test unit would also be possible.

[rolandlo]

One question: Is there any backwards compatibility planned with plugins that use app.uiAction? Or should those plugins (and the plugin API) be adapted to the new action names (which in some cases require an additional parameter, like in the case of arrange-selection-order)?

[bhennion]

Backwards compatibility for the plugin interface has indeed been on my mind recently.
Right now, Control::actionPerformed still exists and maps "old" actions to "new" actions. There are, at this point, two reasons for that:

1. The more evolved toolbar items stilll rely on it (e.g. the togglable combo boxes, the font button, essentially anything that is not a gui/ToolButton). Adapting those should be part of another PR also removing every use of GtkToolbar and GtkToolItem.
2. The plugin interface

In the long run, once 1. is done, I'd imagine the content of Control::actionPerformed should be moved to a plugin compatilibty layer. Ideally, plugins would be given access to the new actions (with parameters and states) and the old ones would be marked as deprecated and eventually retired.

[Febbe]

I am a bit unsure, why we need this database, or what's its exact purpose. Especially, why the Gio.ActionMap interface is not sufficient.

[Febbe]

Nit (opinionated): PHP is not my favorite scripting language, because it requires an interpreter, which is often not common, even on development stations. Python would be my choice.

But doing it via CMake or via the c++ preprocessor might also be a solution:

https://stackoverflow.com/questions/1801363/c-c-any-way-to-get-reflective-enums

With c++20's consteval and std::sort being constexpr, we might be able to generate those arrays via c++ on compile time, and having on top an efficient way to access the Action from its string via binary search / std::lower_bound.

Note, this comment has nearly to zero priority.

[bhennion]

I simply copied the original script we have for ActionType and ActionGroup and adapted it, I don't care for php myself either and any better solution is more than welcome!

[bhennion]

I am a bit unsure, why we need this database, or what's its exact purpose. Especially, why the Gio.ActionMap interface is not sufficient.

My main motivation was to make it possible to define an action and everything we need to know about it in a single place or so (i.e. the specializations in ActionProperties.h).
Most of ActionDatabase.cpp does just that: process the template specializations of ActionProperties to add the suitable GActions to the GtkApplicationWindow's GActionMap interface.
Besides that, I guess having a GActionMap wrapper would be enough indeed, but we might as well remember the GAction's pointers to get faster access (no search though the GActionMap - so something like map<string, GAction*>)

[Febbe]

Besides that, I guess having a GActionMap wrapper would be enough indeed, but we might as well remember the GAction's pointers to get faster access (no search though the GActionMap - so something like map<string, GAction*>)

Such assumptions must be made with benchmarks. Gtk hopefully uses an ordered flatmap or at least a tree based map. Both have a very high mapping speed, when it comes to string mappings. On the other hand with your approach, we can map the actions by the enum which is quite nice, but it doubles the state.

My main motivation was to make it possible to define an action and everything we need to know about it in a single place or so (i.e. the specializations in ActionProperties.h).

Yep, that's anyway a good idea.

What do you think about https://docs.gtk.org/gtk4/method.Application.set_accels_for_action.html can we integrate the initialization of default values also in this structure?

Overall, I think this is useful, when it proofs to be otherwise, we can still refactor it.

[bhennion]

but it doubles the state.

No it does not, we just keep a ref to the same action that has been added to the GActionMap. The state is only stored once.

can we integrate the initialization of default values also in this structure?

Already done. Adding a ActionProperties::accelerators member adds those accelerators automatically. See e.g. Action:UNDO.

[Febbe]

but it doubles the state.

No it does not, we just keep a ref to the same action that has been added to the GActionMap. The state is only stored once.

can we integrate the initialization of default values also in this structure?

Already done. Adding a ActionProperties::accelerators member adds those accelerators automatically. See e.g. Action:UNDO.

Nice!

[bhennion]

Rebased on master: the new pdf marker opacity feature should still work :)
Added a fixed toolbar button padding via CSS: this might change the way things look like for users, depending on their selected theme. This may not be the best way to handle this. Any suggestions would be good

[Febbe]

Rebased on master: the new pdf marker opacity feature should still work :)
Added a fixed toolbar button padding via CSS: this might change the way things look like for users, depending on their selected theme. This may not be the best way to handle this. Any suggestions would be good

Can't we just set all 4 margins of the child widgets?

[bhennion]

Can't we just set all 4 margins of the child widgets?

Sorry I wasn't clear. The problem is not gtk3 vs gtk4. The issue is more simple (and less important): before this PR, the toolbar buttons were styled using the toolbutton selectors in the theme's css (or scss) files. Now, they follow the button entries. This makes them too big in Adwaita.
The last commit simply sets the padding (to 3px) for all buttons in the toolbar, overriding the theme. That works fine, but users may complain if they now find the buttons too big/too small.

What I don't know how to do (I don't think it's possible in a reasonable amount of code) is to have the buttons of the toolbar follow the theme's toolbutton selectors.

All in all, it's not that important and we can leave it like that, IMO.

[bhennion]

Fixed a couple of nitpicks I found in the code (uneeded includes, debug output) and made a cmake flag for debug output concerning the action database.

Is every one happy with this? (I'd be happier if we merged this relatively soon as rebasing whenever a new action is added is a bit cumbersome.)

[Febbe]

This is basically ready to merge right?

[bhennion]

This is basically ready to merge right?

Yes

[Febbe]

Thank you for the work @bhennion