fix(pdf): live ink follows pen + stop zoom jump
All checks were successful
CI / Windows build (push) Successful in 12m51s

Two critical PDF-editor bugs.

1. Live ink only appeared after lifting the pen. The page overlay
   painter captured the live stroke as a build-time snapshot, so
   per-move repaints redrew stale (null) data until commit. Route
   the live stroke through a ValueNotifier the painter reads at
   paint time (repaint: merge(overlayRepaint, liveStrokeVN)).

2. Pinch-zoom jumped on Windows touch. pdfrx's internal forked
   InteractiveViewer scales with an unguarded scaleStart*details.scale
   that pops on a touch-count blip or one-frame spike. Take over the
   pinch: scaleEnabled:false (pdfrx keeps 1-finger scroll + wheel),
   a glitch-guarded ScaleGestureRecognizer drives focal zoom via the
   pdfrx controller, reusing absolutePinchScale + the re-baseline /
   per-frame-clamp / focal-jump guards already proven on the note
   canvas.

Zoom + pen feel are device-validated. analyze clean, tests green.
This commit is contained in:
2026-06-24 20:18:22 +08:00
parent 9bb5c483d6
commit fd102b5703
2 changed files with 396 additions and 23 deletions

View File

@@ -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<PenEditorScreen> {
final ValueNotifier<int> _overlayRepaint = ValueNotifier<int>(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<PenEditorScreen> {
/// In-progress stroke points (normalized to [_liveStrokePage]).
final List<PenPoint> _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<PenEditorScreen> {
scheduler.dispose();
}
_overlayRepaint.dispose();
_liveStrokeVN.dispose();
_penConfig?.dispose();
PenInputService.instance.stop();
DiagnosticLogger.instance.stop();
@@ -481,14 +529,16 @@ class _PenEditorScreenState extends State<PenEditorScreen> {
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<PenEditorScreen> {
}
_liveStrokePage = null;
_livePoints.clear();
_liveStroke = null;
_liveStrokeVN.value = null;
_bumpOverlay();
}
@@ -582,6 +632,68 @@ class _PenEditorScreenState extends State<PenEditorScreen> {
});
}
// ── 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<PenEditorScreen> {
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<PenEditorScreen> {
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<PenEditorScreen> {
),
];
},
// (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<PenEditorScreen> {
}
}
/// 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<int> _pointers = <int>{};
@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<PenStroke> strokes;
final List<Rect> 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<int> overlayRepaint = ValueNotifier<int>(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 {

View File

@@ -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;
}