Mouse handling optimization #5987
Conversation
|
Hmm, maybe the flaky tests are real this time? I don't see any races in a real application though |
|
Hmm, maybe. More likely a race in our test framework than your code, or rather this surfaces those issues more clearly. I think you best bet is looking at those failing tests and seeing if there is any code that runs directly in the test function and not on main. Remember, Go executes each test in its own goroutine so they are not running on the main thread by default like a regular app. |
internal/driver/glfw/window_test.go
Outdated
| ensureCanvasSize(t, w, fyne.NewSize(72, 123)) | ||
|
|
||
| w.mouseMoved(w.viewport, 10, float64(e.Position().Y+10)) | ||
| w.moveMouse(10, float64(e.Position().Y+10)) |
There was a problem hiding this comment.
I suppose lines like this may be the culprit.
There was a problem hiding this comment.
It seems that putting the necessary codeblocks in runOnMain is causing test runtime to exceed 10 minutes. If anyone has time to look into fixing the unit tests races I would be grateful. Otherwise, I'll keep poking at it
There was a problem hiding this comment.
Exceeding the deadline normally means a deadlock. Which indicates that the runOnMain was calling something else that is runOnMain inside it. Note that the "safeCanvas" type and others are attempting to automatically wrap so it's not always needed where it appears it might be.
Hopefully that is a clue. I might get a chance to look tomorrow.
3 participants
Description:
Since glfw gives us mouse position every time the callback is called, we can just save the position, and only call
processMouseMovedright before we draw each frame. This can make a huge difference when mouse movement events are coming in at a faster rate than the frame rate. Same optimization for window resize events.There might be other input events where a similar optimization is possible, this is just a start.
Fixes #5815, #3374 possibly #1114?.
Also drastically improves resizing for https://github.com/fyne-io/solitaire :)
Note: Tests for
mouseMovedhad to be updated, since they expectedprocessMouseMovedto be called internally. I added a helper method for tests that fulfills that assumption.Checklist: