initial conversion to gtkgestures ("gtk4") and other experiments

[dterrahe]

Replacing #29 which became a mess due to rebasing.

This started as a general regexp cleanup to reduce boiler plate in pointer casting (especially in event handlers).

But then I wanted to experiment with transitioning to gtkgestures in preparation for gtk4 (and also to improve support of touchscreens under gtk3). Since there was so much overlap (in the function prototypes) I continued in this branch.

Obviously this has parallels to #16919. So what are some of the differences?

• no memory leak. In gtk3 widgets do not own their associated event controllers, so they need to be cleaned up explicitly when the widget is destroyed. This can be done easily using g_object_weak_ref. The final port to gtk4 would simply adjust the code in dt_gui_connect_... to use gtk_widget_add_controller instead.
• the signal prototypes for gesture button_press and _release do not return a boolean to indicate whether the event has been consumed, like the widget event signals used to do. Instead, one needs to explicitly claim the gesture in order for it to stop propagating (otherwise a right click on a graph might delete a node and call up the presets menu) and to receive further (double) clicks or releases. This also clearly shows the danger when migrating; signal handlers are always cast to GCallback so there is no typechecking that catches changed interfaces. This only shows during testing when functions either malfunction or crash.
• for the converted graphs and bauhaus widgets, both button press/release and motion/enter/leave have been converted.
• an attempt has been made to avoid using constructs that would only work in gtk3. For example there's no explicit mention of GtkGestureMultiPress in the signal handlers, because gtk4 renames it to GtkGestureClick so a final port would have to revisit everything again. Instead, the signal source is passed as a pointer to one of the parents (GtkGesture or GtkGestureSingle, depending on what is convenient). In many cases, rather than continuing to use fields of the event object directly (after retrieving it using gtk_gesture_get_last_event), which will not work anymore in gtk4, alternatives have been used (where their API will be stable). For example using the x and y signal arguments, rather than event fields.

This very limited first step clearly shows how much work is involved. Though there is already an improvement in touch screen behavior (tested/tried only on Windows) clearly much more porting needs to be completed before it becomes useful. However, at the same time it seems infeasible to maintain this as a separate PR. Merges like #17161, besides being mostly unnecessary, cause significant work in rebasing.

Deprecated modules will have to be converted too (and have been, in this limited PR), but testing them is not easy (and won't happen organically before/after merging a PR). "Luckily" a lot of the graph/curve code has been copy/pasted between modules, so much of the conversion was done on the auto pilot and has at least some chance of working.

What this exercise, as many of the previous ones I've worked on, also clearly shows is that any large maintenance work on the codebase benefits from a lot of cleanup work first. Especially deduplication/refactoring and coding style uniformity massively help (and actually are where most of the work goes). Often patches have been applied over patches, when refactored code might have been much more trivially ported. For example some of our custom widgets could have sent their own custom events rather than linking to standard gtk button press/release events (which is often done to catch modifier settings from the event->state, which gtk4 does not allow). I'm specifically thinking of GtkDarktableToggleButton which could support control&shift/right-click/long-press features directly rather than letting all the users deal with it (and possible touches as well). However, restructuring while porting introduces more opportunities for errors to creep in and mindlessly converting 10 cases is often faster than first unifying them and then converting once. Perpetual technical debt.

@turbogit I know you are currently very busy so don't expect any in-depth response, but are you at all interested in this PR even in this limited format? It would have to be prioritised over other PRs that touch significant gtk code in order to be manageable.

@jenshannoschwalm @ralfbrown included you both to see how much interest there would be to carry this forward. Clearly this is not a one man effort (or at least not this man).

[dterrahe]

@turbogit @jenshannoschwalm @ralfbrown

Started an experiment with preparing to migrate gtkbox to gtk4. Again, due to all other gtk related changes here, it made more sense to base it off this "fork". Gtk4 doesn't have the child expand/fill flags anymore that get passed in gtk_box_pack_start. Instead this is managed via gtkwidget hexpand etc. Which makes the process of adding widgets to boxes uniform and allows using generic helper functions that would be the "only" place to make the final change of moving from gtk_container_add to gtk_box_append. Those helper functions can also be nested, so often a multi-step definition of self->widget is no longer needed. See for example https://github.com/dterrahe/darktable/blob/c4fcd6ffdb26734472f49a65b629abe52a732171/src/imageio/storage/piwigo.c#L1149-L1158

To me this new approach would be beneficial even if the migration to gtk4 never happened as it makes defining "standard" ui widgets easier. (I'm sure the more complex ones and very tailored dialog boxes will still be a hassle, as they were before).

The commit "prepare to migrate box" shows a set of examples of what this conversion would look like, but of course there's no point in progressing this further if there is no interest.

[ralfbrown]

I'm about to disappear for two months due to work, but wouldn't mind looking at it again in spring/summer 2025.

[jenshannoschwalm]

1. For me all the "maintenance work" (standardize on g, avoid casting) would be extremely important/helpful as code would be far more readable for us all so i think we should do that asap. There seems to be enough time until next release to fix possible issues ... Will you do a pr on main darktable or would you want me to test on your fork?
2. About Gtk code. As you know i simply don't understand gtk and don't know anything about implications of gtk3/4 specifics. I trust on what you do but unfortunately i won't be able to help you on this part.

[dterrahe]

Not doing a PR, no. Please feel free to cherry-pick whatever you like. I have rebased the maintenance work, but I haven't at all checked if it is still "complete". I.e. if they old patterns have been reintroduced with recent commits, they'll have to be cleaned up again. Also, there's probably more maintenance to do...

I've just been doing the gtk stuff on top of this because there's so much dependency/overlap, but really for gtk3 cleanup/gtk4 preparation I'm just playing around and giving some suggestions on how this could be done without simply doing a line-by-line translation, which would lead to a very inefficient end result. Really, some stuff needs to be redesigned to take advantage of improved gtk4 stuff rather than setting the current status quo in stone. For the gestures, for example, since everything needs to be fixed anyway, why not think about how left/right/ctrl etc clicks (and touch support) really should be done and then implement that everywhere consistently. Looks like popup menu's will need a lot of work too, so same there.

[jenshannoschwalm]

Just a note: your signals work (rebased on current dt code so minor "missings") has also been merged

[jenshannoschwalm]

Diederik, as you won't do the pr i cherry-picked the remaining 3 commits to a local branch, rebased on master without any changes, and tested here. Could not spot a single issue!

Also read through the code, for a non-gtk-speaking person like me it seems to me we reduce and simplify code significantly so for sure it's worth to integrate now, whoever might work on progress to gtk4 (definitely it won't be me and from the commit history related to gtk i fear you are the person knowing by far most how that could be done :-)

I feel a bit uneasy yet about pushing the PR as i might not be able to fix resulting issues or even discuss details. I would certainly need your support! Give me a "GO i will assist" before opening.

[dterrahe]

These GTK4 experiments are just that; experiments. They give an indication of how much work is involved and might give somebody inspiration to get involved. But this should be someone who really knows gtk3&4 and has a thorough understanding of how darktable works and ideally how it abuses gtk to do it. But very few people do anymore, so it may just need somebody to spend a lot of time getting up to speed. It is impossible to test changes if you don't understand how the program is supposed to work; quite often the effects of a ui interaction happen somewhere quite invisible.

So imho merging the commits here is pointless because most of the similar cases have not be converted yet and the way development (especially gtk/ui related) in darktable happens is that the non-converted cases will continue to be copied over.

I toyed with the idea of submitting this as a draft PR marked "RFA" (request for adoption). That would possibly give a somewhat larger chance that somebody will get involved and do the remaining work. But tragically the open PR list gets hardly any attention from anyone besides the three core developers, so I'm not holding out high hope. For this reason I do not believe that a full gtk4 transition has any chance of happening.

For fun I pushed one additional commit here that makes it possible to compile the codebase with Gtk4 instead of Gtk3 (after installing the necessary development libraries, obviously. I've only tested this under debian). Any deprecated/removed calls have been defined as macros in darktable.h so that they don't break compilation. Obviously it immediately crashes when you try to run the resulting binary. But at least this gives some idea of how many different aspects of the codebase need changing. Any of these functions should not be robotically be replaced by their "equivalent" gtk4 version but instead should be restructured to take advantage of new approaches or simply because old workarounds might not longer work or be needed.

I would certainly need your support!

I am still not planning on getting involved to that extend. In hindsight I consider submitting #17451 to have been a mistake.

[TurboGit]

For this reason I do not believe that a full gtk4 transition has any chance of happening.

That means that darktable will not be available in some years from now. So I hope for the opposite :)

[dterrahe]

So I hope for the opposite

I'd like to have hope too, but it is unreasonable to base that on the efforts of one burned out developer. I've indicated what I feel would be required to achieve the goal. It is not impossible for new people to learn gtk, if they are willing to put the effort in. It doesn't require expert level to take the recipes I've suggested in this PR and apply them to the rest of the codebase. I've tried in my past PRs to provide explanations for people to learn along, but have no idea if anybody ever read them. Similarly, anybody can be involved in testing and feedback. But the attitude on github and pixls always is "I'll test it when it's done" and then I'll offer my superior insight and opinions. That may work for smaller enhancements, especially when one is in a position to ignore the feedback and merge anyway, but not for a two year effort.

Have you looked at https://github.com/dterrahe/darktable/blob/13688242e2cc38970e4bbb004312c8b9b331e39e/src/common/darktable.h#L1315-L1556 to understand whats involved? One could JOIN this 242 word list with the codebase to see how many lines (at the minimum) would need to be touched. As mentioned, this should be done while restructuring in order to not end up with an unmaintainable mess full of ancient artifacts that noone knows are needed anymore. Especially in the area of cross-platform compatibility this again comes down to people (on Mac, wherever) being willing to get involved while a transition is ongoing to test solutions as they are being experimented with.

[dterrahe]

@jenshannoschwalm in case you are wondering which changes in ancil are just for my own use and entertainment and which should be generally applicable, I'm listing the relevant ones below:

straight up bug fixes fix defaults for preference combos set default for language combo fix filter range entries tooltips escape log message fix right click slider fine tuning under wayland remove disabled combo items from shortcuts

code base improvements refactor dt_util_dstrcat to dt_util_str_cat save shortcuts dialog pane split only when changed

Uncontroversial (probably) functionality improvements/additions (need testing, obviously) shift+click snapshot to show difference scale and format changed values in history list tooltip don't recalc subset of already available buffer don't recalc on small zoom changes turn toggles into actions in metadata and colorpicker sort shortcuts in h screen by clicking on action header

Opinionated improvements mirror map cursor keys from darkroom in mask manager create new group for active module simplify styles and presets shortcuts tooltip let preview or module name sizes determine style name width in popup in shortcuts list change background to indicate which fields can be changed left align collections combo to avoid jump (what I mean with this last category is that I think these are clear and worthwhile improvements, but others have decided to implement these things differently, so you may get pushback and discussions that may depress you.)

[jenshannoschwalm]

I am surprised i didn't find the reverts in the last part of the list :-)

Just opened a next PR. IIRC we discussed this

   dt_dev_invalidate(dev); // FIXME not needed? redraw should determines if needs new calculation
    dt_control_queue_redraw_center();

before. I thought about it and tested again, you seem to be right, that invalidate is indeed not required.

[dterrahe]

I am surprised i didn't find the reverts in the last part of the list :-)

Those undo an intentional and democratically decided action to break the carefully designed behavior that was intended to gracefully deal with (initialising and switching between) displays with wildly different resolutions (laptops and 4k screens). I like my vectorscope circles and navigation thumbnail to have constant aspect, but if the majority want to adjust their width and height separately, and do it each time they connect an external monitor, I have no desire to get in their way. That's the beauty of maintaining a "fork" for personal use; it's easy to build something exactly to your own liking. Of course there is the little extra hassle of having to revert each fix for now inconvenient behavior in mainline separately too, but it is a very small price to pay. Please don't, in my name, push anything upstream to change this.

Similarly, I did not list "click on expanded but unfocused module grabs focus" as ready for upstream inclusion. I think that's a personal preference and both approaches have drawbacks. It's only in my personal workflow that my local behavior makes more sense. If the PR gets merged as is, in a few months' time you will get people complaining about breakage and bisect will point at a commit with my name. You could label that PR with default-behavior-change, but it's not really just a default.

"mirror map cursor keys from darkroom" is somewhat different, in that I believe that a lot of what people call "intuitive" is actually just "consistency". When CTRL almost everywhere (as far as I've noticed and fixed) means smaller moves, then in this one specific case using it to make larger moves has the potential to break people's motor memory and mistrust their "instinct" everywhere else in the program. I think this is "bad". Also, implementing the feature as a move action with fallbacks, rather than as hardcoded commands, means that it is fully customizable; you can change the step size or add other modifier combinations and you can assign up/down or left/right moves to, for example, a midi knob in one step, automatically inheriting the shift/ctrl speed adjustments. Of course nobody cares about arrow keys in the map view, so it is largely irrelevant, but to me it's the principle that details matter. I'm still somewhat sad about giving in to making "no fallbacks" the default. It means a lot of people will not benefit from the extra functionality this makes automatically/easily available and makes it harder to assume the same behavior for everyone in documentation. But then, some people don't really want flexible shortcuts and see them as a potentially dangerous inconvenience. I think ansel has mostly stripped them out completely for months now and people there seem very happy about it. More power to them!

[jenshannoschwalm]

Please don't, in my name, push anything upstream to change this.

Absolutely not intention to do so and i am fully aware of the decision process - i think i also commented.

Ok, i'll remove the "focus" commit from the PR. Next time i will make me the author so blaming me would be the result. ok?

EDTI: Would you want me to take "ownership" whenever i open a PR with your work so blaming/bisecting would be on me? (Maybe you told me already and i forgot)

[jenshannoschwalm]

But then, some people don't really want flexible shortcuts and see them as a potentially dangerous inconvenience. I think ansel has mostly stripped them out completely for months now and people there seem very happy about it. More power to them!

How do you know what people think? For sure AP strongly disliked it so i guess that's why it got removed.

I sometimes had a look what he was doing in github and couldn't spot anything noteworthy. No idea how stable ansel is now.

[dterrahe]

Would you want me to take "ownership" whenever i open a PR with your work so blaming/bisecting would be on me?

You are doing the integration work so it would only be fair to take the credit. And then people also would know who to turn to for fixes/changes. But tbh it doesn't matter too much to me, so just do what is less work. And not doing contentious stuff saves a lot of work :-)

How do you know what people think?

Anybody who hasn't left yet or continues donating must be happy with the decisions? I'm all for choice and for giving people who don't like dt's direction other options than just complaining.

couldn't spot anything noteworthy

Some of the stuff he does with the cache is "interesting", but his approach often breaks things because they were "stupid" when really the basic design of the cache never meant to support everything that is done with it now, so workarounds are necessary and the only real solution is a full redesign (after analysis of all current, and likely future, use cases). A lot of work but chasing down each individual breakage is work too and unlikely to improve performance in the long run.

[jenshannoschwalm]

Yes , also saw his work on cache. From what I understood he mainly removed the code... May be wrong

[dterrahe]

Added another commit to ancil (04d2e7a968fc7ba1e7b353ec5999c87d4058f1ba) inspired by 13e2050da8d126bd953f02286711d3fe4c315d82 and many like it. This automatically adds a newline to all dt_prints, so there's no need to tediously include it everywhere. I've removed probably like 99% of the explicit \ns, but it is likely I'll have missed some, which would cause empty lines to be printed which I think is less "bad" than multiple lines with their timestamps getting concatenated like might still be happening in mainline sometime (because hard to spot).

There's obviously the possibility that people were using "partial" dt_prints and then used dt_print_nls to complete the line. So far everywhere I spotted this seemed to be the case (i.e. a dt_print without a \n) it seemed to be in error, i.e. the next print was a dt_print (including the timestamp again on the same line). Especially camera_control.c could use a review in this respect. I have not attempted that myself.

It is also reasonable to conclude that this horse has bolted and that large trivial refactorings like this are just not worth it.

[jenshannoschwalm]

I check all changes, seem to be worthwhile. I also made sure, a log line doesn't end with a dot - some did, most not.

Will pr this in combination with the also-many-files-touching "dt_util_str_cat" (BTW your commit included changes in lua scripts, i left that out as it would of course require mods in that git.

[dterrahe]

I check all changes, seem to be worthwhile.

I hope you especially checked for dt_print statements that were not fixed ;-)

[jenshannoschwalm]

I hope you especially checked for dt_print statements that were not fixed

Indeed i missed quite a few, new pr. I think now were done with newlines.

[jenshannoschwalm]

1. A lot of more codebase maintenance got merged. Some dt_print() related
2. Something completely else - gtk related. Cou you have a look at issue 17684 ? There is a log with blue screen mentioned and at the end there are some gdk issue. gdk_seat_default_remove_tool Can you make anything out of that. (The system is definitely under massive system mem stress...)

[dterrahe]

Either some device (mouse, pen, whatever) is physically removed from the system or, more likely, memory is getting corrupted causing a tool driver to fail and remove itself.

I find the mention of DwmEnableBlurBehindWindow just before that more interesting, since it could be the cause of the corruption (or the additional memory strain). Sounds to me like windows is applying some blurring effect for which it might be using the GPU (and its memory) without asking anybody. Don't know anything about that though and trying to figure this thing out with just one user who might have a weird configuration (besides it being windows) without even knowing it is just not fun. If there are more people replicating, then maybe. Of course, a blue screen always means that something else (a display driver or windows itself) is messing up besides dt, so that already puts you at a massive disadvantage if you can't replicate the exact system.

Unclear to me "RAM fills up completely. This status takes very long time" is he talking about a slow increase in memory use (i.e. is there a leak somewhere) or just an immediate increase because he's using (two) large diffuse models?

If it used to work before, he could try installing an old nvidia driver.