diff --git a/lib/editor/canvas/pen_editor_screen.dart b/lib/editor/canvas/pen_editor_screen.dart index 0b7b176..01bfa3b 100644 --- a/lib/editor/canvas/pen_editor_screen.dart +++ b/lib/editor/canvas/pen_editor_screen.dart @@ -20,7 +20,9 @@ // wiring verbatim. Highlights are in-memory only for now — see // TODO(persist-highlights). -import 'package:flutter/gestures.dart' show PointerDeviceKind; +import 'package:flutter/foundation.dart' + show ValueListenable, visibleForTesting; +import 'package:flutter/gestures.dart'; import 'package:flutter/material.dart'; import 'package:pdfrx/pdfrx.dart'; import 'package:uuid/uuid.dart'; @@ -50,6 +52,7 @@ import 'input_diagnostics.dart'; import 'pen_canvas.dart' show CanvasTool; import 'pen_palette_widgets.dart'; import 'pen_stroke.dart'; +import 'pinch_scale_solver.dart'; /// Stable deterministic document-id for a file path (djb2 hash → hex). /// @@ -132,6 +135,42 @@ class _PenEditorScreenState extends State { final ValueNotifier _overlayRepaint = ValueNotifier(0); void _bumpOverlay() => _overlayRepaint.value++; + // ── Glitch-guarded pinch zoom (we own scale; pdfrx owns scroll) ───────────── + // pdfrx's scaleEnabled is off, so we drive 2-finger zoom through the public + // controller (zoomOnLocalPosition) with the SAME guards as the note canvas's + // PenInteractiveViewer: per-frame scale-ratio clamp, pointer-count re-baseline + // to the last APPLIED scale, and a focal-jump guard. Tracking is absolute from + // a gesture-start snapshot via absolutePinchScale() — never a live read-back. + + static const double _kPinchMinScale = 0.5; + static const double _kPinchMaxScale = 8.0; + + /// A real pinch changes scale modestly per frame; a frame demanding far more + /// is a Windows multi-touch glitch and is dropped (so the zoom can't pop). + static const double _kScaleGlitchHi = 1.4; + static const double _kScaleGlitchLo = 1 / _kScaleGlitchHi; + + /// A single-frame focal-midpoint jump beyond this is a touch misread → drop. + static const double _kFocalGlitchPx = 250.0; + + /// Matrix scale captured at the current baseline (gesture start or the last + /// pointer-count re-baseline). Null when no pinch is active. + double? _pinchScaleStart; + + /// Pointer count of the previous accepted pinch frame; a change re-baselines. + int _pinchPointerCount = 0; + + /// The recognizer's cumulative `details.scale` on the previous accepted frame. + double _pinchLastRawScale = 1.0; + + /// The absolute scale we last APPLIED. Re-baseline anchors to THIS (not a live + /// matrix read) so the displayed scale stays continuous across a finger blip. + double _pinchLastAppliedScale = 1.0; + + /// The recognizer's cumulative `details.scale` at the current baseline; the + /// absolute target normalizes against it (see [absolutePinchScale]). + double _pinchRawScaleAtBaseline = 1.0; + // ── Live stroke state (viewer-level pen capture) ──────────────────────────── /// The page index the in-progress stroke belongs to (the page of its first @@ -142,8 +181,16 @@ class _PenEditorScreenState extends State { /// In-progress stroke points (normalized to [_liveStrokePage]). final List _livePoints = []; - /// Live stroke snapshot for the page overlay; null when idle. - PenStroke? _liveStroke; + /// CURRENT live stroke, published to the page overlay painters. The painter + /// reads this at PAINT time (not as a build-time snapshot), so a mid-stroke + /// update repaints immediately — the ink follows the pen instead of only + /// appearing on pointer-up. Null when idle. + /// + /// (The previous design passed a snapshot into the painter's constructor from + /// pageOverlaysBuilder, which only re-runs on setState; _bumpOverlay repainted + /// the painter but it still read the stale build-time snapshot → invisible ink + /// until commit. See `_LiveStrokeData` / `_PageOverlayPainter`.) + final ValueNotifier<_LiveStrokeData?> _liveStrokeVN = ValueNotifier(null); /// Latest pen-event debug readout — shown only when the diagnostic toggle is /// on, to inspect what Windows delivers. @@ -310,6 +357,7 @@ class _PenEditorScreenState extends State { scheduler.dispose(); } _overlayRepaint.dispose(); + _liveStrokeVN.dispose(); _penConfig?.dispose(); PenInputService.instance.stop(); DiagnosticLogger.instance.stop(); @@ -481,14 +529,16 @@ class _PenEditorScreenState extends State { void _updateLiveStroke() { final page = _liveStrokePage; if (page == null || _livePoints.isEmpty) return; - _liveStroke = PenStroke( + final stroke = PenStroke( points: List.of(_livePoints), color: _currentColor().toARGB32(), width: _currentStrokeWidth(), kind: _currentKind(), brush: _currentBrush(), ); - _bumpOverlay(); + // Publish the live stroke so the page overlay painter repaints it NOW + // (notifies its repaint Listenable) — the ink follows the pen. + _liveStrokeVN.value = _LiveStrokeData(page, stroke); } void _endStroke({required bool commit}) { @@ -509,7 +559,7 @@ class _PenEditorScreenState extends State { } _liveStrokePage = null; _livePoints.clear(); - _liveStroke = null; + _liveStrokeVN.value = null; _bumpOverlay(); } @@ -582,6 +632,68 @@ class _PenEditorScreenState extends State { }); } + // ── Pinch zoom (glitch-guarded, drives pdfrx controller) ──────────────────── + + void _onPinchStart(ScaleStartDetails details) { + if (!_controller.isReady) return; + _pinchScaleStart = _controller.currentZoom; + _pinchPointerCount = details.pointerCount; + _pinchLastRawScale = 1.0; + _pinchLastAppliedScale = _pinchScaleStart!; + _pinchRawScaleAtBaseline = 1.0; + } + + void _onPinchUpdate(ScaleUpdateDetails details) { + final scaleStart = _pinchScaleStart; + if (scaleStart == null || !_controller.isReady) return; + + // Re-baseline on any pointer-count change (a finger lands/lifts, or a + // Windows touch 2↔1↔2 dropout). Anchor to the CLEAN tracked scale, not a + // matrix read-back, so the displayed scale is continuous; skip this frame. + if (details.pointerCount != _pinchPointerCount) { + _pinchPointerCount = details.pointerCount; + _pinchScaleStart = _pinchLastAppliedScale; + _pinchLastRawScale = details.scale; + _pinchRawScaleAtBaseline = details.scale; + return; + } + + // Per-frame finger-motion ratio from the recognizer's OWN cumulative scale. + // A ratio outside the glitch band is a multi-touch spike → drop the frame; + // absolute tracking means the next good frame resumes from the true span. + final rawRatio = + _pinchLastRawScale > 0 ? details.scale / _pinchLastRawScale : 1.0; + final scaleDrop = + rawRatio > _kScaleGlitchHi || rawRatio < _kScaleGlitchLo; + final focalDrop = details.focalPointDelta.distance > _kFocalGlitchPx; + if (scaleDrop || focalDrop) return; + + final targetScale = absolutePinchScale( + scaleStart: _pinchScaleStart!, + rawScaleAtBaseline: _pinchRawScaleAtBaseline, + rawScale: details.scale, + minScale: _kPinchMinScale, + maxScale: _kPinchMaxScale, + ); + + // Focal zoom: keep the document point under the live focal (finger midpoint) + // fixed, which also yields 2-finger pan for free as the focal moves. + // localFocalPoint is in the viewer's local coords (the overlay fills it). + _controller.zoomOnLocalPosition( + localPosition: details.localFocalPoint, + newZoom: targetScale, + duration: Duration.zero, + ); + + _pinchLastRawScale = details.scale; + _pinchLastAppliedScale = targetScale; + } + + void _onPinchEnd(ScaleEndDetails details) { + _pinchScaleStart = null; + _pinchPointerCount = 0; + } + // ── Text selection → highlight ───────────────────────────────────────────── void _onTextSelectionChange(PdfTextSelection selection) { @@ -804,6 +916,15 @@ class _PenEditorScreenState extends State { widget.pdfPath, controller: _controller, params: PdfViewerParams( + // pdfrx keeps 1-finger scroll + wheel (panEnabled) but we TAKE OVER the + // 2-finger pinch (scaleEnabled:false): pdfrx's forked InteractiveViewer + // applies `desiredScale = _scaleStart * details.scale` with no per-frame + // glitch guard, so a Windows-touch scale spike or pointer-count blip pops + // the zoom and snaps back. We drive zoom ourselves via the glitch-guarded + // _TwoFingerPinch recognizer in viewerOverlayBuilder → controller. + // zoomOnLocalPosition (focal zoom). See _onPinchUpdate. + panEnabled: true, + scaleEnabled: false, // Native vector text selection. Pen falls through to this only in // select-text mode (PenCaptureRegion.captureEnabled == false). textSelectionParams: PdfTextSelectionParams( @@ -840,11 +961,11 @@ class _PenEditorScreenState extends State { child: IgnorePointer( child: CustomPaint( painter: _PageOverlayPainter( - repaint: _overlayRepaint, + overlayRepaint: _overlayRepaint, + liveStrokeVN: _liveStrokeVN, + pageIndex: pageIndex, strokes: _strokesByPage[pageIndex] ?? const [], highlights: _highlightsByPage[pageIndex] ?? const [], - liveStroke: - _liveStrokePage == pageIndex ? _liveStroke : null, pageSize: pageRectInViewer.size, thinning: _penConfig?.value.pressureSensitivity ?? kDefaultPenThinning, @@ -883,11 +1004,32 @@ class _PenEditorScreenState extends State { ), ]; }, - // (2) Viewer-level pen capture. Stylus is captured ONLY when a pen tool - // is active; touch/mouse (and pen in select-text mode) fall through to - // pdfrx for scroll/zoom/text-selection. + // (2) Viewer-level pen capture + our glitch-guarded pinch. Stylus is + // captured ONLY when a pen tool is active; touch/mouse (and pen in + // select-text mode) fall through to pdfrx for scroll/text-selection. + // The pinch recognizer is touch-only and concedes the arena below 2 + // pointers, so 1-finger scroll still reaches pdfrx and the pen (captured + // outside the arena by PenCaptureRegion) is never stolen. viewerOverlayBuilder: (context, size, handleLinkTap) { return [ + Positioned.fill( + child: RawGestureDetector( + // translucent (NOT opaque): the touch must ALSO hit-test pdfrx + // underneath so its pan recognizer can win the 1-finger case. + behavior: HitTestBehavior.translucent, + gestures: { + _TwoFingerPinch: + GestureRecognizerFactoryWithHandlers<_TwoFingerPinch>( + () => _TwoFingerPinch(debugOwner: this), + (r) => r + ..onStart = _onPinchStart + ..onUpdate = _onPinchUpdate + ..onEnd = _onPinchEnd, + ), + }, + child: const SizedBox.expand(), + ), + ), Positioned.fill( child: PenCaptureRegion( captureEnabled: _penCaptureEnabled, @@ -1151,23 +1293,88 @@ class _PenEditorScreenState extends State { } } +/// A [ScaleGestureRecognizer] that competes ONLY for 2-finger touch gestures. +/// +/// pdfrx's internal pan recognizer runs underneath (scaleEnabled is off, so it +/// still scrolls on 1 finger). A vanilla ScaleGestureRecognizer is eager: with a +/// single uncontested pointer it would win the arena and STEAL 1-finger scroll. +/// Restricting [supportedDevices] to touch keeps the stylus out (the pen is +/// captured outside the arena by [PenCaptureRegion]); rejecting the pointer while +/// fewer than 2 are down hands the 1-finger gesture back to pdfrx immediately, +/// and accepting on the 2nd finger lets us drive the pinch. Guarding inside +/// onUpdate would be too late — by then the arena is already won. +class _TwoFingerPinch extends ScaleGestureRecognizer { + _TwoFingerPinch({super.debugOwner}) + : super(supportedDevices: const {PointerDeviceKind.touch}); + + final Set _pointers = {}; + + @override + void addAllowedPointer(PointerDownEvent event) { + _pointers.add(event.pointer); + super.addAllowedPointer(event); + if (_pointers.length < 2) { + // Concede the 1-finger case now so pdfrx's pan wins without waiting. + resolvePointer(event.pointer, GestureDisposition.rejected); + } else { + // Second finger down: claim the gesture before pdfrx treats it as a pan. + resolve(GestureDisposition.accepted); + } + } + + @override + void handleEvent(PointerEvent event) { + if (event is PointerUpEvent || event is PointerCancelEvent) { + _pointers.remove(event.pointer); + } + super.handleEvent(event); + } + + @override + void rejectGesture(int pointer) { + _pointers.remove(pointer); + super.rejectGesture(pointer); + } +} + +/// The currently in-progress stroke and the page it belongs to. Published via a +/// [ValueNotifier] so [_PageOverlayPainter] can read the live stroke at PAINT +/// time (driven by the notifier) instead of capturing a stale build-time +/// snapshot — that snapshot bug made ink appear only on pointer-up. +class _LiveStrokeData { + const _LiveStrokeData(this.page, this.stroke); + + final int page; + final PenStroke stroke; +} + /// Paints one page's overlay: text highlights (under), committed ink, then the /// live in-progress stroke (over). Strokes are in normalized page coords; the /// painter scales them to the on-screen page rect ([pageSize]) so they stay /// glued to the page under pdfrx's native zoom/scroll. +/// +/// The live stroke is read from [liveStrokeVN] at paint time (not passed as a +/// constructor snapshot) so that mid-stroke updates — which notify the merged +/// repaint Listenable — redraw the in-progress ink immediately. Committed +/// strokes + highlights still arrive via the constructor (they change only on +/// setState, which rebuilds this painter). class _PageOverlayPainter extends CustomPainter { _PageOverlayPainter({ - required Listenable repaint, + required Listenable overlayRepaint, + required this.liveStrokeVN, + required this.pageIndex, required this.strokes, required this.highlights, - required this.liveStroke, required this.pageSize, required this.thinning, - }) : super(repaint: repaint); + }) : super(repaint: Listenable.merge([overlayRepaint, liveStrokeVN])); + /// Live stroke source, read at paint time. Only painted when its page matches + /// [pageIndex]. + final ValueListenable<_LiveStrokeData?> liveStrokeVN; + final int pageIndex; final List strokes; final List highlights; - final PenStroke? liveStroke; final Size pageSize; final double thinning; @@ -1205,16 +1412,16 @@ class _PageOverlayPainter extends CustomPainter { ); } - // 3. Live stroke. - final live = liveStroke; - if (live != null && live.points.isNotEmpty) { - final path = - buildStrokePath(live, size, isComplete: false, thinning: thinning); + // 3. Live stroke — read from the notifier at paint time, only for this page. + final live = liveStrokeVN.value; + if (live != null && live.page == pageIndex && live.stroke.points.isNotEmpty) { + final path = buildStrokePath(live.stroke, size, + isComplete: false, thinning: thinning); if (!path.getBounds().isEmpty) { canvas.drawPath( path, Paint() - ..color = Color(live.color) + ..color = Color(live.stroke.color) ..style = PaintingStyle.fill ..isAntiAlias = true, ); @@ -1224,15 +1431,55 @@ class _PageOverlayPainter extends CustomPainter { @override bool shouldRepaint(_PageOverlayPainter old) => + !identical(old.liveStrokeVN, liveStrokeVN) || + old.pageIndex != pageIndex || !identical(old.strokes, strokes) || old.strokes.length != strokes.length || !identical(old.highlights, highlights) || old.highlights.length != highlights.length || - !identical(old.liveStroke, liveStroke) || old.pageSize != pageSize || old.thinning != thinning; } +/// Test-only handle on the live-stroke painter wiring (Bug 1 regression guard). +/// +/// The live stroke must be read from a [ValueListenable] at PAINT time, so a +/// mid-stroke update repaints WITHOUT a rebuild of `pageOverlaysBuilder`. This +/// seam lets a widget test pump the real painter and assert that pushing a new +/// value into the notifier triggers a repaint (which the old build-time-snapshot +/// design did not). +@visibleForTesting +class LiveStrokeOverlayHarness { + LiveStrokeOverlayHarness({required this.pageIndex}); + + final int pageIndex; + final ValueNotifier overlayRepaint = ValueNotifier(0); + final ValueNotifier<_LiveStrokeData?> _liveStrokeVN = ValueNotifier(null); + + /// Push a live stroke for [page] (or null to clear). Mirrors what + /// `_updateLiveStroke`/`_endStroke` do at runtime. + void setLiveStroke(int page, PenStroke? stroke) { + _liveStrokeVN.value = stroke == null ? null : _LiveStrokeData(page, stroke); + } + + /// The real [CustomPainter] used by the editor, wired to this harness's + /// notifiers exactly as `pageOverlaysBuilder` wires it. + CustomPainter buildPainter() => _PageOverlayPainter( + overlayRepaint: overlayRepaint, + liveStrokeVN: _liveStrokeVN, + pageIndex: pageIndex, + strokes: const [], + highlights: const [], + pageSize: const Size(100, 100), + thinning: kDefaultPenThinning, + ); + + void dispose() { + overlayRepaint.dispose(); + _liveStrokeVN.dispose(); + } +} + /// A small sticky-note "tab" marker glued to a page at a scratch-link anchor. /// Tap opens the anchor's split view; long-press deletes the anchor. class _ScratchLinkMarker extends StatelessWidget { diff --git a/test/pen_editor_live_stroke_test.dart b/test/pen_editor_live_stroke_test.dart new file mode 100644 index 0000000..74465ac --- /dev/null +++ b/test/pen_editor_live_stroke_test.dart @@ -0,0 +1,126 @@ +// Regression guard for Bug 1 ("字迹写完才出现"): the PDF editor's per-page overlay +// painter must read the in-progress stroke from a ValueListenable at PAINT time, +// so a mid-stroke update repaints the live ink WITHOUT re-running +// pageOverlaysBuilder (which only re-runs on setState). The old design captured +// the live stroke as a build-time snapshot, so _bumpOverlay repainted the +// painter but it still drew the stale (null) snapshot — the stroke only appeared +// on pointer-up. +// +// These tests pump the REAL _PageOverlayPainter (via LiveStrokeOverlayHarness) +// and assert (1) paint() reflects the CURRENT notifier value, and (2) updating +// the notifier alone drives a CustomPaint repaint with no widget rebuild. + +import 'dart:ui' as ui; + +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:badnote/editor/canvas/pen_editor_screen.dart'; +import 'package:badnote/editor/canvas/pen_stroke.dart'; +import 'package:badnote/editor/engine/brush.dart'; + +PenStroke _stroke() => PenStroke( + points: const [ + PenPoint(0.2, 0.2, 0.8), + PenPoint(0.5, 0.5, 0.8), + PenPoint(0.8, 0.8, 0.8), + ], + color: 0xFF000000, + width: 0.01, + kind: PenStrokeKind.pen, + brush: BrushKind.fountainPen, + ); + +/// Records the number of drawPath calls a painter issues for the given size. +int _drawPathCount(CustomPainter painter, Size size) { + final recorder = ui.PictureRecorder(); + final canvas = _CountingCanvas(Canvas(recorder)); + painter.paint(canvas, size); + recorder.endRecording().dispose(); + return canvas.drawPathCount; +} + +void main() { + const size = Size(100, 100); + + test('paint() reads the CURRENT live stroke from the notifier at paint time', + () { + final harness = LiveStrokeOverlayHarness(pageIndex: 0); + addTearDown(harness.dispose); + final painter = harness.buildPainter(); + + // Idle: nothing to draw. + expect(_drawPathCount(painter, size), 0); + + // Mid-stroke: pushing a live stroke into the notifier must make the SAME + // painter instance draw it on the next paint (no reconstruction). + harness.setLiveStroke(0, _stroke()); + expect(_drawPathCount(painter, size), 1, + reason: 'painter must read the live stroke at paint time, not a ' + 'build-time snapshot'); + + // Cleared: back to nothing. + harness.setLiveStroke(0, null); + expect(_drawPathCount(painter, size), 0); + }); + + test('a live stroke on a DIFFERENT page is not painted here', () { + final harness = LiveStrokeOverlayHarness(pageIndex: 0); + addTearDown(harness.dispose); + final painter = harness.buildPainter(); + + harness.setLiveStroke(3, _stroke()); // belongs to page 3, not page 0 + expect(_drawPathCount(painter, size), 0); + }); + + testWidgets('updating the live-stroke notifier repaints without a rebuild', + (tester) async { + final harness = LiveStrokeOverlayHarness(pageIndex: 0); + addTearDown(harness.dispose); + + var builds = 0; + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: Builder(builder: (context) { + builds++; + return CustomPaint( + painter: harness.buildPainter(), + size: size, + ); + }), + ), + ); + expect(builds, 1); + + // Pushing a live stroke must trigger a repaint of the CustomPaint via its + // merged repaint Listenable — WITHOUT rebuilding the widget tree (which is + // exactly the path _bumpOverlay/setState would NOT cover mid-stroke). + harness.setLiveStroke(0, _stroke()); + await tester.pump(); + expect(builds, 1, reason: 'no widget rebuild should be needed'); + + // And the painter now draws the live ink. + expect(_drawPathCount(harness.buildPainter(), size), 1); + }); +} + +/// A [Canvas] proxy that counts drawPath calls. +class _CountingCanvas implements Canvas { + _CountingCanvas(this._inner); + + final Canvas _inner; + int drawPathCount = 0; + + @override + void drawPath(ui.Path path, ui.Paint paint) { + drawPathCount++; + _inner.drawPath(path, paint); + } + + @override + void noSuchMethod(Invocation invocation) => + _forward(invocation); + + dynamic _forward(Invocation i) => null; +}