From c64426517e0ff1f7f87a7d603b106ff07c1e4bc0 Mon Sep 17 00:00:00 2001 From: ymote <151983+ymote@users.noreply.github.com> Date: Fri, 16 Oct 2026 23:59:22 +0800 Subject: [PATCH 1/2] Android: recover from GLES buffer stuffing while a finger is down The render thread paints on each Choreographer beat and queues its buffer about 12 ms into a 16.7 ms refresh (OnePlus 6). One slow frame -- often the first after idle or a touch-down -- misses SurfaceFlinger's latch, the next on-time frame queues behind it, and from then on every frame reaches the screen one refresh late until the app goes idle. In a drag the content trails the finger by an extra frame for the rest of the gesture; measured over 288 drags in an hour's soak, 48% of drags ran 16-33 ms behind. The beat now carries its expected presentation time (API 33+: Choreographer.VsyncCallback FrameData). Each frame's display time comes back through EGL_ANDROID_get_frame_timestamps; two frames in a row shown half a refresh or more after their expected time mean the queue holds a frame too many, and while a finger is down one beat is left unpainted to drain it. Before API 33 the shortest recent beat-to-screen time stands in for the expected time. `setprop debug.makepad.stuffing off` disables it; the `android.stuffing` trace topic logs detections and skips. Co-Authored-By: Claude Opus 5.5 --- platform/src/os/linux/android/android.rs | 19 +- platform/src/os/linux/android/android_jni.rs | 17 ++ .../src/os/linux/android/android_stuffing.rs | 247 ++++++++++++++++++ platform/src/os/linux/android/mod.rs | 1 + .../dev/makepad/android/MakepadNative.java | 24 +- 5 files changed, 306 insertions(+), 2 deletions(-) create mode 100644 platform/src/os/linux/android/android_stuffing.rs diff --git a/platform/src/os/linux/android/android.rs b/platform/src/os/linux/android/android.rs index 35e9e10c1d..561a17324d 100644 --- a/platform/src/os/linux/android/android.rs +++ b/platform/src/os/linux/android/android.rs @@ -459,7 +459,16 @@ impl Cx { // — which is what `destroy_surface` leaves us in — is // undefined behavior and crashes Mali/Adreno drivers with // a SIGSEGV inside `render_view`. - if self.os.has_drawable_surface() { + if self.os.has_drawable_surface() + && super::android_stuffing::beat_begin( + super::android_jni::BEAT_FRAME_TIME_NS.load(std::sync::atomic::Ordering::Relaxed), + super::android_jni::BEAT_EXPECTED_PRESENT_NS.load(std::sync::atomic::Ordering::Relaxed), + ) + { + // Buffer-stuffing recovery leaves this beat unpainted + // so the queue drains; what it would paint waits for + // the next beat (android_stuffing.rs). + } else if self.os.has_drawable_surface() { // If we previously skipped frames because the surface // wasn't ready, the redraw request may have been consumed // by an earlier draw cycle that couldn't actually paint. @@ -867,6 +876,9 @@ impl Cx { self.call_event_handler(&e); } FromJavaMessage::Touch(mut touches) => { + super::android_stuffing::set_touch_active( + touches.iter().any(|t| t.state != crate::event::finger::TouchState::Stop), + ); let time = touches[0].time; let window = &self.windows[CxWindowPool::id_zero()]; for touch in &mut touches { @@ -1811,10 +1823,14 @@ impl Cx { let perf_t0 = self.perf_monitor.enabled().then(std::time::Instant::now); if let Some(display) = &mut self.os.display { if display.is_surface_alive() { + super::android_stuffing::before_swap(&display.libegl, display.egl_display, display.surface); let swapped = (display.libegl.eglSwapBuffers.unwrap())( display.egl_display, display.surface, ); + if swapped != 0 { + super::android_stuffing::swapped(); + } // PerfMonitor "wait": eglSwapBuffers blocks on the buffer // queue (and, on tilers, on the GPU finishing the frame). if let Some(t0) = perf_t0 { @@ -4088,6 +4104,7 @@ impl CxAndroidDisplay { (self.libegl.eglDestroySurface.unwrap())(self.egl_display, self.surface); } self.surface = std::ptr::null_mut(); + super::android_stuffing::surface_reset(); } #[cfg(not(use_vulkan))] diff --git a/platform/src/os/linux/android/android_jni.rs b/platform/src/os/linux/android/android_jni.rs index 61a729d81e..a5608f134e 100644 --- a/platform/src/os/linux/android/android_jni.rs +++ b/platform/src/os/linux/android/android_jni.rs @@ -1481,6 +1481,23 @@ extern "C" fn Java_dev_makepad_android_MakepadNative_surfaceOnSafeAreaInsets( }); } +/// The latest Choreographer beat's vsync time and its expected presentation +/// time (CLOCK_MONOTONIC ns; 0 where Java does not provide it), set right +/// before that beat's `RenderLoop` is sent (android_stuffing.rs). +pub(crate) static BEAT_FRAME_TIME_NS: std::sync::atomic::AtomicI64 = std::sync::atomic::AtomicI64::new(0); +pub(crate) static BEAT_EXPECTED_PRESENT_NS: std::sync::atomic::AtomicI64 = std::sync::atomic::AtomicI64::new(0); + +#[no_mangle] +extern "C" fn Java_dev_makepad_android_MakepadNative_onFrameTime( + _: *mut jni_sys::JNIEnv, + _: jni_sys::jclass, + frame_time_nanos: jni_sys::jlong, + expected_present_nanos: jni_sys::jlong, +) { + BEAT_FRAME_TIME_NS.store(frame_time_nanos, std::sync::atomic::Ordering::Relaxed); + BEAT_EXPECTED_PRESENT_NS.store(expected_present_nanos, std::sync::atomic::Ordering::Relaxed); +} + #[no_mangle] extern "C" fn Java_dev_makepad_android_MakepadNative_onRenderLoop( _: *mut jni_sys::JNIEnv, diff --git a/platform/src/os/linux/android/android_stuffing.rs b/platform/src/os/linux/android/android_stuffing.rs new file mode 100644 index 0000000000..a0a7258fcd --- /dev/null +++ b/platform/src/os/linux/android/android_stuffing.rs @@ -0,0 +1,247 @@ +//! Buffer-stuffing recovery for the Choreographer-paced GLES window. +//! +//! The render thread paints on each vsync beat and queues its buffer most of +//! a refresh later (about 12 of 16.7 ms on a OnePlus 6), close to +//! SurfaceFlinger's latch. One slow frame (often the first after idle, or a +//! touch-down) misses the latch, and the next on-time frame queues behind it: +//! from then on every frame waits an extra refresh in the buffer queue, until +//! the app goes idle. While a finger drags content, that is a whole extra +//! frame of lag behind it for the rest of the gesture. +//! +//! Each frame's display time comes back through +//! EGL_ANDROID_get_frame_timestamps a few frames later. A frame is late when +//! it reached the screen half a refresh or more after its beat's expected +//! presentation time (Choreographer FrameData, API 33+; before that, the +//! shortest recent beat-to-screen time stands in). Two late frames from +//! consecutive beats mean the queue holds a frame too many: skipping one beat +//! drains it, and the frames after it are on time again. A drained queue has +//! no spare frame to absorb a slow one, so recovery runs only while a finger +//! is down, where the lag shows; a stuffed queue during a fling's coast is +//! invisible and is left alone. +//! +//! `setprop debug.makepad.stuffing off` turns it off (read once, at the first +//! beat); the `android.stuffing` trace topic logs each detection and skip. +use super::super::egl_sys::{EGLDisplay, EGLSurface, LibEgl}; +use std::{collections::VecDeque, ffi::CStr}; + +const EGL_EXTENSIONS: i32 = 0x3055; +const EGL_TIMESTAMPS_ANDROID: i32 = 0x3430; +const EGL_DISPLAY_PRESENT_TIME_ANDROID: i32 = 0x343a; +const EGL_TIMESTAMP_PENDING_ANDROID: i64 = -2; +/// Recent beat-to-screen times that set the on-time baseline (ten seconds +/// at 60 Hz; idle frames count too, and they are never stuffed). +const BASELINE_FRAMES: usize = 600; + +type NextId = unsafe extern "C" fn(EGLDisplay, EGLSurface, *mut u64) -> u32; +type Timestamps = unsafe extern "C" fn(EGLDisplay, EGLSurface, u64, i32, *const i32, *mut i64) -> u32; +type Supported = unsafe extern "C" fn(EGLDisplay, EGLSurface, i32) -> u32; + +#[derive(Default)] +struct StuffingRecovery { + initialized: bool, + functions: Option<(NextId, Timestamps)>, + /// Swapped frames waiting for their display time: (frame id, beat, + /// the beat's expected presentation time or 0). + pending: VecDeque<(u64, i64, i64)>, + latencies: VecDeque, + /// Frames from beats at or before this were rendered before the last + /// skip and still carry the old queue; they never trigger another. + armed_after: i64, + skip_next_beat: bool, + skips: u64, + /// Consecutive late frames from consecutive beats, and the last observed beat. + late_streak: u32, + last_observed_beat: i64, + /// A finger is on the screen. Only then does lag show (the content + /// trails the finger); after lift-off a stuffed queue is invisible and + /// its extra frame absorbs a slow one, so the queue is left alone. + touch_active: bool, + /// The beat being painted, the one before it, and recent beat intervals + /// (their minimum is the refresh period). + beat: i64, + expected: i64, + last_beat: i64, + intervals: VecDeque, + swapped_this_beat: bool, +} + +impl StuffingRecovery { + fn period(&self) -> i64 { + self.intervals.iter().copied().min().unwrap_or(16_666_667) + } + + unsafe fn initialize(&mut self, egl: &LibEgl, display: EGLDisplay, surface: EGLSurface) { + if self.initialized { + return; + } + self.initialized = true; + let extensions = (egl.eglQueryString.unwrap())(display, EGL_EXTENSIONS); + if extensions.is_null() + || !CStr::from_ptr(extensions).to_bytes().split(|b| *b == b' ').any(|s| s == b"EGL_ANDROID_get_frame_timestamps") + { + return; + } + let next = (egl.eglGetProcAddress.unwrap())(c"eglGetNextFrameIdANDROID".as_ptr()); + let timestamps = (egl.eglGetProcAddress.unwrap())(c"eglGetFrameTimestampsANDROID".as_ptr()); + let supported = (egl.eglGetProcAddress.unwrap())(c"eglGetFrameTimestampSupportedANDROID".as_ptr()); + if next.is_null() || timestamps.is_null() || supported.is_null() { + return; + } + let supported: Supported = std::mem::transmute(supported); + if supported(display, surface, EGL_DISPLAY_PRESENT_TIME_ANDROID) == 0 { + return; + } + if (egl.eglSurfaceAttrib.unwrap())(display, surface, EGL_TIMESTAMPS_ANDROID, 1) == 0 { + return; + } + self.functions = Some((std::mem::transmute(next), std::mem::transmute(timestamps))); + } + + /// Before swapping the frame painted for the vsync `beat` (CLOCK_MONOTONIC + /// ns, 0 when unknown): read back the display times that have arrived and + /// note this frame's id. + unsafe fn before_swap(&mut self, egl: &LibEgl, display: EGLDisplay, surface: EGLSurface) { + let (beat, expected, period) = (self.beat, self.expected, self.period()); + self.initialize(egl, display, surface); + let Some((next, timestamps)) = self.functions else { return }; + while let Some(&(id, frame_beat, frame_expected)) = self.pending.front() { + let mut present = 0i64; + if timestamps(display, surface, id, 1, &EGL_DISPLAY_PRESENT_TIME_ANDROID, &mut present) == 0 { + self.pending.pop_front(); + continue; + } + if present == EGL_TIMESTAMP_PENDING_ANDROID { + break; + } + self.pending.pop_front(); + if present > 0 && frame_beat > 0 { + self.observe(frame_beat, frame_expected, present, period); + } + } + if self.pending.len() >= 16 { + self.pending.pop_front(); + } + let mut id = 0u64; + if beat > 0 && next(display, surface, &mut id) != 0 { + self.pending.push_back((id, beat, expected)); + } + } + + /// A frame painted for `beat` reached the screen at `present`. Late: + /// half a refresh or more after the beat's expected presentation time + /// (Choreographer FrameData, API 33+), or before that API after the + /// shortest recent beat-to-screen time. + fn observe(&mut self, beat: i64, expected: i64, present: i64, period: i64) { + let latency = present - beat; + if latency <= 0 || latency > 8 * period { + return; + } + let late = if expected > 0 { + present - expected >= period / 2 + } else { + self.latencies.push_back(latency); + if self.latencies.len() > BASELINE_FRAMES { + self.latencies.pop_front(); + } + latency >= *self.latencies.iter().min().unwrap() + period / 2 + }; + // One late frame need not stuff the queue (an idle beat after it + // drains it), and its display time arrives frames later, when the + // queue may already be empty again. Stuffing makes EVERY following + // frame late, so act only on two in a row from consecutive beats. + let consecutive = beat - self.last_observed_beat <= period + period / 2; + self.last_observed_beat = beat; + self.late_streak = if late { if consecutive { self.late_streak + 1 } else { 1 } } else { 0 }; + if self.touch_active && beat > self.armed_after && self.late_streak >= 2 { + self.skip_next_beat = true; + self.late_streak = 0; + if crate::makepad_error_log::trace_enabled("android.stuffing") { + crate::log!("[android.stuffing] late beat={} present={} expected={} latency={}", beat, present, expected, latency); + } + } + } + + /// Whether to leave the beat `beat` unpainted. Only a beat that follows a + /// painted one is skipped: an idle app's next frame (a new gesture's + /// first) always paints. + fn take_skip(&mut self, beat: i64, painted_last_beat: bool) -> bool { + let skip = std::mem::take(&mut self.skip_next_beat) && painted_last_beat && self.touch_active; + if skip { + self.armed_after = beat; + self.skips += 1; + } + skip + } +} + +thread_local! { + static RECOVERY: std::cell::RefCell = std::cell::RefCell::new(StuffingRecovery::default()); +} + +fn enabled() -> bool { + static ENABLED: std::sync::OnceLock = std::sync::OnceLock::new(); + *ENABLED.get_or_init(|| { + extern "C" { + fn __system_property_get(name: *const std::ffi::c_char, value: *mut std::ffi::c_char) -> std::ffi::c_int; + } + let mut value = [0u8; 92]; + let len = unsafe { __system_property_get(c"debug.makepad.stuffing".as_ptr(), value.as_mut_ptr() as *mut std::ffi::c_char) }; + let off = &value[..len.max(0) as usize] == b"off"; + crate::log!("[android.stuffing] recovery {}", if off { "off" } else { "on" }); + !off + }) +} + +/// A vsync beat that will paint: whether to leave it unpainted instead. +pub(super) fn beat_begin(beat: i64, expected: i64) -> bool { + RECOVERY.with(|r| { + let mut r = r.borrow_mut(); + let painted_last_beat = std::mem::take(&mut r.swapped_this_beat); + if r.last_beat > 0 && beat > r.last_beat && beat - r.last_beat < 100_000_000 { + let interval = beat - r.last_beat; + r.intervals.push_back(interval); + if r.intervals.len() > 32 { + r.intervals.pop_front(); + } + } + r.last_beat = beat; + r.beat = beat; + r.expected = expected; + if !enabled() || beat <= 0 { + return false; + } + let skip = r.take_skip(beat, painted_last_beat); + if skip && crate::makepad_error_log::trace_enabled("android.stuffing") { + crate::log!("[android.stuffing] skip beat={} total={}", beat, r.skips); + } + skip + }) +} + +/// Right before eglSwapBuffers of the frame painted for the current beat. +pub(super) unsafe fn before_swap(egl: &LibEgl, display: EGLDisplay, surface: EGLSurface) { + if enabled() { + RECOVERY.with(|r| r.borrow_mut().before_swap(egl, display, surface)); + } +} + +/// eglSwapBuffers queued the frame. +pub(super) fn swapped() { + RECOVERY.with(|r| r.borrow_mut().swapped_this_beat = true); +} + +/// The window surface went away: frame ids belong to it. +pub(super) fn surface_reset() { + RECOVERY.with(|r| { + let mut r = r.borrow_mut(); + r.initialized = false; + r.functions = None; + r.pending.clear(); + r.skip_next_beat = false; + }); +} + +/// Whether a finger is down, from each touch update. +pub(super) fn set_touch_active(active: bool) { + RECOVERY.with(|r| r.borrow_mut().touch_active = active); +} diff --git a/platform/src/os/linux/android/mod.rs b/platform/src/os/linux/android/mod.rs index a4a335dfe1..a0e7af0291 100644 --- a/platform/src/os/linux/android/mod.rs +++ b/platform/src/os/linux/android/mod.rs @@ -7,6 +7,7 @@ pub mod android_camera; pub mod android_camera_player; pub mod android_file_dialog; pub mod android_jni; +pub mod android_stuffing; pub mod android_keycodes; pub mod android_media; pub mod android_midi; diff --git a/tools/cargo_makepad/src/android/java/dev/makepad/android/MakepadNative.java b/tools/cargo_makepad/src/android/java/dev/makepad/android/MakepadNative.java index 2f6dc2a8ab..60af8102af 100644 --- a/tools/cargo_makepad/src/android/java/dev/makepad/android/MakepadNative.java +++ b/tools/cargo_makepad/src/android/java/dev/makepad/android/MakepadNative.java @@ -25,17 +25,39 @@ public static native void onAndroidParams(String cache_path, String data_path, f private static final Choreographer.FrameCallback renderLoop = new Choreographer.FrameCallback() { @Override public void doFrame(long frameTimeNanos) { + onFrameTime(frameTimeNanos, 0); if (onRenderLoop()) { Choreographer.getInstance().postFrameCallback(this); } } }; + // API 33+: the same animation-phase beat, with the frame's expected + // presentation time, which buffer-stuffing recovery measures lateness + // against (android_stuffing.rs). + private static final Choreographer.VsyncCallback vsyncLoop = android.os.Build.VERSION.SDK_INT >= 33 + ? new Choreographer.VsyncCallback() { + @Override + public void onVsync(Choreographer.FrameData data) { + onFrameTime(data.getFrameTimeNanos(), + data.getPreferredFrameTimeline().getExpectedPresentationTimeNanos()); + if (onRenderLoop()) { + Choreographer.getInstance().postVsyncCallback(this); + } + } + } + : null; + private static void startRenderLoop() { - Choreographer.getInstance().postFrameCallback(renderLoop); + if (vsyncLoop != null) { + Choreographer.getInstance().postVsyncCallback(vsyncLoop); + } else { + Choreographer.getInstance().postFrameCallback(renderLoop); + } } public native static boolean onRenderLoop(); + public native static void onFrameTime(long frameTimeNanos, long expectedPresentNanos); public native static void onBackPressed(); public native static void onHomeIntent(); From 44568a391ee25d83014abff3a15643bb31a4c7f0 Mon Sep 17 00:00:00 2001 From: ymote <151983+ymote@users.noreply.github.com> Date: Sat, 17 Oct 2026 08:25:47 +0800 Subject: [PATCH 2/2] Android stuffing: frames from before a refresh without a new frame never trigger a skip Most motions end with the queue still stuffed (the coast is left alone), and their last frames' display times are read back only at the next gesture's first swap. By then the app went idle and the queue drained, but those late frames would still count, arm a skip, and drop the new gesture's second beat for nothing. A refresh that showed no new frame (an unpainted beat, or the beats an idle app stops asking for) drains a stuffed queue the way a skip does, so frames painted before one no longer arm recovery. Co-Authored-By: Claude Opus 5.5 --- .../src/os/linux/android/android_stuffing.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/platform/src/os/linux/android/android_stuffing.rs b/platform/src/os/linux/android/android_stuffing.rs index a0a7258fcd..cfa4350f9c 100644 --- a/platform/src/os/linux/android/android_stuffing.rs +++ b/platform/src/os/linux/android/android_stuffing.rs @@ -19,6 +19,12 @@ //! is down, where the lag shows; a stuffed queue during a fling's coast is //! invisible and is left alone. //! +//! Any refresh without a new frame drains the queue the same way (an +//! unpainted beat, or the beats an idle app stops asking for), so frames +//! painted before one never count. Most motions end with the queue still +//! stuffed, and their last frames' display times are read back only at the +//! next gesture's first swap, where they would skip a beat for nothing. +//! //! `setprop debug.makepad.stuffing off` turns it off (read once, at the first //! beat); the `android.stuffing` trace topic logs each detection and skip. use super::super::egl_sys::{EGLDisplay, EGLSurface, LibEgl}; @@ -45,7 +51,8 @@ struct StuffingRecovery { pending: VecDeque<(u64, i64, i64)>, latencies: VecDeque, /// Frames from beats at or before this were rendered before the last - /// skip and still carry the old queue; they never trigger another. + /// skip, or before the last refresh without a new frame, and still carry + /// the old queue; they never trigger a skip. armed_after: i64, skip_next_beat: bool, skips: u64, @@ -197,6 +204,7 @@ pub(super) fn beat_begin(beat: i64, expected: i64) -> bool { RECOVERY.with(|r| { let mut r = r.borrow_mut(); let painted_last_beat = std::mem::take(&mut r.swapped_this_beat); + let period = r.period(); if r.last_beat > 0 && beat > r.last_beat && beat - r.last_beat < 100_000_000 { let interval = beat - r.last_beat; r.intervals.push_back(interval); @@ -204,6 +212,12 @@ pub(super) fn beat_begin(beat: i64, expected: i64) -> bool { r.intervals.pop_front(); } } + // The last beat went unpainted, or beats stopped (an idle app asks for + // none): the screen took a refresh without a new frame, which drained + // the queue, so the frames painted before it no longer describe it. + if r.last_beat > 0 && (!painted_last_beat || beat - r.last_beat > period + period / 2) { + r.armed_after = r.armed_after.max(r.last_beat); + } r.last_beat = beat; r.beat = beat; r.expected = expected;