From 166c93c07950998ddac41c282d028a4f41323565 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sun, 2 Aug 2026 19:52:39 +0200 Subject: [PATCH] fix(android/present): the panel grid can be wrong in both directions, and the margin listens to the latch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects in the 0.23.0 timeline presenter, all found while root-causing the field report that turned out to be the slice wire. None of them is that bug; all three are real, and the first is the one that would still bite once it is fixed. The panel-period learner could only ever narrow. It is seeded from the display mode Kotlin asked for — and `preferredDisplayModeId` is a REQUEST the system may refuse (Smooth Display off, battery saver, thermal, an OEM governor). Ask for 120 Hz on a panel that stays at 60 and the presenter pins an 8.33 ms grid on a 16.67 ms display with no way back, for the rest of the session: it then aims at instants that never arrive and releases faster than the panel scans. The learner moves both ways now, and lives in `punktfunk_core::phase::PanelGrid` where it is host-testable and where the iOS and desktop presenters can share it. The asymmetry is kept and made explicit — narrowing is immediate (a finer real grid is always safe to subdivide onto, and it is the per-uid down-rate case the seed most often gets wrong), widening needs eight consecutive agreeing observations and then takes the narrowest of them, because one wide sample is a missed callback and eight in a row is a display that really did slow down. The glass budget was a prediction with nothing underneath it. `OnFrameRendered` already reports what actually reached glass, but the budget never consulted it, so a wrong grid could hand SurfaceFlinger frames indefinitely: BufferQueue fills, MediaCodec runs out of output buffers, the decoder stalls, and the no-output backstop starts begging for keyframes. Releases are now counted against their confirms and the presenter holds back past six outstanding — loose on purpose, since the callbacks are allowed to arrive batched and a held frame in the newest-wins slot is a dropped one. It self-clears when the confirms catch up, and writes the ledger off after the same 100 ms the stale reopen uses, so a platform that stops confirming can never wedge the stream. `qWait` and `unconfirmed` join the 1 Hz pf.present line, which is what would have made this visible from a log. The adaptive latch margin widened on `paced_drops` — the newest-wins store's own policy evictions, which happen whenever the stream out-runs the panel and say nothing about SurfaceFlinger's latch lead. On a healthy device that walked the margin to its 2.5 ms ceiling and re-imposed the display latency the P2e sweep had just measured away. It now widens on the measured latch exceeding one panel period plus the live margin, which is what a missed vsync actually looks like. Also corrects two doc comments that named `display.refreshRate` as the panel_hz source; it has been the mode table since the A024 down-rate fix. Gates: 278 punktfunk-core lib tests (7 new PanelGrid cases incl. the refused-mode regression), clippy -D warnings and fmt clean, cargo ndk check green on arm64 and armv7. Android clippy reports the same 4 warnings as the base commit and no new ones. NOT yet confirmed on glass. Co-Authored-By: Claude Opus 5 (1M context) --- .../android/native/src/decode/async_loop.rs | 2 +- clients/android/native/src/decode/mod.rs | 11 +- .../android/native/src/decode/presenter.rs | 176 +++++++++++++-- clients/android/native/src/decode/vsync.rs | 54 +++-- crates/punktfunk-core/src/phase.rs | 208 +++++++++++++++++- 5 files changed, 403 insertions(+), 48 deletions(-) diff --git a/clients/android/native/src/decode/async_loop.rs b/clients/android/native/src/decode/async_loop.rs index 68ca50e1..825f2089 100644 --- a/clients/android/native/src/decode/async_loop.rs +++ b/clients/android/native/src/decode/async_loop.rs @@ -392,7 +392,7 @@ pub(super) fn run_async( // even when the choreographer clock is absent. if let Some(p) = presenter.as_mut() { let clock = vsync.as_ref().map(|v| v.shared().as_ref()); - if p.pump(&codec, clock, &tracker, &stats, now_monotonic_ns()) { + if p.pump(&codec, clock, &tracker, &meter, &stats, now_monotonic_ns()) { rendered += 1; } // The 1 Hz window flush doubles as the phase-lock report tick. v3 sensor: the diff --git a/clients/android/native/src/decode/mod.rs b/clients/android/native/src/decode/mod.rs index bf4d3d71..630f3f7f 100644 --- a/clients/android/native/src/decode/mod.rs +++ b/clients/android/native/src/decode/mod.rs @@ -115,9 +115,14 @@ pub(crate) struct DecodeOptions { /// The smoothness buffer depth (`smooth_buffer` setting): 0 = automatic (2), else 1..=3. /// Only meaningful with `present_priority` = smooth. pub smooth_buffer: i32, - /// The display mode's own refresh rate (Kotlin's `display.refreshRate` at stream start; - /// 0 = unknown) — the latch grid the presenter subdivides onto when the app's choreographer - /// stream is down-rated below the panel (see `vsync.rs`). + /// SEED for the panel's refresh period — the latch grid the presenter subdivides onto when + /// the app's choreographer stream is down-rated below the panel (see `vsync.rs`). Kotlin + /// resolves it from the display mode TABLE (`MainActivity.streamPanelFps`), not + /// `display.refreshRate`, which reports a per-uid override rather than the panel. 0 = unknown. + /// + /// ⚠ Only a seed: `preferredDisplayModeId` is a REQUEST the system may refuse, so the mode + /// named here is not necessarily the one the panel ends up in. The measured timeline spacing + /// corrects it in both directions ([`punktfunk_core::phase::PanelGrid`]). pub panel_hz: i32, } diff --git a/clients/android/native/src/decode/presenter.rs b/clients/android/native/src/decode/presenter.rs index 1eda9928..5b372712 100644 --- a/clients/android/native/src/decode/presenter.rs +++ b/clients/android/native/src/decode/presenter.rs @@ -4,10 +4,12 @@ //! * a **newest-wins slot** (or a small smoothing FIFO, by user intent) between decode and //! release, so a burst coalesces in the app — as an explicit, counted drop — instead of //! queueing behind the display; -//! * a **glass budget of exactly one**: at most one undisplayed release in flight to -//! SurfaceFlinger, reopened on the clock-predicted latch (with a 100 ms stale force-open as -//! the liveness backstop, mirroring Apple's `PresentGate.staleAfter`). The BufferQueue can -//! hold at most the frame being scanned out plus one — a standing queue is unconstructible; +//! * a **glass budget of one**: at most one undisplayed release in flight to SurfaceFlinger, +//! reopened on the clock-predicted latch (with a 100 ms stale force-open as the liveness +//! backstop, mirroring Apple's `PresentGate.staleAfter`), and bounded underneath by what +//! `OnFrameRendered` actually confirmed reached glass ([`UNDISPLAYED_CAP`]) — because the +//! prediction is only as good as the panel grid behind it, and 0.23.0 shipped a grid that +//! could be wrong in one direction forever; //! * a **timed release**: `AMediaCodec_releaseOutputBufferAtTime` targeting the platform's own //! frame timeline (API 33+, via [`super::vsync`]), so the latch phase is deterministic instead //! of inheriting network + decode jitter. On the 31/32 fallback the release is ASAP — @@ -20,6 +22,7 @@ use ndk::media::media_codec::MediaCodec; use std::collections::VecDeque; +use std::sync::atomic::{AtomicBool, AtomicI32, Ordering}; use std::sync::Mutex; use std::time::Instant; @@ -36,9 +39,9 @@ use super::vsync::VsyncShared; /// /// 2.5 ms: SF's latch runs ~1-2 ms before present on modern devices (its `sfOffset`), and the /// release itself is a binder call well under a ms. 4 ms measured latch p50 8-10; each ms cut -/// here is a ms off every frame's display stage. If a device misses at this margin the `paced` -/// counter shows it (a miss presents one vsync later, coalescing the next frame) — that is the -/// signal to widen, not stutter. +/// here is a ms off every frame's display stage. A device that misses at the live margin shows it +/// as a measured latch beyond one panel period (see the adaptation in +/// [`Presenter::flush_log`]) — that, not a drop counter, is the signal to widen. const LATCH_MARGIN_NS: i64 = 2_500_000; /// `debug.punktfunk.latch_margin_us` (0..=8000 µs): PIN the submit margin for a sweep — @@ -71,6 +74,26 @@ fn latch_margin_ns() -> Option { /// `forced` — reads 0 on healthy systems (Apple's `PresentGate.staleAfter`, same value). const STALE_REOPEN_NS: i64 = 100_000_000; +/// Releases still unconfirmed by `OnFrameRendered` at which the presenter stops handing +/// SurfaceFlinger more work. +/// +/// The reopen above is a PREDICTION off the learned panel grid. A grid finer than the panel +/// (0.23.0 could pin one permanently — see [`punktfunk_core::phase::PanelGrid`]) reopens the +/// budget before the display has consumed anything, and the presenter then releases faster than +/// the panel scans: the BufferQueue fills, MediaCodec runs out of output buffers, the decoder +/// stalls, and the no-output backstop starts begging for keyframes. The render callback is the +/// ground truth about what actually reached glass, so it bounds the prediction. +/// +/// Six, not one: the platform is explicitly allowed to deliver these callbacks BATCHED, and this +/// module's own `RENDERED_CAP` note records them trailing a release by a vsync or two — so a +/// healthy device sits at 1-3 outstanding and a tight cap would throttle it for nothing (a held +/// frame in the newest-wins slot is a DROPPED frame the moment a fresher one decodes). This is +/// not a pacing knob; it is the "something is structurally wrong" rail, and a presenter genuinely +/// out-running its display climbs past any fixed cap within a second. If a device's BufferQueue +/// is shallower than this the rail simply never engages and the no-output backstop handles it, +/// exactly as before — best-effort, never worse than not having it. +const UNDISPLAYED_CAP: i32 = 6; + /// Fallback latch-prediction period while the vsync clock is unmeasured/absent: one 120 Hz frame. const FALLBACK_PERIOD_NS: i64 = 8_333_333; @@ -121,6 +144,14 @@ struct InFlight { /// a HUD-off wireless A/B readable from logcat. pub(super) struct PresentMeter { inner: Mutex, + /// Frames released to SurfaceFlinger that `OnFrameRendered` has not yet confirmed reached + /// glass. The presenter's structural rail (see [`UNDISPLAYED_CAP`]) and the pf-present line's + /// queue-depth readout. Lock-free because the release side runs on the decode loop and the + /// confirm side on the codec's callback thread, once per frame each. + undisplayed: AtomicI32, + /// This device delivers render callbacks at all (API ≥ 33 and the platform accepted the + /// registration). Until one arrives, `undisplayed` is meaningless and the rail stays down. + confirms: AtomicBool, } struct PresentMeterInner { @@ -147,11 +178,23 @@ impl PresentMeter { codec_us: Vec::with_capacity(256), e2e_us: Vec::with_capacity(256), }), + undisplayed: AtomicI32::new(0), + confirms: AtomicBool::new(false), } } /// One displayed frame's release→displayed latch, µs. Callback thread; poison-proof. + /// + /// Also the glass budget's CONFIRM: this frame left the BufferQueue, so one outstanding + /// release is settled. Clamped at zero — the legacy `arrival` path renders without going + /// through [`Presenter::pump`], so confirms can outnumber counted releases. pub(super) fn note_latch(&self, latch_us: Option) { + self.confirms.store(true, Ordering::Relaxed); + let _ = self + .undisplayed + .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |v| { + Some((v - 1).max(0)) + }); let mut g = self .inner .lock() @@ -164,6 +207,26 @@ impl PresentMeter { } } + /// One frame handed to SurfaceFlinger, awaiting its confirm. Decode thread. + fn note_released(&self) { + self.undisplayed.fetch_add(1, Ordering::Relaxed); + } + + /// Releases still unconfirmed, and whether confirms happen on this device at all. + fn outstanding(&self) -> (i32, bool) { + ( + self.undisplayed.load(Ordering::Relaxed), + self.confirms.load(Ordering::Relaxed), + ) + } + + /// Write off the outstanding releases: the platform stopped confirming (it is allowed to + /// drop callbacks under load) or SurfaceFlinger discarded the buffers without presenting + /// them. Never stall the stream on a ledger we cannot audit. + fn forgive_outstanding(&self) { + self.undisplayed.store(0, Ordering::Relaxed); + } + /// One decoded frame's always-on measurements: the `decode`-stage split (feed = /// received→queued when a receipt stamp matched; codec = queued→decoded when the queued /// stamp did) and the capture→decoded end-to-end, µs. Decode thread; poison-proof. @@ -239,6 +302,13 @@ pub(super) struct Presenter { no_budget: u64, forced: u64, dry: u64, + /// Pump passes that held a frame back because too many earlier releases were still + /// unconfirmed ([`UNDISPLAYED_CAP`]) — reads 0 on a healthy device, and a climbing value is + /// the signature of a presenter out-running its display. + queue_waits: u64, + /// When the unconfirmed-release rail first engaged, so it can be forgiven if the confirms + /// simply stopped coming. `None` while the rail is down. + backed_up_since: Option, pace_us: Vec, last_flush: Instant, /// The live submit margin. Starts at 0 (P2e on-glass: SurfaceFlinger latched every @@ -280,6 +350,8 @@ impl Presenter { no_budget: 0, forced: 0, dry: 0, + queue_waits: 0, + backed_up_since: None, pace_us: Vec::with_capacity(256), last_flush: Instant::now(), margin_ns, @@ -334,6 +406,7 @@ impl Presenter { codec: &MediaCodec, clock: Option<&VsyncShared>, tracker: &DisplayTracker, + meter: &PresentMeter, stats: &crate::stats::VideoStats, now_mono_ns: i64, ) -> bool { @@ -346,6 +419,10 @@ impl Presenter { self.inflight = None; } } + // The measured rail beneath that prediction (see `UNDISPLAYED_CAP`). Evaluated on every + // pass — frame waiting or not — so its forgiveness timer measures real elapsed time + // rather than how often a frame happened to be ready. + let backlogged = self.unconfirmed_backlog(meter, now_mono_ns); // Pick the frame this pump may release. let frame = if self.fifo_capacity == 0 { self.frames.pop_back() // submit() kept it a single slot; back == the newest @@ -373,9 +450,12 @@ impl Presenter { self.frames.pop_front() }; let Some(frame) = frame else { return false }; - if self.inflight.is_some() { + if self.inflight.is_some() || backlogged { // Budget closed — park it back; a fresher submit replaces it (newest-wins), the next // vsync tick / loop pass retries the pairing. + if backlogged { + self.queue_waits += 1; + } self.no_budget += 1; match self.fifo_capacity { 0 => self.frames.push_back(frame), @@ -412,6 +492,7 @@ impl Presenter { released_at_ns: now_mono_ns, }); self.released += 1; + meter.note_released(); let release_real_ns = now_realtime_ns(); let pace_us = ((release_real_ns - frame.decoded_ns).max(0) / 1000) as u64; if self.pace_us.len() < 4096 { @@ -422,6 +503,33 @@ impl Presenter { true } + /// Whether SurfaceFlinger is sitting on too many unconfirmed releases to be handed another. + /// + /// The predicted reopen is only as good as the panel grid behind it; this is the measured + /// rail underneath it (see [`UNDISPLAYED_CAP`]). It self-clears two ways — the confirms catch + /// up, or [`STALE_REOPEN_NS`] passes with the backlog stuck, which means the ledger itself is + /// unreliable (callbacks dropped under load, or SF discarded the buffers) and is written off + /// rather than allowed to wedge the stream. + fn unconfirmed_backlog(&mut self, meter: &PresentMeter, now_ns: i64) -> bool { + let (outstanding, confirms_live) = meter.outstanding(); + if !confirms_live || outstanding < UNDISPLAYED_CAP { + self.backed_up_since = None; + return false; + } + match self.backed_up_since { + Some(t) if now_ns - t > STALE_REOPEN_NS => { + meter.forgive_outstanding(); + self.backed_up_since = None; + self.forced += 1; + false + } + _ => { + self.backed_up_since.get_or_insert(now_ns); + true + } + } + } + /// Release every held buffer unrendered — the teardown path, BEFORE `codec.stop()`. pub(super) fn release_all(&mut self, codec: &MediaCodec) { while let Some(f) = self.frames.pop_front() { @@ -434,7 +542,9 @@ impl Presenter { /// `pf-present` line, so a HUD-off on-device A/B is readable wirelessly: /// `released` (to glass) / `displays` (OnFrameRendered confirms) / `paced` (policy drops) / /// `noBudget` (waits on the closed budget) / `forced` (stale force-opens — 0 when healthy) / - /// `qDry` (FIFO underflows) / `pace` (decoded→release) / `latch` (release→displayed) / + /// `qDry` (FIFO underflows) / `qWait` (pumps held back by unconfirmed releases — 0 when + /// healthy) / `unconfirmed` (releases OnFrameRendered hasn't settled) / + /// `pace` (decoded→release) / `latch` (release→displayed) / /// `feed`+`codec` (the decode stage split: received→queued hand-off/slot wait + the /// codec-pure queued→decoded time) / `e2e` (capture→decoded, skew-corrected — the wireless /// A/B headline) / `vsync` (the measured panel period). @@ -462,14 +572,15 @@ impl Presenter { let circ = clock.and_then(|c| { punktfunk_core::phase::circular_latch(&latch, c.panel_period_ns().max(c.period_ns())) }); + let latch_samples = latch.len(); let (latch_p50, latch_max) = p50_max_ms(latch); let period_ms = clock.map(|c| c.period_ns() as f64 / 1e6).unwrap_or(0.0); - let panel_ms = clock - .map(|c| c.panel_period_ns() as f64 / 1e6) - .unwrap_or(0.0); + let panel_ns = clock.map(|c| c.panel_period_ns()).unwrap_or(0); + let (outstanding, _) = meter.outstanding(); log::info!( target: "pf.present", "released={} displays={} paced={} noBudget={} forced={} qDry={} \ + qWait={} unconfirmed={} \ paceMs p50={:.2} max={:.2} latchMs p50={:.2} max={:.2} \ feedMs p50={:.2} max={:.2} codecMs p50={:.2} max={:.2} \ e2eMs p50={:.2} max={:.2} circ={:.2}ms coh={} \ @@ -480,6 +591,8 @@ impl Presenter { self.no_budget, self.forced, self.dry, + self.queue_waits, + outstanding, pace_p50, pace_max, latch_p50, @@ -493,25 +606,48 @@ impl Presenter { circ.map(|(m, _)| m as f64 / 1e6).unwrap_or(0.0), circ.map(|(_, c)| c).unwrap_or(0), period_ms, - panel_ms, + panel_ns as f64 / 1e6, ); self.released = 0; - // Margin adaptation: repeated latch misses in one window (a miss presents a vsync - // late and coalesces the next frame into `paced`) mean this device's SF does need - // lead — widen toward the pre-sweep ceiling. One-way by design: a margin that once - // proved necessary is never re-gambled mid-stream (the next stream restarts at 0). - if !self.margin_pinned && self.paced_drops > 2 && self.margin_ns < LATCH_MARGIN_NS { + // Margin adaptation, off the MEASURED latch. A release targets the first grid point past + // `now + margin`, so a frame that makes its vsync is on glass within one panel period of + // that margin; beyond it, SurfaceFlinger wanted more lead and the frame waited out an + // extra refresh. Widen toward the pre-sweep ceiling. One-way by design: a margin that + // once proved necessary is never re-gambled mid-stream (the next stream restarts at 0). + // + // ⚠ NOT `paced_drops`, which 0.23.0 used: those are the newest-wins store's own policy + // evictions — a second frame decoding while one is held — which happen whenever the + // stream out-runs the panel and say nothing at all about SF's latch lead. Driving the + // margin from them widened it to the ceiling on healthy devices, re-imposing the 2.5 ms + // of pure display latency the P2e sweep had just measured away. + let latch_p50_ns = (latch_p50 * 1e6) as i64; + if !self.margin_pinned + && self.margin_ns < LATCH_MARGIN_NS + && panel_ns > 0 + && latch_samples >= 8 + && latch_p50_ns > panel_ns + self.margin_ns + { self.margin_ns = (self.margin_ns + 500_000).min(LATCH_MARGIN_NS); log::warn!( - "presenter: {} latch misses in 1s — margin widened to {}us", - self.paced_drops, + "presenter: latch p50 {:.2}ms over the {:.2}ms panel period — margin widened to {}us", + latch_p50, + panel_ns as f64 / 1e6, self.margin_ns / 1_000 ); } + if self.queue_waits > 0 { + log::warn!( + "presenter: {} pump(s) held back — {} release(s) still unconfirmed by \ + OnFrameRendered (the display is not keeping up with the release rate)", + self.queue_waits, + outstanding + ); + } self.paced_drops = 0; self.no_budget = 0; self.forced = 0; self.dry = 0; + self.queue_waits = 0; circ } } diff --git a/clients/android/native/src/decode/vsync.rs b/clients/android/native/src/decode/vsync.rs index fac8bab5..c5361ea3 100644 --- a/clients/android/native/src/decode/vsync.rs +++ b/clients/android/native/src/decode/vsync.rs @@ -58,8 +58,10 @@ pub(super) struct VsyncShared { /// video to THIS rate would cap the stream — hence `panel_period_ns` + the subdivision in /// [`Self::next_target`]. period_ns: AtomicI64, - /// The panel's own refresh period (from the display mode Kotlin resolved at stream start; - /// 0 = unknown). The grid SurfaceFlinger actually latches on. + /// The panel's own refresh period — the grid SurfaceFlinger actually latches on (0 = unknown). + /// Seeded from the display mode Kotlin resolved at stream start and then corrected by + /// measurement; the learner itself is [`punktfunk_core::phase::PanelGrid`], owned by the + /// choreographer thread (see [`CallbackCtx::panel`]) and published here for the decode loop. panel_period_ns: AtomicI64, /// Callback count, for the one-shot cadence diagnostic log. ticks: std::sync::atomic::AtomicU32, @@ -231,6 +233,11 @@ struct CallbackCtx { choreographer: *mut c_void, shared: Arc, on_tick: Box, + /// The panel-period learner. `Cell` rather than an atomic because it is touched from exactly + /// one thread — callbacks only ever fire inside this thread's looper poll (see the struct + /// doc) — and its streak state is nobody else's business; only the settled period is + /// published, to `shared.panel_period_ns`. + panel: std::cell::Cell, } impl CallbackCtx { @@ -240,22 +247,25 @@ impl CallbackCtx { .shared .last_vsync_ns .swap(frame_time_ns, Ordering::Relaxed); - // Panel-grid learner: timeline spacing is SurfaceFlinger's own grid, and the finest - // spacing ever observed is the panel's true period — trustworthy where the configured - // value is not (under a per-uid frame-rate override, `Display.getRefreshRate` REPORTS - // THE OVERRIDE, observed on-glass: a 120 Hz panel read back as 60 while early timelines - // ran at 8.28 ms). Corrects DOWNWARD only: subdividing onto a finer real grid is always - // valid, widening on a later down-rated window never is. + // Panel-grid learner: timeline spacing is SurfaceFlinger's own grid, and therefore the + // only honest witness to what the panel is doing — the configured mode is not (under a + // per-uid frame-rate override `Display.getRefreshRate` REPORTS THE OVERRIDE, observed + // on-glass: a 120 Hz panel read back as 60 while its timelines ran at 8.28 ms), and + // neither is the mode Kotlin *requested* (`preferredDisplayModeId` is a hint the system + // may refuse). Both directions matter and the asymmetry lives in `PanelGrid`. if timelines.len() >= 2 { let spacing = timelines[1].expected_present_ns - timelines[0].expected_present_ns; - if (2_000_000..=42_000_000).contains(&spacing) { - let cur = self.shared.panel_period_ns.load(Ordering::Relaxed); - if cur == 0 || spacing < cur - 200_000 { - self.shared - .panel_period_ns - .store(spacing, Ordering::Relaxed); - } + let mut grid = self.panel.get(); + if grid.observe(spacing) { + self.shared + .panel_period_ns + .store(grid.period_ns(), Ordering::Relaxed); + log::info!( + "vsync: panel grid now {:.2}ms", + grid.period_ns() as f64 / 1e6 + ); } + self.panel.set(grid); } // One-shot cadence diagnostic (3rd tick, once deltas exist): the callback cadence vs the // panel period is exactly the down-rating question, and this line answers it on-glass. @@ -372,8 +382,9 @@ pub(super) struct VsyncClock { impl VsyncClock { /// Spawn the choreographer thread. `on_tick` fires once per vsync ON THAT THREAD — it must /// only do something cheap and `Send` (the decode loop passes an event-channel send). - /// `panel_hz` is the display mode's own refresh rate (0 = unknown), the latch grid that - /// [`VsyncShared::next_target`] subdivides onto. `None` when the platform surface is missing + /// `panel_hz` SEEDS the panel-grid learner (0 = unknown) — the latch grid that + /// [`VsyncShared::next_target`] subdivides onto. A seed, not a fact: it names the display + /// mode Kotlin *requested*, and the observed timeline spacing is what settles it. `None` when the platform surface is missing /// (very old device) — the presenter then runs clock-less (ASAP targets, predicted-latch /// budget). pub(super) fn start(panel_hz: i32, on_tick: Box) -> Option { @@ -383,11 +394,9 @@ impl VsyncClock { stop: AtomicBool::new(false), last_vsync_ns: AtomicI64::new(0), period_ns: AtomicI64::new(0), - panel_period_ns: AtomicI64::new(if panel_hz > 0 { - 1_000_000_000 / panel_hz as i64 - } else { - 0 - }), + panel_period_ns: AtomicI64::new( + punktfunk_core::phase::PanelGrid::seeded(panel_hz).period_ns(), + ), ticks: std::sync::atomic::AtomicU32::new(0), timelines: Mutex::new(Vec::new()), }); @@ -408,6 +417,7 @@ impl VsyncClock { choreographer, shared: thread_shared, on_tick, + panel: std::cell::Cell::new(punktfunk_core::phase::PanelGrid::seeded(panel_hz)), }; ctx.repost(); // The bounded poll doubles as the stop check: no cross-thread wake needed, worst diff --git a/crates/punktfunk-core/src/phase.rs b/crates/punktfunk-core/src/phase.rs index 4a08ede9..a66f6f1d 100644 --- a/crates/punktfunk-core/src/phase.rs +++ b/crates/punktfunk-core/src/phase.rs @@ -1,9 +1,105 @@ //! Circular (directional) statistics for phase-locked capture (design/phase-locked-capture.md): -//! the client-side half of the controller's v2 error signal. Pure math, no features — shared so -//! every vsync-aware presenter (Android today, iOS next) computes the SAME statistic the host +//! the client-side half of the controller's v2 error signal, plus the panel-grid learner every +//! vsync-aware presenter paces against. Pure math, no features — shared so every presenter +//! (Android today, iOS and the desktop session client next) computes the SAME statistic the host //! controller was tuned against, and so the controller's simulation tests can generate their //! synthetic reports through the identical code path. +/// Plausible panel periods: ~24 Hz to ~500 Hz. A spacing outside this is a clock glitch, not a +/// display mode, and must never reach the estimate. +const PANEL_PERIOD_RANGE_NS: std::ops::RangeInclusive = 2_000_000..=42_000_000; + +/// Spacings within this of the estimate are the same grid — absorbs ordinary timeline jitter. +const PANEL_GRID_TOLERANCE_NS: i64 = 200_000; + +/// Consecutive WIDER observations required before the estimate grows. One stray wide sample is a +/// scheduling hiccup; eight in a row (~66 ms at 120 Hz) is a display that really did slow down. +const PANEL_WIDEN_STREAK: u8 = 8; + +/// The panel's true refresh period, learned from observed vsync/frame-timeline spacing. +/// +/// A presenter subdivides its release targets onto this grid, so an estimate FINER than the panel +/// makes it aim at instants that never arrive and release faster than the display consumes — +/// which is why the estimate has to be able to move both ways. +/// +/// Seeding is the reason this is not simply "believe the last sample". The platform's *configured* +/// mode is not the panel: under a per-uid frame-rate override a 120 Hz panel reports 60 +/// (`Display.getRefreshRate` returns the override — observed on-glass, A024), and the app's own +/// choreographer callbacks arrive at the down-rated rate while the panel scans at its own. The +/// mode TABLE is honest about what the panel *can* do, so it is the seed; the timeline spacing is +/// honest about what it is *doing*, so it is the correction. +/// +/// The asymmetry is deliberate. **Narrowing is immediate**: a finer real grid is always safe to +/// subdivide onto, and it is the down-rate case the seed most often gets wrong. **Widening needs +/// [`PANEL_WIDEN_STREAK`] consecutive agreeing observations** and then adopts the *narrowest* of +/// them, because a wide sample is far more likely to be a missed callback than a mode change. +/// +/// ⚠ 0.23.0 shipped this learner as narrow-only, seeded from the display mode the app *requests* +/// (`preferredDisplayModeId` is a hint the system may refuse). A refused 120 Hz switch therefore +/// left the presenter pacing a 60 Hz panel on an 8.33 ms grid with no way back — permanently. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct PanelGrid { + period_ns: i64, + widen_streak: u8, + /// Narrowest wider-than-estimate spacing seen during the current streak. + widen_candidate: i64, +} + +impl PanelGrid { + /// Seed from the display mode's refresh rate (`0` = unknown — the first plausible observation + /// then sets the estimate outright). + pub fn seeded(hz: i32) -> PanelGrid { + PanelGrid { + period_ns: if hz > 0 { 1_000_000_000 / hz as i64 } else { 0 }, + widen_streak: 0, + widen_candidate: 0, + } + } + + /// The learned period, or `0` while unknown. + pub fn period_ns(&self) -> i64 { + self.period_ns + } + + /// Fold one observed grid spacing. Returns `true` when [`period_ns`](Self::period_ns) changed. + pub fn observe(&mut self, spacing_ns: i64) -> bool { + if !PANEL_PERIOD_RANGE_NS.contains(&spacing_ns) { + return false; // implausible — a clock glitch, not a display mode + } + if self.period_ns == 0 { + self.reset_streak(); + self.period_ns = spacing_ns; + return true; + } + if spacing_ns < self.period_ns - PANEL_GRID_TOLERANCE_NS { + self.reset_streak(); + self.period_ns = spacing_ns; + return true; + } + if spacing_ns > self.period_ns + PANEL_GRID_TOLERANCE_NS { + self.widen_streak = self.widen_streak.saturating_add(1); + self.widen_candidate = if self.widen_candidate == 0 { + spacing_ns + } else { + self.widen_candidate.min(spacing_ns) + }; + if self.widen_streak >= PANEL_WIDEN_STREAK { + self.period_ns = self.widen_candidate; + self.reset_streak(); + return true; + } + return false; + } + self.reset_streak(); // this sample agreed — the run of wider ones is broken + false + } + + fn reset_streak(&mut self) { + self.widen_streak = 0; + self.widen_candidate = 0; + } +} + /// Circular (vector-mean) statistics of latch samples against a display period: the mean latch /// mod the period (ns) and the coherence (‰). /// @@ -90,3 +186,111 @@ mod tests { assert!(circular_latch(&[1_000; 16], 0).is_none()); } } + +#[cfg(test)] +mod panel_grid_tests { + use super::*; + + const P120: i64 = 8_333_333; + const P60: i64 = 16_666_666; + + #[test] + fn seeds_from_the_mode_and_reports_unknown_without_one() { + assert_eq!(PanelGrid::seeded(120).period_ns(), 8_333_333); + assert_eq!(PanelGrid::seeded(0).period_ns(), 0); + let mut g = PanelGrid::seeded(0); + assert!( + g.observe(P120), + "the first plausible sample sets an unseeded grid" + ); + assert_eq!(g.period_ns(), P120); + } + + #[test] + fn narrows_immediately_when_the_panel_is_faster_than_the_mode_said() { + // The down-rate case: the mode table read 60, the timelines run at 120. + let mut g = PanelGrid::seeded(60); + assert!(g.observe(P120)); + assert_eq!(g.period_ns(), P120, "a finer real grid is adopted at once"); + } + + /// The 0.23.0 bug: `preferredDisplayModeId` is a request, so a refused 120 Hz switch seeds a + /// 120 Hz grid on a panel that is really running 60. The narrow-only learner could never + /// climb back, and the presenter aimed at instants the panel never reached. + #[test] + fn widens_back_out_when_the_requested_mode_was_refused() { + let mut g = PanelGrid::seeded(120); + for i in 0..PANEL_WIDEN_STREAK - 1 { + assert!(!g.observe(P60), "sample {i} must not widen on its own"); + assert_eq!(g.period_ns(), P120); + } + assert!( + g.observe(P60), + "a sustained run of wider spacings widens the grid" + ); + assert_eq!(g.period_ns(), P60); + } + + #[test] + fn one_stray_wide_sample_never_widens() { + let mut g = PanelGrid::seeded(120); + for _ in 0..40 { + assert!(!g.observe(P60)); + assert!(!g.observe(P120)); // an agreeing sample breaks the run + } + assert_eq!( + g.period_ns(), + P120, + "alternating samples must not accumulate" + ); + } + + #[test] + fn widening_adopts_the_narrowest_of_the_run() { + let mut g = PanelGrid::seeded(120); + // A run of wide spacings that includes some very wide outliers. + let run = [ + P60, + 33_000_000, + P60 + 400_000, + 41_000_000, + P60, + P60, + P60, + P60, + ]; + for s in run { + g.observe(s); + } + assert_eq!( + g.period_ns(), + P60, + "the estimate takes the narrowest of the run, never an outlier" + ); + } + + #[test] + fn implausible_spacings_are_ignored_entirely() { + let mut g = PanelGrid::seeded(120); + for _ in 0..100 { + assert!(!g.observe(0)); + assert!(!g.observe(-1)); + assert!(!g.observe(1_000_000)); // 1000 Hz — below the range floor + assert!(!g.observe(100_000_000)); // 10 Hz — above the ceiling + } + assert_eq!(g.period_ns(), P120); + } + + #[test] + fn a_transient_narrow_glitch_self_heals() { + // Narrowing is immediate, so a glitch DOES poison the estimate — the point is that it is + // no longer permanent (0.23.0's learner had no way back). + let mut g = PanelGrid::seeded(120); + assert!(g.observe(2_100_000), "a glitch narrows the estimate"); + assert_eq!(g.period_ns(), 2_100_000); + for _ in 0..PANEL_WIDEN_STREAK { + g.observe(P120); + } + assert_eq!(g.period_ns(), P120, "and the real grid wins it back"); + } +}