Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start of Gtk3 to Gtk4 #1909

Open
wants to merge 102 commits into
base: stable
Choose a base branch
from
Open

Start of Gtk3 to Gtk4 #1909

wants to merge 102 commits into from

Conversation

Bob-IT
Copy link
Contributor

@Bob-IT Bob-IT commented Apr 7, 2024

I have had a look at moving from Gtk3 to 4 and thought I would push what I have done.

Starting with 5.6 built without any Aqbanking and OFX support to try and make life easier I did some of the pre-migration changes as in the first 7 commits. This was mainly around not accessing the event structure directly but by function calls.

After that I pushed Webkit out of the way and changed to gtk version 4.8.3 and started compiling, on error I tried to replace the offending function and do a global find/replace or comment it out for later investigation. In total this was 69 commits, up to Change gtk_init_check commit. The next commit fixed any errors in the trace file when I tried to start a built version.

There are a lot of changes and some widgets have gone like GtkContainer, GtkToolBar and GtkStatusBar is on it's way out. The main layout widgets are Box and Grid with special functions for adding a child to windows and frames.

Another big change is that there is no gtk_dialog_run function so one would need to use a response callback for possibly 140 dialogs but GtkDialog is on it's way also and being replaced by a GtkWindow.

The migration document is here which shows what is likely to be involved.

I definitely have not got every thing I changed correct but it has enabled me to get a sort of working framework to test the changes and make further ones. There is still plenty to change, not least the ui and glade files. As can be seen below, there is no toolbar as that is depreciated and I have removed that section from a couple of ui files and commented out any reference to it.
Not sure about replacing it or maybe not, need to think about it more.
The other thing I am not sure about is the menu, ALT+ the top first letter works but not on the actual menu or the Ctrl+ entries. I think the latter may be fixed by the use of a GtkShortcutController.

The other thing worth mentioning is that I am not using GtkApplication, see Change gtk_main commit.

The last 6 commits are to do with trying to get a handle on what is needed to have a working chart of accounts page as seen below, the height of the combo is not playing ball so that may need to be changed to a GtkDropDown...

One thing I would like to clarify is what version of Gtk4 are we aiming at, as stated above I am building against 4.8.3, 4.10.5 would be my next choice with more depreciated widgets and was released August last year, the latest version is 4.14.2

gtk4-image1

@jralls
Copy link
Member

jralls commented Apr 8, 2024

I guess it's time for a future branch to base this on.

That aside I think you might be going at this the hard way. Step 1 is to turn on deprecation warnings, still building with Gtk3, and get all of those fixed. We could conceivably merge that into stable.

I think Step 2 has to be to convert to GApplication.

Step 3 is to rewrite as many uses of removed API listed in the migration document as possible while remaining with Gtk3. For example we have to replace GtkToolBar with a GtkBox styled as a Toolbar.

On the subject of menu accelerators, they've removed the old primary hack.

Is this so much work that we should consider putting it into changing GUI toolkits instead?

Gtk4 version shouldn't matter that much at this stage: Anything they deprecate after Gtk-4.0.0 won't be removed until Gtk5 so we can keep using it while we migrate, but we can do ourselves a favor going forward by trying to clean out the deprecations as part of major releases, e.g. if GC6's benchmark distro is Ubuntu 24.04LTS then whatever is in that (I suppose it will be Gtk-4.14.0) then we should fix all deprecations to that Gtk version.

@Bob-IT
Copy link
Contributor Author

Bob-IT commented Apr 9, 2024

Step 1 is to turn on deprecation warnings, still building with Gtk3, and get all of those fixed. We could conceivably merge that into stable.

I did do this first and if I did it correctly there are none in GnuCash. The only one it did find was in WebKit where they are still using GtkAction.

I think Step 2 has to be to convert to GApplication.

Would need to think about that...

Step 3 is to rewrite as many uses of removed API listed in the migration document as possible while remaining with Gtk3. For example we have to replace GtkToolBar with a GtkBox styled as a Toolbar.

I saw that and will have a think about what that would involve. Maybe I misunderstood some of the prep work or just thought I would see what building with gtk4 would throw up and then fix it.

Is this so much work that we should consider putting it into changing GUI toolkits instead?

I do not know, I would like to try/fix a few more changes.

@jralls
Copy link
Member

jralls commented Apr 9, 2024

I saw that and will have a think about what that would involve. Maybe I misunderstood some of the prep work or just thought I would see what building with gtk4 would throw up and then fix it.

No harm (other than time expended) in experimenting. Looks like we'll be doing a lot of that in this migration. My thought on getting as much as possible done in Gtk3 is to reduce the variables at play as we rewrite the code.

I do not know, I would like to try/fix a few more changes.

OK.

@christopherlam
Copy link
Contributor

Whoops sorry didn't see these new conflicts dialog-commodity.c[pp] and gnc-currency-edit.c[pp]

@Bob-IT
Copy link
Contributor Author

Bob-IT commented Apr 10, 2024

Whoops sorry didn't see these new conflicts dialog-commodity.c[pp] and gnc-currency-edit.c[pp]

No worries, I will fix when I next push, just carry on committing as normal.

@fellen
Copy link
Member

fellen commented Apr 10, 2024

built without any Aqbanking

That would require a GTK4 module in
https://github.com/aqbanking/gwenhywfar/tree/master/gui

@jralls
Copy link
Member

jralls commented Apr 11, 2024

That would require a GTK4 module in https://github.com/aqbanking/gwenhywfar/tree/master/gui

Which we'll probably have to do and submit as a PR.

@gjanssens
Copy link
Member

I think Step 2 has to be to convert to GApplication.

Would need to think about that...

This was what I was studying when I ran out of time for gnucash. I had my thoughts on how to go forward with this, but hadn't tried anything in code just yet.

I still would like to work this out and may find some time in the coming weeks. Can you leave the GApplication conversion part aside for me ?

@Bob-IT
Copy link
Contributor Author

Bob-IT commented Apr 13, 2024

Can you leave the GApplication conversion part aside for me ?

Will do, I am just looking at conversion of dialogs and some custom widgets.

@jralls
Copy link
Member

jralls commented Apr 13, 2024

@gjanssens One thing to consider for migrating to GApplication is timing of loading the book in relation to starting the event loop. As you know a long-running complaint from Mac users is that they can't double-click a data file to get GnuCash to open it because macOS uses an event to tell the app what to load and GnuCash loads the last-used file before it can handle events.

@gjanssens
Copy link
Member

@gjanssens One thing to consider for migrating to GApplication is timing of loading the book in relation to starting the event loop. As you know a long-running complaint from Mac users is that they can't double-click a data file to get GnuCash to open it because macOS uses an event to tell the app what to load and GnuCash loads the last-used file before it can handle events.

Indeed. I don't know the insides yet, but from what I remember from my reading so far is that in the GApplication world the request to open a file comes in in the form of an event. I don't know how the MacOS backend implements this, but I was hoping on macos such an event would be generated as well when the user clicks on a GnuCash data book.

So my idea here was to process the pending event queue at the appropriate moment early on in the application startup and once empty check whether or no a file has been opened. If not, open the last opened file as found in our history list.

Would that work ?

@jralls
Copy link
Member

jralls commented Apr 13, 2024

Should do. Here's how gtk-mac-integration (the Gtk2/3 library that preceded GtkApplication) handles it. I'm not as familiar with the GApplication implementation but I expect that it would be pretty similar.

There is, BTW, a UX regression for macOS with Gtk4.

Bob-IT added 18 commits May 3, 2024 14:16
In Gtk4 you are no longer able to access the event structure so use the
accessor functions to retrieve values
In Gtk4 you are no longer able to access the event structure so use the
accessor functions to retrieve values

These files access the event structure with no equivalent accessor
function. Not sure of correct change, may need to wait for Gtk4 change.
In Gtk4 you are no longer able to access the event structure so use the
accessor functions to retrieve values.

Register sheet and header changes
In Gtk4 you are no longer able to access the event structure so use the
accessor functions to retrieve values

Register components.
In Gtk4 you are no longer able to access the event structure so use the
accessor functions to retrieve values

ComboCell and CompletionCell needed more changes and not sure why, just
doing the normal change was not enough as this resulted in characters
appearing twice on my build machine but not on Windows or Fedora38.
By changing to GtkEventControllerKey for combocell and completioncell
fixes issue for double key presses.
Start to use GtkEventControllerkey for some key_press_event's, may need
further changes.
Change some entries to remove state and comment out others not sure
about.
Widgets by default are always visible, may need to change some for
toplevel windows, dialogs and popovers
Use gtk_widget_set_visible instead
Use gtk_widget_set_visible instead
Use gtk_widget_set_visible instead
Add function gnc_entry_get/set_text and use that instead
Add function gnc_entry_get/set_text and use that instead
Add function gnc_entry_get/set_text and use that instead
Bob-IT added 22 commits May 3, 2024 14:50
Rename the gnc_dialog_run function to gnc_warning_dialog_run as it uses
the preference warning settings and make it modal and pause the main
loop so it can return a value.

Create a new gnc_dialog_run along the lines of above and change only
references to gtk_dialog_run used to create message dialogs to this
function.

This may need a rethink but for now is OK if only temporary, eventually
the widget used will be a GtkAlertDialog.
All builder ui signals are connected by default and can use the current
object to pass user data.

This may need changing...
@Bob-IT
Copy link
Contributor Author

Bob-IT commented May 3, 2024

I have been trying some changes on and off as seen here, the first image shows a right mouse menu and the next shows the column menu. The text size can be controlled via CSS but on first looks could not see how to change the height/spacing but that will come to me...

gtk4-image2
gtk4-image3

You will also notice I have added a toolbar, seen below. This image shows the difference in named icon image sizes, most of them are large but the last is normal, the actual size I think is theme based but you can set the size in pixels. This was fairly easy to do with some modification to the existing ui files, each button holds a vertical box packed with an image and label and sizes can be controlled by CSS. There is nothing fancy here so the Gnucash window may increase if there many buttons.

gtk4-image5

The other area I have been looking at is the tools menu, I have done a first attempt at converting those dialogs to windows, still modal but possibly returning values via callbacks.

There are still many changes required moving to gtk4 but am getting some messages in the trace file which appear to of been fixed in the 4.10 series.

My next attempt at changes will be possibly changing a GtkCombo to a GtkDropDown or the dense calendar/register as they use a lot of cairo. If that is successful, I think I will up my gtk4 version to 4.10.5 and see what that shows up.

@jralls
Copy link
Member

jralls commented May 3, 2024

@Bob-IT Nice work. Do you mind if I push a fix to the CI?

@Bob-IT
Copy link
Contributor Author

Bob-IT commented May 3, 2024

@Bob-IT Nice work. Do you mind if I push a fix to the CI?

For stable, yes just do it, for this PR I am building with just -Werror -DGDK_DISABLE_DEPRECATED -DGTK_DISABLE_DEPRECATED. I commented out several calls to functions and did not want the extra hassle of fixing variables/function defined but not used. I will of cause change that to the original parameters and fix any issues.

jralls added 2 commits May 4, 2024 17:42
Macros can't have a space between the name and the opening parenthesis.
@jralls
Copy link
Member

jralls commented May 5, 2024

For stable, yes just do it,

I meant for this PR: jralls@e4bdad8

It turns out that the current Ubuntu-latest image had gtk-4.6 and of course there's other work to get it to build on macOS so I turned off everything except the Arch Linux docker test.

@jralls
Copy link
Member

jralls commented Jun 14, 2024

On the subject of menu accelerators, they've removed the old primary hack.

Progress on this front, of a sort: https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/7260 in which Emmanuele Bassi explains that for the most part it's the application's responsibility to provide the common keyboard shortcuts. Gtk4 only provides shortcuts for its own widgets. I derive from that that apps can pretty easily create alternate versions and Arjen Molenaar redid the MR to provide the alternate versions in Gtk4.

I don't think Emmanuele or Matthias Clasen would go for a macro but nothing keeps us from doing one:

#ifdef __APPLE__ 
#define GNC_PRIMARY_MASK GDK_META_MASK
#else 
#define GNC_PRIMARY_MASK GDK_CONTROL_MASK 
#endif

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants