From d50087247c8d604263b3dd90166a02aa7e1564b0 Mon Sep 17 00:00:00 2001 From: Akiba So Date: Tue, 23 Jun 2026 02:50:53 +0800 Subject: [PATCH] fix(export): single-source freehand recipe screen+export (P0 step 7, R7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pdf_service._buildFreehandPdfPath hardcoded its OWN StrokeOptions (thinning:0.7, streamline:0.5) instead of the shared geometry — the R7 hairline-export divergence. The prior pen-feel commit (streamline 0.5→0.32 on screen) widened the gap: export still rendered at 0.5. Extract the ONE perfect_freehand recipe into stroke_geometry.freehandOutlinePoints (owns thinning/smoothing/streamline/simulatePressure). buildStrokeOutline (screen) and pdf_service (export, via InkStroke→pfPoints) now both call it, so the StrokeOptions live in exactly one place and screen↔export can't drift again. Export now matches screen: thinning 0.85 (kDefaultPenThinning), streamline 0.32. test/export_geometry_test.dart pins it: buildStrokeOutline traces exactly the shared outline; default thinning == kDefaultPenThinning; thinning is wired; empty input is safe. flutter analyze lib/editor clean; 78/78 tests pass (+4). Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/editor/engine/stroke_geometry.dart | 66 +++++++++----- lib/services/pdf_service.dart | 23 +++-- test/export_geometry_test.dart | 117 +++++++++++++++++++++++++ 3 files changed, 173 insertions(+), 33 deletions(-) create mode 100644 test/export_geometry_test.dart diff --git a/lib/editor/engine/stroke_geometry.dart b/lib/editor/engine/stroke_geometry.dart index 8835349..ec6f3a3 100644 --- a/lib/editor/engine/stroke_geometry.dart +++ b/lib/editor/engine/stroke_geometry.dart @@ -32,6 +32,45 @@ const double kDefaultPenThinning = 0.85; const double kPenStreamline = 0.32; const double kPenSmoothing = 0.5; +/// THE single perfect_freehand outline recipe — the raw outline points for a +/// stroke. Both the on-screen painter ([buildStrokeOutline] / the live +/// `ink_painters.buildStrokePath`) and the PDF export +/// (`pdf_service._buildFreehandPdfPath`) call THIS, so the `StrokeOptions` +/// (thinning / smoothing / streamline / simulatePressure) live in exactly one +/// place and screen↔export can never drift again (R7 — the hairline-export bug +/// was pdf_service hardcoding its own `thinning: 0.7, streamline: 0.5`). +/// +/// Callers supply already-pixel-scaled [pfPoints] (because the two stroke +/// models scale differently) plus the per-stroke flags. Returns the closed +/// outline as `List` (perfect_freehand 2.x); empty when freehand +/// produces nothing. +List freehandOutlinePoints({ + required List pfPoints, + required double size, + required bool isHighlighter, + required bool hasRealPressure, + required bool isComplete, + double thinning = kDefaultPenThinning, +}) { + if (pfPoints.isEmpty) return const []; + return pf.getStroke( + pfPoints, + options: pf.StrokeOptions( + size: size, + // Highlighter keeps a constant width (no thinning); pen uses the + // configurable [thinning] so Surface-Pen pressure changes width. + thinning: isHighlighter ? 0.0 : thinning, + smoothing: kPenSmoothing, + streamline: kPenStreamline, + // Real stylus pressure -> don't simulate; no pressure -> let freehand + // fake it based on velocity (highlighter never simulates). perfect_freehand + // 2.x honors real pressure when simulatePressure is false. + simulatePressure: !hasRealPressure && !isHighlighter, + isComplete: isComplete, + ), + ); +} + /// Builds a closed, fillable outline [Path] for one [stroke], scaled into the /// pixel space of [pageSize] (which maps normalized [0,1] coords to pixels). /// @@ -52,11 +91,6 @@ Path buildStrokeOutline( final path = Path(); if (stroke.points.isEmpty) return path; - final pixelWidth = stroke.width * pageSize.width; - - final hasRealPressure = stroke.points.any((p) => p.pressure != null); - final isHighlighter = stroke.tool == EditorTool.highlighter; - final pfPoints = stroke.points .map( (p) => pf.PointVector( @@ -67,21 +101,13 @@ Path buildStrokeOutline( ) .toList(); - final outline = pf.getStroke( - pfPoints, - options: pf.StrokeOptions( - size: pixelWidth, - // Highlighter keeps a constant width (no thinning); pen uses the - // configurable [thinning] so Surface-Pen pressure changes width. - thinning: isHighlighter ? 0.0 : thinning, - smoothing: kPenSmoothing, - streamline: kPenStreamline, - // Real stylus pressure -> don't simulate; no pressure -> let freehand - // fake it based on velocity (highlighter never simulates). perfect_freehand - // 2.x honors real pressure when simulatePressure is false. - simulatePressure: !hasRealPressure && !isHighlighter, - isComplete: isComplete, - ), + final outline = freehandOutlinePoints( + pfPoints: pfPoints, + size: stroke.width * pageSize.width, + isHighlighter: stroke.tool == EditorTool.highlighter, + hasRealPressure: stroke.points.any((p) => p.pressure != null), + isComplete: isComplete, + thinning: thinning, ); if (outline.isEmpty) return path; diff --git a/lib/services/pdf_service.dart b/lib/services/pdf_service.dart index 54c1b0c..355ab39 100644 --- a/lib/services/pdf_service.dart +++ b/lib/services/pdf_service.dart @@ -8,6 +8,7 @@ import 'package:path_provider/path_provider.dart'; import 'package:perfect_freehand/perfect_freehand.dart' as pf; import 'package:syncfusion_flutter_pdf/pdf.dart'; +import '../editor/engine/stroke_geometry.dart' show freehandOutlinePoints; import '../models/ink_stroke.dart'; import '../models/pen_tool.dart'; @@ -304,19 +305,15 @@ class PdfService { ) .toList(); - final outline = pf.getStroke( - pfPoints, - options: pf.StrokeOptions( - size: pixelWidth, - // Highlighter keeps constant width; pen/marker taper via thinning=0.7. - thinning: isHighlighter ? 0.0 : 0.7, - smoothing: 0.5, - streamline: 0.5, - // Real stylus pressure -> don't simulate; no pressure -> let freehand - // fake it based on velocity. Highlighter never simulates. - simulatePressure: !hasRealPressure && !isHighlighter, - isComplete: true, - ), + // ONE shared recipe with the on-screen painter (R7): export can no longer + // drift from screen. Previously this hardcoded thinning:0.7/streamline:0.5, + // which diverged from the screen's 0.85/0.32 → hairline export mismatch. + final outline = freehandOutlinePoints( + pfPoints: pfPoints, + size: pixelWidth, + isHighlighter: isHighlighter, + hasRealPressure: hasRealPressure, + isComplete: true, ); if (outline.isEmpty) return null; diff --git a/test/export_geometry_test.dart b/test/export_geometry_test.dart new file mode 100644 index 0000000..8deb16e --- /dev/null +++ b/test/export_geometry_test.dart @@ -0,0 +1,117 @@ +// R7 regression (P0 step 7/8): the on-screen painter and the PDF export must +// build their freehand outline from ONE shared recipe. The export bug was +// pdf_service hardcoding `thinning: 0.7, streamline: 0.5` while the screen used +// 0.85 / 0.32 — a hairline mismatch. These tests pin the single source so a +// future edit to one side can't silently re-diverge. + +import 'dart:ui'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:perfect_freehand/perfect_freehand.dart' as pf; + +import 'package:badnote/editor/engine/stroke_geometry.dart'; +import 'package:badnote/editor/engine/stroke_model.dart'; + +void main() { + const size = Size(800, 600); + + EditorStroke pen() => EditorStroke.create( + id: 'a', + points: const [ + EditorPoint(x: 0.1, y: 0.1, pressure: 0.9), + EditorPoint(x: 0.4, y: 0.2, pressure: 0.5), + EditorPoint(x: 0.7, y: 0.15, pressure: 0.2), + ], + width: 0.01, + ); + + test('buildStrokeOutline traces exactly the shared freehandOutlinePoints', () { + final stroke = pen(); + final pfPoints = stroke.points + .map((p) => pf.PointVector( + p.x * size.width, p.y * size.height, p.pressure ?? 0.5)) + .toList(); + final shared = freehandOutlinePoints( + pfPoints: pfPoints, + size: stroke.width * size.width, + isHighlighter: false, + hasRealPressure: true, + isComplete: true, + ); + expect(shared, isNotEmpty); + + final ref = Path()..moveTo(shared.first.dx, shared.first.dy); + for (var i = 1; i < shared.length; i++) { + ref.lineTo(shared[i].dx, shared[i].dy); + } + ref.close(); + + final screen = buildStrokeOutline(stroke, size, isComplete: true); + // Same vertices ⇒ identical bounds. (If the screen path used different + // options than the shared core, the outline would differ.) + expect(screen.getBounds(), ref.getBounds()); + }); + + test('default thinning IS kDefaultPenThinning (export passes the default)', () { + final pts = [ + pf.PointVector(10, 10, 0.9), + pf.PointVector(100, 40, 0.5), + pf.PointVector(200, 30, 0.2), + ]; + final byDefault = freehandOutlinePoints( + pfPoints: pts, + size: 8, + isHighlighter: false, + hasRealPressure: true, + isComplete: true, + ); + final explicit = freehandOutlinePoints( + pfPoints: pts, + size: 8, + isHighlighter: false, + hasRealPressure: true, + isComplete: true, + thinning: kDefaultPenThinning, + ); + expect(byDefault, explicit); + }); + + test('thinning actually changes the outline (param is wired, not ignored)', + () { + final pts = [ + pf.PointVector(10, 10, 0.9), + pf.PointVector(100, 40, 0.5), + pf.PointVector(200, 30, 0.2), + ]; + final strong = freehandOutlinePoints( + pfPoints: pts, + size: 8, + isHighlighter: false, + hasRealPressure: true, + isComplete: true, + thinning: kDefaultPenThinning, + ); + final none = freehandOutlinePoints( + pfPoints: pts, + size: 8, + isHighlighter: false, + hasRealPressure: true, + isComplete: true, + thinning: 0.0, + ); + expect(strong, isNot(equals(none))); + }); + + test('empty input yields an empty outline (no crash)', () { + expect( + freehandOutlinePoints( + pfPoints: const [], + size: 8, + isHighlighter: false, + hasRealPressure: false, + isComplete: true, + ), + isEmpty, + ); + }); +}