From d1db06b21e761eaf3eb7d63397b0367436ab68ec Mon Sep 17 00:00:00 2001 From: Eric Levy Date: Tue, 14 Jul 2026 08:17:19 -0400 Subject: [PATCH 1/5] Add pytest test harness with headless Qt support Introduce the first automated test infrastructure for FrameDeck. There were previously no tests; CI only ran compileall. - tests/conftest.py forces Qt's offscreen platform plugin and a single shared QApplication so widgets can be constructed, rendered and pixel-probed with no display server. FRAMEDECK_PROFILE_ROOT is redirected to a temp dir so tests do not write into the user's Documents folder. - tests/helpers.py provides render_widget_to_image / probe_pixel plus synthetic media generators (solid MP4 via PyAV, numbered PNG sequence, flat EXR via OIIO) so later tests need no external assets. - tests/test_smoke.py verifies the harness end to end: offscreen render + pixel probe, FrameDeck package imports, and each media generator produces a decodable/readable file. 10 tests pass locally. - requirements-dev.txt pins pytest and pypdf. - pytest.ini sets testpaths. - ci.yml gains a headless pytest job (installs Qt offscreen system libs plus runtime + dev requirements). Also adds doc/PLAN-reviewapp-parity.md: the staged plan for porting reviewapp features into FrameDeck across separate PRs. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0177bi2WkPjrGytrarheKFDc Claude-Session-Id: 45a11a50-85fe-4349-b853-e1db9cd20276 --- .github/workflows/ci.yml | 21 +++ doc/PLAN-reviewapp-parity.md | 289 +++++++++++++++++++++++++++++++++++ pytest.ini | 3 + requirements-dev.txt | 5 + tests/__init__.py | 0 tests/conftest.py | 42 +++++ tests/helpers.py | 100 ++++++++++++ tests/test_smoke.py | 75 +++++++++ 8 files changed, 535 insertions(+) create mode 100644 doc/PLAN-reviewapp-parity.md create mode 100644 pytest.ini create mode 100644 requirements-dev.txt create mode 100644 tests/__init__.py create mode 100644 tests/conftest.py create mode 100644 tests/helpers.py create mode 100644 tests/test_smoke.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2679f15..f6f5c76 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,3 +28,24 @@ jobs: test -f build-ubuntu-appimage.sh test -f scripts/fetch_aces12.py test -f resources/ocio/ACES_1.2_SOURCE.txt + + pytest: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.10" + - name: Install Qt offscreen system libraries + run: | + sudo apt-get update + sudo apt-get install -y --no-install-recommends \ + libegl1 libgl1 libxkbcommon0 libdbus-1-3 libglib2.0-0 + - name: Install Python dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements-linux.txt -r requirements-dev.txt + - name: Run test suite (headless) + env: + QT_QPA_PLATFORM: offscreen + run: pytest -q diff --git a/doc/PLAN-reviewapp-parity.md b/doc/PLAN-reviewapp-parity.md new file mode 100644 index 0000000..1b103ef --- /dev/null +++ b/doc/PLAN-reviewapp-parity.md @@ -0,0 +1,289 @@ +# FrameDeck <- reviewapp parity: PR plans + +Draft plan for porting the 14 reviewapp features into FrameDeck as separate PRs. +Nothing here is coded yet. Source of truth for ports: `D:\Work\Coding\reviewapp` +(`review.py`, `providers/`, `shot_match.py`). Each PR below lists the reviewapp +source, the FrameDeck integration seam, the approach, and - critically - the +exact automated test evidence I can produce before shipping. + +--- + +## 1. Can these ship with "high confidence they'll work"? Honest answer. + +Environment is capable: Python 3.10, PySide6 6.9.1, OpenCV, OIIO, OCIO, PyAV all +import cleanly, and `QT_QPA_PLATFORM=offscreen` renders widgets to a QPixmap so I +can probe pixels for real evidence. There are currently **zero tests** in either +repo; CI only runs `python -m compileall`. So step one is a test harness (PR-0). + +Confidence splits into three honest tiers: + +**Tier A - fully verifiable here, high confidence.** Deterministic logic I can +assert with real automated tests: CSV export, shot matching, timecode math, CDL +parse+apply, LGG math, LUT apply, undo/redo, ping-pong index order, speed math, +session round-trip, and pixel-probe checks of rendered pins/arrows/annotated +frames and PDF structure (page count, non-empty). + +**Tier B - GUI, automated smoke + pixel probe, you do final visual sign-off.** +Widgets construct offscreen, signals/seek/wiring are asserted, and I pixel-probe +what I can, but the *feel* (slider response, panel layout, HUD readability) needs +your eyes. This matches your existing "render-verify pending" workflow: I give +you automated evidence + a short list of things to eyeball. + +**Tier C - cannot be fully verified without your infrastructure.** The +ftrack/ShotGrid integration (PR-5): **every** operation (search, auto-match, +list/update status, push note, upload thumbnail) hits a live server. I can copy +the providers and unit-test the offline parts with a mock (note-content builder, +auto-match scoring, dialog/threading, JPEG rendering), but I **cannot** prove a +note actually lands in your ftrack/ShotGrid without a test instance + credentials. +I will not claim that one "works" from here. Options are listed in PR-5. + +Bottom line: 13 of 14 I can build and back with automated evidence to a high bar +(with your visual sign-off on the GUI polish). PR-5 I can build and prove offline, +but its live round-trip needs you. + +--- + +## 2. Testing approach ("test before shipping") + +- **Framework:** `pytest`, new `tests/` dir. Add `pytest` (+ `pypdf` for PDF + structure assertions) to a `requirements-dev.txt`. +- **Qt harness:** `tests/conftest.py` sets `QT_QPA_PLATFORM=offscreen` and + `OPENCV_FFMPEG_CAPTURE_OPTIONS=threads;1`, provides a session `QApplication` + fixture and a `render_widget_to_image(widget)` + `probe_pixel(img, x, y)` + helper. Verified working in this environment. +- **Fixtures:** a couple of tiny synthetic media assets generated at test time + (a 16-frame solid-color MP4 via PyAV, a 4-frame PNG sequence, a known 1x1/64x64 + EXR) so tests need no external files. +- **Per-PR evidence:** every PR lists concrete assertions below. I paste the + `pytest` output into the PR. GUI-visual items go on a "please eyeball" list. +- **CI:** extend `.github/workflows/ci.yml` with a `pytest` job (headless via the + offscreen platform) so regressions are caught on every PR. + +This harness (PR-0) is a prerequisite for the "tested before shipping" bar and +should land first. + +--- + +## 3. Build order & dependencies + +``` +PR-0 Test harness .......................... foundation, land first +PR-1 Comment model + sidebar + pin tool ..... foundation for 2,3,5 + PR-2 CSV export ......................... depends on PR-1 + PR-3 PDF report ......................... depends on PR-1 (+ existing frame render) + PR-5 ftrack/ShotGrid push ............... depends on PR-1; needs your creds +PR-4 Grading (4a LUT, 4b CDL, 4c LGG) ...... independent; highest technical risk +PR-6 Shot matching ........................ independent +PR-7 Timecode readout ..................... independent (helps 2,3) +PR-8 Arrow annotation tool ................ independent (button already scaffolded) +PR-9 Annotation redo ...................... depends on existing undo +PR-10 Ping-pong loop mode .................. independent +PR-11 Playback speed multiplier ............ independent +PR-12 Auto-restore last session ............ independent +PR-13 Performance HUD ...................... independent +PR-14 User-selectable proxy scale .......... independent +``` + +Quick wins to build confidence early: PR-8, PR-9, PR-2, PR-7. +Highest impact: PR-1 (unlocks 2/3/5). Highest risk: PR-4 (color pipeline order). + +--- + +## PR-0 - Test harness +- **Goal:** pytest + offscreen Qt fixtures + synthetic media, wired into CI. +- **Files:** `tests/conftest.py`, `tests/helpers.py`, `requirements-dev.txt`, + `.github/workflows/ci.yml` (add pytest job). +- **Test evidence:** `pytest` collects and a trivial offscreen-render smoke test + passes in CI. +- **Confidence:** High. **Size:** S. **Depends on:** nothing. + +## PR-1 - Comment model + sidebar + pin tool (Frame.io-style) +- **Goal:** per-frame text comments with optional positional pin, a comment + sidebar (click-to-seek, delete, done-toggle, count), numbered pin markers on the + viewer, `[` / `]` jump-to-annotated-frame, and sidecar persistence. +- **Port source:** reviewapp `AnnotationStore` comment model (review.py 1614-1806), + `CommentPanel` (3825-4200), pin tool + `_paint_comment_pins` (SW 2285-2301 / GL + 3497-3511), `_nav_annotation` (7234-7249), sidecar format (1627-1683). +- **FrameDeck seams:** `widgets/annotations.py` `Sketch` (strokes are in-memory, + types pencil/rect/ellipse/text; add a parallel `comments` concept + persistence); + `widgets/viewer.py` overlay draw path (add `pin_clicked` signal + pin render); + `widgets/__init__.py` splitter (host the panel; sizes at line ~324). +- **Approach:** new `widgets/commentpanel.py` + a comment store (sidecar JSON, + normalized x/y, timestamp, done). New "pin" viewer tool. Wire seek + refresh. +- **Open item to confirm in-PR:** whether FrameDeck persists ANY annotations today + (appears in-memory only) - the sidecar work may also give existing strokes + persistence; keep scope to comments unless we decide otherwise. +- **Test evidence:** store add/delete/toggle/round-trip via sidecar (unit); + `get_annotated_frames()` merges comments+strokes; `[`/`]` navigation lands on + right frame; offscreen-render the viewer with one pin and probe the pixel at the + pin center for the marker color; panel constructs and emits `seek_requested` on + row click. Visual sign-off: panel layout/typography. +- **Confidence:** High on model/logic; Tier-B on panel polish. **Size:** L. + **Depends on:** PR-0. + +## PR-2 - CSV export +- **Goal:** `Ctrl+E` export of comments+drawings to CSV. +- **Port source:** reviewapp `export_csv` (review.py 1807-1821): columns + `frame,timecode,type,content,color,x,y,timestamp`. +- **FrameDeck seams:** comment store from PR-1 + `Sketch.strokes`; add menu action + in `widgets/__init__.py`. +- **Test evidence:** build a known set of comments+strokes, export, parse the CSV + back, assert exact header + row values + timecode formatting. Fully automated. +- **Confidence:** High. **Size:** S. **Depends on:** PR-1. + +## PR-3 - PDF review report +- **Goal:** `Ctrl+Shift+E` multi-page PDF: title page, per-source section, + per-frame block with annotated screenshot + numbered comments + timecode. +- **Port source:** reviewapp `_export_pdf` (7498-7663) and + `_render_annotated_pixmap` (7418-7496); QPdfWriter A4/150dpi, print-safe colors. +- **FrameDeck seams:** reuse `widgets/viewer.py render_annotated_frame` (2233-2278) + and the `export_notes` iteration pattern (`widgets/__init__.py` 1924-2005) as the + screenshot source; comment data from PR-1. +- **Approach:** new `widgets/pdf_export.py`. Convert FrameDeck strokes+comments to + the render format, paginate, embed. +- **Test evidence:** generate a PDF from fixture annotations; assert file exists, + non-empty, expected page count and that comment text strings appear (via `pypdf` + text extraction). Visual sign-off: layout quality on paper. +- **Confidence:** High on structure; Tier-B on visual layout. **Size:** M. + **Depends on:** PR-1. + +## PR-4 - Per-clip color grading (split: 4a LUT, 4b CDL, 4c LGG + session LUT) +- **Goal:** real (source-affecting) grading, persisted per source: `.cube/.3dl/` + `.lut` clip LUT, ASC-CDL, Lift/Gamma/Gain(+contrast), and a session-wide LUT. +- **Port source:** reviewapp `LGGPanel` (4377-4574) + LGG LUT math (1972-2008); + `_apply_file_lut` via OIIO `ociofiletransform` (978-987); `_apply_cdl` XML + slope/offset/power/sat (989-1055); pipeline order (1057-1087): CDL -> clip LUT -> + session LUT -> OCIO display -> LGG; per-clip persistence (`_clip_state` 6790-6849). +- **FrameDeck seams:** OCIO is applied CPU-side in `playback/player.py` (~1408-1416) + via `ocio/__init__.py process_image`; display-only gamma/exposure lives in + `widgets/viewer.py _display_image` (1292-1387). Grading must be a SOURCE stage in + the float pipeline, not the display path. `.fdplaylist` (widgets/__init__.py + 1092-1235) needs a per-shot `grading` block + top-level `session_lut`. +- **RISK / decision:** pipeline ORDER and color space. reviewapp applies CDL/LUT on + pre-OCIO pixels and LGG on post-OCIO uint8. Getting this wrong is a silent + correctness bug (looks plausible, grades wrong). This is the one PR where I'll + want a reference frame from you to compare against reviewapp output. +- **Test evidence:** parse a known CDL -> assert slope/offset/power; apply CDL/LGG + to a known pixel value -> assert exact numeric output (deterministic numpy); + apply a known `.cube` via OIIO -> assert output pixel; persistence round-trip. + Cross-check: render the same frame+grade in FrameDeck and reviewapp, diff pixels. +- **Confidence:** High on the math per stage; **medium** on pipeline-order parity + until we pick + verify the order against reviewapp. **Size:** L (split into 3). + **Depends on:** PR-0. + +## PR-5 - ftrack + ShotGrid note push (TIER C - needs your infrastructure) +- **Goal:** search/auto-match a shot, push per-comment notes with an annotated + thumbnail, optionally set status. ftrack + ShotGrid. +- **Port source:** reviewapp `providers/` (base/models/ftrack/shotgrid - copy + wholesale) and the dialogs/workers `ServiceSearchWorker`/`ServiceAutoMatchWorker`/ + `ServiceBatchPushWorker`/`ServicePushDialog` (review.py 5483-6026). +- **FrameDeck seams:** `widgets/recaps.py` already has status combo, type combo, + attachments, and a submit flow - re-target its submit to `provider.push_notes(...)`; + reuse the annotated-frame render for the JPEG thumbnail; comment data from PR-1. +- **External deps / creds:** ftrack needs `requests` + `FTRACK_SERVER/API_USER/` + `API_KEY`; ShotGrid needs `shotgun_api3` + `SHOTGRID_SERVER/SCRIPT_NAME/API_KEY`. +- **What I CAN test offline:** note-content/timecode builder, auto-match scoring + (with fixture filenames), status sentinel handling, dialog + worker threading + against a **mock provider**, JPEG thumbnail bytes are valid. +- **What I CANNOT test without you:** any real search/status/note/upload round-trip. +- **Options (pick one):** + 1. You provide a throwaway ftrack/ShotGrid test instance + creds -> I run a real + end-to-end push and paste the created note ID/URL as evidence. (Highest bar.) + 2. I ship it mock-tested with a written live-verification checklist you run once + with your creds. (I will label it "offline-verified only" in the PR.) + 3. Defer PR-5 until later. +- **Confidence:** High offline; live round-trip unverifiable here. **Size:** L. + **Depends on:** PR-1 + your decision above. + +## PR-6 - Shot auto-matching +- **Goal:** normalize VFX filenames, score similarity, scan folders, auto-pair + renders to plates (optionally auto-load as B-side). +- **Port source:** `shot_match.py` (normalize 51-93, extract 96-110, score 113-161, + scan 164-206, match 233-275) - largely a drop-in module. +- **FrameDeck seams:** playlist import in `widgets/__init__.py`; A/B compare start + path already exists. +- **Test evidence:** unit-test normalize + score + greedy match on fixture filename + sets with expected pairings. Fully automated. +- **Confidence:** High. **Size:** M. **Depends on:** PR-0. + +## PR-7 - Timecode readout +- **Goal:** HH:MM:SS:FF display alongside frame number; used by CSV/PDF too. +- **Port source:** reviewapp `frame_to_tc` / `_frame_to_tc` (base.py 109-122). +- **FrameDeck seams:** timeline/transport labels in `widgets/viewer.py` / + `widgets/timeline.py` (currently frame numbers only; no `timecode` in codebase). +- **Test evidence:** unit-test frame<->TC at 23.976/24/25/30/60 incl. drop cases. +- **Confidence:** High. **Size:** S-M. **Depends on:** PR-0. + +## PR-8 - Arrow annotation tool +- **Goal:** enable the arrow tool. +- **FrameDeck seams:** `widgets/viewer.py:422-428` already builds `ArrowButton` but + `setVisible(False)` ("Hidden until arrow support is enabled"); wire it into + `set_draw_enabled("arrow", ...)` (line 572) and add arrow render + hit-test in + `widgets/annotations.py` (mirror rect/ellipse). Arrowhead logic from reviewapp + `_render_annotated_pixmap` (7451-7468). +- **Test evidence:** create an arrow stroke, offscreen-render, probe pixels along + the shaft and at the head; hit-test selects it. Visual sign-off: arrowhead shape. +- **Confidence:** High. **Size:** S. **Depends on:** PR-0. + +## PR-9 - Annotation redo +- **Goal:** add redo to the existing undo. +- **FrameDeck seams:** `widgets/annotations.py` has `undo_history` (line 241) and + undo, but no redo stack. Add `redo_history`, push on undo, clear on new action; + bind Ctrl+Shift+Z / Ctrl+Y. +- **Test evidence:** unit-test create->undo->redo restores exact stroke set; new + action clears redo. Fully automated. +- **Confidence:** High. **Size:** S. **Depends on:** PR-0. + +## PR-10 - Ping-pong loop mode +- **Goal:** add ping-pong to the current on/off loop. +- **FrameDeck seams:** `playback/player.py set_loop` (317/633/1690) - add a mode + enum (stop/loop/pingpong) and reverse direction at range ends. +- **Test evidence:** unit-test the frame-index sequence near both ends reverses as + expected. Automated. +- **Confidence:** High. **Size:** S. **Depends on:** PR-0. + +## PR-11 - Playback speed multiplier +- **Goal:** 0.25x-4x speed independent of source FPS (reviewapp presets). +- **FrameDeck seams:** player timer/interval; FPS selection already exists but is + not a speed multiplier. Add a speed factor applied to the frame timer. +- **Test evidence:** unit-test interval = base/speed for each preset; boundary + clamping. Automated. Visual sign-off: smoothness during playback. +- **Confidence:** High on math; Tier-B on playback feel. **Size:** S-M. + **Depends on:** PR-0. + +## PR-12 - Auto-restore last session +- **Goal:** auto-save a `.last_session` playlist on exit and reload on launch + (unless a file is passed on the CLI). +- **Port source:** reviewapp auto-save/restore (review.py 7897-8000). +- **FrameDeck seams:** existing `.fdplaylist` save/load (widgets/__init__.py + 1092-1235); add app-startup/close hooks in `main.py`. +- **Test evidence:** save session -> new app instance loads -> assert media list + + active + frame restored (logic-level, no window needed). Automated. +- **Confidence:** High. **Size:** S. **Depends on:** PR-0. + +## PR-13 - Performance HUD +- **Goal:** optional overlay: decode/render ms + green/red dropped-frame FPS. +- **Port source:** reviewapp perf HUD (review.py 2056-2069, 2221-2224). +- **FrameDeck seams:** viewer overlay draw in `widgets/viewer.py`; timing around + the decode/display path in `playback/player.py`. +- **Test evidence:** unit-test the FPS/threshold->color logic; HUD toggles on/off. + Visual sign-off: readability. +- **Confidence:** High on logic; Tier-B visual. **Size:** S. **Depends on:** PR-0. + +## PR-14 - User-selectable proxy scale +- **Goal:** expose 1.0x / 0.5x / 0.25x proxy downsample in transport (FrameDeck + currently auto-derives a 2K display proxy). +- **Port source:** reviewapp proxy controls (TransportBar 4677-4803, scale 164-181). +- **FrameDeck seams:** decode/proxy path in `playback/` + a transport control. +- **Test evidence:** assert decoded frame dims match the selected scale; cache key + includes scale. Automated. Visual sign-off: quality tradeoff. +- **Confidence:** High. **Size:** S-M. **Depends on:** PR-0. + +--- + +## Open decisions for you +1. **PR-5 tracker verification:** option 1 (give me a test instance/creds), option + 2 (mock-tested + your one-time live check), or option 3 (defer)? +2. **PR-4 grading:** OK to treat pipeline-order parity as the acceptance gate, and + can you provide one reference frame+grade from reviewapp to diff against? +3. **Scope confirmation:** all 14 as separate PRs in the order above, or reprioritize? diff --git a/pytest.ini b/pytest.ini new file mode 100644 index 0000000..eeb9d41 --- /dev/null +++ b/pytest.ini @@ -0,0 +1,3 @@ +[pytest] +testpaths = tests +addopts = -q diff --git a/requirements-dev.txt b/requirements-dev.txt new file mode 100644 index 0000000..0c206ff --- /dev/null +++ b/requirements-dev.txt @@ -0,0 +1,5 @@ +# Development / test dependencies (not shipped in release builds). +# Install alongside the platform requirements, e.g.: +# pip install -r requirements-windows.txt -r requirements-dev.txt +pytest>=8,<10 +pypdf>=4 diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..63d38b9 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,42 @@ +"""Shared pytest configuration for the FrameDeck test suite. + +The whole suite runs headless: we force Qt's ``offscreen`` platform plugin so +widgets can be constructed, rendered to a ``QImage`` and pixel-probed without a +display. Environment variables are set at import time -- before any PySide6 or +OpenCV import -- because Qt reads ``QT_QPA_PLATFORM`` once at ``QApplication`` +construction and OpenCV reads its FFmpeg options once per ``VideoCapture``. +""" + +import os +import sys +import tempfile +from pathlib import Path + +# Headless Qt + single-threaded FFmpeg decode (same guard the app relies on). +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") +os.environ.setdefault("OPENCV_FFMPEG_CAPTURE_OPTIONS", "threads;1") + +# Keep the app's profile/cache writes inside a throwaway dir instead of the real +# user Documents folder (main.py points FRAMEDECK_PROFILE_ROOT at Documents). +os.environ.setdefault( + "FRAMEDECK_PROFILE_ROOT", + tempfile.mkdtemp(prefix="framedeck-test-"), +) + +# Make the repo root importable so `import constants`, `import widgets...` work +# regardless of where pytest is invoked from. +_REPO_ROOT = Path(__file__).resolve().parent.parent +if str(_REPO_ROOT) not in sys.path: + sys.path.insert(0, str(_REPO_ROOT)) + +import pytest + + +@pytest.fixture(scope="session") +def qapp(): + """A single ``QApplication`` shared by every test that touches Qt widgets.""" + from PySide6.QtWidgets import QApplication + + app = QApplication.instance() or QApplication([]) + yield app + # Do not call app.quit(): a session-wide instance is reused across tests. diff --git a/tests/helpers.py b/tests/helpers.py new file mode 100644 index 0000000..33f1e74 --- /dev/null +++ b/tests/helpers.py @@ -0,0 +1,100 @@ +"""Reusable test helpers: offscreen rendering, pixel probing, synthetic media. + +These utilities let the suite produce real evidence (a rendered widget, a probed +pixel, a decodable clip) without any external asset files or a display server. +""" + +from pathlib import Path + +import numpy as np + + +# --------------------------------------------------------------------------- # +# Offscreen rendering / pixel probing +# --------------------------------------------------------------------------- # +def render_widget_to_image(widget, size=None): + """Render *widget* to a ``QImage`` via ``QWidget.grab`` (works offscreen). + + Pass ``size`` as ``(w, h)`` to force a size; otherwise the widget's current + or hinted size is used. + """ + from PySide6.QtCore import QSize + + if size is not None: + widget.resize(*size) + elif widget.size().isEmpty(): + hint = widget.sizeHint() + widget.resize(hint if hint.isValid() else QSize(200, 200)) + return widget.grab().toImage() + + +def probe_pixel(image, x, y): + """Return ``(r, g, b, a)`` (0-255) for pixel ``(x, y)`` of a ``QImage``.""" + color = image.pixelColor(int(x), int(y)) + return (color.red(), color.green(), color.blue(), color.alpha()) + + +# --------------------------------------------------------------------------- # +# Synthetic media generators +# --------------------------------------------------------------------------- # +def make_solid_mp4(path, frames=16, width=128, height=72, color=(200, 40, 40), fps=24): + """Write a solid-*color* (RGB) MP4 of *frames* length. Returns ``path``. + + Uses mpeg4 for portability across PyAV builds. Colour survives yuv420p only + approximately, so probe decoded frames with a tolerance rather than exactly. + """ + import av + + path = Path(path) + container = av.open(str(path), mode="w") + try: + stream = container.add_stream("mpeg4", rate=fps) + stream.width = width + stream.height = height + stream.pix_fmt = "yuv420p" + + rgb = np.zeros((height, width, 3), dtype=np.uint8) + rgb[:, :] = color + for _ in range(frames): + frame = av.VideoFrame.from_ndarray(rgb, format="rgb24") + for packet in stream.encode(frame): + container.mux(packet) + for packet in stream.encode(): + container.mux(packet) + finally: + container.close() + return path + + +def make_png_sequence(directory, frames=4, width=64, height=64, start=1, pad=4, base="frame"): + """Write a numbered PNG sequence (``base.0001.png`` ...). Returns path list.""" + import cv2 + + directory = Path(directory) + directory.mkdir(parents=True, exist_ok=True) + paths = [] + for i in range(frames): + number = start + i + bgr = np.zeros((height, width, 3), dtype=np.uint8) + bgr[:, :] = ((i * 30) % 256, 40, 200) # BGR: mostly-red, varies per frame + out = directory / f"{base}.{number:0{pad}d}.png" + cv2.imwrite(str(out), bgr) + paths.append(out) + return paths + + +def make_exr(path, width=8, height=8, value=(0.5, 0.25, 0.75)): + """Write a solid linear-*value* RGB float EXR. Returns ``path``.""" + import OpenImageIO as oiio + + path = Path(path) + spec = oiio.ImageSpec(width, height, 3, "float") + out = oiio.ImageOutput.create(str(path)) + if out is None: # pragma: no cover - only if OIIO lacks the EXR plugin + raise RuntimeError("OpenImageIO could not create an EXR writer") + out.open(str(path), spec) + pixels = np.zeros((height, width, 3), dtype=np.float32) + pixels[:, :] = value + out.write_image(pixels) + out.close() + return path diff --git a/tests/test_smoke.py b/tests/test_smoke.py new file mode 100644 index 0000000..25714c9 --- /dev/null +++ b/tests/test_smoke.py @@ -0,0 +1,75 @@ +"""Smoke tests that prove the headless harness itself works. + +If these pass, later PRs can rely on: a live offscreen QApplication, widget +render + pixel probe, FrameDeck package imports, and the synthetic media +generators used as fixtures. +""" + +import importlib + +import pytest + +from tests.helpers import ( + make_exr, + make_png_sequence, + make_solid_mp4, + probe_pixel, + render_widget_to_image, +) + + +def test_offscreen_render_and_probe(qapp): + from PySide6.QtWidgets import QWidget + + widget = QWidget() + widget.setStyleSheet("background-color: rgb(200, 30, 30);") + image = render_widget_to_image(widget, size=(40, 40)) + r, g, b, _a = probe_pixel(image, 20, 20) + assert r > 150 and g < 90 and b < 90 + + +@pytest.mark.parametrize( + "module", + [ + "constants", + "widgets.annotations", + "widgets.buttons", + "playback.player", + "playback.reader", + "ocio", + ], +) +def test_framedeck_module_imports(qapp, module): + importlib.import_module(module) + + +def test_make_solid_mp4_decodes(tmp_path, qapp): + import cv2 + + clip = make_solid_mp4(tmp_path / "solid.mp4", frames=8, color=(200, 40, 40)) + cap = cv2.VideoCapture(str(clip)) + try: + assert cap.get(cv2.CAP_PROP_FRAME_COUNT) >= 1 + ok, frame = cap.read() + assert ok and frame is not None + b, g, r = frame[frame.shape[0] // 2, frame.shape[1] // 2] + assert r > g and r > b # decoded frame is reddish + finally: + cap.release() + + +def test_make_png_sequence(tmp_path): + paths = make_png_sequence(tmp_path / "seq", frames=4) + assert len(paths) == 4 + assert all(p.exists() for p in paths) + assert paths[0].name == "frame.0001.png" + + +def test_make_exr_roundtrip(tmp_path): + import OpenImageIO as oiio + + exr = make_exr(tmp_path / "flat.exr", width=8, height=8, value=(0.5, 0.25, 0.75)) + buf = oiio.ImageBuf(str(exr)) + assert buf.spec().width == 8 and buf.spec().nchannels >= 3 + pixel = buf.getpixel(0, 0) + assert abs(pixel[0] - 0.5) < 1e-3 From 0360ddcce4bfda19e9fdfd718286b1be59880216 Mon Sep 17 00:00:00 2001 From: Eric Levy Date: Tue, 14 Jul 2026 08:38:40 -0400 Subject: [PATCH 2/5] Add redo for annotations (Ctrl+Shift+Z / Ctrl+Y) The Sketch annotation store had undo (create/move/erase) but no redo. Add a snapshot-based redo that reapplies the most recently undone action. - widgets/annotations.py: add redo_history; route the four action-record sites through _record_action(), which appends to the undo stack and clears redo (a fresh edit invalidates redo, standard semantics). undo() now snapshots the full pre-undo stroke state onto the redo stack; redo() restores it in place (mutating the existing dict so held references stay valid) and re-pushes the action onto the undo stack. - widgets/viewer.py: redo_strokes() mirrors undo_strokes(). - widgets/__init__.py: "Redo Note" edit-menu action bound to Ctrl+Shift+Z and Ctrl+Y. Tests: tests/test_annotation_redo.py covers create redo, multi-step undo-then-redo ordering, redo-stack invalidation on a new edit, erase undo/redo, and empty-stack no-op. 5 tests pass (15 with the harness suite). Depends on the test harness (PR #7); branched on top of it. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0177bi2WkPjrGytrarheKFDc Claude-Session-Id: 45a11a50-85fe-4349-b853-e1db9cd20276 --- tests/test_annotation_redo.py | 68 +++++++++++++++++++++++++++++++++++ widgets/__init__.py | 7 ++++ widgets/annotations.py | 52 +++++++++++++++++++++------ widgets/viewer.py | 9 +++++ 4 files changed, 126 insertions(+), 10 deletions(-) create mode 100644 tests/test_annotation_redo.py diff --git a/tests/test_annotation_redo.py b/tests/test_annotation_redo.py new file mode 100644 index 0000000..3cb81a2 --- /dev/null +++ b/tests/test_annotation_redo.py @@ -0,0 +1,68 @@ +"""Unit tests for Sketch undo/redo (widgets.annotations).""" + +from widgets.annotations import Sketch + + +def _add_created_stroke(sketch, frame, stroke_id, stroke_type="pencil"): + """Mimic drawing a stroke: add it and record a 'create' undo action.""" + sketch.strokes.setdefault(frame, []).append( + {"id": stroke_id, "type": stroke_type, "points": []} + ) + sketch._record_action({"type": "create", "frame": frame, "stroke_id": stroke_id}) + + +def _ids(sketch, frame): + return [stroke["id"] for stroke in sketch.strokes.get(frame, [])] + + +def test_redo_restores_created_stroke(qapp): + sketch = Sketch() + _add_created_stroke(sketch, 1, "a") + sketch.undo() + assert 1 not in sketch.strokes + sketch.redo() + assert _ids(sketch, 1) == ["a"] + + +def test_new_action_clears_redo_stack(qapp): + sketch = Sketch() + _add_created_stroke(sketch, 1, "a") + sketch.undo() + assert sketch.redo_history # there is something to redo + _add_created_stroke(sketch, 2, "b") # a fresh edit + assert not sketch.redo_history # ...invalidates redo + sketch.redo() # no-op + assert 1 not in sketch.strokes + + +def test_multiple_undo_then_redo_sequence(qapp): + sketch = Sketch() + _add_created_stroke(sketch, 1, "a") + _add_created_stroke(sketch, 1, "b") + sketch.undo() # remove b + sketch.undo() # remove a + assert 1 not in sketch.strokes + sketch.redo() # restore a + assert _ids(sketch, 1) == ["a"] + sketch.redo() # restore b + assert _ids(sketch, 1) == ["a", "b"] + + +def test_redo_with_empty_stack_is_noop(qapp): + sketch = Sketch() + sketch.redo() # must not raise + assert sketch.strokes == {} + + +def test_erase_undo_and_redo(qapp): + sketch = Sketch() + sketch.strokes[3] = [{"id": "x", "type": "pencil"}] + # Erase records the pre-erase snapshot, then clears the frame. + sketch._record_action( + {"type": "erase", "frame": 3, "strokes": [{"id": "x", "type": "pencil"}]} + ) + sketch.strokes[3] = [] + sketch.undo() # restores x + assert _ids(sketch, 3) == ["x"] + sketch.redo() # re-applies the erase + assert sketch.strokes.get(3) == [] diff --git a/widgets/__init__.py b/widgets/__init__.py index de56f67..860df82 100644 --- a/widgets/__init__.py +++ b/widgets/__init__.py @@ -399,6 +399,13 @@ def setup_review_chrome(self): self.actionUndo.setShortcut(QtGui.QKeySequence("Ctrl+Z")) self.actionUndo.triggered.connect(self.viewframe.viewer.undo_strokes) edit_menu.addAction(self.actionUndo) + self.actionRedo = QtGui.QAction("Redo Note", self) + self.actionRedo.setIcon(NamePixmapIcon("loop")) + self.actionRedo.setShortcuts( + [QtGui.QKeySequence("Ctrl+Shift+Z"), QtGui.QKeySequence("Ctrl+Y")] + ) + self.actionRedo.triggered.connect(self.viewframe.viewer.redo_strokes) + edit_menu.addAction(self.actionRedo) self.actionClearFrame = QtGui.QAction("Clear Notes on Frame", self) self.actionClearFrame.setIcon(NamePixmapIcon("clear")) self.actionClearFrame.triggered.connect(self.viewframe.viewer.clear_strokes) diff --git a/widgets/annotations.py b/widgets/annotations.py index 698db80..6dfdf61 100644 --- a/widgets/annotations.py +++ b/widgets/annotations.py @@ -239,6 +239,7 @@ def __init__(self): # Undo system # ---------------------------- self.undo_history = list() + self.redo_history = list() def set_tool(self, tool): """ @@ -413,7 +414,7 @@ def mousePressEvent(self, point): # Eraser if self.tool == "eraser": # Save current frame state ONCE - self.undo_history.append( + self._record_action( { "type": "erase", "frame": self.current_frame, @@ -438,7 +439,7 @@ def mousePressEvent(self, point): self.strokes.setdefault(self.current_frame, []) self.strokes[self.current_frame].append(stroke) - self.undo_history.append( + self._record_action( { "type": "create", "frame": self.current_frame, @@ -530,7 +531,7 @@ def mouseReleaseEvent(self, point): if self.tool == "move": if self.selected_stroke and self.original_stroke: - self.undo_history.append( + self._record_action( { "type": "move", "stroke_id": self.selected_stroke["id"], @@ -559,7 +560,7 @@ def mouseReleaseEvent(self, point): if typed in ["rectangle", "ellipse"]: self.current_shape["end"] = point - self.undo_history.append( + self._record_action( { "type": "create", "frame": self.current_frame, @@ -954,6 +955,15 @@ def draw_selection(self, painter, stroke, point_converter=None): painter.setPen(pen) painter.drawRect(rect) + def _record_action(self, action): + """Record an undoable action and invalidate the redo stack. + + Any new edit clears the redo history, matching standard undo/redo + semantics -- you cannot redo past a fresh change. + """ + self.undo_history.append(action) + self.redo_history.clear() + def undo(self): """ Undo last action (create, move, erase). @@ -963,6 +973,9 @@ def undo(self): - Move: restore previous stroke state - Erase: restore full frame snapshot + The full pre-undo stroke state is captured onto the redo stack so the + action can be reapplied by :meth:`redo`. + Returns: None """ @@ -972,6 +985,9 @@ def undo(self): action = self.undo_history.pop() + # Snapshot the state we are about to leave so redo() can restore it. + pre_undo = copy.deepcopy(self.strokes) + # Create undo if action["type"] == "create": frame = action["frame"] @@ -986,24 +1002,40 @@ def undo(self): if not self.strokes[frame]: del self.strokes[frame] - return - # Move undo - if action["type"] == "move": + elif action["type"] == "move": stroke = self.find_stroke(action["stroke_id"]) if stroke: stroke.clear() stroke.update(copy.deepcopy(action["old_data"])) - return - # Erase undo - if action["type"] == "erase": + elif action["type"] == "erase": self.strokes[action["frame"]] = copy.deepcopy(action["strokes"]) + self.redo_history.append({"action": action, "strokes": pre_undo}) + + def redo(self): + """ + Reapply the most recently undone action. + + Restores the stroke state captured by :meth:`undo` (mutating the + existing ``strokes`` dict in place so held references stay valid) and + pushes the action back onto the undo stack. + + Returns: + None + """ + + if not self.redo_history: return + entry = self.redo_history.pop() + self.strokes.clear() + self.strokes.update(copy.deepcopy(entry["strokes"])) + self.undo_history.append(entry["action"]) + def find_stroke(self, stroke_id): """ Find a stroke by its unique ID across all frames. diff --git a/widgets/viewer.py b/widgets/viewer.py index 257efca..72ce263 100644 --- a/widgets/viewer.py +++ b/widgets/viewer.py @@ -2208,6 +2208,15 @@ def undo_strokes(self): self.update() + def redo_strokes(self): + """ + Reapply the most recently undone annotation. + """ + + self.annotations.redo() + + self.update() + def clear_strokes(self): """ Clear annotations only on the current frame. From fa840484bdaccc1eea53eb59c8f5e617b6b4d17d Mon Sep 17 00:00:00 2001 From: Eric Levy Date: Tue, 14 Jul 2026 08:57:32 -0400 Subject: [PATCH 3/5] Persist annotations to note sidecars FrameDeck kept annotations only in memory, so pencil/text notes were lost when switching sources or closing the app. Persist them to JSON sidecars so they survive a reload -- the first piece of stronger session/project state, and the storage foundation the comment sidebar (planned) will build on. - widgets/annotations.py: Sketch.serialize() returns a JSON-safe snapshot (frame -> strokes, empty frames omitted); deserialize() restores it, converting list coordinates/colours back to tuples and resetting undo/redo history. - widgets/notestore.py: read/write sidecars under the FrameDeck profile dir (/framedeck/notes/_.fdnotes.json), keyed by a hash of the absolute source path. Empty notes remove a stale sidecar; unreadable/foreign files load as empty. - widgets/__init__.py: openMedia saves the outgoing source's notes before the viewer is cleared and loads the incoming source's notes after it opens; closeEvent saves on exit. All hooks are defensive (a notes error is logged, never breaks media loading). Tests: tests/test_notes_persistence.py covers JSON serialize/deserialize round-trip, history reset on load, sidecar save/load round-trip, empty-removes- sidecar, missing-clears-sketch, and deterministic profile-scoped paths. 6 tests pass (21 with the branch suite). Depends on the test harness (PR #7) and annotation redo (PR #9, for the redo stack reset in deserialize). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0177bi2WkPjrGytrarheKFDc Claude-Session-Id: 45a11a50-85fe-4349-b853-e1db9cd20276 --- tests/test_notes_persistence.py | 90 +++++++++++++++++++++++++++++++ widgets/__init__.py | 28 ++++++++++ widgets/annotations.py | 47 ++++++++++++++++ widgets/notestore.py | 96 +++++++++++++++++++++++++++++++++ 4 files changed, 261 insertions(+) create mode 100644 tests/test_notes_persistence.py create mode 100644 widgets/notestore.py diff --git a/tests/test_notes_persistence.py b/tests/test_notes_persistence.py new file mode 100644 index 0000000..210372f --- /dev/null +++ b/tests/test_notes_persistence.py @@ -0,0 +1,90 @@ +"""Tests for annotation persistence: Sketch serialize/deserialize + notestore.""" + +import json + +from widgets import notestore +from widgets.annotations import Sketch + + +def _sketch_with_strokes(): + sketch = Sketch() + sketch.set_frame(5) + sketch.strokes[5] = [ + { + "id": "a", "type": "pencil", "color": (255, 170, 0), "thickness": 3, + "points": [(0.1, 0.2), (0.3, 0.4)], + }, + { + "id": "b", "type": "rectangle", "color": (0, 255, 0), "thickness": 2, + "start": (0.2, 0.2), "end": (0.5, 0.6), + }, + ] + sketch.strokes[9] = [ + { + "id": "t", "type": "txt", "color": (255, 255, 255), + "position": (0.4, 0.5), "txt": "fix this", + }, + ] + return sketch + + +def test_serialize_deserialize_roundtrip_through_json(qapp): + source = _sketch_with_strokes() + # Force a real JSON round-trip (tuples -> lists -> tuples, int keys -> str). + data = json.loads(json.dumps(source.serialize())) + restored = Sketch() + restored.deserialize(data) + assert restored.strokes == source.strokes + + +def test_deserialize_resets_undo_and_redo(qapp): + sketch = _sketch_with_strokes() + sketch._record_action({"type": "create", "frame": 5, "stroke_id": "a"}) + sketch.undo() # populates redo_history + sketch.deserialize({"5": [{"id": "z", "type": "pencil", "points": [[0.1, 0.1]]}]}) + assert sketch.undo_history == [] + assert sketch.redo_history == [] + assert sketch.strokes[5][0]["points"] == [(0.1, 0.1)] # list -> tuple + + +def test_notestore_save_and_load_roundtrip(tmp_path, monkeypatch, qapp): + monkeypatch.setenv("FRAMEDECK_PROFILE_ROOT", str(tmp_path)) + source = str(tmp_path / "SHOT_010_comp_v001.mov") + + original = _sketch_with_strokes() + path = notestore.save_notes(source, original) + assert path is not None and path.exists() + + loaded_sketch = Sketch() + assert notestore.load_notes(source, loaded_sketch) is True + assert loaded_sketch.strokes == original.strokes + + +def test_notestore_empty_removes_sidecar(tmp_path, monkeypatch, qapp): + monkeypatch.setenv("FRAMEDECK_PROFILE_ROOT", str(tmp_path)) + source = str(tmp_path / "clip.mov") + + path = notestore.save_notes(source, _sketch_with_strokes()) + assert path.exists() + + # Saving an empty sketch clears the stale sidecar. + assert notestore.save_notes(source, Sketch()) is None + assert not path.exists() + + +def test_notestore_load_missing_clears_sketch(tmp_path, monkeypatch, qapp): + monkeypatch.setenv("FRAMEDECK_PROFILE_ROOT", str(tmp_path)) + sketch = _sketch_with_strokes() # starts non-empty + loaded = notestore.load_notes(str(tmp_path / "never_saved.mov"), sketch) + assert loaded is False + assert sketch.strokes == {} + + +def test_notes_path_is_stable_and_scoped_to_profile(tmp_path, monkeypatch, qapp): + monkeypatch.setenv("FRAMEDECK_PROFILE_ROOT", str(tmp_path)) + source = str(tmp_path / "a" / "shot.mov") + p1 = notestore.notes_path_for(source) + p2 = notestore.notes_path_for(source) + assert p1 == p2 # deterministic + assert str(tmp_path) in str(p1) # under the profile dir, not next to media + assert p1.name.startswith("shot_") and p1.suffix == ".json" diff --git a/widgets/__init__.py b/widgets/__init__.py index 860df82..892f9f0 100644 --- a/widgets/__init__.py +++ b/widgets/__init__.py @@ -973,6 +973,9 @@ def openMedia(self, filepath=None, add_to_playlist=True): if added: filepath = added[0]["media"] + # Persist the outgoing source's annotations before the viewer is wiped. + self._save_current_notes() + # Clear current viewer frame self.viewframe.viewer.clear() self.viewframe.viewer.reset_view() @@ -1050,6 +1053,8 @@ def openMedia(self, filepath=None, add_to_playlist=True): self.playlistWidget.set_active_media(source_filepath) self.shotSequenceWidget.set_active_media(source_filepath) self.current_source_filepath = source_filepath + # Restore any saved annotations for the incoming source. + self._load_notes_for_source(source_filepath) if hasattr(self, "sourceStatusLabel"): self.sourceStatusLabel.setText( f" SOURCE | {os.path.basename(source_filepath)} " @@ -1523,8 +1528,31 @@ def _secondary_frame_for(self, primary_frame): ), ) + def _save_current_notes(self): + """Persist the current source's annotations to its note sidecar.""" + source = self.current_source_filepath + if not source: + return + try: + from widgets import notestore + + notestore.save_notes(source, self.viewframe.viewer.annotations) + except Exception: + LOGGER.exception("Unable to save annotation notes") + + def _load_notes_for_source(self, source): + """Restore annotations for *source* from its note sidecar (if any).""" + try: + from widgets import notestore + + notestore.load_notes(source, self.viewframe.viewer.annotations) + self.viewframe.viewer.update() + except Exception: + LOGGER.exception("Unable to load annotation notes") + def closeEvent(self, event): """Stop decoder/cache threads cleanly before application exit.""" + self._save_current_notes() self._release_media_player(self.player) self._release_media_player(self.compare_player) self.media_cache.shutdown() diff --git a/widgets/annotations.py b/widgets/annotations.py index 6dfdf61..464ba90 100644 --- a/widgets/annotations.py +++ b/widgets/annotations.py @@ -377,6 +377,53 @@ def annotated_frames(self): """Return sorted frame numbers that contain at least one note.""" return sorted(frame for frame, strokes in self.strokes.items() if strokes) + def serialize(self): + """Return a JSON-serializable snapshot of every frame's strokes. + + Frame numbers become string keys (JSON object keys must be strings) and + coordinate/color tuples serialize to lists. Empty frames are omitted. + """ + return { + str(frame): strokes + for frame, strokes in self.strokes.items() + if strokes + } + + def deserialize(self, data): + """Replace all strokes from a snapshot produced by :meth:`serialize`. + + Coordinate/color fields are converted back from lists to tuples so the + loaded strokes render identically to freshly drawn ones. Undo/redo + history is reset -- the loaded state becomes the new baseline. + """ + self.strokes = dict() + for frame_key, strokes in (data or dict()).items(): + try: + frame = int(frame_key) + except (TypeError, ValueError): + continue + restored = [self._restore_stroke(stroke) for stroke in strokes] + if restored: + self.strokes[frame] = restored + self.undo_history.clear() + self.redo_history.clear() + + @staticmethod + def _restore_stroke(stroke): + """Convert a stroke's JSON list coordinates/colour back to tuples.""" + restored = dict(stroke) + points = restored.get("points") + if isinstance(points, list): + restored["points"] = [tuple(point) for point in points] + for key in ("start", "end", "position", "move_start"): + value = restored.get(key) + if isinstance(value, list): + restored[key] = tuple(value) + color = restored.get("color") + if isinstance(color, list): + restored["color"] = tuple(color) + return restored + def generate_id(self): """ Generate unique stroke ID. diff --git a/widgets/notestore.py b/widgets/notestore.py new file mode 100644 index 0000000..d660059 --- /dev/null +++ b/widgets/notestore.py @@ -0,0 +1,96 @@ +"""Persist per-source annotation notes to JSON sidecar files. + +Notes live under the FrameDeck profile directory (not next to the media), keyed +by a short hash of the absolute source path, so pencil/text annotations survive +closing and reopening a source or a whole session. + +The store is intentionally decoupled from the Sketch widget: it only calls the +pure ``serialize`` / ``deserialize`` methods, so it is straightforward to test. +""" + +import hashlib +import json +import os +from pathlib import Path + +SCHEMA = "framedeck-notes-v1" + + +def _profile_root(): + return os.environ.get("FRAMEDECK_PROFILE_ROOT") or str(Path.home() / "Documents") + + +def notes_dir(): + """Return the directory that holds note sidecars (not created here).""" + return Path(_profile_root()) / "framedeck" / "notes" + + +def notes_path_for(source): + """Return the sidecar path for a given media source path.""" + absolute = os.path.abspath(str(source)) + digest = hashlib.md5(os.path.normcase(absolute).encode("utf-8")).hexdigest()[:8] + return notes_dir() / f"{Path(absolute).stem}_{digest}.fdnotes.json" + + +def save_notes(source, sketch): + """Write *sketch*'s strokes to *source*'s sidecar. + + An empty annotation set removes any existing sidecar so a later load starts + clean. Returns the sidecar path when written, else ``None``. + """ + if not source: + return None + + path = notes_path_for(source) + data = sketch.serialize() + + if not data: + try: + if path.exists(): + path.unlink() + except OSError: + pass + return None + + document = { + "schema": SCHEMA, + "source": os.path.abspath(str(source)), + "annotations": data, + } + path.parent.mkdir(parents=True, exist_ok=True) + temporary = path.with_suffix(path.suffix + ".tmp") + with open(temporary, "w", encoding="utf-8") as stream: + json.dump(document, stream, ensure_ascii=False, indent=2) + os.replace(temporary, path) + return path + + +def load_notes(source, sketch): + """Load *source*'s sidecar into *sketch*. + + Always leaves *sketch* in a defined state: if there is no sidecar (or it is + unreadable/foreign) the sketch is cleared. Returns ``True`` when notes were + loaded from a valid sidecar. + """ + if not source: + sketch.deserialize({}) + return False + + path = notes_path_for(source) + if not path.exists(): + sketch.deserialize({}) + return False + + try: + with open(path, "r", encoding="utf-8") as stream: + document = json.load(stream) + except (OSError, ValueError): + sketch.deserialize({}) + return False + + if not isinstance(document, dict) or document.get("schema") != SCHEMA: + sketch.deserialize({}) + return False + + sketch.deserialize(document.get("annotations") or {}) + return True From 7cae81e5fcf75e912331b3cc787be4810d98f6d4 Mon Sep 17 00:00:00 2001 From: Eric Levy Date: Tue, 14 Jul 2026 10:40:49 -0400 Subject: [PATCH 4/5] Add per-frame comments with pinned markers Foundation for the Frame.io-style review workflow: per-frame text comments that can optionally be pinned to a point on the image. This is the model, rendering, persistence and navigation layer; the comment sidebar UI follows separately. - widgets/annotations.py: Sketch gains a comments store (frame -> comments) with add/get/delete/toggle-done, commented_frames() and comment_count(). A comment holds text, a timestamp, a done flag and optional normalized x/y. draw_comment_pins() renders numbered markers for the current frame's pinned comments (numbering matches list order; done comments use a distinct fill). annotated_frames() now returns the union of stroke and comment frames, and clear()/clear_all() cover both. - widgets/notestore.py: sidecars persist comments alongside strokes. A sketch holding only comments is still written; sidecars predating comments (no "comments" key) still load. - widgets/__init__.py: jump_to_annotation() with "[" / "]" shortcuts and Edit menu actions, seeking to the previous/next annotated frame. Annotations are keyed by the player's local frame while the timeline is global during playlist playback, so the jump maps local -> timeline explicitly. - constants: COMMENT_PIN_RADIUS and the pin colours. Tests: tests/test_comments.py covers comment CRUD, blank-text rejection, done-toggling, the stroke/comment frame union, per-frame and global clear, pin rendering (pixel-probing the marker fill, and the distinct done colour), unpinned comments drawing nothing, sidecar round-trip, a comment-only sketch still writing a sidecar, legacy sidecars without a comments key, and a missing sidecar clearing stale comments. 12 tests pass (33 with the branch suite). Depends on annotation persistence (PR #10), which is on PR #9 -> PR #7. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0177bi2WkPjrGytrarheKFDc Claude-Session-Id: 45a11a50-85fe-4349-b853-e1db9cd20276 --- constants/__init__.py | 5 + tests/test_comments.py | 201 +++++++++++++++++++++++++++++++++++++++++ widgets/__init__.py | 48 ++++++++++ widgets/annotations.py | 169 +++++++++++++++++++++++++++++++++- widgets/notestore.py | 24 +++-- 5 files changed, 436 insertions(+), 11 deletions(-) create mode 100644 tests/test_comments.py diff --git a/constants/__init__.py b/constants/__init__.py index 0f0f4af..19591fd 100644 --- a/constants/__init__.py +++ b/constants/__init__.py @@ -130,6 +130,11 @@ DEFAULT_SKETCH_COLOR = (255, 170, 0) +# Pinned-comment markers drawn on the frame. +COMMENT_PIN_RADIUS = 10 +COMMENT_PIN_COLOR = (255, 68, 68) +COMMENT_PIN_DONE_COLOR = (76, 175, 80) + OPEN_EXTENSIONS = [ "exr", "png", "jpg", "jpeg", "mp4", "mov", "avi", "fdplaylist" ] diff --git a/tests/test_comments.py b/tests/test_comments.py new file mode 100644 index 0000000..4ad2f3b --- /dev/null +++ b/tests/test_comments.py @@ -0,0 +1,201 @@ +"""Tests for the comment model: CRUD, pin rendering, and sidecar persistence.""" + +from PySide6.QtCore import QPointF +from PySide6.QtGui import QColor, QPainter, QPixmap + +import constants +from tests.helpers import probe_pixel +from widgets import notestore +from widgets.annotations import Sketch + + +def _converter(width, height): + def convert(point): + return QPointF(point[0] * width, point[1] * height) + + return convert + + +def _render(sketch, width=200, height=200): + pixmap = QPixmap(width, height) + pixmap.fill(QColor(0, 0, 0)) + painter = QPainter(pixmap) + try: + sketch.draw(painter, point_converter=_converter(width, height)) + finally: + painter.end() + return pixmap.toImage() + + +# --------------------------------------------------------------------------- # +# CRUD +# --------------------------------------------------------------------------- # +def test_add_comment_frame_level_and_pinned(qapp): + sketch = Sketch() + + plain = sketch.add_comment(10, "looks good") + pinned = sketch.add_comment(10, "fix this edge", x=0.25, y=0.75) + + assert plain["text"] == "looks good" + assert "x" not in plain # frame-level note carries no pin + assert (pinned["x"], pinned["y"]) == (0.25, 0.75) + assert plain["done"] is False + assert plain["timestamp"] + assert len(sketch.get_comments(10)) == 2 + + +def test_blank_comment_is_ignored(qapp): + sketch = Sketch() + assert sketch.add_comment(1, " ") is None + assert sketch.comment_count() == 0 + + +def test_delete_and_toggle_done(qapp): + sketch = Sketch() + comment = sketch.add_comment(4, "note") + + assert sketch.toggle_comment_done(4, comment["id"]) is True + assert sketch.get_comments(4)[0]["done"] is True + assert sketch.toggle_comment_done(4, comment["id"]) is False + + assert sketch.delete_comment(4, comment["id"]) is True + assert sketch.get_comments(4) == [] + assert 4 not in sketch.comments # empty frame entry removed + assert sketch.delete_comment(4, comment["id"]) is False # already gone + + +def test_annotated_frames_unions_strokes_and_comments(qapp): + sketch = Sketch() + sketch.strokes[2] = [{"id": "s", "type": "pencil", "points": [(0.1, 0.1)]}] + sketch.add_comment(7, "a note") + sketch.add_comment(2, "same frame as a stroke") + + assert sketch.annotated_frames() == [2, 7] + assert sketch.commented_frames() == [2, 7] + assert sketch.comment_count() == 2 + + +def test_clear_removes_comments_for_frame_only(qapp): + sketch = Sketch() + sketch.set_frame(3) + sketch.add_comment(3, "on three") + sketch.add_comment(9, "on nine") + + sketch.clear() + assert sketch.get_comments(3) == [] + assert len(sketch.get_comments(9)) == 1 + + sketch.clear_all() + assert sketch.comment_count() == 0 + + +# --------------------------------------------------------------------------- # +# Pin rendering (real pixels) +# --------------------------------------------------------------------------- # +def test_pinned_comment_draws_marker(qapp): + sketch = Sketch() + sketch.set_frame(1) + sketch.add_comment(1, "here", x=0.5, y=0.5) + + image = _render(sketch) + + # Marker centre carries the pin colour (white number sits on top, so probe + # slightly off-centre where the fill shows). + red, green, blue, _a = probe_pixel(image, 100, 106) + assert (red, green, blue) != (0, 0, 0) + assert red > 150 and blue < 120 # reddish pin fill + + # Away from the pin the frame is untouched. + assert probe_pixel(image, 10, 10)[:3] == (0, 0, 0) + + +def test_unpinned_comment_draws_nothing(qapp): + sketch = Sketch() + sketch.set_frame(1) + sketch.add_comment(1, "frame level, no pin") + + image = _render(sketch) + assert probe_pixel(image, 100, 100)[:3] == (0, 0, 0) + + +def test_done_pin_uses_distinct_colour(qapp): + sketch = Sketch() + sketch.set_frame(1) + comment = sketch.add_comment(1, "done one", x=0.5, y=0.5) + sketch.toggle_comment_done(1, comment["id"]) + + red, green, _blue, _a = probe_pixel(_render(sketch), 100, 106) + assert green > red # green "done" fill, not the red default + + +# --------------------------------------------------------------------------- # +# Persistence +# --------------------------------------------------------------------------- # +def test_comments_survive_sidecar_roundtrip(tmp_path, monkeypatch, qapp): + monkeypatch.setenv("FRAMEDECK_PROFILE_ROOT", str(tmp_path)) + source = str(tmp_path / "shot.mov") + + original = Sketch() + original.strokes[1] = [ + {"id": "s", "type": "pencil", "color": (255, 0, 0), + "thickness": 2, "points": [(0.1, 0.2)]} + ] + original.add_comment(1, "pinned note", x=0.3, y=0.4) + original.add_comment(5, "frame note") + + assert notestore.save_notes(source, original) is not None + + loaded = Sketch() + assert notestore.load_notes(source, loaded) is True + assert loaded.serialize_comments() == original.serialize_comments() + assert loaded.strokes == original.strokes + assert loaded.annotated_frames() == [1, 5] + + +def test_comment_only_sketch_still_writes_sidecar(tmp_path, monkeypatch, qapp): + monkeypatch.setenv("FRAMEDECK_PROFILE_ROOT", str(tmp_path)) + source = str(tmp_path / "comment_only.mov") + + sketch = Sketch() + sketch.add_comment(2, "no strokes, just a comment") + + assert notestore.save_notes(source, sketch) is not None # must not be skipped + + loaded = Sketch() + assert notestore.load_notes(source, loaded) is True + assert loaded.get_comments(2)[0]["text"] == "no strokes, just a comment" + + +def test_old_sidecar_without_comments_key_loads(tmp_path, monkeypatch, qapp): + import json + + monkeypatch.setenv("FRAMEDECK_PROFILE_ROOT", str(tmp_path)) + source = str(tmp_path / "legacy.mov") + + path = notestore.notes_path_for(source) + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text( + json.dumps( + { + "schema": notestore.SCHEMA, + "source": source, + "annotations": {"3": [{"id": "s", "type": "pencil", + "points": [[0.1, 0.2]]}]}, + } + ), + encoding="utf-8", + ) + + sketch = Sketch() + assert notestore.load_notes(source, sketch) is True + assert sketch.annotated_frames() == [3] + assert sketch.comment_count() == 0 # absent "comments" key is fine + + +def test_load_missing_clears_comments_too(tmp_path, monkeypatch, qapp): + monkeypatch.setenv("FRAMEDECK_PROFILE_ROOT", str(tmp_path)) + sketch = Sketch() + sketch.add_comment(1, "stale") + + assert notestore.load_notes(str(tmp_path / "nope.mov"), sketch) is False + assert sketch.comment_count() == 0 diff --git a/widgets/__init__.py b/widgets/__init__.py index 892f9f0..880bab7 100644 --- a/widgets/__init__.py +++ b/widgets/__init__.py @@ -406,6 +406,18 @@ def setup_review_chrome(self): ) self.actionRedo.triggered.connect(self.viewframe.viewer.redo_strokes) edit_menu.addAction(self.actionRedo) + self.actionPrevNote = QtGui.QAction("Previous Annotated Frame", self) + self.actionPrevNote.setShortcut(QtGui.QKeySequence("[")) + self.actionPrevNote.triggered.connect( + lambda: self.jump_to_annotation(-1) + ) + edit_menu.addAction(self.actionPrevNote) + self.actionNextNote = QtGui.QAction("Next Annotated Frame", self) + self.actionNextNote.setShortcut(QtGui.QKeySequence("]")) + self.actionNextNote.triggered.connect( + lambda: self.jump_to_annotation(1) + ) + edit_menu.addAction(self.actionNextNote) self.actionClearFrame = QtGui.QAction("Clear Notes on Frame", self) self.actionClearFrame.setIcon(NamePixmapIcon("clear")) self.actionClearFrame.triggered.connect(self.viewframe.viewer.clear_strokes) @@ -1528,6 +1540,42 @@ def _secondary_frame_for(self, primary_frame): ), ) + def _timeline_frame_for_local(self, local_frame): + """Map a player-local frame to its timeline frame. + + Annotations are keyed by the player's local frame, but the timeline is a + global range while a playlist is playing. + """ + if self.playlist_playback_active and 0 <= self.playlist_entry_index < len( + self.playlist_entries + ): + entry = self.playlist_entries[self.playlist_entry_index] + return entry["start"] + int(local_frame) - constants.VL_START_FRAME + return int(local_frame) + + def jump_to_annotation(self, step): + """Seek to the previous/next frame holding a note or comment.""" + annotations = self.viewframe.viewer.annotations + frames = annotations.annotated_frames() + if not frames: + return + + current = annotations.current_frame + if current is None: + current = constants.VL_START_FRAME + + if step > 0: + following = [frame for frame in frames if frame > current] + target = following[0] if following else None + else: + preceding = [frame for frame in frames if frame < current] + target = preceding[-1] if preceding else None + + if target is None: + return + + self.seek(self._timeline_frame_for_local(target)) + def _save_current_notes(self): """Persist the current source's annotations to its note sidecar.""" source = self.current_source_filepath diff --git a/widgets/annotations.py b/widgets/annotations.py index 464ba90..f19a5d8 100644 --- a/widgets/annotations.py +++ b/widgets/annotations.py @@ -106,6 +106,7 @@ from __future__ import absolute_import import copy +import datetime import uuid import constants @@ -204,6 +205,10 @@ def __init__(self): # Stroke Storage self.strokes = dict() + # Comment storage: frame -> list of comment dicts. Comments are text + # notes, optionally pinned to a normalized (x, y) point on the frame. + self.comments = dict() + # ---------------------------- # Image configuration # ---------------------------- @@ -374,8 +379,159 @@ def frame_count(self): return len(self.strokes) def annotated_frames(self): - """Return sorted frame numbers that contain at least one note.""" - return sorted(frame for frame, strokes in self.strokes.items() if strokes) + """Return sorted frames holding at least one stroke or comment.""" + frames = {frame for frame, strokes in self.strokes.items() if strokes} + frames.update( + frame for frame, comments in self.comments.items() if comments + ) + return sorted(frames) + + # ----------------------------------------------------------------------- # + # Comments + # ----------------------------------------------------------------------- # + def add_comment(self, frame, text, x=None, y=None): + """Add a text comment to *frame*. + + Pass normalized ``x``/``y`` (0.0-1.0) to pin the comment to a point on + the frame; omit them for a frame-level note. Empty text is ignored. + + Returns: + dict | None: The stored comment, or None when *text* was blank. + """ + + text = str(text or "").strip() + if not text: + return None + + comment = { + "id": self.generate_id(), + "text": text, + "timestamp": datetime.datetime.now().strftime(constants.DATE_TIME_FORMAT), + "done": False, + } + + if x is not None and y is not None: + comment["x"] = float(x) + comment["y"] = float(y) + + self.comments.setdefault(int(frame), list()).append(comment) + return comment + + def get_comments(self, frame=None): + """Return the comment list for *frame* (defaults to the current frame).""" + if frame is None: + frame = self.current_frame + if frame is None: + return list() + return self.comments.get(int(frame), list()) + + def delete_comment(self, frame, comment_id): + """Remove a comment by id. Returns True when one was removed.""" + frame = int(frame) + existing = self.comments.get(frame) + if not existing: + return False + + remaining = [ + comment for comment in existing if comment.get("id") != comment_id + ] + if len(remaining) == len(existing): + return False + + if remaining: + self.comments[frame] = remaining + else: + del self.comments[frame] + return True + + def toggle_comment_done(self, frame, comment_id): + """Flip a comment's done flag. Returns the new state, or None if absent.""" + for comment in self.comments.get(int(frame), list()): + if comment.get("id") == comment_id: + comment["done"] = not comment.get("done", False) + return comment["done"] + return None + + def commented_frames(self): + """Return sorted frame numbers that hold at least one comment.""" + return sorted( + frame for frame, comments in self.comments.items() if comments + ) + + def comment_count(self): + """Return the total number of comments across every frame.""" + return sum(len(comments) for comments in self.comments.values()) + + def draw_comment_pins(self, painter, point_converter=None): + """Draw numbered markers for the current frame's pinned comments. + + Numbering follows the frame's comment order, so a marker's number + matches its row in the comment list. Done comments use a distinct fill. + """ + + pinned = [ + comment + for comment in self.get_comments() + if "x" in comment and "y" in comment + ] + if not pinned: + return + + radius = constants.COMMENT_PIN_RADIUS + + painter.save() + for number, comment in enumerate(pinned, start=1): + point = (comment["x"], comment["y"]) + point = point_converter(point) if point_converter else QtCore.QPointF(*point) + + fill = ( + constants.COMMENT_PIN_DONE_COLOR + if comment.get("done") + else constants.COMMENT_PIN_COLOR + ) + + painter.setPen(QtGui.QPen(QtGui.QColor(0, 0, 0), 1.5)) + painter.setBrush(QtGui.QColor(*fill)) + painter.drawEllipse(point, radius, radius) + + font = painter.font() + font.setBold(True) + font.setPointSize(8) + painter.setFont(font) + painter.setPen(QtGui.QColor(255, 255, 255)) + painter.drawText( + QtCore.QRectF( + point.x() - radius, + point.y() - radius, + radius * 2, + radius * 2, + ), + QtCore.Qt.AlignCenter, + str(number), + ) + painter.restore() + + def serialize_comments(self): + """Return a JSON-serializable snapshot of every frame's comments.""" + return { + str(frame): comments + for frame, comments in self.comments.items() + if comments + } + + def deserialize_comments(self, data): + """Replace all comments from a :meth:`serialize_comments` snapshot.""" + self.comments = dict() + for frame_key, comments in (data or dict()).items(): + try: + frame = int(frame_key) + except (TypeError, ValueError): + continue + restored = [ + dict(comment) for comment in comments if comment.get("text") + ] + if restored: + self.comments[frame] = restored def serialize(self): """Return a JSON-serializable snapshot of every frame's strokes. @@ -679,6 +835,9 @@ def draw(self, painter, point_converter=None, rect=None): if stroke is self.selected_stroke: self.draw_selection(painter, stroke, point_converter) + # Draw pinned comment markers above the strokes + self.draw_comment_pins(painter, point_converter) + # Draw Watermarks if rect: self.draw_overlays(painter, rect) @@ -1317,7 +1476,7 @@ def hit_pencil_move(self, stroke, point): def clear(self): """ - Remove all strokes from the current frame. + Remove all notes (strokes and comments) from the current frame. This does NOT affect other frames. @@ -1329,10 +1488,11 @@ def clear(self): # Safely remove frame entry if it exists self.strokes.pop(self.current_frame, None) + self.comments.pop(self.current_frame, None) def clear_all(self): """ - Remove all strokes from all frames. + Remove all notes (strokes and comments) from all frames. This resets the entire sketch system. @@ -1341,6 +1501,7 @@ def clear_all(self): """ self.strokes.clear() + self.comments.clear() def draw_overlays(self, painter, rect): """ diff --git a/widgets/notestore.py b/widgets/notestore.py index d660059..45b02cb 100644 --- a/widgets/notestore.py +++ b/widgets/notestore.py @@ -32,8 +32,14 @@ def notes_path_for(source): return notes_dir() / f"{Path(absolute).stem}_{digest}.fdnotes.json" +def _clear(sketch): + """Reset a sketch to an empty, defined state (strokes and comments).""" + sketch.deserialize({}) + sketch.deserialize_comments({}) + + def save_notes(source, sketch): - """Write *sketch*'s strokes to *source*'s sidecar. + """Write *sketch*'s strokes and comments to *source*'s sidecar. An empty annotation set removes any existing sidecar so a later load starts clean. Returns the sidecar path when written, else ``None``. @@ -43,8 +49,9 @@ def save_notes(source, sketch): path = notes_path_for(source) data = sketch.serialize() + comments = sketch.serialize_comments() - if not data: + if not data and not comments: try: if path.exists(): path.unlink() @@ -56,6 +63,7 @@ def save_notes(source, sketch): "schema": SCHEMA, "source": os.path.abspath(str(source)), "annotations": data, + "comments": comments, } path.parent.mkdir(parents=True, exist_ok=True) temporary = path.with_suffix(path.suffix + ".tmp") @@ -66,31 +74,33 @@ def save_notes(source, sketch): def load_notes(source, sketch): - """Load *source*'s sidecar into *sketch*. + """Load *source*'s sidecar (strokes and comments) into *sketch*. Always leaves *sketch* in a defined state: if there is no sidecar (or it is unreadable/foreign) the sketch is cleared. Returns ``True`` when notes were loaded from a valid sidecar. """ if not source: - sketch.deserialize({}) + _clear(sketch) return False path = notes_path_for(source) if not path.exists(): - sketch.deserialize({}) + _clear(sketch) return False try: with open(path, "r", encoding="utf-8") as stream: document = json.load(stream) except (OSError, ValueError): - sketch.deserialize({}) + _clear(sketch) return False if not isinstance(document, dict) or document.get("schema") != SCHEMA: - sketch.deserialize({}) + _clear(sketch) return False + # "comments" is absent in sidecars written before comments existed. sketch.deserialize(document.get("annotations") or {}) + sketch.deserialize_comments(document.get("comments") or {}) return True From ce11708dcd464226d1f949e9d82c5e7f113efd8b Mon Sep 17 00:00:00 2001 From: Eric Levy Date: Tue, 14 Jul 2026 11:28:33 -0400 Subject: [PATCH 5/5] Add comment sidebar and pin tool Turns the comment model into a usable review workflow: a Comments panel listing every note grouped by frame, and a viewer tool that pins a note to a point on the frame. - widgets/commentpanel.py: frame-grouped comment tree. Click a row to seek, tick to mark done, delete the selection, add a frame-level note. The panel is a pure view over Sketch.comments and owns no state, so it cannot drift from the markers drawn on the frame. Pin numbers in the list match the numbers inside the on-frame markers. - Viewer gains a Comment tool. Clicking the frame emits comment_requested with the normalized hit point; the window prompts for text and pins it. - View > Comments Panel (Ctrl+M) toggles the sidebar, which opens to a usable width the first time it is shown. - Sketch.mousePressEvent now returns early for the comment tool. A {type: comment} stroke would render as nothing and be silently dropped by erase(), which only re-appends the stroke types it recognizes. Note toggling a comment done restyles its row in place. Rebuilding the tree from inside itemChanged destroys the item Qt is still emitting the signal for, which is a use-after-free that takes the process down; the tests caught it as an intermittent access violation. 15 tests, including the in-place-restyle regression guard. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01TexnzYbmCjjTDB8zzZuUPb --- tests/test_comment_panel.py | 344 +++++++++++++++++++++++++++++++++++ widgets/__init__.py | 87 ++++++++- widgets/annotations.py | 7 + widgets/buttons.py | 6 + widgets/commentpanel.py | 350 ++++++++++++++++++++++++++++++++++++ widgets/pixmaps.py | 9 + widgets/viewer.py | 30 ++++ 7 files changed, 832 insertions(+), 1 deletion(-) create mode 100644 tests/test_comment_panel.py create mode 100644 widgets/commentpanel.py diff --git a/tests/test_comment_panel.py b/tests/test_comment_panel.py new file mode 100644 index 0000000..2090efa --- /dev/null +++ b/tests/test_comment_panel.py @@ -0,0 +1,344 @@ +"""Tests for the comment sidebar and the viewer's comment pin tool.""" + +from PySide6 import QtCore +from PySide6 import QtGui + +import pytest + +from widgets.annotations import Sketch +from widgets.commentpanel import COMMENT_ROLE, FRAME_ROLE, CommentPanel + + +# Every panel built by a test is registered here and destroyed before the test +# ends. A top-level QWidget that survives into interpreter shutdown is torn down +# after the QApplication is already gone, which kills the process with an access +# violation -- intermittently, so it must be prevented rather than retried. +_PANELS = list() + + +@pytest.fixture(autouse=True) +def _destroy_panels(qapp): + yield + while _PANELS: + panel = _PANELS.pop() + panel.close() + panel.deleteLater() + qapp.processEvents() + + +def _new_panel(): + panel = CommentPanel(None) + _PANELS.append(panel) + return panel + + +def _panel(sketch, frame=None): + panel = _new_panel() + panel.set_sketch(sketch) + if frame is not None: + panel.set_current_frame(frame) + return panel + + +def _rows(panel): + """Return the tree as [(frame_label, [child_label, ...]), ...].""" + tree = panel.commentTree + rows = list() + for index in range(tree.topLevelItemCount()): + parent = tree.topLevelItem(index) + children = [parent.child(i).text(0) for i in range(parent.childCount())] + rows.append((parent.text(0), children)) + return rows + + +# --------------------------------------------------------------------------- # +# Rendering the sketch into the tree +# --------------------------------------------------------------------------- # +def test_panel_groups_comments_by_frame(qapp): + sketch = Sketch() + sketch.add_comment(12, "first on twelve") + sketch.add_comment(12, "second on twelve") + sketch.add_comment(3, "on three") + + panel = _panel(sketch) + + # Frames ascend regardless of insertion order. + labels = [label for label, _children in _rows(panel)] + assert labels == ["Frame 0003 (1)", "Frame 0012 (2)"] + assert panel.titleLabel.text() == "Comments (3)" + + +def test_pinned_comments_are_numbered_like_their_markers(qapp): + sketch = Sketch() + sketch.add_comment(1, "frame level note") # unpinned: no number + sketch.add_comment(1, "first pin", x=0.2, y=0.2) + sketch.add_comment(1, "second pin", x=0.8, y=0.8) + + _label, children = _rows(_panel(sketch))[0] + + # Numbering counts pinned comments only, matching draw_comment_pins(). + assert children == ["frame level note", "1. first pin", "2. second pin"] + + +def test_empty_sketch_renders_empty_tree(qapp): + panel = _panel(Sketch()) + + assert panel.commentTree.topLevelItemCount() == 0 + assert panel.titleLabel.text() == "Comments" + assert panel.deleteButton.isEnabled() is False + + +def test_panel_without_sketch_does_not_crash(qapp): + panel = _new_panel() + panel.refresh() + + assert panel.commentTree.topLevelItemCount() == 0 + + +# --------------------------------------------------------------------------- # +# Interaction +# --------------------------------------------------------------------------- # +def test_clicking_a_row_requests_a_seek(qapp): + sketch = Sketch() + sketch.add_comment(42, "seek me") + panel = _panel(sketch) + + seeks = list() + panel.seek_requested.connect(seeks.append) + + parent = panel.commentTree.topLevelItem(0) + panel.commentTree.itemClicked.emit(parent.child(0), 0) + panel.commentTree.itemClicked.emit(parent, 0) # frame row seeks too + + assert seeks == [42, 42] + + +def test_checkbox_toggles_done_on_the_sketch(qapp): + sketch = Sketch() + comment = sketch.add_comment(5, "fix the edge") + panel = _panel(sketch) + + changed = list() + panel.comments_changed.connect(lambda: changed.append(True)) + + child = panel.commentTree.topLevelItem(0).child(0) + assert child.checkState(0) == QtCore.Qt.CheckState.Unchecked + + child.setCheckState(0, QtCore.Qt.CheckState.Checked) + + assert sketch.get_comments(5)[0]["done"] is True + assert changed == [True] + + # The row must be restyled IN PLACE. Rebuilding the tree from inside + # itemChanged destroys the item Qt is still emitting for -- a use-after-free + # that crashes the process. Same object == no rebuild happened. + assert panel.commentTree.topLevelItem(0).child(0) is child + + # It reflects the new state, and restyling did not re-emit as a user edit. + assert child.checkState(0) == QtCore.Qt.CheckState.Checked + assert child.font(0).strikeOut() is True + assert changed == [True] + + # Unchecking flips it back. + child.setCheckState(0, QtCore.Qt.CheckState.Unchecked) + assert sketch.get_comments(5)[0]["done"] is False + assert child.font(0).strikeOut() is False + assert changed == [True, True] + + assert comment["id"] == sketch.get_comments(5)[0]["id"] + + +def test_refresh_does_not_emit_comments_changed(qapp): + """Rebuilding sets check states; that must not look like a user edit.""" + sketch = Sketch() + done = sketch.add_comment(2, "already done") + sketch.toggle_comment_done(2, done["id"]) + + panel = _new_panel() + changed = list() + panel.comments_changed.connect(lambda: changed.append(True)) + + panel.set_sketch(sketch) + panel.refresh() + + assert changed == [] + + +def test_delete_selected_removes_the_comment(qapp): + sketch = Sketch() + sketch.add_comment(7, "keep me") + sketch.add_comment(7, "delete me") + panel = _panel(sketch) + + changed = list() + panel.comments_changed.connect(lambda: changed.append(True)) + + victim = panel.commentTree.topLevelItem(0).child(1) + victim.setSelected(True) + panel.selection_changed() + assert panel.deleteButton.isEnabled() is True + + panel.delete_selected() + + remaining = [comment["text"] for comment in sketch.get_comments(7)] + assert remaining == ["keep me"] + assert changed == [True] + assert panel.deleteButton.isEnabled() is False + + +def test_selecting_a_frame_row_does_not_enable_delete(qapp): + sketch = Sketch() + sketch.add_comment(7, "a note") + panel = _panel(sketch) + + panel.commentTree.topLevelItem(0).setSelected(True) + panel.selection_changed() + + assert panel.selected_comment() is None + assert panel.deleteButton.isEnabled() is False + + +def test_add_note_targets_the_current_frame(qapp): + sketch = Sketch() + panel = _panel(sketch, frame=9) + + changed = list() + panel.comments_changed.connect(lambda: changed.append(True)) + + panel.commentLineEdit.setText(" a note with padding ") + panel.add_note() + + assert [c["text"] for c in sketch.get_comments(9)] == ["a note with padding"] + assert "x" not in sketch.get_comments(9)[0] # frame-level, not pinned + assert panel.commentLineEdit.text() == "" + assert changed == [True] + + +def test_add_note_ignores_blank_text_and_missing_frame(qapp): + sketch = Sketch() + panel = _panel(sketch, frame=1) + + panel.commentLineEdit.setText(" ") + panel.add_note() + assert sketch.comment_count() == 0 + + # No current frame: nothing to attach to. + panel.set_current_frame(None) + panel.commentLineEdit.setText("orphan") + panel.add_note() + assert sketch.comment_count() == 0 + assert panel.commentLineEdit.isEnabled() is False + + +def test_rows_carry_frame_and_comment_identity(qapp): + sketch = Sketch() + comment = sketch.add_comment(4, "identified") + panel = _panel(sketch) + + parent = panel.commentTree.topLevelItem(0) + child = parent.child(0) + + assert parent.data(0, FRAME_ROLE) == 4 + assert parent.data(0, COMMENT_ROLE) is None # frame rows hold no comment + assert child.data(0, FRAME_ROLE) == 4 + assert child.data(0, COMMENT_ROLE) == comment["id"] + + +# --------------------------------------------------------------------------- # +# The viewer's pin tool +# +# ViewerWidget is a QOpenGLWidget, and constructing one under the offscreen +# platform crashes Qt on teardown (access violation) roughly one run in three -- +# it cannot create a GL context. So the real ViewerWidget.mousePressEvent is +# invoked against a stand-in carrying only the attributes that method reads. +# The code under test is the shipped code; only the GL shell is stubbed out. +# --------------------------------------------------------------------------- # +class _ViewerStub: + """Minimal stand-in for ViewerWidget, holding a real Sketch.""" + + def __init__(self): + from widgets.viewer import ViewerWidget + + self.gamma_check_enabled = False + self.exposure_check_enabled = False + self.compare_enabled = False + self.compare_qimage = None + self.compare_mode = "wipe_vertical" + self.display_rect = QtCore.QRect(0, 0, 200, 100) + + self.annotations = Sketch() + self.annotations.set_frame(1) + self.annotations.set_enabled(True) + + self.emitted = list() + self.comment_requested = type( + "_Signal", (), {"emit": lambda _self, value: self.emitted.append(value)} + )() + + self.updates = 0 + + # Bind the real implementations under test. + self.mousePressEvent = ViewerWidget.mousePressEvent.__get__(self) + self.widget_to_image_point = ViewerWidget.widget_to_image_point.__get__(self) + + def update(self): + self.updates += 1 + + +def _press(viewer, x, y, button=QtCore.Qt.MouseButton.LeftButton): + event = QtGui.QMouseEvent( + QtCore.QEvent.Type.MouseButtonPress, + QtCore.QPointF(x, y), + QtCore.QPointF(x, y), + button, + button, + QtCore.Qt.KeyboardModifier.NoModifier, + ) + viewer.mousePressEvent(event) + + +def test_comment_tool_click_emits_the_hit_point(qapp): + viewer = _ViewerStub() + viewer.annotations.set_tool("comment") + + _press(viewer, 50, 25) + + assert len(viewer.emitted) == 1 + x, y = viewer.emitted[0] + assert x == pytest.approx(0.25) + assert y == pytest.approx(0.25) + + # The click must not become a stroke. + assert viewer.annotations.strokes == {} + assert viewer.annotations.drawing is False + + +def test_pencil_tool_still_draws(qapp): + viewer = _ViewerStub() + viewer.annotations.set_tool("pencil") + + _press(viewer, 50, 25) + + assert viewer.emitted == [] + assert len(viewer.annotations.strokes[1]) == 1 + assert viewer.annotations.strokes[1][0]["type"] == "pencil" + + +def test_comment_tool_never_creates_a_stroke(qapp): + """Guard in the model itself, independent of the viewer. + + A {"type": "comment"} stroke would draw as nothing and would be silently + dropped by erase(), which only re-appends stroke types it knows. + """ + sketch = Sketch() + sketch.set_frame(1) + sketch.set_enabled(True) + sketch.set_tool("comment") + + sketch.mousePressEvent((0.5, 0.5)) + sketch.mouseMoveEvent((0.6, 0.6)) + sketch.mouseReleaseEvent((0.7, 0.7)) + + assert sketch.strokes == {} + assert sketch.drawing is False + assert sketch.undo_history == [] diff --git a/widgets/__init__.py b/widgets/__init__.py index 880bab7..d46a931 100644 --- a/widgets/__init__.py +++ b/widgets/__init__.py @@ -57,6 +57,7 @@ from playback.reader import SequenceReader from widgets.recaps import RecapsWidget +from widgets.commentpanel import CommentPanel from widgets.styles import SetStylesheet from widgets.playlist import PlaylistWidget from widgets.shotstrip import ShotSequenceWidget @@ -181,6 +182,10 @@ def setupUi(self): self.viewframe = ViewFrame(self) self.splitter.addWidget(self.viewframe) + # Local per-frame comments (sidecar-backed, no tracker involved). + self.commentPanel = CommentPanel(self) + self.splitter.addWidget(self.commentPanel) + self.recapsWidget = RecapsWidget(self) self.splitter.addWidget(self.recapsWidget) @@ -281,6 +286,14 @@ def setupUi(self): ) self.viewframe.viewer.fullscreen_requested.connect(self.toggle_fullscreen) + # -------------------------------------------------------------------- + # Comment Panel Signal Connections + # -------------------------------------------------------------------- + self.commentPanel.set_sketch(self.viewframe.viewer.annotations) + self.viewframe.viewer.comment_requested.connect(self.add_pinned_comment) + self.commentPanel.seek_requested.connect(self.seek_to_comment) + self.commentPanel.comments_changed.connect(self.handle_comments_changed) + # self.recapsWidget.inputWidget.trigger_snapshot.connect(self.render_snapshot) # -------------------------------------------------------------------- @@ -321,7 +334,7 @@ def setupUi(self): self.apply_review_styles() # Initial splitter sizes - self.splitter.setSizes([320, 1100, 0]) + self.splitter.setSizes([320, 1100, 0, 0]) def setupIcons(self): """ @@ -445,6 +458,11 @@ def setup_review_chrome(self): self.actionShotTimeline.setChecked(True) self.actionShotTimeline.toggled.connect(self.shotSequenceWidget.setVisible) view_menu.addAction(self.actionShotTimeline) + self.actionCommentPanel = QtGui.QAction("Comments Panel", self, checkable=True) + self.actionCommentPanel.setIcon(NamePixmapIcon("comment")) + self.actionCommentPanel.setShortcut(QtGui.QKeySequence("Ctrl+M")) + self.actionCommentPanel.toggled.connect(self.set_comment_panel_visible) + view_menu.addAction(self.actionCommentPanel) self.actionRecaps = QtGui.QAction("Review Notes Panel", self, checkable=True) self.actionRecaps.setIcon(NamePixmapIcon("txt")) self.actionRecaps.toggled.connect(self.recapsWidget.set_current_recaps) @@ -823,6 +841,9 @@ def _on_primary_frame_changed(self, local_frame): else: self.viewframe.timeline.set_current_frame(local_frame) + # New notes attach to the frame the viewer is actually showing. + self.commentPanel.set_current_frame(int(local_frame)) + def _on_primary_cache_changed(self, local_frames): if self.playlist_playback_active and 0 <= self.playlist_entry_index < len(self.playlist_entries): offset = self.playlist_entries[self.playlist_entry_index]["start"] - constants.VL_START_FRAME @@ -1576,6 +1597,68 @@ def jump_to_annotation(self, step): self.seek(self._timeline_frame_for_local(target)) + def set_comment_panel_visible(self, enabled): + """Show or hide the comment sidebar, giving it width on first open.""" + self.commentPanel.set_visible_state(enabled) + + if not enabled: + return + + # The panel starts collapsed to zero width in the splitter; open it to a + # usable size the first time it is shown, but never fight a user resize. + sizes = self.splitter.sizes() + index = self.splitter.indexOf(self.commentPanel) + if index < 0 or sizes[index] > 0: + return + + width = 320 + viewer_index = self.splitter.indexOf(self.viewframe) + if viewer_index >= 0 and sizes[viewer_index] > width * 2: + sizes[viewer_index] -= width + sizes[index] = width + self.splitter.setSizes(sizes) + + def add_pinned_comment(self, point): + """Prompt for text and pin the resulting comment to *point* on the frame. + + Args: + point (tuple): + Normalized (x, y) hit point emitted by the viewer. + """ + + annotations = self.viewframe.viewer.annotations + frame = annotations.current_frame + if frame is None: + return + + text, accepted = QtWidgets.QInputDialog.getMultiLineText( + self, + "Pin Comment", + "Note for frame {0}:".format( + str(frame).zfill(constants.VL_FRAME_PADDING) + ), + ) + if not accepted: + return + + if annotations.add_comment(frame, text, x=point[0], y=point[1]) is None: + return + + # Make the note visible immediately, even if the panel was closed. + self.actionCommentPanel.setChecked(True) + + self.commentPanel.refresh() + self.handle_comments_changed() + + def seek_to_comment(self, frame): + """Seek to the frame a comment row points at.""" + self.seek(self._timeline_frame_for_local(frame)) + + def handle_comments_changed(self): + """Repaint the pins and persist the change to the note sidecar.""" + self.viewframe.viewer.update() + self._save_current_notes() + def _save_current_notes(self): """Persist the current source's annotations to its note sidecar.""" source = self.current_source_filepath @@ -1595,6 +1678,7 @@ def _load_notes_for_source(self, source): notestore.load_notes(source, self.viewframe.viewer.annotations) self.viewframe.viewer.update() + self.commentPanel.refresh() except Exception: LOGGER.exception("Unable to load annotation notes") @@ -1872,6 +1956,7 @@ def set_fullscreen(self, enabled): self.reviewToolbar, self.statusBar(), self.playlistWidget, + self.commentPanel, self.recapsWidget, self.shotSequenceWidget, ) diff --git a/widgets/annotations.py b/widgets/annotations.py index f19a5d8..6166dc4 100644 --- a/widgets/annotations.py +++ b/widgets/annotations.py @@ -601,6 +601,13 @@ def mousePressEvent(self, point): if self.current_frame is None: return + # Comment tool: the viewer pins a note at this point instead. Bail out + # before any stroke is created -- a {"type": "comment"} stroke would + # render as nothing and be silently discarded by erase(), which only + # re-appends the stroke types it recognizes. + if self.tool == "comment": + return + # Enable drag operation self.drawing = True diff --git a/widgets/buttons.py b/widgets/buttons.py index 81544c9..63039ae 100644 --- a/widgets/buttons.py +++ b/widgets/buttons.py @@ -270,6 +270,12 @@ class AttachButton(IconButton): name = "attach" +class CommentButton(IconButton): + """Pin a comment to a point on the frame.""" + + name = "comment" + + class RecapsButton(IconButton): """Recaps button.""" diff --git a/widgets/commentpanel.py b/widgets/commentpanel.py new file mode 100644 index 0000000..5c6760e --- /dev/null +++ b/widgets/commentpanel.py @@ -0,0 +1,350 @@ +""" +Copyright (c) 2026, Motion-Craft Technology All rights reserved. + +Module: + ./widgets/commentpanel.py + +Description: + Sidebar listing every comment held by the active Sketch, grouped by frame. + + The panel is a thin view over ``Sketch.comments`` -- it owns no comment + state of its own. Every edit is applied straight to the sketch and the tree + is rebuilt from it, so the panel and the pinned markers drawn on the frame + can never drift apart. + +Responsibilities: + - List comments grouped by frame, newest frame last + - Seek the player to a comment's frame on click + - Toggle a comment's "done" flag from its checkbox + - Delete the selected comment + - Add a frame-level comment to the current frame + +Notes: + Pinned comments (those carrying x/y) are numbered per frame to match the + numbers drawn inside the on-frame markers by ``Sketch.draw_comment_pins``. +""" + +from __future__ import absolute_import + +from PySide6 import QtCore +from PySide6 import QtGui +from PySide6 import QtWidgets + +import constants + +from widgets.layouts import HorizontalLayout +from widgets.layouts import VerticalLayout + +# Item roles carrying the identity of the comment behind a tree row. +FRAME_ROLE = QtCore.Qt.ItemDataRole.UserRole +COMMENT_ROLE = QtCore.Qt.ItemDataRole.UserRole + 1 + + +class CommentPanel(QtWidgets.QWidget): + """Frame-grouped comment list for the active sketch. + + Signals: + seek_requested (int): + A row was activated; the player should move to this frame. + + comments_changed (): + A comment was added, deleted, or toggled. The viewer should + repaint and the notes should be re-saved. + + Example: + >>> panel = CommentPanel(parent) + >>> panel.set_sketch(viewer.annotations) + >>> panel.refresh() + """ + + seek_requested = QtCore.Signal(int) + comments_changed = QtCore.Signal() + + def __init__(self, parent=None, *args, **kwargs): + super(CommentPanel, self).__init__(parent, *args, **kwargs) + + # Hidden until the user asks for it, like the recaps panel. + self.setVisible(False) + + # The sketch whose comments are displayed. None until a media is open. + self.sketch = None + + # Target frame for newly added frame-level comments. + self.current_frame = None + + # Guard: suppresses itemChanged while the tree is being rebuilt. + self._loading = False + + self.setupUi() + + def setupUi(self): + """Build the panel user interface.""" + + self.mainlayout = VerticalLayout(self, space=6, margins=(6, 6, 6, 6)) + + # ----------------------------------------------------------------- # + # Header + # ----------------------------------------------------------------- # + self.titleLabel = QtWidgets.QLabel("Comments", self) + self.titleLabel.setStyleSheet("font-weight: bold;") + self.mainlayout.addWidget(self.titleLabel) + + # ----------------------------------------------------------------- # + # Comment tree: frame parents, comment children + # ----------------------------------------------------------------- # + self.commentTree = QtWidgets.QTreeWidget(self) + self.commentTree.setHeaderHidden(True) + self.commentTree.setColumnCount(1) + self.commentTree.setRootIsDecorated(True) + self.commentTree.setAlternatingRowColors(True) + self.commentTree.setSelectionMode( + QtWidgets.QAbstractItemView.SelectionMode.SingleSelection + ) + self.commentTree.setWordWrap(True) + self.mainlayout.addWidget(self.commentTree) + + # ----------------------------------------------------------------- # + # Add a note to the current frame + # ----------------------------------------------------------------- # + self.inputLayout = HorizontalLayout(None, space=4, margins=(0, 0, 0, 0)) + + self.commentLineEdit = QtWidgets.QLineEdit(self) + self.commentLineEdit.setPlaceholderText("Add a note to this frame...") + self.inputLayout.addWidget(self.commentLineEdit) + + self.addButton = QtWidgets.QPushButton("Add", self) + self.addButton.setFixedWidth(56) + self.inputLayout.addWidget(self.addButton) + + self.mainlayout.addLayout(self.inputLayout) + + # ----------------------------------------------------------------- # + # Row actions + # ----------------------------------------------------------------- # + self.deleteButton = QtWidgets.QPushButton("Delete Selected", self) + self.deleteButton.setEnabled(False) + self.mainlayout.addWidget(self.deleteButton) + + # ----------------------------------------------------------------- # + # Signals + # ----------------------------------------------------------------- # + self.commentTree.itemClicked.connect(self.item_clicked) + self.commentTree.itemChanged.connect(self.item_changed) + self.commentTree.itemSelectionChanged.connect(self.selection_changed) + + self.addButton.clicked.connect(self.add_note) + self.commentLineEdit.returnPressed.connect(self.add_note) + self.deleteButton.clicked.connect(self.delete_selected) + + # --------------------------------------------------------------------- # + # State + # --------------------------------------------------------------------- # + def set_sketch(self, sketch): + """Bind the panel to a sketch (pass None when no media is open).""" + self.sketch = sketch + self.refresh() + + def set_current_frame(self, frame): + """Set the frame that newly added notes are attached to.""" + self.current_frame = frame + self.commentLineEdit.setEnabled(frame is not None) + + def set_visible_state(self, enabled): + """Show or hide the panel (mirrors RecapsWidget.set_current_recaps).""" + self.setVisible(enabled) + + # --------------------------------------------------------------------- # + # Rendering + # --------------------------------------------------------------------- # + def refresh(self): + """Rebuild the tree from the bound sketch's comments.""" + + # Rebuilding sets check states; do not treat those as user edits. + self._loading = True + try: + self.commentTree.clear() + + total = 0 + if self.sketch: + for frame in self.sketch.commented_frames(): + comments = self.sketch.get_comments(frame) + total += len(comments) + self.commentTree.addTopLevelItem( + self.build_frame_item(frame, comments) + ) + + self.commentTree.expandAll() + finally: + self._loading = False + + self.titleLabel.setText( + "Comments ({0})".format(total) if total else "Comments" + ) + self.deleteButton.setEnabled(False) + + def build_frame_item(self, frame, comments): + """Return a frame row holding one child row per comment.""" + + padded = str(frame).zfill(constants.VL_FRAME_PADDING) + parent = QtWidgets.QTreeWidgetItem( + ["Frame {0} ({1})".format(padded, len(comments))] + ) + parent.setData(0, FRAME_ROLE, int(frame)) + parent.setFlags( + QtCore.Qt.ItemFlag.ItemIsEnabled | QtCore.Qt.ItemFlag.ItemIsSelectable + ) + + font = parent.font(0) + font.setBold(True) + parent.setFont(0, font) + + # Pin numbers count only pinned comments, matching the on-frame markers. + pin_number = 0 + for comment in comments: + pinned = "x" in comment and "y" in comment + if pinned: + pin_number += 1 + + label = comment["text"] + if pinned: + label = "{0}. {1}".format(pin_number, label) + + child = QtWidgets.QTreeWidgetItem([label]) + child.setData(0, FRAME_ROLE, int(frame)) + child.setData(0, COMMENT_ROLE, comment["id"]) + child.setFlags( + QtCore.Qt.ItemFlag.ItemIsEnabled + | QtCore.Qt.ItemFlag.ItemIsSelectable + | QtCore.Qt.ItemFlag.ItemIsUserCheckable + ) + child.setToolTip(0, comment.get("timestamp", "")) + + self.style_comment_item(child, comment, pinned) + + parent.addChild(child) + + return parent + + def style_comment_item(self, item, comment, pinned): + """Apply a comment's done state to its row (check mark, strike, colour). + + Callers must hold the ``_loading`` guard: every setter here emits + ``itemChanged``. + """ + + item.setCheckState( + 0, + QtCore.Qt.CheckState.Checked + if comment.get("done") + else QtCore.Qt.CheckState.Unchecked, + ) + + font = item.font(0) + font.setStrikeOut(bool(comment.get("done"))) + item.setFont(0, font) + + if comment.get("done"): + color = constants.COMMENT_PIN_DONE_COLOR + elif pinned: + color = constants.COMMENT_PIN_COLOR + else: + color = None + + if color is None: + item.setData(0, QtCore.Qt.ItemDataRole.ForegroundRole, None) + else: + item.setForeground(0, QtGui.QBrush(QtGui.QColor(*color))) + + # --------------------------------------------------------------------- # + # Interaction + # --------------------------------------------------------------------- # + def item_clicked(self, item, column=0): + """Seek to the frame behind the clicked row.""" + frame = item.data(0, FRAME_ROLE) + if frame is not None: + self.seek_requested.emit(int(frame)) + + def item_changed(self, item, column=0): + """Apply a checkbox change to the sketch as a done-toggle.""" + if self._loading or not self.sketch: + return + + comment_id = item.data(0, COMMENT_ROLE) + frame = item.data(0, FRAME_ROLE) + if comment_id is None or frame is None: + return + + checked = item.checkState(0) == QtCore.Qt.CheckState.Checked + + comment = self.find_comment(int(frame), comment_id) + if comment is None or bool(comment.get("done")) == checked: + return + + comment["done"] = checked + + # Restyle this row in place. Calling refresh() here would clear the tree + # -- destroying the very item Qt is still emitting itemChanged for, which + # is a use-after-free that takes the whole process down. The comment + # count is unchanged by a toggle, so there is nothing else to rebuild. + self._loading = True + try: + self.style_comment_item(item, comment, "x" in comment and "y" in comment) + finally: + self._loading = False + + self.comments_changed.emit() + + def find_comment(self, frame, comment_id): + """Return the comment dict behind a row, or None if it is gone.""" + for comment in self.sketch.get_comments(int(frame)): + if comment.get("id") == comment_id: + return comment + return None + + def selection_changed(self): + """Enable Delete only while a comment row (not a frame row) is selected.""" + self.deleteButton.setEnabled(self.selected_comment() is not None) + + def selected_comment(self): + """Return (frame, comment_id) for the selected comment row, else None.""" + for item in self.commentTree.selectedItems(): + comment_id = item.data(0, COMMENT_ROLE) + if comment_id is not None: + return int(item.data(0, FRAME_ROLE)), comment_id + return None + + def add_note(self): + """Add the line edit's text to the current frame as a frame-level note.""" + if not self.sketch or self.current_frame is None: + return + + text = self.commentLineEdit.text().strip() + if not text: + return + + if self.sketch.add_comment(self.current_frame, text) is None: + return + + self.commentLineEdit.clear() + self.refresh() + self.comments_changed.emit() + + def delete_selected(self): + """Delete the selected comment from the sketch.""" + if not self.sketch: + return + + selected = self.selected_comment() + if not selected: + return + + frame, comment_id = selected + if not self.sketch.delete_comment(frame, comment_id): + return + + self.refresh() + self.comments_changed.emit() + + +if __name__ == "__main__": + pass diff --git a/widgets/pixmaps.py b/widgets/pixmaps.py index 69abfdb..db7610f 100644 --- a/widgets/pixmaps.py +++ b/widgets/pixmaps.py @@ -156,6 +156,15 @@ def path(points, closed=False): elif name == "attach": painter.setPen(QtGui.QPen(accent, 4)); painter.drawArc(QtCore.QRectF(15, 8, 34, 48), -35 * 16, 245 * 16) painter.drawArc(QtCore.QRectF(23, 14, 18, 34), -45 * 16, 235 * 16) + elif name == "comment": + painter.setPen(QtGui.QPen(accent, 4)) + painter.setBrush(QtGui.QColor("#3a3f45")) + painter.drawEllipse(QtCore.QRectF(16, 8, 32, 32)) + painter.setBrush(QtGui.QColor("#3a3f45")) + path([(23, 35), (32, 56), (41, 35)], True) + painter.setBrush(warm) + painter.setPen(QtCore.Qt.PenStyle.NoPen) + painter.drawEllipse(QtCore.QRectF(27, 19, 10, 10)) elif name == "recaps": painter.setPen(QtGui.QPen(accent, 4)); path([(10, 13), (54, 13), (54, 44), (31, 44), (20, 54), (20, 44), (10, 44)], True) line(19, 24, 45, 24); line(19, 33, 38, 33) diff --git a/widgets/viewer.py b/widgets/viewer.py index 72ce263..757983f 100644 --- a/widgets/viewer.py +++ b/widgets/viewer.py @@ -151,6 +151,7 @@ from widgets.buttons import ClearButton from widgets.buttons import ArrowButton from widgets.buttons import PencilButton +from widgets.buttons import CommentButton from widgets.buttons import NavigateButton from widgets.buttons import EraserButton from widgets.buttons import RenderButton @@ -439,6 +440,16 @@ def setupUi(self): ) self.addWidget(self.rectangleButton) + # Comment pin tool + self.commentButton = CommentButton( + None, + tooltip="Pin Comment (click the frame to place a note)", + checkable=True, + width=22, + height=22, + ) + self.addWidget(self.commentButton) + # Eraser tool self.eraserButton = EraserButton( None, tooltip="Erasier Tool", checkable=True, width=22, height=22 @@ -570,6 +581,9 @@ def setupUi(self): self.pencilButton.toggled.connect(lambda enabled: self.set_draw_enabled("pencil", enabled)) self.navigateButton.clicked.connect(self.deactivate_tools) self.arrowButton.toggled.connect(lambda enabled: self.set_draw_enabled("arrow", enabled)) + self.commentButton.toggled.connect( + lambda enabled: self.set_draw_enabled("comment", enabled) + ) self.ellipseButton.toggled.connect( lambda enabled: self.set_draw_enabled("ellipse", enabled) ) @@ -724,6 +738,7 @@ def set_draw_enabled(self, tool, enabled): buttons = [ self.pencilButton, self.arrowButton, + self.commentButton, self.ellipseButton, self.rectangleButton, self.eraserButton, @@ -789,6 +804,7 @@ def deactivate_tools(self): for button in ( self.pencilButton, self.arrowButton, + self.commentButton, self.ellipseButton, self.rectangleButton, self.eraserButton, @@ -1203,6 +1219,11 @@ class ViewerWidget(QtOpenGLWidgets.QOpenGLWidget): annotation_tool_finished = QtCore.Signal(str) fullscreen_requested = QtCore.Signal() + # Emitted when the comment tool is clicked on the frame, carrying the + # normalized (x, y) hit point. The window prompts for the note text; the + # viewer deliberately owns no dialog of its own. + comment_requested = QtCore.Signal(tuple) + def __init__(self, parent=None): """ Initialize viewer widget. @@ -2046,6 +2067,15 @@ def mousePressEvent(self, event): point = self.widget_to_image_point(event.position().toPoint()) + # The comment tool pins a note instead of drawing a stroke. Hand the hit + # point to the window (which collects the text) and never let it reach + # the sketch, which would otherwise store a bogus "comment" stroke. + if self.annotations.tool == "comment": + if event.button() == QtCore.Qt.MouseButton.LeftButton: + self.comment_requested.emit(point) + event.accept() + return + self.annotations.mousePressEvent(point) self.update()