Merge pull request 'fix(gif): make the contact sheet an actual contact sheet (closes mort#1519)' (#3) from fix/1519-contact-sheet-time-samples into main

gif/SKILL.md

@@ -149,9 +149,16 @@ gifsicle are already in the sandbox.
 ## 5. Self-critique (AT MOST 3 render passes)
 
 Before delivering, LOOK at what you made: call `image_describe` on the
-**`still.png` file_id** — the harness already built it as a labelled contact
+**`still.png` file_id** — the harness already built it as a contact sheet of
-sheet, one tile per scene. Ask specifically about subject size, caption
+frames sampled evenly ACROSS the animation, each tile labelled with its `t`
-readability and whether the depicted action matches the request. Then answer:
+(0.00 = first frame). Ask specifically about subject size, caption readability
+and whether the depicted action matches the request.
+
+**A thing missing from one tile is not a thing missing from the animation.**
+The tiles are time points, so a transient — a tear rolling, a blink, a wipe —
+appears in some and not others. If the critique says something is absent, check
+the other tiles for it before re-rendering; that mistake once cost a worker
+three full re-renders chasing a tear that was there all along. Then answer:
 
 1. Does it depict what was asked — every named character, prop and action?
 2. Is the subject **big enough to read** (≥ ⅓ frame height)?

gif/scripts/gifsmith.py

@@ -67,6 +67,11 @@ GIF_MAX_BYTES = 9 * 1024 * 1024
 # turns a borderline short piece into an MP4.
 PNG_BYTES_PER_PIXEL = 0.29
 
+# Contact-sheet budget. Nine tiles is a 3x3 grid that vision models read
+# reliably; spread across TIME for one scene, across scenes for many.
+CONTACT_SHEET_MAX_TILES = 9
+CONTACT_TILE_W = 300
+
 _FONT_CANDIDATES = [
     "/usr/share/fonts/truetype/dejavu/DejaVuSans-Bold.ttf",
     "/usr/share/fonts/truetype/dejavu/DejaVuSans.ttf",
@@ -110,7 +115,10 @@ class _Scene:
         self.mode, self.hold = mode, float(hold)
         self.frames = 0
         self.error = None
-        self.first_frame = None
+        # (t, thumbnail) pairs for the contact sheet. Thumbnails, not full
+        # frames: nine 640x480 RGB copies is ~8 MiB of RAM held for the whole
+        # render, and the sheet downscales them anyway.
+        self.samples = []
 
 
 class Animation:
@@ -241,20 +249,31 @@ class Animation:
         print(f"[gifsmith] {len(self._scenes)} scene(s), {total_s:.1f}s at "
               f"{self.fps}fps, canvas {self.width}x{self.height}", flush=True)
 
+        # Tile budget for the contact sheet, spread across the scenes. One scene
+        # gets the whole budget in TIME, which is the case that was broken: a
+        # loop's interesting content is by construction not at any single
+        # instant.
+        per_scene = max(1, CONTACT_SHEET_MAX_TILES // len(self._scenes))
+
         for i, sc in enumerate(self._scenes):
             start = self._count
             n = max(1, int(round(sc.seconds * self.fps)))
             try:
+                want = self._sample_frames(n, per_scene)
                 for k in range(n):
                     t = k / max(n - 1, 1)
                     img = (self._render_pil if sc.mode == "pil" else self._render_mpl)(sc, t)
-                    if sc.first_frame is None and k >= n // 2:
+                    if k in want:
-                        sc.first_frame = img.copy()
+                        sc.samples.append((t, img.resize(
+                            (CONTACT_TILE_W, self._tile_h()), Image.LANCZOS)))
                     self._emit(img)
                 for _ in range(int(round(sc.hold * self.fps))):
                     self._emit(img)
             except Exception:  # noqa: BLE001
                 sc.error = traceback.format_exc(limit=6)
+                # Its partial frames are dropped below; its samples must go too,
+                # or the sheet advertises a scene that never shipped.
+                sc.samples = []
                 if not self._streaming:
                     # Drop this scene's partial frames so it can't ship half-drawn.
                     # (Streamed frames are already in the encoder — a broken scene
@@ -292,15 +311,19 @@ class Animation:
                     os.remove(leftover)
             shutil.rmtree(self._scratch, ignore_errors=True)
             raise
-        self._contact_sheet()
+        sheet_tiles = self._contact_sheet() or 0
         broken = [f"{i}:{s.title!r}" for i, s in enumerate(self._scenes) if s.error]
         print(f"[gifsmith] {self._count} frames, scratch "
               + ("streamed (0 MiB on disk)" if self._streaming
                  else f"peak {self._peak_mb:.1f} MiB"))
         print(f"[gifsmith] scene errors: {', '.join(broken) if broken else 'none'}")
         print(f"[gifsmith] ART {art}")
-        print("[gifsmith] still: /workspace/still.png (contact sheet — call "
+        ts = ", ".join(f"{t:.2f}" for sc in self._scenes for (t, _th) in sc.samples)
-              "image_describe on ITS file_id from files_out)")
+        print(f"[gifsmith] still: /workspace/still.png — {sheet_tiles} frames "
+              f"sampled across the animation at t={ts} (each tile is labelled "
+              "with its t). Call image_describe on ITS file_id from files_out; "
+              "if something is missing from one tile, check whether it is "
+              "present in another before re-rendering.")
         shutil.rmtree(self._scratch, ignore_errors=True)
         return art
 
@@ -411,25 +434,64 @@ class Animation:
 
     # ------------------------------------------------------------- critique aid
 
+    def _tile_h(self):
+        return max(1, round(self.height * CONTACT_TILE_W / self.width))
+
+    @staticmethod
+    def _sample_frames(n, count):
+        """Frame indices to keep for the contact sheet, spread evenly over the
+        scene: t = 0, 1/count, ... (count-1)/count.
+
+        A scene with very few frames yields fewer samples than asked for, since
+        distinct t values collide onto the same frame index; the sheet simply
+        has fewer tiles, which is the honest outcome.
+
+        Deliberately NOT ending at t=1.0. A loop is built to return to its
+        starting state, so its last frame is the one instant guaranteed to show
+        none of the motion — which is exactly how a worker spent three
+        re-renders chasing a tear its critic could not see, because the still
+        was the frame after the tear had faded."""
+        if n <= 1:
+            return {0}
+        count = max(1, min(count, n))
+        return {min(n - 1, round(j / count * (n - 1))) for j in range(count)}
+
     def _contact_sheet(self, path="/workspace/still.png"):
-        tiles = [(i, s) for i, s in enumerate(self._scenes) if s.first_frame is not None]
+        """A grid sampled across TIME, one labelled tile per sample.
+
+        It used to be one frame per SCENE, and a single-scene animation — the
+        common case — short-circuited to a bare frame save. Every single-scene
+        worker in two production runs told us so unprompted, in the words of
+        the vision model: "this is a single frame (not a contact sheet)". The
+        critique loop was being fed one time point out of ~84 and asked to
+        judge motion.
+        """
+        tiles = [(i, sc, t, thumb)
+                 for i, sc in enumerate(self._scenes)
+                 for (t, thumb) in sc.samples]
         if not tiles:
             return
-        if len(tiles) == 1:
-            tiles[0][1].first_frame.save(path)
-            return
         cols = min(3, len(tiles))
         rows = math.ceil(len(tiles) / cols)
-        tw = 300
+        tw, th = CONTACT_TILE_W, self._tile_h()
-        th = max(1, round(self.height * tw / self.width))
         sheet = Image.new("RGB", (cols * tw, rows * (th + 20)), (20, 20, 24))
         d = ImageDraw.Draw(sheet)
         f = self.font(14)
-        for k, (i, sc) in enumerate(tiles):
+        multi = len(self._scenes) > 1
+        for k, (i, sc, t, thumb) in enumerate(tiles):
             x, y = (k % cols) * tw, (k // cols) * (th + 20)
-            sheet.paste(sc.first_frame.resize((tw, th), Image.LANCZOS), (x, y + 20))
+            sheet.paste(thumb, (x, y + 20))
-            d.text((x + 4, y + 3), f"scene {i}: {sc.title[:38]}", fill=(235, 235, 240), font=f)
+            # The t label is the point: it tells the critic WHEN it is looking,
+            # so "the tear is missing" can be read as "missing at t=0.75"
+            # rather than as a fault in the drawing.
+            label = f"t={t:.2f}"
+            if multi:
+                label = f"scene {i} {label}: {sc.title[:26]}"
+            elif sc.title:
+                label = f"{label}  {sc.title[:30]}"
+            d.text((x + 4, y + 3), label, fill=(235, 235, 240), font=f)
         sheet.save(path)
+        return len(tiles)
 
 
 def _run(cmd):