Skip to content

Conversation

@kernelkind
Copy link
Member

This introduces noticeable jitter/stutter when scrolling on android, I'm not sure why.

Also, take a look at kernelkind/egui@2e78dbf
I think what happened was the WgpuWinitRunning hadn't been initialized yet but the WgpuWinitApp::window_event already got called and that state usually meant we should close the app. So I just made it use the EventResult::Wait, but that is surely incorrect. Not sure how to proceed with that, it's going over my head a bit.

@alltheseas
Copy link
Contributor

Regarding the latter:

kernelkind/egui/crates/eframe/src/native/wgpu_integration.rs:466 now returns EventResult::Wait whenever self.running is None. That state also occurs right after CloseRequested: handle_event_result tears down the app by calling save_and_destroy() (which does self.running.take()) before winit delivers the follow-up Destroyed event. Because the code never emits EventResult::Exit anymore, native/run.rs:120-166 never sets exit = true, so run_native (or the Android activity) keeps spinning with a dead event loop.
    
Closing the root window/backing out of the activity now hangs instead of exiting cleanly, and all GPU resources leak. Fix: detect the difference between “not initialized yet” (self.app_creator.is_some()) and “already torn down” and only return Wait in the former case; once self.running was taken due to shutdown, the next window event must still propagate EventResult::Exit so winit can finish the loop.

@alltheseas
Copy link
Contributor

On Android jitter:

Android Scroll Jitter Analysis

  - ScrollArea motion still comes straight from input.pointer.delta() (crates/egui/src/containers/scroll_area.rs:786-812), so any
    stutter is upstream in the pointer pipeline. In 0.33 that pipeline changed substantially: PointerState::begin_pass now rebuilds
    its pos_history, velocity and drag heuristics in crates/egui/src/input_state/mod.rs:1163-1310. Android’s TouchPhase::Cancelled
    fires frequently while you drag near the navigation bar; every cancel path now postpones clearing until after pos_history.flush
    and then wipes the history entirely (lines 1254-1305). The end result is that each cancel discards all motion samples collected
    earlier in the frame, so the next pointer.delta() is computed only from the single “fresh” touch event that survived, giving
    you the big every-other-frame jumps you observe. On 0.31 the history was cleared immediately when the touch ended, so the
    earlier motion samples were still available when the frame’s delta was computed. Two ways to prove this: (1) log ctx.input(|i|
    i.pointer.delta()) plus ctx.input(|i| i.pointer.pos_history.len()) around the scroll view and watch the history drop to 0 whenever
    Android emits TouchPhase::Cancelled; (2) temporarily comment out the clear_history_after_velocity_calculation block and rebuild—
    kinetic scrolling will regress but the stutter disappears, matching what you see on 0.31.
  - There is a second Android‑only churn introduced with commit ea49f182 (“android: add inset changed events”). Every time
    WindowEvent::InsetsChanged fires (which on Android happens continuously as you drag near the gesture bar), egui-winit pushes
    egui::Event::InsetsChanged and RawInput::safe_area_insets stays unset. InputState::content_rect() therefore keeps toggling
    between the unclipped viewport and the “safe” rect on alternate frames (crates/egui/src/input_state/mod.rs:505-560). ScrollArea
    clamps state.offset against that rect each frame, so if the rect moves while you are applying the pointer delta, the freshly-
    updated offset is immediately snapped back, which looks like the scroll lunges backward. You can confirm by printing ctx.input(|
    i| i.viewport_rect()) while dragging: it oscillates by the height of the gesture inset. As a quick experiment you can drop the
    WindowEvent::InsetsChanged branch in crates/egui-winit/src/lib.rs:602-610 or ignore egui::Event::InsetsChanged inside Notedeck to
    verify the stutter goes away (at the cost of missing inset updates).
  - Frame pacing itself is still governed by NativeOptions.wgpu_options.present_mode, which defaults to AutoVsync (crates/egui-
    wgpu/src/lib.rs:329). On Android GPUs that support FifoRelaxed, wgpu will pick that mode first (see wgpu-core-27.0.3/src/
    device/global.rs:1820-1850). FifoRelaxed is known to alternate between bursty “late” presents and immediate presents, which
    amplifies any pointer jitter because kinetic scrolling runs ctx.request_repaint() based on dt. For diagnosis, try forcing
    options.wgpu_options.present_mode = wgpu::PresentMode::Fifo; options.wgpu_options.desired_maximum_frame_latency = Some(1); in
    crates/notedeck_chrome/src/android.rs:43-55. If that stabilizes the motion, keep the override; otherwise fall back to AutoNoVsync
    to see whether removing the compositor wait fixes it.

  What to do next

  1. Instrument ScrollArea to print pointer history length, delta, and the current viewport_rect to prove whether the sample purge or
     inset toggling is responsible on your hardware.
  2. Patch crates/egui-winit/src/lib.rs locally to skip WindowEvent::InsetsChanged handling on Android and rebuild; if the jitter
     disappears you can upstream a guard that only pushes InsetsChanged when the reported rect actually differs.
  3. If the issue persists even with inset handling disabled, apply the clear_history_after_velocity_calculation fix selectively
     (e.g., only clear when touch.phase == Ended) so kinetic scrolling keeps the final velocity without throwing away intermediate
     samples.
  4. Lock the renderer to PresentMode::Fifo for Android builds to eliminate FifoRelaxed’s uneven cadence as a contributing factor.

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