nandi/jolt-native · a9f3673

⑂ a9f3673

Spend the frame's idle time watching for the resize, not asleep

Dragging a window's edge left the picture behind the edge. Part of that
is XWayland and stays; part of it was ours.

`end_frame` finished by sleeping out whatever was left of the frame
budget. The event loop is not pumped during that sleep, so a resize
arriving 1ms into it was not *seen* for another 15, and the frame after
that laid itself out against a window it had already outgrown. The same
idle time is now spent pumping in 2ms slices, abandoned the moment the
window stops matching the frame just presented, so the next frame starts
against the size the window actually is. Measured on a synthetic resize
storm the pacing is unchanged — 16.8ms median, 60fps — for about 2% of a
core more, and the retry path in `vidya_tree_frame` was already sound:
zero frames presented at a stale size.

Native Wayland would remove the rest of the lag by removing XWayland from
under it, and does not work. Presenting from inside winit's own
`RedrawRequested` dispatch — which the docs here described and the code
did not do — is not enough: traced, the surface receives exactly one
callback, at configure, and every later `request_redraw` goes unanswered
until `PRESENT_TIMEOUT`, 253ms a frame with nothing on the screen. The
callbacks do not survive a loop the caller owns, and owning it is what
this ABI is. So X11 stays, and the comment now says what was measured
rather than what was feared.

Two bugs found while looking, neither of them about resizing:

The fallback to Wayland could never have run. winit marks the process as
having built an event loop even when the build *fails*, so the second
attempt answered "EventLoop can't be recreated" — a session with no
XWayland got no window at all, after printing a message promising one.
The backend is chosen from the environment now and built once, with
`VIDYA_BACKEND` to override.

Closing a Wayland window aborted the process. `App`'s fields drop in
declaration order, so the event loop went first and took the display
connection with it, while egui-winit's clipboard — a smithay-clipboard
thread holding its own proxies — was still alive to destroy them against
nothing. The window's half comes down first now.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

nandi committed 2026-09-01T21:55:29-07:00 Browse files

a9f3673 parent: 757d8e7

modified crates/jolt-vidya/src/app.rs +75 -17 Viewed

@@ -47,26 +47,47 @@ const PRESENT_TIMEOUT: Duration = Duration::from_millis(250);
47	47	/// Pacing floor, so a caller that never sets a target FPS still yields.
48	48	const DEFAULT_FRAME_BUDGET: Duration = Duration::from_micros(16_666);
49	49
50		-/// Build the event loop, preferring X11 where both backends exist.
	50	+/// Build the event loop: X11 (XWayland included) unless asked otherwise.
51	51	///
52		-/// Native Wayland does not survive this ABI's shape: the caller owns the loop,
	52	+/// Native Wayland does not survive this ABI's shape. The caller owns the loop,
53	53	/// so winit is driven with `pump_app_events`, and a surface driven that way
54		-/// stops receiving frame callbacks after its first commit — the window never
55		-/// gets a second frame and the session stalls behind it. Under X11 (XWayland
56		-/// included) presentation does not depend on those callbacks, and the same loop
57		-/// runs at full frame rate. Falls back to the default backend when X11 is
58		-/// unavailable, so a compositor without XWayland still gets a window.
	54	+/// receives exactly one frame callback: the configure. Presenting inside
	55	+/// winit's own `RedrawRequested` dispatch is not enough to keep them coming —
	56	+/// measured, every later `request_redraw` goes unanswered and each frame waits
	57	+/// out `PRESENT_TIMEOUT` instead. Under X11 presentation depends on no such
	58	+/// callback, and the same loop runs at the display's rate.
	59	+///
	60	+/// There is exactly one attempt to spend, which is why this chooses rather than
	61	+/// tries: winit marks the process as having built an event loop even when the
	62	+/// build fails, so the old try-X11-then-fall-back arrangement could not fall
	63	+/// back — the second build answered "EventLoop can't be recreated", and a
	64	+/// session with no XWayland got no window at all rather than the Wayland one it
	65	+/// was promised.
	66	+///
	67	+/// `VIDYA_BACKEND=wayland` asks for the native surface anyway, for whoever is
	68	+/// fixing the above; `x11` forces X11 where `WAYLAND_DISPLAY` is set but
	69	+/// `DISPLAY` is what works.
59	70	#[cfg(all(unix, not(target_os = "macos"), not(target_os = "android")))]
60	71	fn build_event_loop() -> Result<EventLoop<()>, String> {
	72	+ use winit::platform::wayland::EventLoopBuilderExtWayland as _;
61	73	use winit::platform::x11::EventLoopBuilderExtX11 as _;
62	74
63		- if let Ok(el) = EventLoop::builder().with_x11().build() {
64		- return Ok(el);
65		- }
66		- eprintln!("vidya: X11 unavailable; falling back to Wayland (expect stalls)");
67		- EventLoop::builder()
68		- .build()
69		- .map_err(\|e\| format!("event loop: {e}"))
	75	+ let wayland = match std::env::var("VIDYA_BACKEND").unwrap_or_default().as_str() {
	76	+ "wayland" => true,
	77	+ // X11 unless there is no X server to reach at all, in which case a
	78	+ // window that stalls beats no window.
	79	+ _ => std::env::var_os("DISPLAY").is_none(),
	80	+ };
	81	+
	82	+ let built = if wayland {
	83	+ EventLoop::builder().with_wayland().build()
	84	+ } else {
	85	+ EventLoop::builder().with_x11().build()
	86	+ };
	87	+ built.map_err(\|e\| {
	88	+ let backend = if wayland { "wayland" } else { "x11" };
	89	+ format!("event loop ({backend}): {e} — VIDYA_BACKEND selects the other")
	90	+ })
70	91	}
71	92
72	93	/// Android has no display connection to choose: the activity already owns one,
@@ -730,6 +751,17 @@ impl App {
730	751	self.handler.slot_waited = true;
731	752	}
732	753
	754	+ /// Whether the window is no longer the size the last pass was measured
	755	+ /// for. Unlike [`Self::resized_mid_frame`] it asks nothing of the open
	756	+ /// pass, so it also answers between frames.
	757	+ fn window_outgrew_frame(&self) -> bool {
	758	+ let Some(gl) = self.handler.gl.as_ref() else {
	759	+ return false;
	760	+ };
	761	+ let size = gl.window.inner_size();
	762	+ [size.width.max(1), size.height.max(1)] != self.handler.frame_dims
	763	+ }
	764	+
733	765	/// Whether the window changed size after this frame started laying out.
734	766	///
735	767	/// True means the pass now open measured a window that no longer exists,
@@ -818,19 +850,45 @@ impl App {
818	850	}
819	851	}
820	852
821		- // Floor for platforms that do not throttle presents at all.
822		- if let Some(left) = self.frame_budget.checked_sub(self.frame_started.elapsed()) {
823		- std::thread::sleep(left);
	853	+ // Floor for platforms that do not throttle presents at all — spent
	854	+ // pumping rather than asleep, and abandoned the moment the window
	855	+ // changes size.
	856	+ //
	857	+ // A drag's next size lands in exactly this window. Sleeping through it
	858	+ // means the resize is not even seen until the sleep ends, and the
	859	+ // frame after that paints a layout the window outgrew a whole budget
	860	+ // ago — the edge running ahead of the picture. Waiting on the event
	861	+ // loop instead makes the same idle time responsive: the resize wakes
	862	+ // it, and the next frame is laid out against the size the window
	863	+ // already is.
	864	+ while let Some(left) = self.frame_budget.checked_sub(self.frame_started.elapsed()) {
	865	+ if left.is_zero() {
	866	+ break;
	867	+ }
	868	+ self.pump(left.min(Duration::from_millis(2)));
	869	+ if self.window_outgrew_frame() {
	870	+ break;
	871	+ }
824	872	}
825	873	}
826	874	}
827	875
828	876	impl Drop for App {
	877	+ /// Order matters, and the field order would get it exactly backwards.
	878	+ ///
	879	+ /// `event_loop` is declared first, so it is dropped first — taking the
	880	+ /// display connection with it while `Gl` still holds what was built on
	881	+ /// top. Under Wayland that is fatal rather than untidy: egui-winit's
	882	+ /// clipboard is a smithay-clipboard thread holding its own proxies, and
	883	+ /// destroying them against a connection that has already gone aborts the
	884	+ /// process inside libwayland. So the window's half comes down here, in
	885	+ /// full, before anything else is allowed to.
829	886	fn drop(&mut self) {
830	887	self.stack.unwind();
831	888	if let Some(gl) = self.handler.gl.as_mut() {
832	889	gl.painter.destroy();
833	890	}
	891	+ drop(self.handler.gl.take());
834	892	}
835	893	}
836	894

  /// Pacing floor, so a caller that never sets a target FPS still yields.
  const DEFAULT_FRAME_BUDGET: Duration = Duration::from_micros(16_666);
--/// Build the event loop, preferring X11 where both backends exist.
++/// Build the event loop: X11 (XWayland included) unless asked otherwise.
  ///
--/// Native Wayland does not survive this ABI's shape: the caller owns the loop,
++/// Native Wayland does not survive this ABI's shape. The caller owns the loop,
  /// so winit is driven with `pump_app_events`, and a surface driven that way
--/// stops receiving frame callbacks after its first commit — the window never
++/// receives exactly one frame callback: the configure. Presenting inside
--/// gets a second frame and the session stalls behind it. Under X11 (XWayland
++/// winit's own `RedrawRequested` dispatch is not enough to keep them coming —
--/// included) presentation does not depend on those callbacks, and the same loop
++/// measured, every later `request_redraw` goes unanswered and each frame waits
--/// runs at full frame rate. Falls back to the default backend when X11 is
++/// out `PRESENT_TIMEOUT` instead. Under X11 presentation depends on no such
--/// unavailable, so a compositor without XWayland still gets a window.
++/// callback, and the same loop runs at the display's rate.
++///
++/// There is exactly one attempt to spend, which is why this chooses rather than
++/// tries: winit marks the process as having built an event loop even when the
++/// build *fails*, so the old try-X11-then-fall-back arrangement could not fall
++/// back — the second build answered "EventLoop can't be recreated", and a
++/// session with no XWayland got no window at all rather than the Wayland one it
++/// was promised.
++///
++/// `VIDYA_BACKEND=wayland` asks for the native surface anyway, for whoever is
++/// fixing the above; `x11` forces X11 where `WAYLAND_DISPLAY` is set but
++/// `DISPLAY` is what works.
  #[cfg(all(unix, not(target_os = "macos"), not(target_os = "android")))]
  fn build_event_loop() -> Result<EventLoop<()>, String> {
++    use winit::platform::wayland::EventLoopBuilderExtWayland as _;
      use winit::platform::x11::EventLoopBuilderExtX11 as _;
--    if let Ok(el) = EventLoop::builder().with_x11().build() {
++    let wayland = match std::env::var("VIDYA_BACKEND").unwrap_or_default().as_str() {
--        return Ok(el);
++        "wayland" => true,
--    }
++        // X11 unless there is no X server to reach at all, in which case a
--    eprintln!("vidya: X11 unavailable; falling back to Wayland (expect stalls)");
++        // window that stalls beats no window.
--    EventLoop::builder()
++        _ => std::env::var_os("DISPLAY").is_none(),
--        .build()
++    };
--        .map_err(|e| format!("event loop: {e}"))
++
++    let built = if wayland {
++        EventLoop::builder().with_wayland().build()
++    } else {
++        EventLoop::builder().with_x11().build()
++    };
++    built.map_err(|e| {
++        let backend = if wayland { "wayland" } else { "x11" };
++        format!("event loop ({backend}): {e} — VIDYA_BACKEND selects the other")
++    })
  }
  /// Android has no display connection to choose: the activity already owns one,
          self.handler.slot_waited = true;
      }
++    /// Whether the window is no longer the size the last pass was measured
++    /// for. Unlike [`Self::resized_mid_frame`] it asks nothing of the open
++    /// pass, so it also answers between frames.
++    fn window_outgrew_frame(&self) -> bool {
++        let Some(gl) = self.handler.gl.as_ref() else {
++            return false;
++        };
++        let size = gl.window.inner_size();
++        [size.width.max(1), size.height.max(1)] != self.handler.frame_dims
++    }
++
      /// Whether the window changed size after this frame started laying out.
      ///
      /// True means the pass now open measured a window that no longer exists,
              }
          }
--        // Floor for platforms that do not throttle presents at all.
++        // Floor for platforms that do not throttle presents at all — spent
--        if let Some(left) = self.frame_budget.checked_sub(self.frame_started.elapsed()) {
++        // pumping rather than asleep, and abandoned the moment the window
--            std::thread::sleep(left);
++        // changes size.
++        //
++        // A drag's next size lands in exactly this window. Sleeping through it
++        // means the resize is not even *seen* until the sleep ends, and the
++        // frame after that paints a layout the window outgrew a whole budget
++        // ago — the edge running ahead of the picture. Waiting on the event
++        // loop instead makes the same idle time responsive: the resize wakes
++        // it, and the next frame is laid out against the size the window
++        // already is.
++        while let Some(left) = self.frame_budget.checked_sub(self.frame_started.elapsed()) {
++            if left.is_zero() {
++                break;
++            }
++            self.pump(left.min(Duration::from_millis(2)));
++            if self.window_outgrew_frame() {
++                break;
++            }
          }
      }
  }
  impl Drop for App {
++    /// Order matters, and the field order would get it exactly backwards.
++    ///
++    /// `event_loop` is declared first, so it is dropped first — taking the
++    /// display connection with it while `Gl` still holds what was built on
++    /// top. Under Wayland that is fatal rather than untidy: egui-winit's
++    /// clipboard is a smithay-clipboard thread holding its own proxies, and
++    /// destroying them against a connection that has already gone aborts the
++    /// process inside libwayland. So the window's half comes down here, in
++    /// full, before anything else is allowed to.
      fn drop(&mut self) {
          self.stack.unwind();
          if let Some(gl) = self.handler.gl.as_mut() {
              gl.painter.destroy();
          }
++        drop(self.handler.gl.take());
      }
  }