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

Preparation for GTK4 #3306

Merged
merged 6 commits into from
Apr 29, 2022
Merged

Conversation

Febbe
Copy link
Collaborator

@Febbe Febbe commented Aug 6, 2021

@reviewers, please review commit by commit.

https://docs.gtk.org/gtk4/migrating-3to4.html

  • Do not use deprecated symbols
  • Enable diagnostic warnings
  • Do not use GTK-specific command line arguments
  • Do not use widget style properties
  • Review your window creation flags
  • Stop using direct access to GdkEvent structs
  • Stop using gdk_pointer_warp()
  • Stop using non-RGBA visuals
  • Stop using gtk_widget_set_app_paintable
  • Stop using GtkBox padding, fill and expand child properties
  • Stop using the state argument of GtkStyleContext getters
  • Stop using gdk_pixbuf_get_from_window() and gdk_cairo_set_source_window()
  • Stop using GtkButton's image-related API
  • Stop using GtkWidget event signals
  • Set a proper application ID
  • Stop using gtk_main() and related APIs
  • Reduce the use of gtk_widget_destroy()
  • Stop using the GtkWidget.destroy vfunc
  • Reduce the use of generic container APIs
  • Review your use of icon resources

==== HArder to fullfill, since the API is not fully replaced ====

  • Stop using GdkScreen

@Febbe Febbe changed the title remove later: disable deprecated gtk3 API Preparation for GTK4 Aug 6, 2021
@rolandlo
Copy link
Member

rolandlo commented Aug 6, 2021

* [ ]  Stop using `gdk_pixbuf_get_from_window()` and `gdk_cairo_set_source_window()`

We already don't use it. There is only a comment about using gdk_pixbuf_get_from_window()in pixbuf-utils.cpp

@Febbe
Copy link
Collaborator Author

Febbe commented Aug 6, 2021

* [ ]  Stop using `gdk_pixbuf_get_from_window()` and `gdk_cairo_set_source_window()`

We already don't use it. There is only a comment about using gdk_pixbuf_get_from_window()in pixbuf-utils.cpp

Right, I also stumbled over this, maybe mark this after I removed the comment.

@Febbe
Copy link
Collaborator Author

Febbe commented Aug 22, 2021

Stop using direct access to GdkEvent structs

That will be very hard, since the new input system hardly relies on that.
@LittleHuba any suggestions how to fix that?
We basically must(?) write own GDKInputHandlers for that.

@LittleHuba
Copy link
Member

As I see it, this only means that we must use getters to retrieve the information from GdkEvent structs. The input system is rigged to move all data from these GdkEvent structs to our own struct. All our input handlers work on our own structs. So we would only need to make sure that this conversion uses the appropriate getters.

See

auto InputEvents::translateEvent(GdkEvent* sourceEvent, Settings* settings) -> InputEvent {

@Febbe
Copy link
Collaborator Author

Febbe commented Aug 25, 2021

@LittleHuba oh meant actually this: Stop using GtkWidget event signals Gtk4 completely changed the event system / handling. It is not wanted to parse Events outside own event Controllers.

@LittleHuba
Copy link
Member

The description here does not sound that different from what we currently do...
https://docs.gtk.org/gtk4/input-handling.html

@LittleHuba
Copy link
Member

Could you provide me with some resources on the changes that cause your concerns?

@Febbe
Copy link
Collaborator Author

Febbe commented Aug 25, 2021

That the way how we should do it, is to write our own eventControllers, if the existing ones don't suffice. But the only way to port the NewInput System is to connect it to a LegacyInputController. Dunno, which implications this has.

@LittleHuba
Copy link
Member

Can't we reuse the same concept we currently use and simply forward all events from a new eventController to our input system? As the events are propagated from parent to child, this controller would sit on the widget of the drawing area. Same as before as far as I remember.

@Febbe
Copy link
Collaborator Author

Febbe commented Aug 25, 2021

Yes, that's the same as using a LegacyInputController. But it is very dirty. Most of the things we do in the input system and what we also planned to do in the future, is already part of the new EventControler system. So we should transform the input system to adapt those.

@LittleHuba
Copy link
Member

In that case, I would go with the LegacyInputController for now and switch to the new concept in a follow-up PR. Otherwise, this PR will grow very big and stop progress for other PRs as it influences many parts of the source.

@Febbe
Copy link
Collaborator Author

Febbe commented Aug 25, 2021

I think this PR is already ready to merge, most of all changes will require a switch to gtk4 directly.

@Technius
Copy link
Member

Technius commented Feb 6, 2022

Needs to be rebased.

@Febbe Febbe force-pushed the gtk4_remove_deprecated_gtk3 branch 3 times, most recently from 6af1ffd to 7821d24 Compare February 6, 2022 18:25
@Febbe Febbe marked this pull request as ready for review February 6, 2022 18:43
@Febbe
Copy link
Collaborator Author

Febbe commented Feb 6, 2022

@Technius it's rebased, I also rebased the gtk4 branch, but I suppose I broke many lines of code there.

@Febbe Febbe force-pushed the gtk4_remove_deprecated_gtk3 branch from 7f1db49 to d3a36bb Compare April 25, 2022 10:14
@Febbe
Copy link
Collaborator Author

Febbe commented Apr 25, 2022

@Technius I'll merge this in 24h hours if there are no objections

@rolandlo
Copy link
Member

On commit 3625e7b: There are more GdkEvents structs that are accessed directly, e.g. in SidebarPreviewLayerEntry.cpp, SidebarPreviewBaseEntry.cpp, SetsquareInputHandler.cpp,

@Febbe
Copy link
Collaborator Author

Febbe commented Apr 25, 2022

Yes, that issue is too big for this first step. Also many Event structs aren't even implented in gtk3.

@Febbe
Copy link
Collaborator Author

Febbe commented Apr 25, 2022

Honestly it would be better to rewrite the application from scratch than porting it😔

@LittleHuba
Copy link
Member

Honestly it would be better to rewrite the application from scratch than porting it😔

We can always consider restarting on the GUI. The file format and input handling are quite decoupled from it.

@Febbe
Copy link
Collaborator Author

Febbe commented Apr 28, 2022

input handling are quite decoupled from it

We also have to restructure the input handling, since gtk3 events are deprecated and should be replaced by Event controllers.
But this might be easier than I expect.

It defines replacement functions for gtk3-gtk4
@Febbe Febbe force-pushed the gtk4_remove_deprecated_gtk3 branch from d3a36bb to 4f8642a Compare April 28, 2022 21:28
@Febbe Febbe merged commit 377b20b into xournalpp:master Apr 29, 2022
@tattsan tattsan mentioned this pull request May 1, 2022
@bhennion bhennion mentioned this pull request Oct 2, 2022
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.

None yet

4 participants