Implement timers for the X11 platform

[psychon]

This fixes half of #934, but does not deal with #934 (comment)

Timers are collected in a BinaryHeap per window, sorted by deadline. The main loop gets the first deadline of each window and uses the minimum of that for the timeout used in the call to poll. I am least comfortable with my changes to poll_with_timeout(). Hopefully, this function can be simplified a lot when someone (else) handles #934 (comment).

This was tested with cargo run --example invalidate --no-default-features --features x11 (and with nothing else). The result looks similar to the version without --features x11, which should be GTK. Before this PR, --features x11 lead to an empty window instead.

[jneem]

Indeed, I think the timeouts in poll_with_timeout aren't quite right. It looks good otherwise, though!

[luleyleo]

I did not yet have a look at the code, but I tried it with the timer example and noticed two problems:

• The square seems to move slower than in the GTK example
• The square accelerates when I move the cursor to roughly GTK speed

[psychon]

The square seems to move slower than in the GTK example

Yup. No idea yet what happens. I'll investigate.

To make this a bit less subjective, I did:

diff --git a/druid/examples/timer.rs b/druid/examples/timer.rs
index f31637a..5949fed 100644
--- a/druid/examples/timer.rs
+++ b/druid/examples/timer.rs
@@ -15,6 +15,7 @@
 //! An example of a timer.
 
 use std::time::Duration;
+use std::time::Instant;
 
 use druid::widget::prelude::*;
 use druid::widget::BackgroundBrush;
@@ -24,6 +25,7 @@ static TIMER_INTERVAL: Duration = Duration::from_millis(10);
 
 struct TimerWidget {
     timer_id: TimerToken,
+    timeout_requested: Instant,
     simple_box: WidgetPod<u32, SimpleBox>,
     pos: Point,
 }
@@ -49,12 +51,20 @@ impl Widget<u32> for TimerWidget {
         match event {
             Event::WindowConnected => {
                 // Start the timer when the application launches
+                self.timeout_requested = Instant::now();
                 self.timer_id = ctx.request_timer(TIMER_INTERVAL);
             }
             Event::Timer(id) => {
                 if *id == self.timer_id {
+                    let elapsed = self.timeout_requested.elapsed();
+                    if elapsed < TIMER_INTERVAL {
+                        println!("Timeout is {:?} early", TIMER_INTERVAL - elapsed);
+                    } else {
+                        println!("Timeout is {:?} late", elapsed - TIMER_INTERVAL);
+                    }
                     self.adjust_box_pos(ctx.size());
                     ctx.request_layout();
+                    self.timeout_requested = Instant::now();
                     self.timer_id = ctx.request_timer(TIMER_INTERVAL);
                 }
             }
@@ -119,6 +129,7 @@ impl Widget<u32> for SimpleBox {
 pub fn main() {
     let window = WindowDesc::new(|| TimerWidget {
         timer_id: TimerToken::INVALID,
+        timeout_requested: Instant::now(),
         simple_box: WidgetPod::new(SimpleBox),
         pos: Point::ZERO,
     })

Output is:
Gtk:

Timeout is 89.263µs early
Timeout is 563.821µs early
Timeout is 360.599µs early
Timeout is 338.149µs early
Timeout is 587.387µs early
Timeout is 77.686µs early
Timeout is 621.249µs early
Timeout is 234.995µs early
Timeout is 408.812µs early
Timeout is 41.96µs late
Timeout is 414.332µs early
Timeout is 748.723µs early
Timeout is 528.43µs early
Timeout is 780.295µs early

X11:

Timeout is 129.242µs late
Timeout is 1.397298ms late
Timeout is 1.619458ms late
Timeout is 6.540825ms late
Timeout is 6.784348ms late
Timeout is 164.414µs late
Timeout is 1.38442ms late
Timeout is 1.589043ms late
Timeout is 6.516359ms late
Timeout is 6.808551ms late
Timeout is 132.782µs late
Timeout is 1.398832ms late
Timeout is 1.599091ms late
Timeout is 6.582464ms late
Timeout is 6.730138ms late
Timeout is 131.859µs late

[psychon]

Something is being slow and I do not know what. I also added

diff --git a/druid-shell/src/platform/x11/application.rs b/druid-shell/src/platform/x11/application.rs
index 8d879eb..a1cc61f 100644
--- a/druid-shell/src/platform/x11/application.rs
+++ b/druid-shell/src/platform/x11/application.rs
@@ -569,6 +569,9 @@ fn poll_with_timeout(conn: &Rc<XCBConnection>, idle: RawFd, timer_timeout: Optio
                 .contains(PollFlags::POLLIN)
         };
 
+        let before = Instant::now();
+        let p = poll(poll_fds, poll_timeout);
+        println!("Poll for {} returned {:?} in time {:?}", poll_timeout, p, before.elapsed());
         match poll(poll_fds, poll_timeout) {
             Ok(_) => {
                 if readable(poll_fds[0]) {

Output is:

Poll for 9 returned Ok(0) in time 9.121401ms
Timeout is 6.617062ms late
Poll for 9 returned Ok(0) in time 9.117681ms
Timeout is 6.450287ms late
Poll for 9 returned Ok(0) in time 9.123917ms
Timeout is 6.895794ms late
Poll for 9 returned Ok(0) in time 9.124663ms
Timeout is 6.599921ms late
Poll for 9 returned Ok(0) in time 9.125751ms

The timer example uses a timeout of 10 ms. This ends up as a 9 ms timeout for poll() and poll() seems to be obey this request quite well. No idea where the other 6 ms are spent. The output above also suggests that there is exactly one call to poll() before the timer is run, so this is also not due to something "forgetting" to run the timeouts.

No idea what to make of this.

[psychon]

Found it. It's something like a rounding issue.

When the timer_timeout is 9.5 ms in the future, poll() is called with a timeout of 9 ms. It then wakes up before the timeout, thus poll() is called again. However, this new call still uses the old timeout.

Recomputing the timeout in each loop iteration "does the trick".

[psychon]

I guess it would be best if this would round up instead of round down (in as_millis()). Since that does not seem to be easily possible: How about adding a +1 to the result? That way, poll should sleep until "something interesting" happens. Otherwise, it is possible that Instant::now() is less than 1 ms before "something interesting" and the code here would busy loop with poll() with a timeout of zero...

[jneem]

I think adding 1 ms is a good approach.

[jneem]

I think adding 1 ms is a good approach.

[jneem]

Why not just break here?

[psychon]

A combination of "historic reasons" and "I do not follow understand the idle drawing code". The previous code definitely did not return early and might have went into a short busy loop. I think. I am not sure.

I'll just change it to a break (and also to <= instead of <, but that shouldn't make much of a difference).