From 1ad8275ab51abd095a2f046b6a39d70f9ef9073b Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Thu, 17 Sep 2026 08:51:14 -0400 Subject: [PATCH] fix(scroll): restart the stats window when an idle gap is dropped by size #582 dropped an idle gap from the frame stats two ways: the reset_scroll() sentinel, which also restarts the 5s window timer, and a size guard for scrollers that never call reset_scroll(), which did not. On that path the first real frame after the gap found the boundary overdue and logged a stats line for a one-frame window. Both paths now share one seeding helper. Co-Authored-By: Claude Opus 5 --- src/common/scroll_helper.py | 23 +++++++++++++++-------- test/test_scroll_helper.py | 13 +++++++++++++ 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/common/scroll_helper.py b/src/common/scroll_helper.py index 71b9080a..24cb1e68 100644 --- a/src/common/scroll_helper.py +++ b/src/common/scroll_helper.py @@ -985,11 +985,7 @@ class ScrollHelper: # the real stall rates being measured, so the number could not be # trusted at all. Seed the clock and take no sample. if self.last_frame_time is None: - self.last_frame_time = current_time - # Restart the window with the scroll. Otherwise the boundary is - # already long overdue when the second frame arrives, and the new - # scroll opens by reporting a window of exactly one frame. - self.last_fps_log_time = current_time + self._restart_stats_window(current_time) return # Calculate instantaneous frame time @@ -998,9 +994,10 @@ class ScrollHelper: # A caller that scrolls without ever calling reset_scroll() never arms # the sentinel above, so catch the same gap by its size. Nothing that # renders a scroll produces a frame longer than the log interval; a - # sample that large is an idle period, not a frame. + # sample that large is an idle period, not a frame. It starts a new + # scroll exactly as the sentinel does, window timer included. if frame_time >= FPS_LOG_INTERVAL: - self.last_frame_time = current_time + self._restart_stats_window(current_time) return self.frame_times.append(frame_time) @@ -1034,7 +1031,17 @@ class ScrollHelper: self.last_frame_time = current_time self.frame_count += 1 - + + def _restart_stats_window(self, current_time: float) -> None: + """Seed the frame clock at the start of a scroll, taking no sample. + + The window timer restarts with it. Otherwise the 5s boundary is + already long overdue when the next frame arrives, and the new scroll + opens by reporting a window of exactly one frame. + """ + self.last_frame_time = current_time + self.last_fps_log_time = current_time + def clear_cache(self) -> None: """ Clear the cached scrolling image. diff --git a/test/test_scroll_helper.py b/test/test_scroll_helper.py index 8d77edca..a04eabfe 100644 --- a/test/test_scroll_helper.py +++ b/test/test_scroll_helper.py @@ -460,6 +460,19 @@ class TestIdleGapIsNotAFrame: assert not info.called assert len(helper._window) == 1 + def test_a_dropped_gap_restarts_the_window_timer_too(self, helper): + """The size-guard path is the one scrollers that never call + reset_scroll() take, so it must restart the window like the sentinel + does, or their next scroll opens with a one-frame stats line.""" + helper.log_frame_rate() # seeds + helper.last_frame_time = time.time() - 137.0 + helper.last_fps_log_time = 0.0 # boundary long overdue + with patch.object(helper.logger, "info") as info: + helper.log_frame_rate() # the gap, dropped + helper.log_frame_rate() # first real frame + assert not info.called, "a one-frame window was reported" + assert len(helper._window) == 1 + def test_a_normal_frame_still_counts(self, helper): helper.last_frame_time = time.time() - 0.010 helper.log_frame_rate()