September - GTK4 migration - dboles

22 septembre 2023 15:07

Here’s the report on the work I’ve done on Inkscape in September, of course primarily focused on getting us GTK4-ready – but with a fair bit of bugfixes, cleanups & optimisations into the bargain!

1st change was an overhaul of our widget to edit gradients/stops. This had very old-style size- requesting code that would be a pain to migrate to GTK4, so I rewrote it to be a Box and hence get  ‘normal’ GTK size-requesting. This let me move the request to CSS, which is simpler and means user themes will be able to change sizing. In this I resolved an existing TODO that we didn’t show a focus rectangle around the bar. Next I realised it was a bit underwhelming that Tab keys could not change focus among stops (only in/out of widget overall), so I added code to make it happen. Nice!

I fixed a bug in the Path Effect menu, retaining items from past selections & thus allowing activation of items that should be disabled. I also fixed a memory leak of widgets here. Also, we saw that if an invalid activation did somehow occur, the app could crash, which of course is always wrong! So I tracked down and fixed some bugs, so we now warn if something like that does happen, not crash.

A user on IRC channel pointed out that, since migrating GdkEvents to controllers, the ability to hold Alt and hover items in the Layers & Objects dialog, to highlight on the canvas, did not work right. Alt detection was inverted due to a bug in GTK, and the ability to do this when the dialog didn’t have the key-focus had been lost. I fixed these regressions, then noticed another: hovering without key-focus failied to instantly update if Alt state changed (only upon more motion), so I fixed that too, with the help of a new utility function to dis/connect a key handler to the root window on un/map.

In other testing, I noticed the Export dialog had cosmetic gremlins. I added/unified margins, Title Case, and tooltips; removed a frame inside a popover; and replaced GtkButtonBox with GtkBox to be ready for GTK4. I also saw our helper to align icons with check/radio indicators in menus had a bug in a calculation & so was offsetting a bit too far, so I fixed this to get everything lined up again.

Back to a main task, migrating GdkEvents to Controllers – I removed all GdkEvents in SpinButton, GradientSelector & PointParam widgets; & FontVariants/Selectors/FontCollectionSelector dialogs. In the latter dialog I also found and fixed some issues: deleting by key didn’t work, adding one could cause GTK errors, and we were allocating widgets with `new` & so leaking, so now Gtk::manage().

Further GdkEvents were removed later from ComboBoxEnum, Desktop, and InkscapeWindow – the latter two being some of our main classes. Code that cannot avoid GdkEvents on GTK3 in the latter classes was rewritten in the (recently revitalised!) GTK4 branch and now seems to build OK!

I updated my PopoverMenu, which we are using to replace our more ‘widget-y’ GtkMenus to get ready for GTK4. It had a regression stopping the selected item being updated on motion, and it was using GTK3’s Widget.get_children(), which won’t exist in GTK4, and which may anyway have been vulnerable to unspecified child order – that is replaced with our own vector to ensure predictability!

I fixed a cosmetic bugbear: we had `namespace Dialog` and `namespace Dialogs`: these are now one `namespace Dialog`. In this I found and fixed leaks, raw new/delete, unneeded dynamic alloc, double lookup in maps, and opportunities to use auto_connection. Later Tav saw a build error with C++20, which I tracked to a redundant template argument list in the ctor declaration, which C++20 starts rejecting, so I fixed that. I also did a lot to clean-up our desktop and avoid the huge gtkmm.h, which I hope will speed build time. And finally binned the deprecated, GTK4-unfriendly LPE Gallery!

The update to stop using GtkBox.pack_*() functions mentioned in a previous blog has been kindly tested extensively by PBS, who noticed several regressions for me to fix – and is now merged! Hopefully we are mostly there with that, and being merged to master means more keen eyes on it.  Also, straggling uses of Gtk::Bin, gone in GTK4, were replaced or encapsulated to get us ahead of that for migration later, and to ease the merge from the current GTK3 master branch to GTK4 one.

We spent more time trying to build the GTK4 branch, instead of just preparing for it in the `master` branch. This is a mix of work: stuff that can only be done on GTK4, and things it flags that aren’t errors on GTK3 but can be improved there, to get more eyes on them and minimise Git work later.

A big task is replacing GTK3 Container.get_children(). In GTK/mm 4 this is gone, but we use it a lot. Thus, like UI::pack_*(), I made a function to do the same thing, but which we can use instead of Container.get_children(), so once building on GTK4 we only need swap the implementation, instead of having to update all callers. Plus, having a function that takes a Widget instead of a Container – let me later remove nearly all remaining dependencies on Gtk::Container in the `master` branch.

Another thing we can do here is stop using per-widget style/CSS providers, since in GTK4 that ability is removed and all styles must be added on the display/screen level only. Thankfully we only had one usage of this pattern in Inkscape, and it is fairly easy to replace while still on GTK3 so that we won’t have to remember/worry about it later on GTK4. Handily, this makes the code shorter too!

I resumed everyone’s favourite task: replacing Gtk:: Menu et al. with GTK4- ready alternatives, as GtkMenu* are gone there. A good start: several files with Menu members didn’t use them, and one was in an old, unused DebugDialog. Then I got to used menus! Eventually, our LPE Editor, Page Properties and XML dialog were migrated to Gio::Menu/Actions. GTK could document menus far better, but I found the GTK4 doc of PopoverMenu the best ‘at a glance’ source and worked it all out.

The menu for page size/template in our Document Properties was very fun, as it has 76 items… which (A) is not appealing to slowly scroll through and (B) can’t be scrolled in Gio::Menu as GTK doesn’t give scrolling there. I fixed it by splitting into submenus: US, ISO A-E and Others – IMO an overdue tidy-up. I also added the missing arrow and a tooltip for long page size names. I converted the Gradient Editor to our PopoverMenu as it has icons, which sadly GTK doesn’t want us to see…

I then took a break from Menus to fix some memory leaks reported by ASAN, which I’d enabled on my local build to debug a different issue some weeks ago. Always worth doing to save your RAM!

Then, to my brain’s delight, back to migrating from GtkMenu. First, another easy win: our combo- box-tool-item widget had members of a couple MenuItem types, but they evidently were a relic of a “radio” mode which has since been removed. The members were now unused, so I remove them. Then I tidied/renamed and fixed a bug in our function that shifts menu items with icons into place, renaming it to reflect that it also sets their tooltip, and fixing my bug that means only the 1st got set.

On to our ContextMenu, when right-clicking the canvas. This already used Gio::Menu, so making it a Popover was easy. But GTK really don’t want to ever show our icons in popover menus. To fix this, at least until someone wants to try to win an argument with GTK about it… I wrote a function to walk a widget tree, find non-empty Images (which luckily for us, GTK creates but leaves hidden) – and force said images visible. I updated the ‘icon shift’ code to work on the modelbuttons in these Popovers too. Finally for menus (for now?) I found and binned various unused *menuitem includes.

I fixed a visual bug in our Ink/Spin/Scale, as seen in the Filter Editor, where if drawing a label over the scale (most don’t!), we drew the ‘filled’ part of the text as dim or invisible, depending on theme. I then turned to building our GTK4 branch again and, coincidentally, started with various fixes to said widgets needed to make them work there. I continued this for most remaining time, picking away at each file revealed to have build errors by `ninja -k0`, fixing these errors by updating them for GTK4. This included moving to the new Gtk::FileDialog, to replace the deprecated FileChooserDialog et al.

I then tacked a FUN change: gtkmm3’s TreeIter was a normal class, used as both the TreeModel:: itererator and ::const_iterator classes. But in gtkmm 4, TreeIter is a template, on either TreeRow or TreeConstRow, so we can’t just use it as-is. So I updated all uses of TreeIter to TreeModel::iterator. We won’t be as const-correct as gtkmm4 allows since TreeView et al. are deprecated, so no point!