Skip to content

test(ui-egui): wait for the page raster before dragging an image corner - #3129

Closed
skylartaylor wants to merge 1 commit into
storytold:mainfrom
skylartaylor:fix/image-drag-preview-flake
Closed

skylartaylor wants to merge 1 commit into
storytold:mainfrom
skylartaylor:fix/image-drag-preview-flake

Conversation

@skylartaylor

Copy link
Copy Markdown
Contributor

What this changes

image_pixels_follow_the_frame_while_a_corner_is_dragged fails intermittently on test (windows-latest), and passes on rerun:

the picture is drawn in the frame [[704.0 245.2] - [828.8 322.0]], not only at []

I found 17 failures between 2026-10-15 and 2026-10-17 on unrelated PR branches. All were Windows and all were first attempts. Linux had none. The test has been flaky since it landed in #1677.

Cause. The live drag preview stretches the whole-page raster (DocView::page_raster) into the dragged frame, and draws nothing until that texture exists. The render pool's worker threads produce that raster asynchronously. The test only ran a fixed number of frames before dragging, about 50 ms of wall time for the whole test. When a busy Windows runner hadn't parsed and rendered the page by then, no picture mesh was drawn. The frame is drawn from geometry alone, so it was still correct. That's why the message shows a right frame and an empty list.

Fix. This is a test-only change. It adds a bounded wait (up to 10 s, until render_pending() is false, then asserts) after the zoom change and after each undo, the same settle pattern ui.rs, view.rs and comments.rs use. The assertions are unchanged. With no raster yet, the app shows the frame without pixels. That's the intended fallback, so the product code stays as it is.

How you verified it

  • cargo fmt --check
  • clippy with -D warnings on the crates I changed (cargo clippy -p pdfcraft-ui-egui --all-targets -- -D warnings)
  • tests pass on the crates I changed (cargo test -p pdfcraft-ui-egui --test editing: 129 passed)

To show the test fails before the fix, I added a temporary sleep before the render worker's first parse, gated on an env var and not committed:

  • before the fix, no delay: 3/3 pass
  • before the fix, 1500 ms delay: 3/3 fail, with the exact CI message and the same rectangle
  • after the fix, 1500 ms delay: 5/5 pass
  • after the fix, no delay: 50/50 pass, plus 20/20 after rebasing on 0.9.0
  • after the fix, the whole editing file with 100, 300 and 700 ms delays, and with none: all 129 pass

These runs were on Linux. I didn't rerun on Windows, because the delay reproduces the Windows failure deterministically.

Anything a reviewer should look at first

The second wait, after each undo, isn't needed for this failure: an undo keeps the stale raster on screen until the new one arrives. I kept it so the second drag starts from the same settled state as the first.

image_pixels_follow_the_frame_while_a_corner_is_dragged checks that the
live drag preview stretches the page raster into the blue frame. That
raster comes from the render workers asynchronously, but the test only
ran a fixed number of frames (about 50 ms of wall time) before dragging.
On a loaded Windows runner the raster had not arrived yet, no preview
mesh was drawn, and the assertion failed with "not only at []".

Wait, bounded, until no render is pending after the zoom change and after
each undo, as the other GUI tests do. What the test checks is unchanged.
@mtlcl410

Copy link
Copy Markdown
Contributor

For coordination: #3077 (opened 2026-10-17) fixes the same flaky test the same way. It waits on render_pending() with a bounded loop before the drag, and asserts "the page raster arrives".

This PR does one thing #3077 doesn't: it also settles after each undo inside the loop, which covers later iterations whose raster is re-rendered after the undo. Only one of the two needs to land, so I'm happy to defer to whichever the maintainers prefer. If it's this one, I'll close #3077.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants