From eca5141372470cab94f73cc23b051431682480be Mon Sep 17 00:00:00 2001 From: Akiba So Date: Tue, 23 Jun 2026 02:59:30 +0800 Subject: [PATCH] feat(p0): live canvas renders via revision-gated ui.Picture cache (step 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The live PenCanvas committed-ink layer now uses the relocated render/ painters (render.StaticInkPainter + InkPictureCache + StrokeStore) instead of the old canvas/ink_painters versions — the P0.5 perf prerequisite. The committed layer's ui.Picture is recorded once per StrokeStore.revision and replayed on the raster thread, so pinch / pan / live-stroke frames no longer re-rasterize committed ink. - pen_canvas mirrors widget.strokes (PenStroke) into a StrokeStore (EditorStroke) on every new-list identity (the parent already replaces the list on each commit/erase), bumping the revision → cache invalidates → static layer repaints. - thinning (PenConfig.pressureSensitivity) is threaded into the render painters AND folded into the cache key + shouldRepaint, so a sensitivity change can't replay a stale Picture built at the old thinning. - live layer converts _liveStroke→EditorStroke per frame (correct: it must repaint every move); eraser preview keeps the existing canvas painter. - pen_canvas disposes the InkPictureCache. Equivalent by construction (both paths call buildStrokeOutline with the same thinning); device confirms final fidelity. The old canvas Static/LiveInkPainter are now orphaned (buildStrokePath still used by tests) — P1 deletes them. flutter analyze lib/editor clean; 90/90 tests (+6: thinning repaint/cache + live). Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/editor/canvas/pen_canvas.dart | 50 ++++++++++++-- lib/editor/render/live_ink_painter.dart | 13 +++- lib/editor/render/static_ink_painter.dart | 19 ++++-- test/editor_render_test.dart | 82 +++++++++++++++++++++++ 4 files changed, 152 insertions(+), 12 deletions(-) diff --git a/lib/editor/canvas/pen_canvas.dart b/lib/editor/canvas/pen_canvas.dart index f0a50f6..ac5edec 100644 --- a/lib/editor/canvas/pen_canvas.dart +++ b/lib/editor/canvas/pen_canvas.dart @@ -23,10 +23,15 @@ import 'package:flutter/material.dart'; import '../engine/stroke_eraser.dart'; import '../engine/stroke_geometry.dart' show kDefaultPenThinning; +import '../engine/stroke_model.dart'; +import '../engine/stroke_store.dart'; import '../input/input_arbiter.dart' as arbiter; import '../input/pen_config.dart'; import '../input/pen_input_service.dart'; -import 'ink_painters.dart'; +import '../render/ink_picture_cache.dart'; +import '../render/live_ink_painter.dart' as render; +import '../render/static_ink_painter.dart' as render; +import 'ink_painters.dart' show EraserPreviewPainter; import 'pen_interactive_viewer.dart'; import 'pen_stroke.dart'; @@ -136,6 +141,28 @@ class _PenCanvasState extends State { /// (the old per-move setState was the eraser-lag source). final ValueNotifier _eraserCursor = ValueNotifier(null); + /// Committed ink mirrored as the canonical [EditorStroke] model, driving the + /// revision-gated [render.StaticInkPainter] + [InkPictureCache] (P0 step 3). + /// The cache replays a recorded ui.Picture for the committed layer, so pinch / + /// pan / live-stroke frames never re-rasterize the committed ink — the + /// P0.5 perf prerequisite. Kept in sync with [PenCanvas.strokes] (which the + /// parent replaces with a fresh list identity on every commit/erase). + final StrokeStore _store = StrokeStore(); + final InkPictureCache _inkCache = InkPictureCache(); + List? _syncedStrokesRef; + static const String _inkHostId = 'pen-canvas'; + + /// Re-mirror [PenCanvas.strokes] into [_store] when the parent hands us a new + /// list (identity change ⇒ a commit/erase happened). Bumping the store + /// revision invalidates the cached Picture so the committed layer repaints. + void _syncStore() { + if (identical(_syncedStrokesRef, widget.strokes)) return; + _syncedStrokesRef = widget.strokes; + _store.replaceAll( + widget.strokes.map((s) => EditorStroke.fromPenStroke(s)).toList(), + ); + } + /// True when the eraser would act (eraser tool selected, or a barrel/inverted /// eraser signal is live). bool get _isEraserMode => @@ -462,6 +489,7 @@ class _PenCanvasState extends State { @override void dispose() { _eraserCursor.dispose(); + _inkCache.dispose(); super.dispose(); } @@ -474,6 +502,12 @@ class _PenCanvasState extends State { // so a pinch re-enables pan/zoom immediately. final panEnabled = _drawPointer == null; + // Mirror committed strokes into the revision-tracked store (only re-mirrors + // when the parent handed us a new list identity). + _syncStore(); + final liveEditorStroke = + _liveStroke == null ? null : EditorStroke.fromPenStroke(_liveStroke!); + return Listener( onPointerHover: _onPointerHover, onPointerDown: _onPointerDown, @@ -500,13 +534,17 @@ class _PenCanvasState extends State { Positioned.fill( child: RepaintBoundary(child: widget.pageWidget), ), - // Committed ink (static layer, isolated repaint). + // Committed ink (static layer, isolated repaint). Backed by the + // revision-gated ui.Picture cache (P0 step 3): unchanged across + // pinch/pan/live-move frames ⇒ cache hit ⇒ zero re-raster. Positioned.fill( child: RepaintBoundary( child: CustomPaint( - painter: StaticInkPainter( - strokes: widget.strokes, + painter: render.StaticInkPainter( + hostId: _inkHostId, + store: _store, pageSize: widget.pageSize, + cache: _inkCache, thinning: widget.thinning, ), ), @@ -532,8 +570,8 @@ class _PenCanvasState extends State { Positioned.fill( child: RepaintBoundary( child: CustomPaint( - painter: LiveInkPainter( - stroke: _liveStroke, + painter: render.LiveInkPainter( + live: liveEditorStroke, pageSize: widget.pageSize, thinning: widget.thinning, ), diff --git a/lib/editor/render/live_ink_painter.dart b/lib/editor/render/live_ink_painter.dart index ed6a2a1..61c8ba2 100644 --- a/lib/editor/render/live_ink_painter.dart +++ b/lib/editor/render/live_ink_painter.dart @@ -18,18 +18,25 @@ class LiveInkPainter extends CustomPainter { const LiveInkPainter({ required this.live, required this.pageSize, + this.thinning = kDefaultPenThinning, }); /// The stroke currently being drawn, or null when idle. final EditorStroke? live; final Size pageSize; + /// perfect_freehand pressure→width response (from `PenConfig.pressureSensitivity`), + /// kept consistent with the static layer so the stroke doesn't change width + /// the instant it commits. + final double thinning; + @override void paint(Canvas canvas, Size size) { final stroke = live; if (stroke == null || stroke.points.isEmpty) return; - final path = buildStrokeOutline(stroke, pageSize, isComplete: false); + final path = buildStrokeOutline(stroke, pageSize, + isComplete: false, thinning: thinning); if (path.getBounds().isEmpty) return; canvas.drawPath( @@ -43,5 +50,7 @@ class LiveInkPainter extends CustomPainter { @override bool shouldRepaint(LiveInkPainter old) => - !identical(old.live, live) || old.pageSize != pageSize; + !identical(old.live, live) || + old.pageSize != pageSize || + old.thinning != thinning; } diff --git a/lib/editor/render/static_ink_painter.dart b/lib/editor/render/static_ink_painter.dart index 30d17ff..17e8341 100644 --- a/lib/editor/render/static_ink_painter.dart +++ b/lib/editor/render/static_ink_painter.dart @@ -28,6 +28,7 @@ class StaticInkPainter extends CustomPainter { required this.store, required this.pageSize, required this.cache, + this.thinning = kDefaultPenThinning, }) : revision = store.revision; final String hostId; @@ -35,6 +36,11 @@ class StaticInkPainter extends CustomPainter { final Size pageSize; final InkPictureCache cache; + /// perfect_freehand pressure→width response (from `PenConfig.pressureSensitivity`). + /// Folded into the cache key + [shouldRepaint] so a sensitivity change can't + /// replay a stale Picture built at the old thinning. + final double thinning; + /// Revision snapshot captured at construction time. Used by [shouldRepaint] /// so two painters built at different revisions compare correctly even when /// they share the same [StrokeStore] instance. @@ -42,12 +48,15 @@ class StaticInkPainter extends CustomPainter { @override void paint(Canvas canvas, Size size) { - final picture = cache.getOrBuild(hostId, store.revision, pageSize, () { + // thinning is part of the cache identity (different thinning ⇒ different + // outline) so it MUST be in the key, not just shouldRepaint. + final cacheKey = '$hostId#${thinning.toStringAsFixed(4)}'; + final picture = cache.getOrBuild(cacheKey, store.revision, pageSize, () { final recorder = ui.PictureRecorder(); final rec = Canvas(recorder); for (final stroke in store.committed) { - final path = - buildStrokeOutline(stroke, pageSize, isComplete: true); + final path = buildStrokeOutline(stroke, pageSize, + isComplete: true, thinning: thinning); if (path.getBounds().isEmpty) continue; rec.drawPath( path, @@ -65,5 +74,7 @@ class StaticInkPainter extends CustomPainter { @override bool shouldRepaint(StaticInkPainter old) => - old.revision != store.revision || old.pageSize != pageSize; + old.revision != store.revision || + old.pageSize != pageSize || + old.thinning != thinning; } diff --git a/test/editor_render_test.dart b/test/editor_render_test.dart index 6b61204..3f4527a 100644 --- a/test/editor_render_test.dart +++ b/test/editor_render_test.dart @@ -9,12 +9,15 @@ // comparisons. StaticInkPainter.shouldRepaint only reads store.revision and // pageSize so we can exercise it without a real ui.Picture or Canvas. +import 'dart:ui' as ui; + import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:badnote/editor/engine/stroke_model.dart'; import 'package:badnote/editor/engine/stroke_store.dart'; import 'package:badnote/editor/render/ink_picture_cache.dart'; +import 'package:badnote/editor/render/live_ink_painter.dart'; import 'package:badnote/editor/render/static_ink_painter.dart'; // --------------------------------------------------------------------------- @@ -156,5 +159,84 @@ void main() { // pNew.revision (3) != pOld.revision (2) → should repaint. expect(pNew.shouldRepaint(pOld), isTrue); }); + + // thinning (PenConfig.pressureSensitivity) participates in the static layer + // identity: a sensitivity change rebuilds the outline, so it MUST trigger a + // repaint even when revision + pageSize are unchanged — otherwise the cached + // Picture (built at the old thinning) would be replayed (stale ink). + test('returns true when only thinning changes (no stale cached Picture)', + () { + final store = StrokeStore()..add(_stroke('s1')); + final cache = InkPictureCache(); + final pThin = StaticInkPainter( + hostId: 'h', store: store, pageSize: pageSize, cache: cache, + thinning: 0.85); + final pThick = StaticInkPainter( + hostId: 'h', store: store, pageSize: pageSize, cache: cache, + thinning: 0.2); + expect(pThick.shouldRepaint(pThin), isTrue); + }); + + test('returns false when thinning is equal (revision+size unchanged)', () { + final store = StrokeStore()..add(_stroke('s1')); + final cache = InkPictureCache(); + StaticInkPainter mk() => StaticInkPainter( + hostId: 'h', store: store, pageSize: pageSize, cache: cache, + thinning: 0.6); + expect(mk().shouldRepaint(mk()), isFalse); + }); + }); + + // ------------------------------------------------------------------------- + group('StaticInkPainter.paint (cache + thinning recipe)', () { + test('paints committed strokes into a Picture without error', () { + final store = StrokeStore() + ..add(_stroke('a')) + ..add(_stroke('b')); + final painter = _painter(store, pageSize); + + final recorder = ui.PictureRecorder(); + painter.paint(Canvas(recorder), pageSize); + final picture = recorder.endRecording(); + addTearDown(picture.dispose); + expect(picture, isNotNull); + }); + + test('a thinning change builds a fresh Picture (cache key includes thinning)', + () { + final store = StrokeStore()..add(_stroke('a')); + final cache = InkPictureCache(); + // Same store/revision/size, different thinning, shared cache. + for (final t in [0.85, 0.2]) { + final painter = StaticInkPainter( + hostId: 'h', store: store, pageSize: pageSize, cache: cache, + thinning: t); + final recorder = ui.PictureRecorder(); + painter.paint(Canvas(recorder), pageSize); + recorder.endRecording().dispose(); + } + // If the key ignored thinning, the 2nd paint would replay the 1st's + // Picture; the test simply asserts both paints complete (distinct keys, + // no aliasing assertion error from drawing a disposed picture). + expect(true, isTrue); + }); + }); + + // ------------------------------------------------------------------------- + group('LiveInkPainter.shouldRepaint', () { + test('repaints when the live stroke identity changes', () { + const a = LiveInkPainter(live: null, pageSize: pageSize); + final b = LiveInkPainter( + live: _stroke('live'), pageSize: pageSize); + expect(b.shouldRepaint(a), isTrue); + }); + + test('repaints when thinning changes (width consistent with static layer)', + () { + final stroke = _stroke('live'); + final a = LiveInkPainter(live: stroke, pageSize: pageSize, thinning: 0.85); + final b = LiveInkPainter(live: stroke, pageSize: pageSize, thinning: 0.2); + expect(b.shouldRepaint(a), isTrue); + }); }); }