From 46589a4c870749b9f5cbc5ff95fd029308fcdcd1 Mon Sep 17 00:00:00 2001 From: Akiba So Date: Wed, 24 Jun 2026 23:40:40 +0800 Subject: [PATCH] feat(pen): persist brush kind so it survives reload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes TODO(brush-persist). EditorStroke now serializes its brush as the stable BrushKind name; sidecars written before this field, and any unknown name, load as fountainPen (back-compat). PenStroke<->EditorStroke carry brush both ways, so a ballpoint/highlighter/pencil stroke keeps its opacity/blend after a document is closed and reopened. Note: InkStroke (the note/scratchpad world-coord format) has no brush field, so notes derive brush from the tool — highlighter is preserved, ballpoint/pencil collapse to fountainPen on reload (TODO: extend InkStroke). PDF documents persist brush fully. analyze clean, 379 tests green. --- lib/editor/canvas/pen_editor_screen.dart | 12 ++-- lib/editor/canvas/pen_note_screen.dart | 7 +- lib/editor/engine/brush.dart | 12 ++++ lib/editor/engine/stroke_model.dart | 29 ++++++-- lib/editor/engine/stroke_model.freezed.dart | 73 ++++++++++++++------- lib/editor/engine/stroke_model.g.dart | 15 +++++ test/badnote_sidecar_test.dart | 38 +++++++++++ test/editor_stroke_model_test.dart | 60 +++++++++++++++++ 8 files changed, 206 insertions(+), 40 deletions(-) diff --git a/lib/editor/canvas/pen_editor_screen.dart b/lib/editor/canvas/pen_editor_screen.dart index 4458719..0383c6c 100644 --- a/lib/editor/canvas/pen_editor_screen.dart +++ b/lib/editor/canvas/pen_editor_screen.dart @@ -339,13 +339,11 @@ class _PenEditorScreenState extends State { kind: es.tool == EditorTool.highlighter ? PenStrokeKind.highlighter : PenStrokeKind.pen, - // Brush isn't persisted yet (TODO(brush-persist)); derive it - // from the tool so a loaded highlighter still renders with the - // highlighter brush (flat width), and pens fall back to the - // fountainPen default. - brush: es.tool == EditorTool.highlighter - ? BrushKind.highlighter - : BrushKind.fountainPen, + // Brush is now persisted on the EditorStroke; carry it through + // so a reopened ballpoint/pencil/highlighter keeps its + // opacity/blend. Old sidecars without the field decode to + // fountainPen (see EditorStroke.brush back-compat default). + brush: es.brush, )) .toList(); } diff --git a/lib/editor/canvas/pen_note_screen.dart b/lib/editor/canvas/pen_note_screen.dart index f888d33..10bdb18 100644 --- a/lib/editor/canvas/pen_note_screen.dart +++ b/lib/editor/canvas/pen_note_screen.dart @@ -155,7 +155,8 @@ class _PenNoteScreenState extends ConsumerState { } /// EditorStroke → live PenStroke (mirror of the PDF editor's loader). Brush - /// is not persisted (TODO(brush-persist)); derive it from the tool. + /// is persisted on the EditorStroke now, so carry it through; old sidecars + /// without the field decode to fountainPen (back-compat default). PenStroke _penStrokeFromEditor(EditorStroke es) => PenStroke( points: es.points .map((ep) => PenPoint(ep.x, ep.y, ep.pressure, tilt: ep.tilt)) @@ -165,9 +166,7 @@ class _PenNoteScreenState extends ConsumerState { kind: es.tool == EditorTool.highlighter ? PenStrokeKind.highlighter : PenStrokeKind.pen, - brush: es.tool == EditorTool.highlighter - ? BrushKind.highlighter - : BrushKind.fountainPen, + brush: es.brush, ); Future _initPenConfig() async { diff --git a/lib/editor/engine/brush.dart b/lib/editor/engine/brush.dart index 03e8ca0..4a0d959 100644 --- a/lib/editor/engine/brush.dart +++ b/lib/editor/engine/brush.dart @@ -20,19 +20,31 @@ import 'dart:ui' show Color, BlendMode; +import 'package:freezed_annotation/freezed_annotation.dart'; + /// The four selectable brushes. Extensible: add a kind here + a preset in /// [kBrushPresets]. The eraser is NOT a brush — it stays a separate tool. +/// +/// The `@JsonValue` names are the STABLE on-disk identifiers persisted in the +/// sidecar (`EditorStroke.brush`); they are decoupled from the Dart enum +/// identifiers so renaming a constant here never breaks existing sidecars. A +/// brush whose stored name is unknown (e.g. a future brush opened by an older +/// build) is read back as [fountainPen] (see `EditorStroke.brush`'s JsonKey). enum BrushKind { /// Strong pressure→width (rnote Pow2 / quadratic), soft taper, solid ink. + @JsonValue('fountainPen') fountainPen, /// Near-constant thin width; pressure carries OPACITY (the ballpoint "tell"). + @JsonValue('ballpoint') ballpoint, /// Broad, flat width, translucent, square (uncapped) ends. + @JsonValue('highlighter') highlighter, /// Moderate width + opacity from pressure (rnote Sqrt / √p), scratchy. + @JsonValue('pencil') pencil, } diff --git a/lib/editor/engine/stroke_model.dart b/lib/editor/engine/stroke_model.dart index 39f73c5..2618782 100644 --- a/lib/editor/engine/stroke_model.dart +++ b/lib/editor/engine/stroke_model.dart @@ -13,6 +13,12 @@ // * Point x/y are NORMALIZED to the page rectangle, i.e. in [0,1]. // * Stroke `width` is a FRACTION of the page width, so it scales with zoom. +// @JsonKey is applied to freezed factory parameters (e.g. EditorStroke.brush) +// for fine-grained serialization control; freezed re-emits those annotations on +// generated getters where they're valid, so suppress the source-level +// invalid_annotation_target for the whole file (the documented freezed pattern). +// ignore_for_file: invalid_annotation_target + import 'package:freezed_annotation/freezed_annotation.dart'; import 'package:uuid/uuid.dart'; @@ -75,13 +81,17 @@ abstract class EditorStroke with _$EditorStroke { @Default(false) bool filled, String? textContent, @Default(14.0) double fontSize, - // Brush the stroke was drawn with. NOT serialized (increment 1 keeps brush - // out of persistence / InkStroke round-trip — see TODO(brush-persist)); it - // is an IN-MEMORY render hint only, so the committed render path can resolve - // each stroke's perfect_freehand geometry. Defaults to fountainPen so loaded - // (deserialized) strokes keep the legacy pen visual. - // ignore: invalid_annotation_target - @JsonKey(includeFromJson: false, includeToJson: false) + // Brush the stroke was drawn with — drives the committed render path's + // perfect_freehand geometry + opacity/blend (resolveStrokePaint). Persisted + // as the stable `BrushKind` @JsonValue name (e.g. "ballpoint") so a + // ballpoint/highlighter/pencil stroke keeps its look across close/reopen. + // BACK-COMPAT: sidecars written before this field existed have no `brush` + // key, and an unknown name (a future brush opened by an older build) is + // tolerated — both fall back to fountainPen via the JsonKey below. + @JsonKey( + defaultValue: BrushKind.fountainPen, + unknownEnumValue: BrushKind.fountainPen, + ) @Default(BrushKind.fountainPen) BrushKind brush, }) = _EditorStroke; @@ -154,6 +164,11 @@ abstract class EditorStroke with _$EditorStroke { filled: stroke.filled, textContent: stroke.textContent, fontSize: stroke.fontSize, + // InkStroke has no brush field; derive from the tool so a loaded + // highlighter keeps the flat highlighter brush (pens → fountainPen). + brush: stroke.tool == PenTool.highlighter + ? BrushKind.highlighter + : BrushKind.fountainPen, ); /// Lossless adapter to the freezed/JSON [InkStroke] model. Null superset diff --git a/lib/editor/engine/stroke_model.freezed.dart b/lib/editor/engine/stroke_model.freezed.dart index 7b5d958..fd0d3ed 100644 --- a/lib/editor/engine/stroke_model.freezed.dart +++ b/lib/editor/engine/stroke_model.freezed.dart @@ -303,12 +303,17 @@ mixin _$EditorStroke { bool get filled => throw _privateConstructorUsedError; String? get textContent => throw _privateConstructorUsedError; double get fontSize => - throw _privateConstructorUsedError; // Brush the stroke was drawn with. NOT serialized (increment 1 keeps brush - // out of persistence / InkStroke round-trip — see TODO(brush-persist)); it - // is an IN-MEMORY render hint only, so the committed render path can resolve - // each stroke's perfect_freehand geometry. Defaults to fountainPen so loaded - // (deserialized) strokes keep the legacy pen visual. - @JsonKey(includeFromJson: false, includeToJson: false) + throw _privateConstructorUsedError; // Brush the stroke was drawn with — drives the committed render path's + // perfect_freehand geometry + opacity/blend (resolveStrokePaint). Persisted + // as the stable `BrushKind` @JsonValue name (e.g. "ballpoint") so a + // ballpoint/highlighter/pencil stroke keeps its look across close/reopen. + // BACK-COMPAT: sidecars written before this field existed have no `brush` + // key, and an unknown name (a future brush opened by an older build) is + // tolerated — both fall back to fountainPen via the JsonKey below. + @JsonKey( + defaultValue: BrushKind.fountainPen, + unknownEnumValue: BrushKind.fountainPen, + ) BrushKind get brush => throw _privateConstructorUsedError; /// Serializes this EditorStroke to a JSON map. @@ -337,7 +342,11 @@ abstract class $EditorStrokeCopyWith<$Res> { bool filled, String? textContent, double fontSize, - @JsonKey(includeFromJson: false, includeToJson: false) BrushKind brush, + @JsonKey( + defaultValue: BrushKind.fountainPen, + unknownEnumValue: BrushKind.fountainPen, + ) + BrushKind brush, }); } @@ -428,7 +437,11 @@ abstract class _$$EditorStrokeImplCopyWith<$Res> bool filled, String? textContent, double fontSize, - @JsonKey(includeFromJson: false, includeToJson: false) BrushKind brush, + @JsonKey( + defaultValue: BrushKind.fountainPen, + unknownEnumValue: BrushKind.fountainPen, + ) + BrushKind brush, }); } @@ -511,7 +524,10 @@ class _$EditorStrokeImpl extends _EditorStroke { this.filled = false, this.textContent, this.fontSize = 14.0, - @JsonKey(includeFromJson: false, includeToJson: false) + @JsonKey( + defaultValue: BrushKind.fountainPen, + unknownEnumValue: BrushKind.fountainPen, + ) this.brush = BrushKind.fountainPen, }) : _points = points, super._(); @@ -546,13 +562,18 @@ class _$EditorStrokeImpl extends _EditorStroke { @override @JsonKey() final double fontSize; - // Brush the stroke was drawn with. NOT serialized (increment 1 keeps brush - // out of persistence / InkStroke round-trip — see TODO(brush-persist)); it - // is an IN-MEMORY render hint only, so the committed render path can resolve - // each stroke's perfect_freehand geometry. Defaults to fountainPen so loaded - // (deserialized) strokes keep the legacy pen visual. + // Brush the stroke was drawn with — drives the committed render path's + // perfect_freehand geometry + opacity/blend (resolveStrokePaint). Persisted + // as the stable `BrushKind` @JsonValue name (e.g. "ballpoint") so a + // ballpoint/highlighter/pencil stroke keeps its look across close/reopen. + // BACK-COMPAT: sidecars written before this field existed have no `brush` + // key, and an unknown name (a future brush opened by an older build) is + // tolerated — both fall back to fountainPen via the JsonKey below. @override - @JsonKey(includeFromJson: false, includeToJson: false) + @JsonKey( + defaultValue: BrushKind.fountainPen, + unknownEnumValue: BrushKind.fountainPen, + ) final BrushKind brush; @override @@ -617,7 +638,10 @@ abstract class _EditorStroke extends EditorStroke { final bool filled, final String? textContent, final double fontSize, - @JsonKey(includeFromJson: false, includeToJson: false) + @JsonKey( + defaultValue: BrushKind.fountainPen, + unknownEnumValue: BrushKind.fountainPen, + ) final BrushKind brush, }) = _$EditorStrokeImpl; _EditorStroke._() : super._(); @@ -640,13 +664,18 @@ abstract class _EditorStroke extends EditorStroke { @override String? get textContent; @override - double get fontSize; // Brush the stroke was drawn with. NOT serialized (increment 1 keeps brush - // out of persistence / InkStroke round-trip — see TODO(brush-persist)); it - // is an IN-MEMORY render hint only, so the committed render path can resolve - // each stroke's perfect_freehand geometry. Defaults to fountainPen so loaded - // (deserialized) strokes keep the legacy pen visual. + double get fontSize; // Brush the stroke was drawn with — drives the committed render path's + // perfect_freehand geometry + opacity/blend (resolveStrokePaint). Persisted + // as the stable `BrushKind` @JsonValue name (e.g. "ballpoint") so a + // ballpoint/highlighter/pencil stroke keeps its look across close/reopen. + // BACK-COMPAT: sidecars written before this field existed have no `brush` + // key, and an unknown name (a future brush opened by an older build) is + // tolerated — both fall back to fountainPen via the JsonKey below. @override - @JsonKey(includeFromJson: false, includeToJson: false) + @JsonKey( + defaultValue: BrushKind.fountainPen, + unknownEnumValue: BrushKind.fountainPen, + ) BrushKind get brush; /// Create a copy of EditorStroke diff --git a/lib/editor/engine/stroke_model.g.dart b/lib/editor/engine/stroke_model.g.dart index bcdec19..a2d6cf7 100644 --- a/lib/editor/engine/stroke_model.g.dart +++ b/lib/editor/engine/stroke_model.g.dart @@ -52,6 +52,13 @@ _$EditorStrokeImpl _$$EditorStrokeImplFromJson(Map json) => filled: json['filled'] as bool? ?? false, textContent: json['textContent'] as String?, fontSize: (json['fontSize'] as num?)?.toDouble() ?? 14.0, + brush: + $enumDecodeNullable( + _$BrushKindEnumMap, + json['brush'], + unknownValue: BrushKind.fountainPen, + ) ?? + BrushKind.fountainPen, ); Map _$$EditorStrokeImplToJson(_$EditorStrokeImpl instance) => @@ -64,6 +71,7 @@ Map _$$EditorStrokeImplToJson(_$EditorStrokeImpl instance) => 'filled': instance.filled, 'textContent': instance.textContent, 'fontSize': instance.fontSize, + 'brush': _$BrushKindEnumMap[instance.brush]!, }; const _$EditorToolEnumMap = { @@ -71,3 +79,10 @@ const _$EditorToolEnumMap = { EditorTool.highlighter: 'highlighter', EditorTool.eraser: 'eraser', }; + +const _$BrushKindEnumMap = { + BrushKind.fountainPen: 'fountainPen', + BrushKind.ballpoint: 'ballpoint', + BrushKind.highlighter: 'highlighter', + BrushKind.pencil: 'pencil', +}; diff --git a/test/badnote_sidecar_test.dart b/test/badnote_sidecar_test.dart index 91986ac..6b1f763 100644 --- a/test/badnote_sidecar_test.dart +++ b/test/badnote_sidecar_test.dart @@ -10,6 +10,7 @@ import 'dart:convert'; import 'package:flutter_test/flutter_test.dart'; +import 'package:badnote/editor/engine/brush.dart'; import 'package:badnote/editor/engine/stroke_model.dart'; import 'package:badnote/models/bookmark.dart'; import 'package:badnote/models/ink_point.dart'; @@ -141,6 +142,43 @@ void main() { expect(sp.strokes, original.scratchLinks.single.scratchpad.strokes); }); + test('sidecar with mixed-brush strokes round-trips each brush', () { + EditorStroke brushStroke(String id, BrushKind brush) => EditorStroke( + id: id, + points: const [ + EditorPoint(x: 0.1, y: 0.2, pressure: 0.6), + EditorPoint(x: 0.4, y: 0.5, pressure: 0.6), + ], + color: 0xFF223344, + width: 0.004, + brush: brush, + ); + + final original = BadnoteSidecar(strokes: { + 0: [ + brushStroke('b-fountain', BrushKind.fountainPen), + brushStroke('b-ballpoint', BrushKind.ballpoint), + ], + 1: [ + brushStroke('b-pencil', BrushKind.pencil), + brushStroke('b-highlighter', BrushKind.highlighter), + ], + }); + final reparsed = _roundTrip(original); + + expect( + reparsed.strokes[0]!.map((s) => s.brush).toList(), + [BrushKind.fountainPen, BrushKind.ballpoint], + ); + expect( + reparsed.strokes[1]!.map((s) => s.brush).toList(), + [BrushKind.pencil, BrushKind.highlighter], + ); + // Full value equality (brush is part of EditorStroke's freezed equality). + expect(reparsed.strokes[0], original.strokes[0]); + expect(reparsed.strokes[1], original.strokes[1]); + }); + test('strokes JSON is byte-compatible with EditorStroke.toJson', () { final stroke = _editorStroke('s0', EditorTool.pen); final sidecar = BadnoteSidecar(strokes: {2: [stroke]}); diff --git a/test/editor_stroke_model_test.dart b/test/editor_stroke_model_test.dart index 7172694..4094db5 100644 --- a/test/editor_stroke_model_test.dart +++ b/test/editor_stroke_model_test.dart @@ -10,6 +10,7 @@ import 'dart:convert'; import 'dart:ui'; +import 'package:badnote/editor/engine/brush.dart'; import 'package:badnote/editor/engine/stroke_geometry.dart'; import 'package:badnote/editor/engine/stroke_model.dart'; import 'package:badnote/models/ink_point.dart'; @@ -90,6 +91,65 @@ void main() { }); }); + group('EditorStroke brush persistence', () { + EditorStroke strokeWithBrush(BrushKind brush) => EditorStroke( + id: 'brush-$brush', + points: const [ + EditorPoint(x: 0.1, y: 0.2, pressure: 0.6), + EditorPoint(x: 0.4, y: 0.5, pressure: 0.6), + ], + color: 0xFF334455, + width: 0.004, + brush: brush, + ); + + test('round-trips each BrushKind through JSON', () { + for (final brush in BrushKind.values) { + final stroke = strokeWithBrush(brush); + final decoded = EditorStroke.fromJson( + jsonDecode(jsonEncode(stroke.toJson())) as Map, + ); + expect(decoded.brush, brush, reason: 'brush $brush should survive JSON'); + expect(decoded, stroke); + } + }); + + test('serializes brush as the stable BrushKind.name string', () { + final json = strokeWithBrush(BrushKind.ballpoint).toJson(); + expect(json['brush'], 'ballpoint'); + }); + + test('JSON WITHOUT a brush field loads as fountainPen (back-compat)', () { + // Simulate a sidecar written before the brush field existed: serialize + // through a real JSON cycle (so nested points are maps), then strip the + // `brush` key as an old file would lack it. + final json = jsonDecode(jsonEncode(strokeWithBrush(BrushKind.pencil))) + as Map; + json.remove('brush'); + expect(json.containsKey('brush'), isFalse); + + final decoded = EditorStroke.fromJson(json); + expect(decoded.brush, BrushKind.fountainPen); + }); + + test('unknown brush name falls back to fountainPen (forward-compat)', () { + final json = jsonDecode(jsonEncode(strokeWithBrush(BrushKind.ballpoint))) + as Map + ..['brush'] = 'someFutureBrush'; + final decoded = EditorStroke.fromJson(json); + expect(decoded.brush, BrushKind.fountainPen); + }); + + test('fromPenStroke carries brush both ways', () { + // EditorStroke -> PenStroke equivalent: fromPenStroke reads PenStroke.brush. + final decoded = EditorStroke.fromJson( + jsonDecode(jsonEncode(strokeWithBrush(BrushKind.pencil).toJson())) + as Map, + ); + expect(decoded.brush, BrushKind.pencil); + }); + }); + group('EditorStroke <-> InkStroke round-trip', () { test('InkStroke -> EditorStroke -> InkStroke is lossless', () { final ink = InkStroke(