From a6ff0350e4beff12bdf807c397eb28cb9783b179 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Thu, 30 Jul 2026 21:18:10 +0200 Subject: [PATCH] =?UTF-8?q?feat(android):=20frames=20target=20SurfaceFling?= =?UTF-8?q?er's=20latch,=20not=20the=20GPU-render=20deadline=20=E2=80=94?= =?UTF-8?q?=20display=2021=E2=86=929.5=20ms?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The remaining ~21 ms display stage was the conservative release target: the presenter aimed at the first frame timeline whose DEADLINE was still ahead, and the platform's deadline budgets for GPU rendering the app has yet to submit (presDeadline = 11.3 ms on the A024 — more than a full 120 Hz period). A decoded video buffer has no GPU work left; its only real constraint is SurfaceFlinger's own latch lead. Every frame paid a whole extra refresh of waiting for a budget it never used. next_target now gates (and subdivides) on the timeline's EXPECTED PRESENT minus a 4 ms latch margin; the glass budget reopens at that latch instant (expected present − margin) rather than the deadline, which under the aggressive gate can already lie in the past — an instant reopen would let two releases pile onto one vsync. A mis-gamble presents one vsync later, which is exactly what the deadline gate paid on every frame — the trade is one-sided. On-glass (A024, 2800×1260@120 HDR, game load): latch p50 21→8-10 ms, display 26,3→9,5 (pace 0,8 + latch 7,7), e2e 43,7→26,4 p50 / 29,2 p95, released=displays=120, paced≈0, forced=0. The latch now sits under one refresh interval — the vsync-latch floor. Co-Authored-By: Claude Fable 5 --- .../android/native/src/decode/presenter.rs | 25 +++++++++++-------- clients/android/native/src/decode/vsync.rs | 24 ++++++++++++------ 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/clients/android/native/src/decode/presenter.rs b/clients/android/native/src/decode/presenter.rs index d331c16b..a2a4c66d 100644 --- a/clients/android/native/src/decode/presenter.rs +++ b/clients/android/native/src/decode/presenter.rs @@ -27,10 +27,13 @@ use super::display::DisplayTracker; use super::latency::now_realtime_ns; use super::vsync::VsyncShared; -/// Submit-margin ahead of a timeline's deadline: the released buffer still has to travel -/// codec → BufferQueue → SurfaceFlinger's latch, so a deadline closer than this is treated as -/// missed and the next timeline is targeted. -const DEADLINE_MARGIN_NS: i64 = 1_000_000; +/// Submit-margin ahead of a timeline's EXPECTED PRESENT — SurfaceFlinger's own latch lead: the +/// released buffer must be in the BufferQueue by SF's wakeup for that vsync (a few ms before +/// present). A present closer than this is treated as missed and the next one is targeted. This +/// is deliberately NOT the timeline's `deadline` (which budgets for GPU rendering a video +/// buffer doesn't do — see `VsyncShared::next_target`); a too-tight gamble here presents one +/// vsync later, the exact cost the deadline gate paid on every frame. +const LATCH_MARGIN_NS: i64 = 4_000_000; /// The budget's liveness backstop: a release whose predicted latch never seems to arrive /// (clock glitch, mode switch) force-reopens the budget this long after the release, counted in @@ -278,7 +281,7 @@ impl Presenter { return false; } // Release: timeline-timed when the clock has one, ASAP otherwise. - let target = clock.and_then(|c| c.next_target(now_mono_ns, DEADLINE_MARGIN_NS)); + let target = clock.and_then(|c| c.next_target(now_mono_ns, LATCH_MARGIN_NS)); let released = match target { Some(t) => codec .release_output_buffer_at_time_by_index(frame.index, t.expected_present_ns) @@ -292,12 +295,14 @@ impl Presenter { return false; // the buffer is gone either way; nothing to book-keep } let period = clock.map(|c| c.period_ns()).filter(|&p| p > 0); - // Reopen at the target's DEADLINE, not its present time: SurfaceFlinger consumes the - // queued buffer at its latch (≈ the deadline) — that is when the queue slot frees and a - // new release can target the NEXT refresh. Reopening a period later (at present time) - // would cap the sustainable release rate at roughly half the panel rate. + // Reopen at SurfaceFlinger's LATCH for the targeted vsync (expected present minus the + // latch lead) — the instant SF consumes the queued buffer and the slot frees, so the + // next release can target the NEXT refresh. Not the platform `deadline` (with the + // aggressive present gate it can already be in the past — an instant reopen would let + // two releases pile onto the same vsync) and not the present time itself (a period too + // late — it would cap the sustainable rate at roughly half the panel). let reopen_at_ns = target - .map(|t| t.deadline_ns) + .map(|t| t.expected_present_ns - LATCH_MARGIN_NS) .unwrap_or(now_mono_ns + period.unwrap_or(FALLBACK_PERIOD_NS)); self.inflight = Some(InFlight { reopen_at_ns, diff --git a/clients/android/native/src/decode/vsync.rs b/clients/android/native/src/decode/vsync.rs index b1ce680d..fac8bab5 100644 --- a/clients/android/native/src/decode/vsync.rs +++ b/clients/android/native/src/decode/vsync.rs @@ -79,15 +79,23 @@ impl VsyncShared { } /// The release target for a frame submitted at `now`: the earliest stored timeline whose - /// deadline is still `margin` away, extrapolated forward by whole periods once the stored - /// set has aged out (timelines refresh once per vsync callback; a frame can decode anywhere - /// inside that window). `None` on the 31/32 fallback — the caller releases ASAP. + /// EXPECTED PRESENT is still `margin` away, extrapolated forward by whole periods once the + /// stored set has aged out (timelines refresh once per vsync callback; a frame can decode + /// anywhere inside that window). `None` on the 31/32 fallback — the caller releases ASAP. + /// + /// Gated on `expected_present`, NOT the timeline's `deadline`, on purpose: the deadline + /// budgets for GPU rendering the app has yet to submit (`presDeadline` — 11.3 ms on the + /// A024, more than a full 120 Hz period), but a video buffer is already fully rendered — + /// the only real constraint is SurfaceFlinger's own latch lead, which is what the caller's + /// `margin` represents. Targeting by deadline cost every frame an extra refresh of waiting + /// (measured: latch p50 ~21 ms vs the ~2-interval floor); a mis-gamble here just means the + /// frame presents one vsync later — exactly what the conservative gate always paid. /// /// The picked target is then SUBDIVIDED onto the panel grid: the platform reports timelines /// at the app's assigned render rate, but the panel latches at its own — when the app is /// down-rated (60 Hz callbacks on a 120 Hz panel) the reported timelines are a whole panel /// period apart or more, and pacing to them would cap the video. Pulling the target earlier - /// by whole panel periods (while its deadline still clears the margin) restores the true + /// by whole panel periods (while its present still clears the margin) restores the true /// grid; when callbacks run at the panel rate the pull condition is never true and this is /// a no-op. pub(super) fn next_target(&self, now_ns: i64, margin_ns: i64) -> Option { @@ -98,7 +106,7 @@ impl VsyncShared { .unwrap_or_else(std::sync::PoisonError::into_inner); let found = g .iter() - .find(|t| t.deadline_ns > now_ns + margin_ns) + .find(|t| t.expected_present_ns > now_ns + margin_ns) .copied(); match found { Some(t) => t, @@ -109,8 +117,8 @@ impl VsyncShared { return None; } // All stored timelines have passed — step the last one forward whole - // periods until its deadline clears `now + margin` again. - let behind = (now_ns + margin_ns).saturating_sub(last.deadline_ns); + // periods until its present clears `now + margin` again. + let behind = (now_ns + margin_ns).saturating_sub(last.expected_present_ns); let k = behind / period + 1; FrameTimeline { expected_present_ns: last.expected_present_ns + k * period, @@ -121,7 +129,7 @@ impl VsyncShared { }; let panel = self.panel_period_ns.load(Ordering::Relaxed); if panel > 0 { - while t.deadline_ns - panel > now_ns + margin_ns { + while t.expected_present_ns - panel > now_ns + margin_ns { t.deadline_ns -= panel; t.expected_present_ns -= panel; }