From 18eae6dc52f26f8c68005533546fd032cf013f72 Mon Sep 17 00:00:00 2001 From: Bruce Mitchener Date: Fri, 3 Jul 2026 03:35:38 +0700 Subject: [PATCH] frameclock: let drivers own frame indices Remove `frame_index` from `FrameTick` so backend ticks describe platform wake facts only: `now`, predicted present, refresh interval, output, and previous actual-present feedback. Make low-level lifecycle identity explicit by changing `Scheduler::plan` to take a `frame_index` argument and replacing `FrameTickEvent::from` with `FrameTickEvent::new(frame_index, &tick)`. Direct scheduler integrations keep their own content-frame id and pass the same value into planning and diagnostics. Move retained identity ownership into `FrameDriver`. The driver now stamps `FramePlan::frame_index` when pending demand is consumed into a plan, retains that id through queued plans, submissions, deferred feedback, drops, and summaries, and removes backend wake counters from Apple, Web, Wayland, and Windows tick helpers. Update examples and docs so `FrameTick` stays backend-neutral while `FrameDriver` and low-level hosts own content-frame lifecycle identity. --- examples/frameclock_simulated/src/main.rs | 5 +- examples/trace_demo/src/main.rs | 5 +- examples/wayland_layers/src/main.rs | 3 +- examples/wayland_lotta_layers/src/main.rs | 3 +- examples/web_video/src/lib.rs | 2 +- examples/windows_layers/src/main.rs | 15 ++-- examples/windows_lotta_layers/src/main.rs | 15 ++-- examples/winit_paced_redraw/src/main.rs | 18 +---- frameclock/README.md | 19 ++--- frameclock/src/diagnostics.rs | 59 ++++++++++----- frameclock/src/driver.rs | 88 +++++++++++++++++++---- frameclock/src/lib.rs | 1 - frameclock/src/scheduler.rs | 53 +++++++++++--- frameclock/src/timing.rs | 31 ++------ frameclock_apple/src/ca_display_link.rs | 10 +-- frameclock_apple/src/cv_display_link.rs | 11 +-- frameclock_apple/src/lib.rs | 2 - frameclock_wayland/src/lib.rs | 2 - frameclock_wayland/src/tick.rs | 46 ++++++------ frameclock_web/README.md | 8 +-- frameclock_web/src/lib.rs | 1 - frameclock_web/src/raf.rs | 7 -- frameclock_windows/README.md | 2 +- frameclock_windows/src/tick.rs | 14 ++-- subduction_backend_windows/src/lib.rs | 9 +-- subduction_backend_windows/src/tick.rs | 8 +-- subduction_core/src/trace.rs | 3 +- 27 files changed, 252 insertions(+), 188 deletions(-) diff --git a/examples/frameclock_simulated/src/main.rs b/examples/frameclock_simulated/src/main.rs index 58eb270..1683f1a 100644 --- a/examples/frameclock_simulated/src/main.rs +++ b/examples/frameclock_simulated/src/main.rs @@ -87,11 +87,10 @@ fn main() { now, predicted_present: Some(predicted_present), refresh_interval: Some(REFRESH_INTERVAL.ticks()), - frame_index, output, prev_actual_present: if frame_index > 0 { Some(now) } else { None }, }; - let tick_event = FrameTickEvent::from(&tick); + let tick_event = FrameTickEvent::new(frame_index, &tick); summary.record_frame_tick(&tick_event); diagnostics.frame_tick(&tick_event); @@ -107,7 +106,7 @@ fn main() { hints, DisplayTiming::from_tick(&tick, REFRESH_INTERVAL), ); - let plan = scheduler.plan(opportunity, FrameDemand::ANIMATION); + let plan = scheduler.plan(opportunity, FrameDemand::ANIMATION, frame_index); let plan_event = FramePlanEvent::new(&plan, scheduler.safety_margin_ticks()); summary.record_frame_plan(&plan_event); diagnostics.frame_plan(&plan_event); diff --git a/examples/trace_demo/src/main.rs b/examples/trace_demo/src/main.rs index a7e024e..1276109 100644 --- a/examples/trace_demo/src/main.rs +++ b/examples/trace_demo/src/main.rs @@ -51,7 +51,6 @@ fn main() { now: HostTime(now_ticks), predicted_present: Some(HostTime(now_ticks + refresh_interval)), refresh_interval: Some(refresh_interval), - frame_index, output: OutputId(0), prev_actual_present: if frame_index > 0 { // Previous frame presented on time. @@ -61,7 +60,7 @@ fn main() { }, }; - let tick_event = FrameTickEvent::from(&tick); + let tick_event = FrameTickEvent::new(frame_index, &tick); pretty.on_frame_tick(&tick_event); recorder.on_frame_tick(&tick_event); @@ -85,7 +84,7 @@ fn main() { hints, DisplayTiming::from_tick(&tick, Duration(refresh_interval)), ); - let plan = scheduler.plan(opportunity, FrameDemand::ANIMATION); + let plan = scheduler.plan(opportunity, FrameDemand::ANIMATION, frame_index); let plan_end = HostTime(now_ticks + 100_000); let plan_event = FramePlanEvent::new(&plan, scheduler.safety_margin_ticks()); diff --git a/examples/wayland_layers/src/main.rs b/examples/wayland_layers/src/main.rs index 1b0c9d6..aac4541 100644 --- a/examples/wayland_layers/src/main.rs +++ b/examples/wayland_layers/src/main.rs @@ -29,6 +29,7 @@ const DEFAULT_W: u32 = 800; const DEFAULT_H: u32 = 600; const NUM_LAYERS: usize = 5; const LAYER_SIZE: u32 = 80; +const UNOBSERVED_FRAME_INDEX: u64 = 0; /// ARGB colors for the five layers. const COLORS: [[u8; 4]; NUM_LAYERS] = [ @@ -164,7 +165,7 @@ fn main() { let build_start = frameclock_wayland::now(); let opportunity = frameclock_wayland::frame_opportunity(tick, Duration(16_666_667)); - let plan = scheduler.plan(opportunity, FrameDemand::ANIMATION); + let plan = scheduler.plan(opportunity, FrameDemand::ANIMATION, UNOBSERVED_FRAME_INDEX); let elapsed_nanos = plan.sample_time.ticks().saturating_sub(start_nanos); let t = elapsed_nanos as f64 / 1_000_000_000.0; diff --git a/examples/wayland_lotta_layers/src/main.rs b/examples/wayland_lotta_layers/src/main.rs index 47483cd..387621e 100644 --- a/examples/wayland_lotta_layers/src/main.rs +++ b/examples/wayland_lotta_layers/src/main.rs @@ -29,6 +29,7 @@ const DEFAULT_W: u32 = 1024; const DEFAULT_H: u32 = 768; const NUM_GROUPS: usize = 10; const LAYERS_PER_GROUP: usize = 10; +const UNOBSERVED_FRAME_INDEX: u64 = 0; /// Returns an `[r, g, b]` triple in 0–255 for a given index using /// golden-angle hue spacing. @@ -194,7 +195,7 @@ fn main() { let build_start = frameclock_wayland::now(); let opportunity = frameclock_wayland::frame_opportunity(tick, Duration(16_666_667)); - let plan = scheduler.plan(opportunity, FrameDemand::ANIMATION); + let plan = scheduler.plan(opportunity, FrameDemand::ANIMATION, UNOBSERVED_FRAME_INDEX); let elapsed_nanos = plan.sample_time.ticks().saturating_sub(start_nanos); let t = elapsed_nanos as f64 / 1_000_000_000.0; diff --git a/examples/web_video/src/lib.rs b/examples/web_video/src/lib.rs index db6bf00..d6f0b8a 100644 --- a/examples/web_video/src/lib.rs +++ b/examples/web_video/src/lib.rs @@ -688,7 +688,7 @@ fn on_tick(state: &Rc>, tick: FrameTick) { let present_bucket = (phase_target * emu_refresh_hz).floor().max(0.0) as u64; let timecode_text = format!( "F {:06} | PT_BUCKET {:08} | beat {:05} | timing {}", - tick.frame_index, present_bucket, beat_idx, presentation_timing_label + plan.frame_index, present_bucket, beat_idx, presentation_timing_label ); s.ui.timecode.set_text_content(Some(&timecode_text)); diff --git a/examples/windows_layers/src/main.rs b/examples/windows_layers/src/main.rs index ae4e018..37c9416 100644 --- a/examples/windows_layers/src/main.rs +++ b/examples/windows_layers/src/main.rs @@ -270,23 +270,24 @@ unsafe extern "system" fn wnd_proc( fn on_tick() { let Some(s) = state_mut() else { return }; - s.frame_index += 1; - let tick = make_tick(REFRESH_NS, s.frame_index, s.prev_present_time); - let frame_index = tick.frame_index; + let frame_index = s.frame_index; + s.frame_index = s.frame_index.saturating_add(1); + let tick = make_tick(REFRESH_NS, s.prev_present_time); // Resolve previous frame's feedback. if let Some(pending) = s.pending_feedback.take() { + let pending_frame_index = pending.plan.frame_index; let feedback = pending.resolve(tick.prev_actual_present); s.scheduler.observe(&feedback); s.recorder.on_present_feedback(&PresentFeedbackEvent { - frame_index: frame_index.saturating_sub(1), + frame_index: pending_frame_index, actual_present: tick.prev_actual_present, missed_deadline: feedback.missed_deadline, pacing_overrun: feedback.pacing_overrun, }); } - let tick_event = FrameTickEvent::from(&tick); + let tick_event = FrameTickEvent::new(frame_index, &tick); s.recorder.on_frame_tick(&tick_event); // --- Plan phase --- @@ -303,7 +304,9 @@ fn on_tick() { hints, DisplayTiming::from_tick(&tick, Duration(16_666_667)), ); - let plan = s.scheduler.plan(opportunity, FrameDemand::ANIMATION); + let plan = s + .scheduler + .plan(opportunity, FrameDemand::ANIMATION, frame_index); let plan_end = backend::now(); s.recorder.on_phase_end(&PhaseEndEvent { diff --git a/examples/windows_lotta_layers/src/main.rs b/examples/windows_lotta_layers/src/main.rs index 6a45c06..1fd8b4d 100644 --- a/examples/windows_lotta_layers/src/main.rs +++ b/examples/windows_lotta_layers/src/main.rs @@ -272,23 +272,24 @@ unsafe extern "system" fn wnd_proc( fn on_tick() { let Some(s) = state_mut() else { return }; - s.frame_index += 1; - let tick = make_tick(REFRESH_NS, s.frame_index, s.prev_present_time); - let frame_index = tick.frame_index; + let frame_index = s.frame_index; + s.frame_index = s.frame_index.saturating_add(1); + let tick = make_tick(REFRESH_NS, s.prev_present_time); // Resolve previous frame's feedback. if let Some(pending) = s.pending_feedback.take() { + let pending_frame_index = pending.plan.frame_index; let feedback = pending.resolve(tick.prev_actual_present); s.scheduler.observe(&feedback); s.recorder.on_present_feedback(&PresentFeedbackEvent { - frame_index: frame_index.saturating_sub(1), + frame_index: pending_frame_index, actual_present: tick.prev_actual_present, missed_deadline: feedback.missed_deadline, pacing_overrun: feedback.pacing_overrun, }); } - let tick_event = FrameTickEvent::from(&tick); + let tick_event = FrameTickEvent::new(frame_index, &tick); s.recorder.on_frame_tick(&tick_event); // --- Plan phase --- @@ -305,7 +306,9 @@ fn on_tick() { hints, DisplayTiming::from_tick(&tick, Duration(16_666_667)), ); - let plan = s.scheduler.plan(opportunity, FrameDemand::ANIMATION); + let plan = s + .scheduler + .plan(opportunity, FrameDemand::ANIMATION, frame_index); let plan_end = backend::now(); s.recorder.on_phase_end(&PhaseEndEvent { diff --git a/examples/winit_paced_redraw/src/main.rs b/examples/winit_paced_redraw/src/main.rs index 89080b1..ff1fd02 100644 --- a/examples/winit_paced_redraw/src/main.rs +++ b/examples/winit_paced_redraw/src/main.rs @@ -52,7 +52,6 @@ struct WindowState { struct SurfaceFrameClock { driver: FrameDriver, - frame_index: u64, output: OutputId, } @@ -188,12 +187,8 @@ impl WindowState { // Plain winit does not expose a future present timestamp here, so this // example uses a pacing-only opportunity with a conservative "submit // by around the next refresh" boundary. - let opportunity = FrameOpportunity::pacing_only( - now, - REFRESH_INTERVAL, - self.surface_clock.frame_index, - self.surface_clock.output, - ); + let opportunity = + FrameOpportunity::pacing_only(now, REFRESH_INTERVAL, self.surface_clock.output); self.surface_clock.driver.begin_frame(opportunity).result } @@ -221,7 +216,6 @@ impl WindowState { // content frame and ask for fresh demand if the work still // matters. let retry_demand = summary.demand; - self.surface_clock.frame_index += 1; if !retry_demand.is_empty() { self.request_frame(retry_demand); } else if self.surface_clock.driver.has_pending_demand() { @@ -263,7 +257,7 @@ impl WindowState { .summary .expect("pacing-only submission should resolve immediately"); - if self.surface_clock.frame_index.is_multiple_of(60) { + if plan.frame_index.is_multiple_of(60) { self.window.set_title(&format!( "Frameclock + winit: mode={} sample={}ms x={}", work_mode.label(), @@ -283,11 +277,6 @@ impl WindowState { ); } - // `frame_index` is a per-output content-frame id. Advance it after the - // active frame is submitted, not for every frame-start wake that only - // releases a queued plan. - self.surface_clock.frame_index += 1; - // If lower-priority demand was retained behind the queued frame we // just consumed, wake winit again so the driver can plan it on a fresh // turn. The host does not inspect or rank those demand bits itself. @@ -333,7 +322,6 @@ impl SurfaceFrameClock { // policy can still raise depth if repeated overruns show that this is // too aggressive. driver: FrameDriver::new(config), - frame_index: 0, output, } } diff --git a/frameclock/README.md b/frameclock/README.md index 9f30d81..9d9c6ae 100644 --- a/frameclock/README.md +++ b/frameclock/README.md @@ -32,7 +32,7 @@ Use `FrameDemand` as the host-owned reason a frame is needed. Request dragging, `ANIMATION` while a visual timeline is active, and `BACKGROUND` for deferrable visual work. With `FrameDriver`, call `request(demand)` when those causes arrive; with the low-level scheduler, pass the demand to -`Scheduler::plan(opportunity, demand)`. +`Scheduler::plan(opportunity, demand, frame_index)`. Demand also remains attached to the selected `FramePlan`. Once `FrameBeginResult::Ready` returns, use `frame.plan().demand` to choose the app's @@ -66,10 +66,13 @@ redraw requests, renderer submission, and native presentation resources. Use `FrameDriver::next_frame_start` as one wake source to merge with app timers. After submitting or discarding an `ActiveFrame`, hosts should request another redraw when `FrameDriver::has_pending_demand()` is still true. -`FrameTick::frame_index` is host-owned per output and identifies one planned -content frame. Hosts using `FrameDriver` normally increment it after an -`ActiveFrame` is submitted or discarded, not every time a frame-start wake -fires while a plan is queued. + +`FrameDriver` also owns the retained content-frame counter. It advances that +counter only when a planned frame becomes ready or expires, so queued plans +that are preempted or cleared do not leave diagnostic gaps. The reported +`FramePlan::frame_index` is used for submit/drop summaries. Low-level +`Scheduler` integrations own their own content-frame id and pass it explicitly +to `Scheduler::plan` and `FrameTickEvent::new`. The lower-level `Scheduler` remains available for custom integrations. Event structs and `FrameTimingSummaryBuilder` live under `frameclock::diagnostics` @@ -117,7 +120,6 @@ driver.request(FrameDemand::ANIMATION); let opportunity = FrameOpportunity::pacing_only( HostTime(1_000_000), Duration(16_666_667), - 1, OutputId(0), ); @@ -198,8 +200,9 @@ The split also tightens names around timing semantics: - `FramePlan::present_time` is now `FramePlan::target_present`. - `FramePlan::frame_start` is now the scheduler-selected time to wake or start app-side frame work before `FramePlan::commit_deadline`. -- `Scheduler::plan` now takes a `FrameOpportunity` plus `FrameDemand` so - display timing facts and demand remain explicit policy inputs. +- `Scheduler::plan` now takes a `FrameOpportunity`, `FrameDemand`, and explicit + frame index so display timing facts, demand, and lifecycle identity remain + explicit policy inputs. - `FrameDemand::dominant_class` and `FrameDemand::preempts` expose the demand ordering used by the scheduler. - `FrameDriver` owns pending demand and queued frame-start plans for hosts that diff --git a/frameclock/src/diagnostics.rs b/frameclock/src/diagnostics.rs index 9783448..eeea763 100644 --- a/frameclock/src/diagnostics.rs +++ b/frameclock/src/diagnostics.rs @@ -22,12 +22,12 @@ use crate::timing::{FrameDemand, FramePlan, FrameTick, PresentFeedback, Presenta /// Diagnostics event created from a [`FrameTick`] when a frame opportunity arrives. /// -/// Platform adapters or [`FrameDriver`](crate::FrameDriver) create this with -/// [`FrameTickEvent::from`] and pass it to a [`DiagnosticsSink`] or +/// Platform adapters or [`FrameDriver`](crate::FrameDriver) create this with a +/// lifecycle-owned frame index and pass it to a [`DiagnosticsSink`] or /// [`FrameTimingSummaryBuilder`]. #[derive(Clone, Copy, Debug)] pub struct FrameTickEvent { - /// Monotonic frame counter. + /// Monotonic content-frame counter. pub frame_index: u64, /// Target output for this tick. pub output: OutputId, @@ -39,10 +39,18 @@ pub struct FrameTickEvent { pub refresh_interval: Option, } -impl From<&FrameTick> for FrameTickEvent { - fn from(tick: &FrameTick) -> Self { +impl FrameTickEvent { + /// Creates a diagnostics event from a frame index and platform tick facts. + /// + /// `frame_index` is the content-frame id carried by the matching + /// [`FramePlan`]. [`FrameDriver`](crate::FrameDriver) assigns this + /// internally for reported frames; low-level integrations that call + /// [`Scheduler::plan`](crate::scheduler::Scheduler::plan) directly should + /// pass the same index to both `Scheduler::plan` and this constructor. + #[must_use] + pub const fn new(frame_index: u64, tick: &FrameTick) -> Self { Self { - frame_index: tick.frame_index, + frame_index, output: tick.output, now: tick.now, predicted_present: tick.predicted_present, @@ -59,7 +67,7 @@ impl From<&FrameTick> for FrameTickEvent { /// to create it manually for frame summaries. #[derive(Clone, Copy, Debug)] pub struct FramePlanEvent { - /// Monotonic frame counter. + /// Monotonic content-frame counter. pub frame_index: u64, /// Target output for this plan. pub output: OutputId, @@ -111,7 +119,7 @@ impl FramePlanEvent { /// from [`FrameSubmission`](crate::FrameSubmission). #[derive(Clone, Copy, Debug)] pub struct SubmitEvent { - /// Monotonic frame counter. + /// Monotonic content-frame counter. pub frame_index: u64, /// Host time of submission. pub submitted_at: HostTime, @@ -127,7 +135,7 @@ pub struct SubmitEvent { /// it internally during [`submit_frame`](crate::FrameDriver::submit_frame). #[derive(Clone, Copy, Debug)] pub struct PresentFeedbackEvent { - /// Monotonic frame counter. + /// Monotonic content-frame counter. pub frame_index: u64, /// Actual presentation time, if reported by the platform. pub actual_present: Option, @@ -174,7 +182,7 @@ pub enum FrameDropReason { /// [`Scheduler::observe`](crate::scheduler::Scheduler::observe). #[derive(Clone, Copy, Debug)] pub struct FrameDropEvent { - /// Monotonic frame counter. + /// Monotonic content-frame counter. pub frame_index: u64, /// Target output for the dropped frame. pub output: OutputId, @@ -229,7 +237,7 @@ pub enum FrameTimingBasis { /// renderer-owned concepts. #[derive(Clone, Copy, Debug, PartialEq)] pub struct FrameTimingSummary { - /// Monotonic frame counter. + /// Monotonic content-frame counter. pub frame_index: u64, /// Target output for this frame. pub output: OutputId, @@ -361,14 +369,23 @@ impl FrameTimingSummaryBuilder { /// Produces a frame timing summary. /// - /// Returns `None` when the required tick and plan are missing or refer to - /// different frames. Optional submit and feedback events are included only - /// when their frame index matches the plan. + /// Returns `None` when the required tick and plan are missing. When both + /// are present but refer to different frames, debug builds assert and + /// release builds return `None`. Optional submit and feedback events are + /// included only when their frame index matches the plan. #[must_use] pub fn finish(self) -> Option { let tick = self.tick?; let plan = self.plan?; if tick.frame_index != plan.frame_index || tick.output != plan.output { + debug_assert!( + tick.frame_index == plan.frame_index && tick.output == plan.output, + "frame timing summary requires matching tick and plan: tick frame_index={} output={:?}, plan frame_index={} output={:?}", + tick.frame_index, + tick.output, + plan.frame_index, + plan.output + ); return None; } @@ -634,14 +651,24 @@ mod tests { assert!(builder.finish().is_none()); } + #[cfg_attr( + debug_assertions, + should_panic(expected = "frame timing summary requires matching tick and plan") + )] #[test] fn timing_summary_rejects_mismatched_tick_and_plan() { let mut tick = sample_tick(); tick.frame_index = 8; - let summary = FrameTimingSummaryBuilder::from_tick_and_plan(&tick, &sample_plan()).finish(); + #[cfg(debug_assertions)] + let _ = FrameTimingSummaryBuilder::from_tick_and_plan(&tick, &sample_plan()).finish(); - assert!(summary.is_none()); + #[cfg(not(debug_assertions))] + { + let summary = + FrameTimingSummaryBuilder::from_tick_and_plan(&tick, &sample_plan()).finish(); + assert!(summary.is_none()); + } } #[test] diff --git a/frameclock/src/driver.rs b/frameclock/src/driver.rs index 70fed05..2e50982 100644 --- a/frameclock/src/driver.rs +++ b/frameclock/src/driver.rs @@ -336,8 +336,11 @@ enum DriverBeginResult { /// another redraw so weaker retained demand can be planned on a fresh turn. /// /// This keeps demand preemption, feedback observation, and summary construction -/// inside `frameclock` while leaving timer queues, event-loop wake mechanics, -/// renderer submission, and native surface lifecycle in the host. +/// inside `frameclock`. The driver also owns `FramePlan::frame_index`, and it +/// advances the counter only when a planned frame becomes ready or expires. +/// Queued plans that are preempted or explicitly cleared do not consume an id. +/// Timer queues, event-loop wake mechanics, renderer submission, and native +/// surface lifecycle stay in the host. /// /// # Queued plans /// @@ -358,6 +361,7 @@ pub struct FrameDriver { pending_demand: FrameDemand, pending_frame: Option, pending_feedback: Option, + next_frame_index: u64, } #[derive(Debug)] @@ -392,6 +396,7 @@ impl FrameDriver { pending_demand: FrameDemand::NONE, pending_frame: None, pending_feedback: None, + next_frame_index: 0, } } @@ -417,6 +422,9 @@ impl FrameDriver { } /// Returns the currently queued frame, if any. + /// + /// The queued plan carries the next candidate frame index. The driver + /// consumes that id only if the queued plan later becomes ready or expires. #[must_use] pub const fn pending_frame(&self) -> Option { self.pending_frame @@ -595,8 +603,8 @@ impl FrameDriver { submitted_at: HostTime, feedback: &PresentFeedback, ) -> FrameTimingSummary { - let tick_event = FrameTickEvent::from(&planned.tick); let plan = planned.plan; + let tick_event = FrameTickEvent::new(plan.frame_index, &planned.tick); let plan_event = FramePlanEvent::new(&plan, planned.safety_margin_ticks); let submit_event = SubmitEvent { frame_index: plan.frame_index, @@ -647,8 +655,8 @@ impl FrameDriver { frame: ActiveFrame, reason: FrameDropReason, ) -> FrameTimingSummary { - let tick_event = FrameTickEvent::from(&frame.tick()); let plan = frame.plan(); + let tick_event = FrameTickEvent::new(plan.frame_index, &frame.tick()); let plan_event = FramePlanEvent::new(&plan, frame.safety_margin_ticks()); let drop_event = FrameDropEvent::new(&plan, reason); let state_event = SchedulerStateEvent { @@ -677,6 +685,7 @@ impl FrameDriver { } self.pending_frame = None; + let frame = self.consume_frame_index(frame); if tick.now > frame.plan.commit_deadline { return DriverBeginResult::Expired(frame); } @@ -689,7 +698,9 @@ impl FrameDriver { let demand = self.pending_demand; self.pending_demand = FrameDemand::NONE; - let plan = self.scheduler.plan(opportunity, demand); + let plan = self + .scheduler + .plan(opportunity, demand, self.next_frame_index); let frame = PlannedFrame::new( tick, plan, @@ -701,12 +712,20 @@ impl FrameDriver { self.pending_frame = Some(frame); DriverBeginResult::WaitUntil(frame_start) } else if tick.now > frame.plan.commit_deadline { + let frame = self.consume_frame_index(frame); DriverBeginResult::Expired(frame) } else { + let frame = self.consume_frame_index(frame); DriverBeginResult::Ready(frame) } } + fn consume_frame_index(&mut self, mut frame: PlannedFrame) -> PlannedFrame { + frame.plan.frame_index = self.next_frame_index; + self.next_frame_index = self.next_frame_index.saturating_add(1); + frame + } + /// Drops the queued frame and returns whether one existed. /// /// This does not clear [`pending_demand`](Self::pending_demand). Demand @@ -744,12 +763,11 @@ mod tests { FrameDriver::new(config) } - fn tick(now: u64, frame_index: u64) -> FrameTick { + fn tick(now: u64, _frame_index: u64) -> FrameTick { FrameTick { now: HostTime(now), predicted_present: None, refresh_interval: Some(REFRESH_INTERVAL.ticks()), - frame_index, output: OutputId(0), prev_actual_present: None, } @@ -769,7 +787,7 @@ mod tests { fn predictive_opportunity( now: u64, - frame_index: u64, + _frame_index: u64, desired_present: u64, latest_commit: u64, ) -> FrameOpportunity { @@ -777,7 +795,6 @@ mod tests { now: HostTime(now), predicted_present: Some(HostTime(desired_present)), refresh_interval: Some(REFRESH_INTERVAL.ticks()), - frame_index, output: OutputId(0), prev_actual_present: None, }; @@ -812,7 +829,7 @@ mod tests { #[test] fn pacing_only_opportunity_fills_common_host_defaults() { let opportunity = - FrameOpportunity::pacing_only(HostTime(12), REFRESH_INTERVAL, 42, OutputId(9)); + FrameOpportunity::pacing_only(HostTime(12), REFRESH_INTERVAL, OutputId(9)); assert_eq!(opportunity.tick.now, HostTime(12)); assert_eq!(opportunity.tick.predicted_present, None); @@ -820,7 +837,6 @@ mod tests { opportunity.tick.refresh_interval, Some(REFRESH_INTERVAL.ticks()) ); - assert_eq!(opportunity.tick.frame_index, 42); assert_eq!(opportunity.tick.output, OutputId(9)); assert_eq!( opportunity.hints.presentation_timing(), @@ -885,6 +901,20 @@ mod tests { assert_eq!(frame.plan().sample_time, HostTime(100)); } + #[test] + fn begin_frame_assigns_sequential_frame_indices() { + let mut driver = driver(); + driver.request(FrameDemand::INPUT); + + let first = ready_at(&mut driver, 10); + assert_eq!(first.plan().frame_index, 0); + let _ = driver.discard_frame(first); + + driver.request(FrameDemand::INPUT); + let second = ready_at(&mut driver, 20); + assert_eq!(second.plan().frame_index, 1); + } + #[test] fn expired_queued_frame_returns_drop_summary() { let mut driver = driver(); @@ -948,6 +978,23 @@ mod tests { assert_eq!(frame.plan().frame_start, HostTime(1)); } + #[test] + fn preempted_queued_plan_does_not_consume_frame_index() { + let mut driver = driver(); + driver.request(FrameDemand::ANIMATION); + assert!(matches!( + begin_at(&mut driver, 0), + FrameBeginResult::WaitUntil(HostTime(90)) + )); + + driver.request(FrameDemand::INPUT); + + let frame = ready_at(&mut driver, 1); + assert_eq!(frame.plan().frame_index, 0); + assert!(frame.plan().demand.contains(FrameDemand::INPUT)); + assert!(frame.plan().demand.contains(FrameDemand::ANIMATION)); + } + #[test] fn weaker_demand_waits_behind_queued_plan() { let mut driver = driver(); @@ -1006,6 +1053,23 @@ mod tests { assert!(!driver.clear_pending_frame()); } + #[test] + fn cleared_queued_plan_does_not_consume_frame_index() { + let mut driver = driver(); + driver.request(FrameDemand::ANIMATION); + assert!(matches!( + begin_at(&mut driver, 0), + FrameBeginResult::WaitUntil(HostTime(90)) + )); + + assert!(driver.clear_pending_frame()); + driver.request(FrameDemand::INPUT); + + let frame = ready_at(&mut driver, 10); + assert_eq!(frame.plan().frame_index, 0); + assert_eq!(frame.plan().demand, FrameDemand::INPUT); + } + #[test] fn weaker_retained_demand_survives_stronger_frame_submission() { let mut driver = driver(); @@ -1040,8 +1104,8 @@ mod tests { submission.submitted_at, submission.actual_present(), ); - let tick_event = FrameTickEvent::from(&frame.tick()); let plan = frame.plan(); + let tick_event = FrameTickEvent::new(plan.frame_index, &frame.tick()); let plan_event = FramePlanEvent::new(&plan, frame.safety_margin_ticks()); let submit_event = SubmitEvent { frame_index: plan.frame_index, diff --git a/frameclock/src/lib.rs b/frameclock/src/lib.rs index 02dace9..583970f 100644 --- a/frameclock/src/lib.rs +++ b/frameclock/src/lib.rs @@ -66,7 +66,6 @@ //! let opportunity = FrameOpportunity::pacing_only( //! HostTime(1_000_000), //! Duration(16_666_667), -//! 1, //! OutputId(0), //! ); //! diff --git a/frameclock/src/scheduler.rs b/frameclock/src/scheduler.rs index 4eb7770..806832c 100644 --- a/frameclock/src/scheduler.rs +++ b/frameclock/src/scheduler.rs @@ -249,7 +249,7 @@ fn f64_ticks_to_u64(ticks: f64) -> u64 { /// # Usage /// /// ```rust,ignore -/// let plan = scheduler.plan(opportunity, demand); +/// let plan = scheduler.plan(opportunity, demand, frame_index); /// // ... build and submit frame ... /// scheduler.observe(&feedback); /// ``` @@ -288,14 +288,24 @@ impl Scheduler { } } - /// Produces a [`FramePlan`] from a frame opportunity and demand. + /// Produces a [`FramePlan`] from a frame opportunity, demand, and frame id. /// /// Hosts should usually call this only with non-empty [`FrameDemand`]. /// `FrameDemand::NONE` is accepted for passive pacing diagnostics or /// backend bookkeeping, but it should not be treated as ordinary render /// demand. + /// + /// `frame_index` identifies the planned content frame for diagnostics and + /// summaries. [`FrameDriver`](crate::FrameDriver) owns this counter for + /// retained hosts; low-level scheduler integrations pass their own + /// lifecycle id here. #[must_use] - pub fn plan(&mut self, opportunity: FrameOpportunity, demand: FrameDemand) -> FramePlan { + pub fn plan( + &mut self, + opportunity: FrameOpportunity, + demand: FrameDemand, + frame_index: u64, + ) -> FramePlan { let tick = opportunity.tick; let hints = opportunity.hints; let source_interval = self.source_interval(opportunity); @@ -348,7 +358,7 @@ impl Scheduler { commit_deadline, pipeline_depth: self.pipeline_depth, output: tick.output, - frame_index: tick.frame_index, + frame_index, } } @@ -566,7 +576,6 @@ mod tests { now: HostTime(now), predicted_present: predicted.map(HostTime), refresh_interval: Some(REFRESH_INTERVAL.ticks()), - frame_index: 0, output: OutputId(0), prev_actual_present: None, } @@ -613,6 +622,7 @@ mod tests { let plan = sched.plan( make_opportunity(PresentationTiming::Predictive, 1000, Some(2000), 1800), FrameDemand::ANIMATION, + 0, ); assert_eq!(plan.demand, FrameDemand::ANIMATION); @@ -623,6 +633,20 @@ mod tests { assert_eq!(plan.frame_start, HostTime(1000)); } + #[test] + fn plan_uses_explicit_frame_index() { + let config = SchedulerConfig::predictive(); + let mut sched = Scheduler::new(config); + + let plan = sched.plan( + make_opportunity(PresentationTiming::Predictive, 1000, Some(2000), 1800), + FrameDemand::ANIMATION, + 42, + ); + + assert_eq!(plan.frame_index, 42); + } + #[test] fn pacing_only_plan_has_no_target_present() { let config = SchedulerConfig::pacing_only(); @@ -631,6 +655,7 @@ mod tests { let plan = sched.plan( make_opportunity(PresentationTiming::PacingOnly, 1_000_000, None, 17_000_000), FrameDemand::ANIMATION, + 0, ); assert_eq!(plan.target_present, None); @@ -648,6 +673,7 @@ mod tests { let plan = sched.plan( make_opportunity(PresentationTiming::Predictive, 1000, Some(2000), 1800), FrameDemand::ANIMATION, + 0, ); assert_eq!(plan.frame_start, HostTime(1550)); @@ -672,6 +698,7 @@ mod tests { let plan = sched.plan( make_opportunity(PresentationTiming::Predictive, 1_000, Some(2_000), 2_000), FrameDemand::ANIMATION, + 0, ); assert_eq!(sched.safety_margin_ticks(), 400); @@ -707,6 +734,7 @@ mod tests { let plan = sched.plan( make_opportunity(PresentationTiming::Predictive, 1_500, Some(2_000), 1_800), FrameDemand::ANIMATION, + 0, ); assert_eq!(plan.frame_start, HostTime(1_500)); @@ -724,7 +752,7 @@ mod tests { display_timing: DisplayTiming::fixed(REFRESH_INTERVAL), }; - let plan = sched.plan(opportunity, FrameDemand::ANIMATION); + let plan = sched.plan(opportunity, FrameDemand::ANIMATION, 0); assert_eq!(plan.target_present, None); assert_eq!(plan.sample_time, HostTime(2_000) + REFRESH_INTERVAL); @@ -741,6 +769,7 @@ mod tests { let plan = sched.plan( make_opportunity(PresentationTiming::Predictive, 1_000, Some(2_000), 1_800), FrameDemand::INPUT, + 0, ); assert_eq!(plan.demand, FrameDemand::INPUT); @@ -772,6 +801,7 @@ mod tests { 16_666_667, ), FrameDemand::ANIMATION, + 0, ); assert_eq!(plan.frame_interval, REFRESH_INTERVAL.saturating_mul(2)); @@ -805,6 +835,7 @@ mod tests { 50_000_000, ), FrameDemand::ANIMATION, + 0, ); assert_eq!(sched.safety_margin_ticks(), 40_000_000); @@ -832,7 +863,6 @@ mod tests { now: HostTime(1_000_000), predicted_present: Some(HostTime(9_333_333)), refresh_interval: Some(8_333_333), - frame_index: 0, output: OutputId(0), prev_actual_present: None, }; @@ -846,7 +876,7 @@ mod tests { ), }; - let plan = sched.plan(opportunity, FrameDemand::ANIMATION); + let plan = sched.plan(opportunity, FrameDemand::ANIMATION, 0); assert_eq!(plan.frame_interval, Duration(16_666_666)); assert_eq!(plan.target_present, Some(HostTime(17_666_666))); @@ -873,7 +903,6 @@ mod tests { now: HostTime(1_000_000), predicted_present: Some(HostTime(9_333_333)), refresh_interval: Some(8_333_333), - frame_index: 0, output: OutputId(0), prev_actual_present: None, }; @@ -887,7 +916,7 @@ mod tests { ), }; - let plan = sched.plan(opportunity, FrameDemand::ANIMATION); + let plan = sched.plan(opportunity, FrameDemand::ANIMATION, 0); assert_eq!(plan.frame_interval, Duration(15_000_000)); assert_eq!(plan.target_present, Some(HostTime(16_000_000))); @@ -928,6 +957,7 @@ mod tests { let plan = sched.plan( make_opportunity(PresentationTiming::Predictive, 1_000, Some(2_000), 1_800), FrameDemand::ANIMATION, + 0, ); let lookahead = REFRESH_INTERVAL.saturating_mul(2); @@ -952,6 +982,7 @@ mod tests { let plan = sched.plan( make_opportunity(PresentationTiming::Predictive, 1_000, Some(2_000), 1_800), FrameDemand::INPUT, + 0, ); assert_eq!(plan.pipeline_depth, 3); @@ -971,6 +1002,7 @@ mod tests { let plan = sched.plan( make_opportunity(PresentationTiming::Predictive, 1_000, Some(2_000), 1_800), FrameDemand::CONTINUOUS_INPUT, + 0, ); assert_eq!(plan.pipeline_depth, 3); @@ -1122,6 +1154,7 @@ mod tests { let plan = sched.plan( make_opportunity(PresentationTiming::Estimated, 1000, Some(2000), 1800), FrameDemand::ANIMATION, + 0, ); // Estimated behaves like Predictive for target selection; hosts choose diff --git a/frameclock/src/timing.rs b/frameclock/src/timing.rs index d685b91..7cdd231 100644 --- a/frameclock/src/timing.rs +++ b/frameclock/src/timing.rs @@ -248,14 +248,6 @@ pub struct FrameTick { pub predicted_present: Option, /// Display refresh interval in host-time ticks, if known. pub refresh_interval: Option, - /// Host-owned monotonically increasing frame counter for this output. - /// - /// Keep this stable for the full lifecycle of one planned content frame: - /// tick, plan, submit/feedback, and drop diagnostics all use this value to - /// join events. With [`FrameDriver`](crate::FrameDriver), increment it - /// after an [`ActiveFrame`](crate::ActiveFrame) is submitted or discarded, - /// not every time a frame-start wake fires while a plan is queued. - pub frame_index: u64, /// Which output this tick is for. pub output: OutputId, /// Actual present time of the *previous* frame, if the backend can report @@ -275,12 +267,6 @@ pub struct FrameTick { /// when using the retained lifecycle API, or to /// [`Scheduler::plan`](crate::scheduler::Scheduler::plan) when using the /// lower-level scheduler directly. -/// -/// `frame_index` lives on [`FrameTick`] and is owned by the host/backend. When -/// using [`FrameDriver`](crate::FrameDriver), advance it after an -/// [`ActiveFrame`](crate::ActiveFrame) is submitted or discarded. Do not -/// advance it merely because a frame-start wake fired while an older planned -/// frame was still queued. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub struct FrameOpportunity { /// Platform frame opportunity. @@ -318,17 +304,11 @@ impl FrameOpportunity { /// - [`DisplayTiming::fixed`] with `refresh_interval`. #[inline] #[must_use] - pub fn pacing_only( - now: HostTime, - refresh_interval: Duration, - frame_index: u64, - output: OutputId, - ) -> Self { + pub fn pacing_only(now: HostTime, refresh_interval: Duration, output: OutputId) -> Self { let tick = FrameTick { now, predicted_present: None, refresh_interval: Some(refresh_interval.ticks()), - frame_index, output, prev_actual_present: None, }; @@ -373,10 +353,12 @@ pub struct FramePlan { pub pipeline_depth: u8, /// Which output this frame targets. pub output: OutputId, - /// Frame counter, carried from the originating [`FrameTick`]. + /// Monotonic content-frame counter used for diagnostics and summaries. /// - /// This identifies the planned content frame, not necessarily the host - /// wake that eventually made the queued frame ready. + /// Retained hosts get this from [`FrameDriver`](crate::FrameDriver), which + /// advances its counter when a planned frame becomes ready or expires. + /// Low-level scheduler integrations pass the value explicitly to + /// [`Scheduler::plan`](crate::scheduler::Scheduler::plan). pub frame_index: u64, } @@ -698,7 +680,6 @@ mod tests { now: HostTime(now), predicted_present: predicted.map(HostTime), refresh_interval, - frame_index: 0, output: OutputId(0), prev_actual_present: None, } diff --git a/frameclock_apple/src/ca_display_link.rs b/frameclock_apple/src/ca_display_link.rs index c3e5748..1a65142 100644 --- a/frameclock_apple/src/ca_display_link.rs +++ b/frameclock_apple/src/ca_display_link.rs @@ -19,7 +19,7 @@ use crate::{PreferredFrameRateRange, mach_time, preferred_frame_rate_range}; struct DisplayLinkTargetIvars { callback: Box, - frame_counter: Cell, + has_previous_timestamp: Cell, output: OutputId, timebase: Timebase, } @@ -49,7 +49,7 @@ impl DisplayLinkTarget { let tb = mach_time::timebase(); let this = mtm.alloc::().set_ivars(DisplayLinkTargetIvars { callback: Box::new(callback), - frame_counter: Cell::new(0), + has_previous_timestamp: Cell::new(false), output, timebase: tb, }); @@ -71,10 +71,7 @@ impl DisplayLinkTarget { mach_time::media_time_to_host_time(target_ts, now, ca_now, ivars.timebase); let refresh_interval = mach_time::seconds_to_ticks(duration, ivars.timebase); - let frame_index = ivars.frame_counter.get(); - ivars.frame_counter.set(frame_index + 1); - - let prev_actual_present = if frame_index > 0 { + let prev_actual_present = if ivars.has_previous_timestamp.replace(true) { mach_time::media_time_to_host_time(timestamp, now, ca_now, ivars.timebase) } else { None @@ -84,7 +81,6 @@ impl DisplayLinkTarget { now, predicted_present, refresh_interval: Some(refresh_interval), - frame_index, output: ivars.output, prev_actual_present, }; diff --git a/frameclock_apple/src/cv_display_link.rs b/frameclock_apple/src/cv_display_link.rs index 853d49d..0b3276e 100644 --- a/frameclock_apple/src/cv_display_link.rs +++ b/frameclock_apple/src/cv_display_link.rs @@ -8,7 +8,6 @@ use core::ffi::c_void; use core::fmt; use core::pin::Pin; use core::ptr::NonNull; -use core::sync::atomic::{AtomicU64, Ordering}; use frameclock::time::Timebase; use frameclock::{FrameTick, HostTime, OutputId}; @@ -47,7 +46,6 @@ impl core::error::Error for DisplayLinkError {} struct CallbackState { sender: TickSender, - frame_counter: AtomicU64, output: OutputId, } @@ -88,11 +86,7 @@ impl DisplayLink { reason = "CVDisplayLink API is deprecated by Apple but still functional" )] pub fn new(sender: TickSender, output: OutputId) -> Result { - let state = Box::pin(CallbackState { - sender, - frame_counter: AtomicU64::new(0), - output, - }); + let state = Box::pin(CallbackState { sender, output }); let mut link_ptr: *mut CVDisplayLinkRaw = core::ptr::null_mut(); let ret = unsafe { @@ -218,13 +212,10 @@ unsafe extern "C-unwind" fn display_link_callback( None }; - let frame_index = state.frame_counter.fetch_add(1, Ordering::Relaxed); - let tick = FrameTick { now, predicted_present: Some(predicted_present), refresh_interval, - frame_index, output: state.output, prev_actual_present: None, }; diff --git a/frameclock_apple/src/lib.rs b/frameclock_apple/src/lib.rs index 5745688..b087a2f 100644 --- a/frameclock_apple/src/lib.rs +++ b/frameclock_apple/src/lib.rs @@ -291,7 +291,6 @@ mod tests { now: HostTime(1_000_000), predicted_present, refresh_interval: Some(16_666_667), - frame_index: 7, output: OutputId(0), prev_actual_present: None, } @@ -334,7 +333,6 @@ mod tests { now: HostTime(2_000_000), predicted_present: Some(HostTime(1_900_000)), refresh_interval: Some(16_666_667), - frame_index: 7, output: OutputId(0), prev_actual_present: None, }; diff --git a/frameclock_wayland/src/lib.rs b/frameclock_wayland/src/lib.rs index e4816ad..59672fe 100644 --- a/frameclock_wayland/src/lib.rs +++ b/frameclock_wayland/src/lib.rs @@ -196,7 +196,6 @@ mod tests { now: HostTime(1_000_000), predicted_present, refresh_interval: Some(16_666_667), - frame_index: 7, output: OutputId(0), prev_actual_present: None, } @@ -239,7 +238,6 @@ mod tests { now: HostTime(2_000_000), predicted_present: Some(HostTime(1_900_000)), refresh_interval: Some(16_666_667), - frame_index: 7, output: OutputId(0), prev_actual_present: None, }; diff --git a/frameclock_wayland/src/tick.rs b/frameclock_wayland/src/tick.rs index 467b3c0..dd92b96 100644 --- a/frameclock_wayland/src/tick.rs +++ b/frameclock_wayland/src/tick.rs @@ -90,7 +90,6 @@ impl Default for TickQueue { #[derive(Debug)] pub struct TickerState { queue: TickQueue, - tick_index: u64, callback_in_flight: bool, last_observed_actual_present: Option, last_observed_refresh_interval: Option, @@ -102,7 +101,6 @@ impl TickerState { pub fn new() -> Self { Self { queue: TickQueue::default(), - tick_index: 0, callback_in_flight: false, last_observed_actual_present: None, last_observed_refresh_interval: None, @@ -112,9 +110,8 @@ impl TickerState { /// Records that a `wl_callback.done` event has arrived. /// /// If a callback is in flight, builds a [`FrameTick`] for `output` with the - /// current time read from `clock`, enqueues it, increments the tick index, - /// and clears the in-flight flag. If no callback is in flight, debug-asserts - /// and returns. + /// current time read from `clock`, enqueues it, and clears the in-flight + /// flag. If no callback is in flight, debug-asserts and returns. /// /// When a previous actual-present time and refresh interval have been /// observed, the tick carries a predicted next-vsync @@ -142,13 +139,11 @@ impl TickerState { now, predicted_present, refresh_interval, - frame_index: self.tick_index, output, prev_actual_present: last_actual, }; self.queue.push(tick); - self.tick_index += 1; self.callback_in_flight = false; } @@ -233,12 +228,11 @@ mod tests { use frameclock::HostTime; use frameclock::OutputId; - fn test_tick(frame_index: u64) -> FrameTick { + fn test_tick(now: u64) -> FrameTick { FrameTick { - now: HostTime(frame_index), + now: HostTime(now), predicted_present: None, refresh_interval: None, - frame_index, output: OutputId(0), prev_actual_present: None, } @@ -253,8 +247,8 @@ mod tests { queue.push(test_tick(2)); queue.push(test_tick(3)); - assert_eq!(queue.pop().map(|tick| tick.frame_index), Some(2)); - assert_eq!(queue.pop().map(|tick| tick.frame_index), Some(3)); + assert_eq!(queue.pop().map(|tick| tick.now), Some(HostTime(2))); + assert_eq!(queue.pop().map(|tick| tick.now), Some(HostTime(3))); assert_eq!(queue.pop(), None); assert_eq!(queue.dropped_count(), 1); } @@ -287,7 +281,6 @@ mod tests { assert!(tick.now.ticks() > 0); assert_eq!(tick.predicted_present, None); assert_eq!(tick.refresh_interval, None); - assert_eq!(tick.frame_index, 0); assert_eq!(tick.output, OutputId(0)); assert_eq!(tick.prev_actual_present, None); } @@ -310,14 +303,20 @@ mod tests { } #[test] - fn tick_index_increments_monotonically() { + fn callback_done_emits_one_tick_per_request() { let mut ticker = TickerState::new(); + let mut previous_now = None; - for expected in 0..5 { + for output in 0..5 { assert!(ticker.mark_callback_requested()); - ticker.on_callback_done(Clock::Monotonic, OutputId(0)); + ticker.on_callback_done(Clock::Monotonic, OutputId(output)); let tick = ticker.poll_tick().unwrap(); - assert_eq!(tick.frame_index, expected); + assert_eq!(tick.output, OutputId(output)); + if let Some(previous_now) = previous_now { + assert!(tick.now >= previous_now); + } + previous_now = Some(tick.now); + assert_eq!(ticker.poll_tick(), None); } } @@ -387,14 +386,17 @@ mod tests { // is dropped. let mut ticker = TickerState::new(); - for _ in 0..9 { + for output in 0..9 { assert!(ticker.mark_callback_requested()); - ticker.on_callback_done(Clock::Monotonic, OutputId(0)); + ticker.on_callback_done(Clock::Monotonic, OutputId(output)); } - // First available tick should be index 1 (index 0 was dropped). - let tick = ticker.poll_tick().unwrap(); - assert_eq!(tick.frame_index, 1); + assert_eq!(ticker.queue.dropped_count(), 1); + for output in 1..9 { + let tick = ticker.poll_tick().expect("queued tick should remain"); + assert_eq!(tick.output, OutputId(output)); + } + assert_eq!(ticker.poll_tick(), None); } // --- Present prediction tests --- diff --git a/frameclock_web/README.md b/frameclock_web/README.md index 8e580b1..eb74c97 100644 --- a/frameclock_web/README.md +++ b/frameclock_web/README.md @@ -103,10 +103,10 @@ diagnostics stay in `frameclock`. `TIMEBASE` uses microsecond ticks: `1 tick = 1_000 ns`. This matches browser `DOMHighResTimeStamp` values after converting milliseconds to microseconds. -`RafLoop` increments `FrameTick::frame_index` once per delivered RAF callback. -Applications that bypass `RafLoop` and create their own ticks should keep the -same per-output monotonic ownership rule: the frame index identifies a -delivered browser frame opportunity for one output or surface. +`RafLoop` emits pacing facts for each delivered RAF callback. It does not assign +content-frame identity; retained hosts get `FramePlan::frame_index` from +`FrameDriver`, while low-level `Scheduler` integrations pass their own frame id +to `Scheduler::plan` and `FrameTickEvent::new`. Because RAF is pacing-only, `PresentHints::desired_present` is `None` and `PresentHints::latest_commit` is one fallback refresh interval after the RAF diff --git a/frameclock_web/src/lib.rs b/frameclock_web/src/lib.rs index cdefff7..70ec3de 100644 --- a/frameclock_web/src/lib.rs +++ b/frameclock_web/src/lib.rs @@ -107,7 +107,6 @@ mod tests { now: HostTime(16_000), predicted_present: None, refresh_interval: None, - frame_index: 0, output: OutputId(0), prev_actual_present: None, } diff --git a/frameclock_web/src/raf.rs b/frameclock_web/src/raf.rs index 0dd13aa..22f70f3 100644 --- a/frameclock_web/src/raf.rs +++ b/frameclock_web/src/raf.rs @@ -46,7 +46,6 @@ type RafClosure = Closure; struct RafInner { closure: RefCell>, callback: RefCell>, - frame_counter: Cell, output: OutputId, running: Cell, raf_id: Cell, @@ -63,7 +62,6 @@ impl RafLoop { inner: Rc::new(RafInner { closure: RefCell::new(None), callback: RefCell::new(Box::new(callback)), - frame_counter: Cell::new(0), output, running: Cell::new(false), raf_id: Cell::new(0), @@ -93,14 +91,10 @@ impl RafLoop { )] let now = HostTime((timestamp_ms * 1000.0) as u64); - let frame_index = inner.frame_counter.get(); - inner.frame_counter.set(frame_index + 1); - let tick = FrameTick { now, predicted_present: None, refresh_interval: None, - frame_index, output: inner.output, prev_actual_present: None, }; @@ -150,7 +144,6 @@ impl core::fmt::Debug for RafLoop { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { f.debug_struct("RafLoop") .field("running", &self.inner.running.get()) - .field("frame_counter", &self.inner.frame_counter.get()) .field("output", &self.inner.output) .finish() } diff --git a/frameclock_windows/README.md b/frameclock_windows/README.md index 64e1950..df46161 100644 --- a/frameclock_windows/README.md +++ b/frameclock_windows/README.md @@ -26,7 +26,7 @@ Use `now` and `timebase` to read the QPC clock as a `HostTime` / `Timebase` pair (`nanos = ticks * timebase.numer / timebase.denom`). Call `make_tick` from a `VSync`-paced tick handler (for example, one driven by `DwmFlush` or a swapchain frame-latency waitable) to build a `FrameTick` from -the refresh interval, frame index, and previous actual present time. +the refresh interval and previous actual present time. ## Timing Model diff --git a/frameclock_windows/src/tick.rs b/frameclock_windows/src/tick.rs index cfedfa6..9385f63 100644 --- a/frameclock_windows/src/tick.rs +++ b/frameclock_windows/src/tick.rs @@ -7,11 +7,7 @@ use frameclock::{FrameTick, HostTime, OutputId}; /// Build a [`FrameTick`] from QPC. Call inside a `VSync`-paced tick handler. #[must_use] -pub fn make_tick( - refresh_interval_ns: u64, - frame_index: u64, - prev_present_time: Option, -) -> FrameTick { +pub fn make_tick(refresh_interval_ns: u64, prev_present_time: Option) -> FrameTick { let timebase = crate::time::timebase(); let interval_ticks = if refresh_interval_ns > 0 { refresh_interval_ns * u64::from(timebase.denom) / u64::from(timebase.numer) @@ -39,7 +35,6 @@ pub fn make_tick( } else { None }, - frame_index, output: OutputId(0), prev_actual_present: prev_present_time, } @@ -53,8 +48,7 @@ mod tests { #[test] fn make_tick_with_refresh_and_prev() { let prev = HostTime(1_000_000); - let tick = make_tick(16_666_667, 5, Some(prev)); - assert_eq!(tick.frame_index, 5); + let tick = make_tick(16_666_667, Some(prev)); assert_eq!(tick.prev_actual_present, Some(prev)); assert!(tick.predicted_present.is_some()); assert!(tick.refresh_interval.is_some()); @@ -62,7 +56,7 @@ mod tests { #[test] fn make_tick_zero_refresh() { - let tick = make_tick(0, 1, None); + let tick = make_tick(0, None); assert_eq!(tick.predicted_present, None); assert_eq!(tick.refresh_interval, None); assert_eq!(tick.prev_actual_present, None); @@ -70,7 +64,7 @@ mod tests { #[test] fn make_tick_first_frame_predicts_from_now() { - let tick = make_tick(16_666_667, 0, None); + let tick = make_tick(16_666_667, None); // First frame with no prev: predicted_present = now + interval let predicted = tick.predicted_present.unwrap(); assert!(predicted.ticks() > tick.now.ticks()); diff --git a/subduction_backend_windows/src/lib.rs b/subduction_backend_windows/src/lib.rs index f4a96f7..64e6e63 100644 --- a/subduction_backend_windows/src/lib.rs +++ b/subduction_backend_windows/src/lib.rs @@ -103,7 +103,6 @@ mod tests { now: HostTime(1_000_000), predicted_present: Some(HostTime(2_000_000)), refresh_interval: Some(16_666_667), - frame_index: 0, output: OutputId(0), prev_actual_present: None, }; @@ -118,8 +117,7 @@ mod tests { #[test] fn make_tick_with_refresh_and_prev() { let prev = HostTime(1_000_000); - let tick = make_tick(16_666_667, 5, Some(prev)); - assert_eq!(tick.frame_index, 5); + let tick = make_tick(16_666_667, Some(prev)); assert_eq!(tick.prev_actual_present, Some(prev)); assert!(tick.predicted_present.is_some()); assert!(tick.refresh_interval.is_some()); @@ -127,7 +125,7 @@ mod tests { #[test] fn make_tick_zero_refresh() { - let tick = make_tick(0, 1, None); + let tick = make_tick(0, None); assert_eq!(tick.predicted_present, None); assert_eq!(tick.refresh_interval, None); assert_eq!(tick.prev_actual_present, None); @@ -135,7 +133,7 @@ mod tests { #[test] fn make_tick_first_frame_predicts_from_now() { - let tick = make_tick(16_666_667, 0, None); + let tick = make_tick(16_666_667, None); // First frame with no prev: predicted_present = now + interval let predicted = tick.predicted_present.unwrap(); assert!(predicted.ticks() > tick.now.ticks()); @@ -147,7 +145,6 @@ mod tests { now: HostTime(1_000_000), predicted_present: None, refresh_interval: None, - frame_index: 0, output: OutputId(0), prev_actual_present: None, }; diff --git a/subduction_backend_windows/src/tick.rs b/subduction_backend_windows/src/tick.rs index 7f3b14b..2a70403 100644 --- a/subduction_backend_windows/src/tick.rs +++ b/subduction_backend_windows/src/tick.rs @@ -178,12 +178,8 @@ impl Drop for FrameEventTickSource { /// Build a [`FrameTick`] from QPC. Call inside the `WM_APP_TICK` handler. #[must_use] -pub fn make_tick( - refresh_interval_ns: u64, - frame_index: u64, - prev_present_time: Option, -) -> FrameTick { - frameclock_windows::make_tick(refresh_interval_ns, frame_index, prev_present_time) +pub fn make_tick(refresh_interval_ns: u64, prev_present_time: Option) -> FrameTick { + frameclock_windows::make_tick(refresh_interval_ns, prev_present_time) } /// Compute presentation hints from a tick and safety margin (nanoseconds). diff --git a/subduction_core/src/trace.rs b/subduction_core/src/trace.rs index 5e9bc8a..9390d09 100644 --- a/subduction_core/src/trace.rs +++ b/subduction_core/src/trace.rs @@ -541,11 +541,10 @@ mod tests { now: HostTime(100), predicted_present: Some(HostTime(200)), refresh_interval: Some(16_666_667), - frame_index: 7, output: OutputId(1), prev_actual_present: None, }; - let evt = FrameTickEvent::from(&tick); + let evt = FrameTickEvent::new(7, &tick); assert_eq!(evt.frame_index, 7); assert_eq!(evt.output, OutputId(1)); assert_eq!(evt.now, HostTime(100));