Ask about the size only once the answer can have changed
A drag's resizes arrive while we wait for the compositor, so a frame that checked its layout before that wait was asking about a window nobody had told to move yet, and went out measured for a size the buffer no longer was. Split the wait out of `end_frame` so the tree can take it before it asks, and remember it happened: pumping a second time between the check and the present would let one more resize through, into the gap the check just proved empty. The synthetic resize moves with it, from after the paint to after the frame's input is taken — where a real drag lands, mid-walk, rather than between frames, which was never the case that broke. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
69b5de4 parent: b1758f5 modified
crates/jolt-vidya/src/app.rs +75 -19 | @@ -288,6 +288,14 @@ struct Handler { | ||
| 288 | 288 | /// that arms every later one is only requested *by* presenting — waiting |
| 289 | 289 | /// for it before the first present deadlocks. |
| 290 | 290 | may_present: bool, |
| 291 | + /// Whether the wait for the compositor already happened this frame. | |
| 292 | + /// | |
| 293 | + /// `end_frame` waits unless it did. That matters because the wait is where | |
| 294 | + /// a drag's resizes arrive, and a caller that has already waited has also | |
| 295 | + /// already checked its layout against the result — pumping again between | |
| 296 | + /// that check and the present would let one more resize through, into the | |
| 297 | + /// gap the check just proved empty. | |
| 298 | + slot_waited: bool, | |
| 291 | 299 | frames: u32, |
| 292 | 300 | /// The size, in pixels, the frame now being built laid itself out against. |
| 293 | 301 | /// |
| @@ -326,6 +334,17 @@ impl Handler { | ||
| 326 | 334 | let size = gl.window.inner_size(); |
| 327 | 335 | let dims = [size.width.max(1), size.height.max(1)]; |
| 328 | 336 | |
| 337 | + if std::env::var_os("VIDYA_DEBUG_RESIZE").is_some() { | |
| 338 | + let r = self.egui_ctx.screen_rect(); | |
| 339 | + eprintln!( | |
| 340 | + "frame {} buffer={}x{} laid_out={}x{}", | |
| 341 | + self.frames + 1, | |
| 342 | + dims[0], | |
| 343 | + dims[1], | |
| 344 | + (r.width() * job.pixels_per_point).round() as u32, | |
| 345 | + (r.height() * job.pixels_per_point).round() as u32 | |
| 346 | + ); | |
| 347 | + } | |
| 329 | 348 | gl.painter.clear(dims, job.clear); |
| 330 | 349 | gl.painter.paint_and_update_textures( |
| 331 | 350 | dims, |
| @@ -335,17 +354,6 @@ impl Handler { | ||
| 335 | 354 | ); |
| 336 | 355 | |
| 337 | 356 | self.frames += 1; |
| 338 | - // A resize the compositor never sent, for testing a layout that only | |
| 339 | - // goes wrong when the window changes size under it. `VIDYA_RESIZE_AT` | |
| 340 | - // is `frame:WIDTHxHEIGHT`, and asks winit for the size the way a drag | |
| 341 | - // of the window's edge would. | |
| 342 | - for &(at, w, h) in &self.resize_at { | |
| 343 | - if self.frames == at { | |
| 344 | - let _ = gl | |
| 345 | - .window | |
| 346 | - .request_inner_size(winit::dpi::LogicalSize::new(w, h)); | |
| 347 | - } | |
| 348 | - } | |
| 349 | 357 | // Third frame: fonts and layout have settled by then. |
| 350 | 358 | // A click the desktop never sent: `VIDYA_CLICK_AT=frame:x,y` presses and |
| 351 | 359 | // releases the left button at a point, so an interaction can be tested |
| @@ -520,6 +528,7 @@ impl App { | ||
| 520 | 528 | should_close: false, |
| 521 | 529 | error: None, |
| 522 | 530 | may_present: true, |
| 531 | + slot_waited: false, | |
| 523 | 532 | frames: 0, |
| 524 | 533 | frame_dims: [0, 0], |
| 525 | 534 | capture: std::env::var("VIDYA_CAPTURE").ok(), |
| @@ -658,6 +667,9 @@ impl App { | ||
| 658 | 667 | pub fn begin_frame(&mut self) { |
| 659 | 668 | self.pump(Duration::ZERO); |
| 660 | 669 | self.frame_started = Instant::now(); |
| 670 | + // Per-frame, and cleared here rather than after a present: a frame | |
| 671 | + // dropped because nobody was compositing never reaches one. | |
| 672 | + self.handler.slot_waited = false; | |
| 661 | 673 | |
| 662 | 674 | if self.handler.gl.is_none() || self.stack.is_active() { |
| 663 | 675 | // No window, or the caller skipped `vidya_end_frame`. |
| @@ -673,12 +685,49 @@ impl App { | ||
| 673 | 685 | // has finished walking its tree. |
| 674 | 686 | let size = gl.window.inner_size(); |
| 675 | 687 | let dims = [size.width.max(1), size.height.max(1)]; |
| 688 | + // A resize the compositor never sent, for testing a layout that only | |
| 689 | + // goes wrong when the window changes size under it. `VIDYA_RESIZE_AT` | |
| 690 | + // is `frame:WIDTHxHEIGHT`, and asks winit for the size the way a drag | |
| 691 | + // of the window's edge would. | |
| 692 | + // | |
| 693 | + // Asked for here, after the input for this frame has been taken, so it | |
| 694 | + // lands where a drag actually lands it: in the middle of the walk, | |
| 695 | + // with the pass already measured against the old size. Requesting it | |
| 696 | + // from `present` instead would only ever resize between frames, which | |
| 697 | + // is the one case that was never a problem. | |
| 698 | + let building = self.handler.frames + 1; | |
| 699 | + for &(at, w, h) in &self.handler.resize_at { | |
| 700 | + if building == at { | |
| 701 | + let _ = gl | |
| 702 | + .window | |
| 703 | + .request_inner_size(winit::dpi::LogicalSize::new(w, h)); | |
| 704 | + } | |
| 705 | + } | |
| 706 | + | |
| 676 | 707 | let ctx = &self.handler.egui_ctx; |
| 677 | 708 | ctx.begin_pass(input); |
| 678 | 709 | self.stack.push_root(ctx); |
| 679 | 710 | self.handler.frame_dims = dims; |
| 680 | 711 | } |
| 681 | 712 | |
| 713 | + /// Wait for the compositor to want a frame, dispatching events while we | |
| 714 | + /// wait — never inside `swap_buffers`, which would park the thread that | |
| 715 | + /// owes the compositor its replies and stall the session. This wait is | |
| 716 | + /// also what paces the caller's loop to the display. | |
| 717 | + /// | |
| 718 | + /// Callable before the pass is closed as well as from `end_frame`, and the | |
| 719 | + /// second call is free. That is the point: the resizes a drag produces | |
| 720 | + /// arrive *in here*, so a caller that wants to know whether its layout is | |
| 721 | + /// still the right size has to wait first and ask afterwards. Asking | |
| 722 | + /// before the wait sees a window that has not been told to move yet. | |
| 723 | + pub fn await_present_slot(&mut self) { | |
| 724 | + let deadline = Instant::now() + PRESENT_TIMEOUT; | |
| 725 | + while !self.handler.may_present && Instant::now() < deadline { | |
| 726 | + self.pump(Duration::from_millis(2)); | |
| 727 | + } | |
| 728 | + self.handler.slot_waited = true; | |
| 729 | + } | |
| 730 | + | |
| 682 | 731 | /// Whether the window changed size after this frame started laying out. |
| 683 | 732 | /// |
| 684 | 733 | /// True means the pass now open measured a window that no longer exists, |
| @@ -695,7 +744,19 @@ impl App { | ||
| 695 | 744 | return false; |
| 696 | 745 | }; |
| 697 | 746 | let size = gl.window.inner_size(); |
| 698 | - [size.width.max(1), size.height.max(1)] != self.handler.frame_dims | |
| 747 | + let now = [size.width.max(1), size.height.max(1)]; | |
| 748 | + if std::env::var_os("VIDYA_DEBUG_RESIZE").is_some() { | |
| 749 | + eprintln!( | |
| 750 | + " check frames={} now={}x{} frame_dims={}x{} -> {}", | |
| 751 | + self.handler.frames, | |
| 752 | + now[0], | |
| 753 | + now[1], | |
| 754 | + self.handler.frame_dims[0], | |
| 755 | + self.handler.frame_dims[1], | |
| 756 | + now != self.handler.frame_dims | |
| 757 | + ); | |
| 758 | + } | |
| 759 | + now != self.handler.frame_dims | |
| 699 | 760 | } |
| 700 | 761 | |
| 701 | 762 | /// Close the open pass and throw its output away, presenting nothing. |
| @@ -746,13 +807,8 @@ impl App { | ||
| 746 | 807 | .handle_platform_output(&gl.window, platform_output); |
| 747 | 808 | } |
| 748 | 809 | |
| 749 | - // Wait for the compositor to want a frame, dispatching events while we | |
| 750 | - // wait — never inside `swap_buffers`, which would park the thread that | |
| 751 | - // owes the compositor its replies and stall the session. This wait is | |
| 752 | - // also what paces the caller's loop to the display. | |
| 753 | - let deadline = Instant::now() + PRESENT_TIMEOUT; | |
| 754 | - while !self.handler.may_present && Instant::now() < deadline { | |
| 755 | - self.pump(Duration::from_millis(2)); | |
| 810 | + if !self.handler.slot_waited { | |
| 811 | + self.await_present_slot(); | |
| 756 | 812 | } |
| 757 | 813 | |
| 758 | 814 | if self.handler.may_present { |
| @@ -288,6 +288,14 @@ struct Handler { | |||
| 288 | /// that arms every later one is only requested *by* presenting — waiting | 288 | /// that arms every later one is only requested *by* presenting — waiting |
| 289 | /// for it before the first present deadlocks. | 289 | /// for it before the first present deadlocks. |
| 290 | may_present: bool, | 290 | may_present: bool, |
| 291 | + /// Whether the wait for the compositor already happened this frame. | ||
| 292 | + /// | ||
| 293 | + /// `end_frame` waits unless it did. That matters because the wait is where | ||
| 294 | + /// a drag's resizes arrive, and a caller that has already waited has also | ||
| 295 | + /// already checked its layout against the result — pumping again between | ||
| 296 | + /// that check and the present would let one more resize through, into the | ||
| 297 | + /// gap the check just proved empty. | ||
| 298 | + slot_waited: bool, | ||
| 291 | frames: u32, | 299 | frames: u32, |
| 292 | /// The size, in pixels, the frame now being built laid itself out against. | 300 | /// The size, in pixels, the frame now being built laid itself out against. |
| 293 | /// | 301 | /// |
| @@ -326,6 +334,17 @@ impl Handler { | |||
| 326 | let size = gl.window.inner_size(); | 334 | let size = gl.window.inner_size(); |
| 327 | let dims = [size.width.max(1), size.height.max(1)]; | 335 | let dims = [size.width.max(1), size.height.max(1)]; |
| 328 | 336 | ||
| 337 | + if std::env::var_os("VIDYA_DEBUG_RESIZE").is_some() { | ||
| 338 | + let r = self.egui_ctx.screen_rect(); | ||
| 339 | + eprintln!( | ||
| 340 | + "frame {} buffer={}x{} laid_out={}x{}", | ||
| 341 | + self.frames + 1, | ||
| 342 | + dims[0], | ||
| 343 | + dims[1], | ||
| 344 | + (r.width() * job.pixels_per_point).round() as u32, | ||
| 345 | + (r.height() * job.pixels_per_point).round() as u32 | ||
| 346 | + ); | ||
| 347 | + } | ||
| 329 | gl.painter.clear(dims, job.clear); | 348 | gl.painter.clear(dims, job.clear); |
| 330 | gl.painter.paint_and_update_textures( | 349 | gl.painter.paint_and_update_textures( |
| 331 | dims, | 350 | dims, |
| @@ -335,17 +354,6 @@ impl Handler { | |||
| 335 | ); | 354 | ); |
| 336 | 355 | ||
| 337 | self.frames += 1; | 356 | self.frames += 1; |
| 338 | - // A resize the compositor never sent, for testing a layout that only | ||
| 339 | - // goes wrong when the window changes size under it. `VIDYA_RESIZE_AT` | ||
| 340 | - // is `frame:WIDTHxHEIGHT`, and asks winit for the size the way a drag | ||
| 341 | - // of the window's edge would. | ||
| 342 | - for &(at, w, h) in &self.resize_at { | ||
| 343 | - if self.frames == at { | ||
| 344 | - let _ = gl | ||
| 345 | - .window | ||
| 346 | - .request_inner_size(winit::dpi::LogicalSize::new(w, h)); | ||
| 347 | - } | ||
| 348 | - } | ||
| 349 | // Third frame: fonts and layout have settled by then. | 357 | // Third frame: fonts and layout have settled by then. |
| 350 | // A click the desktop never sent: `VIDYA_CLICK_AT=frame:x,y` presses and | 358 | // A click the desktop never sent: `VIDYA_CLICK_AT=frame:x,y` presses and |
| 351 | // releases the left button at a point, so an interaction can be tested | 359 | // releases the left button at a point, so an interaction can be tested |
| @@ -520,6 +528,7 @@ impl App { | |||
| 520 | should_close: false, | 528 | should_close: false, |
| 521 | error: None, | 529 | error: None, |
| 522 | may_present: true, | 530 | may_present: true, |
| 531 | + slot_waited: false, | ||
| 523 | frames: 0, | 532 | frames: 0, |
| 524 | frame_dims: [0, 0], | 533 | frame_dims: [0, 0], |
| 525 | capture: std::env::var("VIDYA_CAPTURE").ok(), | 534 | capture: std::env::var("VIDYA_CAPTURE").ok(), |
| @@ -658,6 +667,9 @@ impl App { | |||
| 658 | pub fn begin_frame(&mut self) { | 667 | pub fn begin_frame(&mut self) { |
| 659 | self.pump(Duration::ZERO); | 668 | self.pump(Duration::ZERO); |
| 660 | self.frame_started = Instant::now(); | 669 | self.frame_started = Instant::now(); |
| 670 | + // Per-frame, and cleared here rather than after a present: a frame | ||
| 671 | + // dropped because nobody was compositing never reaches one. | ||
| 672 | + self.handler.slot_waited = false; | ||
| 661 | 673 | ||
| 662 | if self.handler.gl.is_none() || self.stack.is_active() { | 674 | if self.handler.gl.is_none() || self.stack.is_active() { |
| 663 | // No window, or the caller skipped `vidya_end_frame`. | 675 | // No window, or the caller skipped `vidya_end_frame`. |
| @@ -673,12 +685,49 @@ impl App { | |||
| 673 | // has finished walking its tree. | 685 | // has finished walking its tree. |
| 674 | let size = gl.window.inner_size(); | 686 | let size = gl.window.inner_size(); |
| 675 | let dims = [size.width.max(1), size.height.max(1)]; | 687 | let dims = [size.width.max(1), size.height.max(1)]; |
| 688 | + // A resize the compositor never sent, for testing a layout that only | ||
| 689 | + // goes wrong when the window changes size under it. `VIDYA_RESIZE_AT` | ||
| 690 | + // is `frame:WIDTHxHEIGHT`, and asks winit for the size the way a drag | ||
| 691 | + // of the window's edge would. | ||
| 692 | + // | ||
| 693 | + // Asked for here, after the input for this frame has been taken, so it | ||
| 694 | + // lands where a drag actually lands it: in the middle of the walk, | ||
| 695 | + // with the pass already measured against the old size. Requesting it | ||
| 696 | + // from `present` instead would only ever resize between frames, which | ||
| 697 | + // is the one case that was never a problem. | ||
| 698 | + let building = self.handler.frames + 1; | ||
| 699 | + for &(at, w, h) in &self.handler.resize_at { | ||
| 700 | + if building == at { | ||
| 701 | + let _ = gl | ||
| 702 | + .window | ||
| 703 | + .request_inner_size(winit::dpi::LogicalSize::new(w, h)); | ||
| 704 | + } | ||
| 705 | + } | ||
| 706 | + | ||
| 676 | let ctx = &self.handler.egui_ctx; | 707 | let ctx = &self.handler.egui_ctx; |
| 677 | ctx.begin_pass(input); | 708 | ctx.begin_pass(input); |
| 678 | self.stack.push_root(ctx); | 709 | self.stack.push_root(ctx); |
| 679 | self.handler.frame_dims = dims; | 710 | self.handler.frame_dims = dims; |
| 680 | } | 711 | } |
| 681 | 712 | ||
| 713 | + /// Wait for the compositor to want a frame, dispatching events while we | ||
| 714 | + /// wait — never inside `swap_buffers`, which would park the thread that | ||
| 715 | + /// owes the compositor its replies and stall the session. This wait is | ||
| 716 | + /// also what paces the caller's loop to the display. | ||
| 717 | + /// | ||
| 718 | + /// Callable before the pass is closed as well as from `end_frame`, and the | ||
| 719 | + /// second call is free. That is the point: the resizes a drag produces | ||
| 720 | + /// arrive *in here*, so a caller that wants to know whether its layout is | ||
| 721 | + /// still the right size has to wait first and ask afterwards. Asking | ||
| 722 | + /// before the wait sees a window that has not been told to move yet. | ||
| 723 | + pub fn await_present_slot(&mut self) { | ||
| 724 | + let deadline = Instant::now() + PRESENT_TIMEOUT; | ||
| 725 | + while !self.handler.may_present && Instant::now() < deadline { | ||
| 726 | + self.pump(Duration::from_millis(2)); | ||
| 727 | + } | ||
| 728 | + self.handler.slot_waited = true; | ||
| 729 | + } | ||
| 730 | + | ||
| 682 | /// Whether the window changed size after this frame started laying out. | 731 | /// Whether the window changed size after this frame started laying out. |
| 683 | /// | 732 | /// |
| 684 | /// True means the pass now open measured a window that no longer exists, | 733 | /// True means the pass now open measured a window that no longer exists, |
| @@ -695,7 +744,19 @@ impl App { | |||
| 695 | return false; | 744 | return false; |
| 696 | }; | 745 | }; |
| 697 | let size = gl.window.inner_size(); | 746 | let size = gl.window.inner_size(); |
| 698 | - [size.width.max(1), size.height.max(1)] != self.handler.frame_dims | 747 | + let now = [size.width.max(1), size.height.max(1)]; |
| 748 | + if std::env::var_os("VIDYA_DEBUG_RESIZE").is_some() { | ||
| 749 | + eprintln!( | ||
| 750 | + " check frames={} now={}x{} frame_dims={}x{} -> {}", | ||
| 751 | + self.handler.frames, | ||
| 752 | + now[0], | ||
| 753 | + now[1], | ||
| 754 | + self.handler.frame_dims[0], | ||
| 755 | + self.handler.frame_dims[1], | ||
| 756 | + now != self.handler.frame_dims | ||
| 757 | + ); | ||
| 758 | + } | ||
| 759 | + now != self.handler.frame_dims | ||
| 699 | } | 760 | } |
| 700 | 761 | ||
| 701 | /// Close the open pass and throw its output away, presenting nothing. | 762 | /// Close the open pass and throw its output away, presenting nothing. |
| @@ -746,13 +807,8 @@ impl App { | |||
| 746 | .handle_platform_output(&gl.window, platform_output); | 807 | .handle_platform_output(&gl.window, platform_output); |
| 747 | } | 808 | } |
| 748 | 809 | ||
| 749 | - // Wait for the compositor to want a frame, dispatching events while we | 810 | + if !self.handler.slot_waited { |
| 750 | - // wait — never inside `swap_buffers`, which would park the thread that | 811 | + self.await_present_slot(); |
| 751 | - // owes the compositor its replies and stall the session. This wait is | ||
| 752 | - // also what paces the caller's loop to the display. | ||
| 753 | - let deadline = Instant::now() + PRESENT_TIMEOUT; | ||
| 754 | - while !self.handler.may_present && Instant::now() < deadline { | ||
| 755 | - self.pump(Duration::from_millis(2)); | ||
| 756 | } | 812 | } |
| 757 | 813 | ||
| 758 | if self.handler.may_present { | 814 | if self.handler.may_present { |
modified
crates/jolt-vidya/src/lib.rs +6 -0 | @@ -551,6 +551,12 @@ pub extern "C" fn vidya_tree_frame() { | ||
| 551 | 551 | tree.paint(ui, theme); |
| 552 | 552 | } |
| 553 | 553 | }); |
| 554 | + // Wait for the compositor's go-ahead before asking whether the | |
| 555 | + // window moved. The resizes a drag produces arrive during that | |
| 556 | + // wait, so asking first would answer about a window that has not | |
| 557 | + // been told to change yet — and the frame would go out measured | |
| 558 | + // for a size the buffer no longer is. | |
| 559 | + app.await_present_slot(); | |
| 554 | 560 | // The last attempt keeps whatever it measured. Discarding here |
| 555 | 561 | // instead would leave no open pass for `end_frame` to present, and |
| 556 | 562 | // a drag long enough to exhaust the retries would stop painting |
| @@ -551,6 +551,12 @@ pub extern "C" fn vidya_tree_frame() { | |||
| 551 | tree.paint(ui, theme); | 551 | tree.paint(ui, theme); |
| 552 | } | 552 | } |
| 553 | }); | 553 | }); |
| 554 | + // Wait for the compositor's go-ahead before asking whether the | ||
| 555 | + // window moved. The resizes a drag produces arrive during that | ||
| 556 | + // wait, so asking first would answer about a window that has not | ||
| 557 | + // been told to change yet — and the frame would go out measured | ||
| 558 | + // for a size the buffer no longer is. | ||
| 559 | + app.await_present_slot(); | ||
| 554 | // The last attempt keeps whatever it measured. Discarding here | 560 | // The last attempt keeps whatever it measured. Discarding here |
| 555 | // instead would leave no open pass for `end_frame` to present, and | 561 | // instead would leave no open pass for `end_frame` to present, and |
| 556 | // a drag long enough to exhaust the retries would stop painting | 562 | // a drag long enough to exhaust the retries would stop painting |