-
Notifications
You must be signed in to change notification settings - Fork 7
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
Gtk4 port #53
Gtk4 port #53
Conversation
Gtk4 only rounds when the value is shown, which is not the default
I think this is ready to go now -- I don't see how to simulate combinations of mouse clicks and keyboard modifiers, which are used in tests and precompiling in the GTK3 version. Maybe doing so would involve GtkEventControllerLegacy (which I really don't want to go near) and even that may not work. Anyway, these types of events work fine for me (in the "imageviewer.jl" example and the corresponding ports of ProfileView and ImageView). |
I will hope to get to this over the weekend. The line-change-count is intimidating but I'm sure it was far harder to execute than it will be to review! I would want to spend a bit of time seeing if I can figure out any way to restore automation of those tests, but if you haven't found anything I suspect there's very little chance that I'll discover anything. It does seem weird, though; how does Gtk4 do its own internal testing if these things can't be automated? |
Codecov Report
@@ Coverage Diff @@
## master #53 +/- ##
==========================================
+ Coverage 91.62% 94.49% +2.86%
==========================================
Files 6 6
Lines 1098 1089 -9
==========================================
+ Hits 1006 1029 +23
+ Misses 92 60 -32
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Thanks! It looks like there is a problem with tests on nightly related to timers, which I saw mention of in the precompile code. I'll look into it. Yeah, the most difficult part of this port was adapting to the changes to event handling, and those are unfortunately the parts that aren't currently being tested. I'll take a closer look at how to simulate events. You're right, they must do this somehow within GTK and it would be worth examining how they do testing within GTK. I'm waist deep in this so I think it's up to me to get this working. I tried not to make gratuitous edits, but I did make a few changes that are not directly related to the port to Gtk4.jl: I tried to fix some of the doctests, which weren't working (and oops, still aren't I guess), and updated some versions in CI that lead to warnings. If it would make your task easier I could split those into separate PR's, but it's not going to reduce this PR by much. |
Make life easy on yourself and leave them here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the issue about testing discussed earlier, I see nothing but goodness here. Merge when ready.
@@ -2,11 +2,10 @@ | |||
|
|||
Let's create a `slider` object: | |||
```jldoctest demo1 | |||
julia> using Gtk.ShortNames, GtkObservables |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't have to be fixed in this PR, but we'd want a path towards resolving doc errors too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed -- it looks like we need to do some pattern matching to get them to pass. I will submit a new PR once I figure out how to do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, definitely don't worry about that for this PR, unless you know of something that will make that really hard. One of us can tackle that later.
@@ -34,7 +33,7 @@ We're going to set this up so that a new line is started when the user | |||
clicks with the left mouse button; when the user releases the mouse | |||
button, the line is finished and added to a list of previously-drawn | |||
lines. Consequently, we need a place to store user data. We'll use | |||
Signals, so that our Canvas will be notified when there is new | |||
Observables, so that our Canvas will be notified when there is new |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
|
||
# Re-exports | ||
export set_coordinates, BoundingBox, SHIFT, CONTROL, MOD1, UP, DOWN, LEFT, RIGHT, | ||
BUTTON_PRESS, DOUBLE_BUTTON_PRESS, destroy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the replacement for double-click?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In GTK4 it's one of the arguments in the signal handler.
step_back = button(; widget=builder["step_back$id"]::Gtk4.GtkButton) | ||
stop = button(; widget=builder["stop$id"]::Gtk4.GtkButton) | ||
step_forward = button(; widget=builder["step_forward$id"]::Gtk4.GtkButton) | ||
play_forward = button(; widget=builder["play_forward$id"]::Gtk4.GtkButton) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much nicer!
destroy(p.entry) | ||
destroy(p.scale) | ||
destroy(frame(p)) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I take it these are handled automatically now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, there's no destroy
for widgets now, just for windows. If you destroy windows typically all the widgets are destroyed too.
Ugh, I just tried capturing the modifier state in a way that could be tested and precompiled, only to run into a GTK4 bug: https://gitlab.gnome.org/GNOME/gtk/-/issues/5139 I didn't find any tests for this stuff inside the C library, but maybe I didn't look in the right place. I will keep looking but I'm nearing my limit. |
I think we can proceed here anyway if it's not testable and you think it's an improved experience. (I should say I haven't tested myself.) At some point, if there's a gtk dev forum maybe one of us could ask them how they do testing? |
In other words: if you're to the point of wanting to click merge, just do so. If you still want to play with it for a bit more, that's fine too. |
This introduces a way to override the modifier state for the purposes of testing. It's better than nothing.
OK, I got something to work -- it doesn't generate real GTK events but it exercises the Julia code. I just want to add precompiles using that too, then I'll merge. |
Hooray! Let's get this out there. I presume this should be 2.0.0? E.g., changes like |
Yup, this should definitely be 2.0! Go for it, otherwise I'll have time tomorrow evening (assuming I have that access level). |
You do, but I'll do it now. |
Thanks again, I'm moving my whole lab but when the dust settles I'm excited to start playing with this! |
This is a port to use Gtk4.jl instead of Gtk.jl.
The primary benefit of Gtk4.jl is that it's based on the most actively maintained version of GTK. My impression is that it works better on Windows and MacOS than version 3, but I should note that there are a few annoying bugs on Windows. Other advantages: Gtk4.jl doesn't cause horrendous REPL lag on Windows, it works well with PackageCompiler, at least in my limited experience, it allows GL-based rendering on all three platforms (including Makie via Gtk4Makie), and it supports the vast majority of widgets and methods in the underlying C libraries through GObject introspection.
I have been using Gtk4.jl in my own projects for a while now, as has @tknopp. Documentation needs work and that is a priority, but the current docs are about the same level as Gtk.jl.