diff --git a/constants/__init__.py b/constants/__init__.py index 6aff060..ab92f77 100644 --- a/constants/__init__.py +++ b/constants/__init__.py @@ -135,6 +135,13 @@ COMMENT_PIN_COLOR = (255, 68, 68) COMMENT_PIN_DONE_COLOR = (76, 175, 80) +# Annotation markers drawn on the timeline. A frame holding a comment reads as +# a comment even if it also carries drawings: the words are the reviewable part, +# and a drawing-only frame is a different kind of note. +TIMELINE_COMMENT_MARKER_COLOR = (79, 195, 247) +TIMELINE_DRAWING_MARKER_COLOR = (171, 71, 188) +TIMELINE_MARKER_WIDTH = 3 + # Container support is provided by the FFmpeg libraries bundled with PyAV. # Keep these lists centralized so import, drag/drop and reader selection never # disagree about a valid movie. Codec support is detected from the stream, not diff --git a/tests/test_notes_persistence.py b/tests/test_notes_persistence.py index c54c934..63e1815 100644 --- a/tests/test_notes_persistence.py +++ b/tests/test_notes_persistence.py @@ -67,6 +67,46 @@ def test_notestore_save_and_load_roundtrip(tmp_path, monkeypatch, qapp): assert loaded_sketch.strokes == original.strokes +def test_notestore_reads_marker_frames_without_mutating_a_sketch( + tmp_path, monkeypatch, qapp +): + monkeypatch.setenv("FRAMEDECK_PROFILE_ROOT", str(tmp_path)) + source = str(tmp_path / "SHOT_020_comp_v002.mov") + sketch = _sketch_with_strokes() + sketch.add_comment(5, "Check edge") + sketch.comments[12] = [] + notestore.save_notes(source, sketch) + + comments, drawings = notestore.annotation_frames(source) + + assert comments == {5} + assert drawings == {5, 9} + + +def test_notestore_marker_frames_reject_corrupt_or_foreign_sidecars( + tmp_path, monkeypatch, qapp +): + monkeypatch.setenv("FRAMEDECK_PROFILE_ROOT", str(tmp_path)) + source = str(tmp_path / "shot.mov") + path = notestore.notes_path_for(source) + path.parent.mkdir(parents=True) + path.write_text("{broken", encoding="utf-8") + assert notestore.annotation_frames(source) == (set(), set()) + + path.write_text( + json.dumps( + { + "schema": notestore.SCHEMA, + "source": str(tmp_path / "other.mov"), + "annotations": {"5": [{"type": "pencil"}]}, + "comments": {"9": [{"text": "note"}]}, + } + ), + encoding="utf-8", + ) + assert notestore.annotation_frames(source) == (set(), set()) + + def test_notestore_empty_removes_sidecar(tmp_path, monkeypatch, qapp): monkeypatch.setenv("FRAMEDECK_PROFILE_ROOT", str(tmp_path)) source = str(tmp_path / "clip.mov") diff --git a/tests/test_timeline_markers.py b/tests/test_timeline_markers.py new file mode 100644 index 0000000..e74767e --- /dev/null +++ b/tests/test_timeline_markers.py @@ -0,0 +1,207 @@ +"""Tests for the timeline's annotation markers. + +The timeline is a plain QWidget, so these render it offscreen and probe the +actual pixels -- the assertions are about what a reviewer sees, not about what +the widget believes it stored. +""" + +from types import SimpleNamespace + +import pytest + +import constants + +from tests.helpers import probe_pixel +from widgets import MainWindow, notestore +from widgets.annotations import Sketch +from widgets.timeline import TimelineWidget + + +MARKER_Y = int(60 * 0.45) + 6 # inside the marker band + + +@pytest.fixture +def timeline(qapp): + widget = TimelineWidget(None) + widget.resize(900, 60) + widget.set_range(1, 100) + widget.set_current_frame(1) + + yield widget + + widget.close() + widget.deleteLater() + qapp.processEvents() + + +def _marker_color(widget, frame): + image = widget.grab().toImage() + x = int(widget.frame_to_pos(frame)) + return probe_pixel(image, x, MARKER_Y)[:3] + + +class _MarkerSink: + def set_annotated_frames(self, comments, drawings): + self.comment_frames = set(comments) + self.drawing_frames = set(drawings) + + +def _window_controller(source, sketch, entries): + """Build only the controller surface refresh_timeline_markers requires.""" + timeline = _MarkerSink() + window = SimpleNamespace( + viewframe=SimpleNamespace( + viewer=SimpleNamespace(annotations=sketch), timeline=timeline + ), + playlist_playback_active=True, + playlist_entries=entries, + current_source_filepath=source, + ) + return window, timeline + + +def _entry(source, start, count): + return { + "context": {"media": source}, + "start": start, + "end": start + count - 1, + "count": count, + } + + +# --------------------------------------------------------------------------- # +# What gets marked +# --------------------------------------------------------------------------- # +def test_a_commented_frame_is_marked(timeline): + timeline.set_annotated_frames(comment_frames=[20], drawing_frames=[]) + + assert _marker_color(timeline, 20) == constants.TIMELINE_COMMENT_MARKER_COLOR + + +def test_a_drawn_frame_is_marked_differently(timeline): + timeline.set_annotated_frames(comment_frames=[], drawing_frames=[60]) + + assert _marker_color(timeline, 60) == constants.TIMELINE_DRAWING_MARKER_COLOR + + +def test_a_frame_with_both_reads_as_a_comment(timeline): + """The words are the reviewable part; the drawing marker must not hide them.""" + timeline.set_annotated_frames(comment_frames=[80], drawing_frames=[80]) + + assert timeline.drawing_frames == set() # subsumed, not drawn twice + assert _marker_color(timeline, 80) == constants.TIMELINE_COMMENT_MARKER_COLOR + + +def test_an_unannotated_frame_is_not_marked(timeline): + timeline.set_annotated_frames(comment_frames=[20], drawing_frames=[60]) + + color = _marker_color(timeline, 40) + + assert color != constants.TIMELINE_COMMENT_MARKER_COLOR + assert color != constants.TIMELINE_DRAWING_MARKER_COLOR + + +def test_clearing_the_markers_removes_them(timeline): + timeline.set_annotated_frames(comment_frames=[20], drawing_frames=[]) + assert _marker_color(timeline, 20) == constants.TIMELINE_COMMENT_MARKER_COLOR + + timeline.set_annotated_frames(comment_frames=[], drawing_frames=[]) + + assert _marker_color(timeline, 20) != constants.TIMELINE_COMMENT_MARKER_COLOR + + +# --------------------------------------------------------------------------- # +# Robustness +# --------------------------------------------------------------------------- # +def test_markers_outside_the_range_are_not_drawn(timeline): + """A stale marker from a longer clip must not smear onto the edge.""" + timeline.set_annotated_frames(comment_frames=[500], drawing_frames=[-20]) + + # Renders without error, and nothing lands at either end of the track. + image = timeline.grab().toImage() + left = probe_pixel(image, timeline.timeline_margin, MARKER_Y)[:3] + right = probe_pixel(image, timeline.width() - timeline.timeline_margin, MARKER_Y)[:3] + + assert left != constants.TIMELINE_COMMENT_MARKER_COLOR + assert right != constants.TIMELINE_COMMENT_MARKER_COLOR + + +def test_no_markers_renders_cleanly(timeline): + timeline.set_annotated_frames(comment_frames=[], drawing_frames=[]) + + assert timeline.comment_frames == set() + assert timeline.drawing_frames == set() + assert not timeline.grab().toImage().isNull() + + +def test_none_is_treated_as_empty(timeline): + timeline.set_annotated_frames(None, None) + + assert timeline.comment_frames == set() + assert timeline.drawing_frames == set() + + +def test_markers_accept_any_iterable(timeline): + timeline.set_annotated_frames(comment_frames=(5, 6), drawing_frames=range(9, 11)) + + assert timeline.comment_frames == {5, 6} + assert timeline.drawing_frames == {9, 10} + + +def test_playlist_timeline_aggregates_active_memory_and_other_sidecars( + tmp_path, monkeypatch, qapp +): + monkeypatch.setenv("FRAMEDECK_PROFILE_ROOT", str(tmp_path)) + active_source = str(tmp_path / "active.mov") + other_source = str(tmp_path / "other.mov") + + active = Sketch() + active.comments[2] = [{"id": "active", "text": "memory edit"}] + active.strokes[4] = [{"id": "draw-active", "type": "pencil"}] + other = Sketch() + other.comments[3] = [{"id": "other", "text": "saved note"}] + other.strokes[8] = [{"id": "draw-other", "type": "pencil"}] + notestore.save_notes(other_source, other) + + window, sink = _window_controller( + active_source, + active, + [_entry(active_source, 1, 10), _entry(other_source, 11, 10)], + ) + MainWindow.refresh_timeline_markers(window) + + assert sink.comment_frames == {2, 13} + assert sink.drawing_frames == {4, 18} + + +def test_duplicate_playlist_shots_each_receive_the_source_markers(qapp): + source = "same-shot.mov" + sketch = Sketch() + sketch.comments[2] = [{"id": "review", "text": "same source"}] + sketch.strokes[6] = [{"id": "draw", "type": "pencil"}] + window, sink = _window_controller( + source, + sketch, + [_entry(source, 1, 10), _entry(source, 11, 10)], + ) + + MainWindow.refresh_timeline_markers(window) + + assert sink.comment_frames == {2, 12} + assert sink.drawing_frames == {6, 16} + + +# --------------------------------------------------------------------------- # +# The playhead stays readable +# --------------------------------------------------------------------------- # +def test_the_playhead_is_drawn_over_a_marker_on_the_same_frame(timeline): + """A note must never hide the frame the reviewer is actually sitting on.""" + timeline.set_current_frame(20) + timeline.set_annotated_frames(comment_frames=[20], drawing_frames=[]) + + image = timeline.grab().toImage() + x = int(timeline.frame_to_pos(20)) + red, green, blue = probe_pixel(image, x, MARKER_Y)[:3] + + # The playhead is red; the marker is blue. Red wins on the shared pixel. + assert red > green and red > blue diff --git a/widgets/__init__.py b/widgets/__init__.py index f7ddef7..5b69455 100644 --- a/widgets/__init__.py +++ b/widgets/__init__.py @@ -321,6 +321,11 @@ def setupUi(self): ) self.viewframe.viewer.fullscreen_requested.connect(self.toggle_fullscreen) self.viewframe.viewer.comment_pin_clicked.connect(self.add_pinned_comment) + # Drawing a stroke changes what the timeline should show, and the viewer + # is the only thing that knows it happened. + self.viewframe.viewer.annotations_changed.connect( + self.refresh_timeline_markers + ) self.commentSidebar.add_requested.connect(self.add_frame_comment) self.commentSidebar.pin_requested.connect(self.begin_pinned_comment) self.commentSidebar.pin_cancel_requested.connect(self.cancel_pinned_comment) @@ -929,6 +934,7 @@ def apply_review_styles(self): def set_current_project(self, project): self.current_project = project self.viewframe.viewer.clear() + self.viewframe.timeline.set_annotated_frames([], []) @staticmethod def _playlist_frame_count(context): @@ -990,6 +996,8 @@ def _playlist_changed(self, contexts): self.viewframe.timeline.set_range( constants.VL_START_FRAME, constants.VL_START_FRAME ) + if hasattr(self, "viewframe"): + self.refresh_timeline_markers() def _playlist_entry_for_context(self, context): instance = context.get("playlist_instance_id") @@ -1222,6 +1230,7 @@ def openMedia(self, filepath=None, add_to_playlist=True): # Clear current viewer frame self.viewframe.viewer.clear() + self.viewframe.timeline.set_annotated_frames([], []) self.viewframe.viewer.reset_view() self.commentSidebar.set_source_available(False) self.commentSidebar.refresh() @@ -1532,6 +1541,7 @@ def handle_active_media_removed(self, replacement): self.media_cache.set_active(None) self.primary_compare_frame = None self.viewframe.viewer.clear() + self.viewframe.timeline.set_annotated_frames([], []) self.viewframe.viewer.reset_view() self.commentSidebar.set_source_available(False) self.commentSidebar.refresh() @@ -1871,6 +1881,61 @@ def _timeline_frame_for_local(self, local_frame): return entry["start"] + int(local_frame) - constants.VL_START_FRAME return int(local_frame) + def refresh_timeline_markers(self): + """Redraw note markers for the current source or complete playlist. + + Annotations are keyed by the player's LOCAL frame, but the timeline is a + global range while a playlist is playing. The active source is read from + memory so unsaved edits appear immediately; other shots are inspected + through their lightweight note sidecars. + """ + annotations = self.viewframe.viewer.annotations + memory_comments = set(annotations.commented_frames()) + memory_drawings = { + int(frame) for frame, strokes in annotations.strokes.items() if strokes + } + + if not self.playlist_playback_active or not self.playlist_entries: + self.viewframe.timeline.set_annotated_frames( + memory_comments, memory_drawings + ) + return + + from widgets import notestore + + current_source = self.current_source_filepath + current_identity = ( + os.path.normcase(os.path.abspath(str(current_source))) + if current_source + else None + ) + comments = set() + drawings = set() + for entry in self.playlist_entries: + source = entry["context"].get("media") + identity = ( + os.path.normcase(os.path.abspath(str(source))) if source else None + ) + if identity is not None and identity == current_identity: + local_comments, local_drawings = memory_comments, memory_drawings + else: + local_comments, local_drawings = notestore.annotation_frames(source) + + first = constants.VL_START_FRAME + last = first + entry["count"] - 1 + offset = entry["start"] - first + comments.update( + int(frame) + offset + for frame in local_comments + if first <= int(frame) <= last + ) + drawings.update( + int(frame) + offset + for frame in local_drawings + if first <= int(frame) <= last + ) + self.viewframe.timeline.set_annotated_frames(comments, drawings) + def add_frame_comment(self, text): """Add an unpinned comment to the viewer's current local frame.""" frame = self.viewframe.viewer.annotations.current_frame @@ -1881,6 +1946,7 @@ def add_frame_comment(self, text): self._save_current_notes() self.commentSidebar.clear_editor() self.commentSidebar.refresh() + self.refresh_timeline_markers() self.viewframe.viewer.update() self.statusBar().showMessage(f"Comment added at frame {frame}", 2500) @@ -1908,6 +1974,7 @@ def add_pinned_comment(self, x, y): self._save_current_notes() self.commentSidebar.clear_editor() self.commentSidebar.refresh() + self.refresh_timeline_markers() self.viewframe.viewer.update() self.statusBar().showMessage(f"Pinned comment added at frame {frame}", 2500) self.viewframe.viewer.set_comment_pin_mode(False) @@ -1926,6 +1993,7 @@ def toggle_comment_done(self, frame, comment_id): return self._save_current_notes() self.commentSidebar.refresh() + self.refresh_timeline_markers() self.viewframe.viewer.update() def delete_comment(self, frame, comment_id): @@ -1933,12 +2001,14 @@ def delete_comment(self, frame, comment_id): return self._save_current_notes() self.commentSidebar.refresh() + self.refresh_timeline_markers() self.viewframe.viewer.update() def annotation_state_changed(self): """Persist and refresh after a toolbar/menu Clear Notes action.""" self._save_current_notes() self.commentSidebar.refresh() + self.refresh_timeline_markers() self.viewframe.viewer.update() def jump_to_annotation(self, step): @@ -2027,6 +2097,7 @@ def _load_notes_for_source(self, source): self.commentSidebar.set_current_frame( self.viewframe.viewer.annotations.current_frame ) + self.refresh_timeline_markers() self.viewframe.viewer.update() except Exception: LOGGER.exception("Unable to load annotation notes") diff --git a/widgets/notestore.py b/widgets/notestore.py index 0055a82..299f8ec 100644 --- a/widgets/notestore.py +++ b/widgets/notestore.py @@ -43,6 +43,67 @@ def _clear(sketch): sketch.deserialize_comments({}) +def _read_document(source): + """Return a validated note document for *source*, or ``None``. + + Keeping validation in one place lets lightweight timeline queries inspect + sidecars without constructing or mutating a :class:`Sketch` instance. + """ + if not source: + return None + + path = notes_path_for(source) + if not path.exists(): + return None + + try: + with open(path, "r", encoding="utf-8") as stream: + document = json.load(stream) + except (OSError, ValueError): + return None + + if not isinstance(document, dict) or document.get("schema") != SCHEMA: + return None + + saved_source = document.get("source") + if not saved_source or os.path.normcase(os.path.abspath(str(saved_source))) != ( + os.path.normcase(os.path.abspath(str(source))) + ): + return None + return document + + +def _populated_frame_keys(records): + """Return integer frame keys whose record list is non-empty and valid.""" + frames = set() + if not isinstance(records, dict): + return frames + for frame, items in records.items(): + if not isinstance(items, list) or not items: + continue + try: + frames.add(int(frame)) + except (TypeError, ValueError): + continue + return frames + + +def annotation_frames(source): + """Return ``(comment_frames, drawing_frames)`` without loading a sketch. + + Missing, corrupt, or foreign sidecars safely read as two empty sets. This is + used to populate a playlist-wide timeline while only the active shot's full + annotation payload remains in memory. + """ + document = _read_document(source) + if document is None: + return set(), set() + return ( + _populated_frame_keys(document.get("comments")), + _populated_frame_keys(document.get("annotations")), + ) + + def save_notes(source, sketch): """Write *sketch*'s strokes and comments to *source*'s sidecar. @@ -97,26 +158,8 @@ def load_notes(source, sketch): _clear(sketch) return False - path = notes_path_for(source) - if not path.exists(): - _clear(sketch) - return False - - try: - with open(path, "r", encoding="utf-8") as stream: - document = json.load(stream) - except (OSError, ValueError): - _clear(sketch) - return False - - if not isinstance(document, dict) or document.get("schema") != SCHEMA: - _clear(sketch) - return False - - saved_source = document.get("source") - if not saved_source or os.path.normcase(os.path.abspath(str(saved_source))) != ( - os.path.normcase(os.path.abspath(str(source))) - ): + document = _read_document(source) + if document is None: _clear(sketch) return False diff --git a/widgets/timeline.py b/widgets/timeline.py index 58e5b82..465023d 100644 --- a/widgets/timeline.py +++ b/widgets/timeline.py @@ -103,6 +103,10 @@ def __init__(self, parent=None): # Cached frame storage self.cached_frames = set() + # Annotated frame storage, in timeline (not player-local) frames. + self.comment_frames = set() + self.drawing_frames = set() + self.setMouseTracking(True) def set_range(self, start, end): @@ -154,6 +158,72 @@ def set_cached_frames(self, frames): # Refresh widget self.update() + def set_annotated_frames(self, comment_frames, drawing_frames): + """ + Update the annotation markers. + + A frame carrying both a comment and a drawing counts as a comment: the + words are the reviewable part, and the marker should not hide them. + + Args: + comment_frames (iterable[int]): + Timeline frames holding at least one comment. + + drawing_frames (iterable[int]): + Timeline frames holding at least one stroke. + """ + + self.comment_frames = set(int(frame) for frame in comment_frames or []) + self.drawing_frames = set( + int(frame) for frame in drawing_frames or [] + ) - self.comment_frames + + # Refresh widget + self.update() + + def draw_annotation_markers(self, painter, height): + """ + Draw a tick on the timeline for every annotated frame. + + Args: + painter (QtGui.QPainter): + Active painter. + + height (int): + Widget height. + """ + + if not self.comment_frames and not self.drawing_frames: + return + + # Sit below the frame-number labels, above the playhead readout. The + # playhead is drawn afterwards so it always stays legible on top. + top = int(height * 0.45) + marker_height = max(10, int(height * 0.45)) + width = constants.TIMELINE_MARKER_WIDTH + + painter.save() + painter.setPen(QtCore.Qt.PenStyle.NoPen) + + for frames, color in ( + (self.drawing_frames, constants.TIMELINE_DRAWING_MARKER_COLOR), + (self.comment_frames, constants.TIMELINE_COMMENT_MARKER_COLOR), + ): + painter.setBrush(QtGui.QColor(*color)) + for frame in frames: + if frame < self.start_frame or frame > self.end_frame: + continue + x = self.frame_to_pos(frame) + painter.drawRoundedRect( + QtCore.QRectF( + x - (width / 2.0), top, width, marker_height + ), + 1, + 1, + ) + + painter.restore() + def paintEvent(self, event): """ Paint timeline widget. @@ -215,6 +285,10 @@ def paintEvent(self, event): painter.drawLine(x, 0, x, 25) painter.drawText(x + 2, 40, str(frame)) + # Annotation markers sit under the playhead, so a note never hides the + # frame the reviewer is actually on. + self.draw_annotation_markers(painter, height) + # Calculate playhead position current_x = int( self.timeline_margin + ((self.current_frame - self.start_frame) * pixels_per_frame) diff --git a/widgets/viewer.py b/widgets/viewer.py index 01e3d61..5559865 100644 --- a/widgets/viewer.py +++ b/widgets/viewer.py @@ -1211,6 +1211,11 @@ class ViewerWidget(QtOpenGLWidgets.QOpenGLWidget): fullscreen_requested = QtCore.Signal() comment_pin_clicked = QtCore.Signal(float, float) + # Emitted whenever the stroke set changes (drawn, undone, redone, cleared), + # so the timeline markers follow the annotations instead of drifting from + # them until the next reload. + annotations_changed = QtCore.Signal() + def __init__(self, parent=None): """ Initialize viewer widget. @@ -2269,6 +2274,8 @@ def mouseReleaseEvent(self, event): self.annotations.set_enabled(False) self.annotation_tool_finished.emit("txt") + self.annotations_changed.emit() + self.update() def wheelEvent(self, event): @@ -2327,6 +2334,8 @@ def undo_strokes(self): self.annotations.undo() + self.annotations_changed.emit() + self.update() def redo_strokes(self): @@ -2336,6 +2345,8 @@ def redo_strokes(self): self.annotations.redo() + self.annotations_changed.emit() + self.update() def clear_strokes(self): @@ -2345,6 +2356,8 @@ def clear_strokes(self): self.annotations.clear() + self.annotations_changed.emit() + self.update() def render_current_frame(self):