Qt/Winit backend: Unified quit_event_loop() followed by run() behavior#11022
Qt/Winit backend: Unified quit_event_loop() followed by run() behavior#11022LeonMatthes wants to merge 5 commits intoslint-ui:masterfrom
quit_event_loop() followed by run() behavior#11022Conversation
ogoffart
left a comment
ogoffart
left a comment
There was a problem hiding this comment.
QCoreApplication::exit(0); is documented to not be thread safe. But the Proxy can be moved between threads (that's actually one of the point) and slint::quit_event_loop can be called from any thread.
LeonMatthes
force-pushed
the
lm/qt-backend-quit-event-loop
branch
from
14692e5 to
cf715c8
Compare
I actually cannot find that documentation but I've now moved it over to an event on the event loop with a generational index the same as with the winit backend. |
LeonMatthes
changed the title
quit_event_loop() followed by run() behavior
Before, this used a quit event which would call the on_close_requested handler which could prevent the quit. Note that simply storing a flag to prevent the on_close_requested callback and then accpeting the quit event did not work. In that case, all windows would be closed and not all windows would reopen if the event loop was restarted. This works correctly with the call to exit(), though. Closes slint-ui#10966.
Previously calling `quit_event_loop` twice caused the next call to `run_event_loop` to exit immediately. This was especially problematic because window hide calls `quit_event_loop` as well, which is called at the end of `run`.
LeonMatthes
force-pushed
the
lm/qt-backend-quit-event-loop
branch
from
cf715c8 to
5e27c06
Compare
ogoffart
left a comment
ogoffart
left a comment
There was a problem hiding this comment.
I actually cannot find that documentation
https://doc.qt.io/qt-6/qcoreapplication.html#exit
Note also that this function is not thread-safe. It should be called only from the main thread
For the test, i'm thinking it is a bit confusing to have internal/backends/testing and internal/backends/tests.
May I suggest moving the tests to tests/backends maybe? (no strong opinion)
|
|
||
| pub struct Backend; | ||
| pub struct Backend { | ||
| event_loop_generation: Arc<AtomicUsize>, |
There was a problem hiding this comment.
Could you add a documentation comment on what that it and why is this necessary?
| #[test] | ||
| fn multiple_quit_event_loop_calls() { | ||
| crate::init(); | ||
|
|
There was a problem hiding this comment.
I'm afraid this won't work well unfortunately. Because #[test] is run in a thread. and neither Qt nor winit support being run twice from different thread.
(This just works now because there is only one test)
2 participants
Before, this used a quit event which would call the on_close_requested
handler which could prevent the quit.
As an additional issue, the winit backend would exit immediately if
run()was called a second time.In both of these cases, a programmatic close needs to be handled differently than a normal close event and now uses a generational index to determine if a quit_event_loop call is stale/duplicate from a previous event loop run.
Closes #10966.