fix(display): only mtime-touch a snapshot frame that is actually on disk

From review (CodeRabbit): the digest was recorded when a frame was queued,
so while the writer hadn't saved it yet, an unchanged frame could take
the TOUCH path and refresh the mtime of the older file on disk. Track the
saved frame's digest separately (set by the writer and the inline path
after a successful save) and write, not touch, when it isn't the current
frame.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Chuck
2026-09-24 18:39:49 -04:00
co-authored by Claude Opus 5.5
parent 00e14ea55f
commit 98ebc859e0
2 changed files with 44 additions and 7 deletions
+18 -7
View File
@@ -194,10 +194,14 @@ class DisplayManager:
self._last_snapshot_ts = 0.0 self._last_snapshot_ts = 0.0
self._last_snapshot_touch_ts = 0.0 self._last_snapshot_touch_ts = 0.0
self._last_snapshot_digest: Optional[int] = None self._last_snapshot_digest: Optional[int] = None
# The frame actually on disk. _last_snapshot_digest moves when a frame
# is handed to the writer; this only once it has been saved, so an
# mtime touch never vouches for a frame still waiting to be written.
self._saved_snapshot_digest: Optional[int] = None
self._snapshot_dir_prepared = False self._snapshot_dir_prepared = False
# Background writer used mid-scroll; see _write_snapshot_if_due. # Background writer used mid-scroll; see _write_snapshot_if_due.
self._snapshot_cond = threading.Condition() self._snapshot_cond = threading.Condition()
self._snapshot_pending: Optional[Image.Image] = None self._snapshot_pending: Optional[Tuple[Image.Image, Optional[int]]] = None
self._snapshot_thread: Optional[threading.Thread] = None self._snapshot_thread: Optional[threading.Thread] = None
self._snapshot_stop = False self._snapshot_stop = False
# Held for the whole of each PNG write, by the writer thread and by # Held for the whole of each PNG write, by the writer thread and by
@@ -1600,12 +1604,16 @@ class DisplayManager:
viewer_fresh, digest != self._last_snapshot_digest) viewer_fresh, digest != self._last_snapshot_digest)
if action is snapshot_policy.SnapshotAction.SKIP: if action is snapshot_policy.SnapshotAction.SKIP:
return return
if action is snapshot_policy.SnapshotAction.TOUCH: if (action is snapshot_policy.SnapshotAction.TOUCH
and self._saved_snapshot_digest == digest):
# mtime bump only: keeps the health check (snapshot age) # mtime bump only: keeps the health check (snapshot age)
# green without paying for a PNG encode of an unchanged frame # green without paying for a PNG encode of an unchanged frame
os.utime(self._snapshot_path, None) os.utime(self._snapshot_path, None)
self._last_snapshot_touch_ts = now self._last_snapshot_touch_ts = now
return return
# (A TOUCH for a frame that isn't on disk yet -- still queued, or
# its write failed -- is written instead: touching would make the
# older file on disk look current.)
# WRITE. Mid-scroll the PNG encode goes to a background thread: at # WRITE. Mid-scroll the PNG encode goes to a background thread: at
# 512x64 it takes 12-14ms on a Pi 4, longer than a 95Hz refresh, # 512x64 it takes 12-14ms on a Pi 4, longer than a 95Hz refresh,
@@ -1615,7 +1623,7 @@ class DisplayManager:
# it compresses, so the encode no longer holds the loop up. Static # it compresses, so the encode no longer holds the loop up. Static
# frames still write inline: nothing is moving to disturb. # frames still write inline: nothing is moving to disturb.
if self.is_currently_scrolling(): if self.is_currently_scrolling():
self._queue_snapshot(self.image.copy()) self._queue_snapshot(self.image.copy(), digest)
else: else:
# A scroll that just ended can leave its last frame queued or # A scroll that just ended can leave its last frame queued or
# mid-write; it must not land on top of this newer one. # mid-write; it must not land on top of this newer one.
@@ -1623,6 +1631,7 @@ class DisplayManager:
with self._snapshot_cond: with self._snapshot_cond:
self._snapshot_pending = None self._snapshot_pending = None
self._save_snapshot(self.image) self._save_snapshot(self.image)
self._saved_snapshot_digest = digest
self._last_snapshot_ts = now self._last_snapshot_ts = now
self._last_snapshot_touch_ts = now self._last_snapshot_touch_ts = now
self._last_snapshot_digest = digest self._last_snapshot_digest = digest
@@ -1688,7 +1697,7 @@ class DisplayManager:
except Exception: except Exception:
pass pass
def _queue_snapshot(self, image: Image.Image) -> None: def _queue_snapshot(self, image: Image.Image, digest: Optional[int] = None) -> None:
"""Hand a frame to the snapshot writer thread; the newest frame wins. """Hand a frame to the snapshot writer thread; the newest frame wins.
One slot, not a queue: if the writer is still encoding when the next One slot, not a queue: if the writer is still encoding when the next
@@ -1696,7 +1705,7 @@ class DisplayManager:
the latest frame, and a backlog would only cost memory and CPU. the latest frame, and a backlog would only cost memory and CPU.
""" """
with self._snapshot_cond: with self._snapshot_cond:
self._snapshot_pending = image self._snapshot_pending = (image, digest)
if self._snapshot_thread is None or not self._snapshot_thread.is_alive(): if self._snapshot_thread is None or not self._snapshot_thread.is_alive():
self._snapshot_thread = threading.Thread( self._snapshot_thread = threading.Thread(
target=self._snapshot_writer, daemon=True, target=self._snapshot_writer, daemon=True,
@@ -1716,11 +1725,13 @@ class DisplayManager:
# clears the slot, so an older frame never lands on a newer one. # clears the slot, so an older frame never lands on a newer one.
with self._snapshot_write_lock: with self._snapshot_write_lock:
with self._snapshot_cond: with self._snapshot_cond:
image, self._snapshot_pending = self._snapshot_pending, None pending, self._snapshot_pending = self._snapshot_pending, None
if image is None: if pending is None:
continue continue
image, digest = pending
try: try:
self._save_snapshot(image) self._save_snapshot(image)
self._saved_snapshot_digest = digest
except Exception as e: except Exception as e:
# The frame was recorded as written when it was queued. # The frame was recorded as written when it was queued.
# Forget that, so an unchanged frame is written again # Forget that, so an unchanged frame is written again
+26
View File
@@ -425,6 +425,32 @@ class TestSnapshotOffRenderThread:
dm.set_scrolling_state(False) dm.set_scrolling_state(False)
assert dm._last_snapshot_digest is None assert dm._last_snapshot_digest is None
def test_a_frame_not_yet_on_disk_is_written_not_touched(
self, dm, tmp_path, monkeypatch):
# The digest is recorded when a frame is queued. Until the writer has
# saved it, an unchanged frame must not mtime-touch the older file on
# disk into looking current.
import zlib
from src.common import snapshot_policy
touched, saved = [], []
self._due(dm, tmp_path, (9, 9, 9))
digest = zlib.adler32(dm.image.tobytes())
dm._last_snapshot_digest = digest # queued earlier...
dm._saved_snapshot_digest = 12345 # ...but an older frame is on disk
monkeypatch.setattr(snapshot_policy, "decide",
lambda *a, **k: snapshot_policy.SnapshotAction.TOUCH)
monkeypatch.setattr(os, "utime", lambda *a, **k: touched.append(a))
monkeypatch.setattr(dm, "_save_snapshot", lambda image: saved.append(image))
dm.set_scrolling_state(False)
dm._write_snapshot_if_due(digest)
assert touched == []
assert len(saved) == 1
assert dm._saved_snapshot_digest == digest
# Once it is on disk, the same frame is only touched.
dm._write_snapshot_if_due(digest)
assert len(touched) == 1 and len(saved) == 1
def test_a_static_frame_lands_after_a_queued_one_still_being_written( def test_a_static_frame_lands_after_a_queued_one_still_being_written(
self, dm, tmp_path, monkeypatch): self, dm, tmp_path, monkeypatch):
# The last frame of a scroll can still be encoding when the first # The last frame of a scroll can still be encoding when the first