feat(pen): persist brush kind so it survives reload
Some checks failed
CI / Windows build (push) Has been cancelled

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.
This commit is contained in:
2026-06-24 23:40:40 +08:00
parent 6c2dd71b82
commit 46589a4c87
8 changed files with 206 additions and 40 deletions

View File

@@ -339,13 +339,11 @@ class _PenEditorScreenState extends State<PenEditorScreen> {
kind: es.tool == EditorTool.highlighter kind: es.tool == EditorTool.highlighter
? PenStrokeKind.highlighter ? PenStrokeKind.highlighter
: PenStrokeKind.pen, : PenStrokeKind.pen,
// Brush isn't persisted yet (TODO(brush-persist)); derive it // Brush is now persisted on the EditorStroke; carry it through
// from the tool so a loaded highlighter still renders with the // so a reopened ballpoint/pencil/highlighter keeps its
// highlighter brush (flat width), and pens fall back to the // opacity/blend. Old sidecars without the field decode to
// fountainPen default. // fountainPen (see EditorStroke.brush back-compat default).
brush: es.tool == EditorTool.highlighter brush: es.brush,
? BrushKind.highlighter
: BrushKind.fountainPen,
)) ))
.toList(); .toList();
} }

View File

@@ -155,7 +155,8 @@ class _PenNoteScreenState extends ConsumerState<PenNoteScreen> {
} }
/// EditorStroke → live PenStroke (mirror of the PDF editor's loader). Brush /// 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( PenStroke _penStrokeFromEditor(EditorStroke es) => PenStroke(
points: es.points points: es.points
.map((ep) => PenPoint(ep.x, ep.y, ep.pressure, tilt: ep.tilt)) .map((ep) => PenPoint(ep.x, ep.y, ep.pressure, tilt: ep.tilt))
@@ -165,9 +166,7 @@ class _PenNoteScreenState extends ConsumerState<PenNoteScreen> {
kind: es.tool == EditorTool.highlighter kind: es.tool == EditorTool.highlighter
? PenStrokeKind.highlighter ? PenStrokeKind.highlighter
: PenStrokeKind.pen, : PenStrokeKind.pen,
brush: es.tool == EditorTool.highlighter brush: es.brush,
? BrushKind.highlighter
: BrushKind.fountainPen,
); );
Future<void> _initPenConfig() async { Future<void> _initPenConfig() async {

View File

@@ -20,19 +20,31 @@
import 'dart:ui' show Color, BlendMode; 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 /// The four selectable brushes. Extensible: add a kind here + a preset in
/// [kBrushPresets]. The eraser is NOT a brush — it stays a separate tool. /// [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 { enum BrushKind {
/// Strong pressure→width (rnote Pow2 / quadratic), soft taper, solid ink. /// Strong pressure→width (rnote Pow2 / quadratic), soft taper, solid ink.
@JsonValue('fountainPen')
fountainPen, fountainPen,
/// Near-constant thin width; pressure carries OPACITY (the ballpoint "tell"). /// Near-constant thin width; pressure carries OPACITY (the ballpoint "tell").
@JsonValue('ballpoint')
ballpoint, ballpoint,
/// Broad, flat width, translucent, square (uncapped) ends. /// Broad, flat width, translucent, square (uncapped) ends.
@JsonValue('highlighter')
highlighter, highlighter,
/// Moderate width + opacity from pressure (rnote Sqrt / √p), scratchy. /// Moderate width + opacity from pressure (rnote Sqrt / √p), scratchy.
@JsonValue('pencil')
pencil, pencil,
} }

View File

@@ -13,6 +13,12 @@
// * Point x/y are NORMALIZED to the page rectangle, i.e. in [0,1]. // * 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. // * 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:freezed_annotation/freezed_annotation.dart';
import 'package:uuid/uuid.dart'; import 'package:uuid/uuid.dart';
@@ -75,13 +81,17 @@ abstract class EditorStroke with _$EditorStroke {
@Default(false) bool filled, @Default(false) bool filled,
String? textContent, String? textContent,
@Default(14.0) double fontSize, @Default(14.0) double fontSize,
// Brush the stroke was drawn with. NOT serialized (increment 1 keeps brush // Brush the stroke was drawn with — drives the committed render path's
// out of persistence / InkStroke round-trip — see TODO(brush-persist)); it // perfect_freehand geometry + opacity/blend (resolveStrokePaint). Persisted
// is an IN-MEMORY render hint only, so the committed render path can resolve // as the stable `BrushKind` @JsonValue name (e.g. "ballpoint") so a
// each stroke's perfect_freehand geometry. Defaults to fountainPen so loaded // ballpoint/highlighter/pencil stroke keeps its look across close/reopen.
// (deserialized) strokes keep the legacy pen visual. // BACK-COMPAT: sidecars written before this field existed have no `brush`
// ignore: invalid_annotation_target // key, and an unknown name (a future brush opened by an older build) is
@JsonKey(includeFromJson: false, includeToJson: false) // tolerated — both fall back to fountainPen via the JsonKey below.
@JsonKey(
defaultValue: BrushKind.fountainPen,
unknownEnumValue: BrushKind.fountainPen,
)
@Default(BrushKind.fountainPen) @Default(BrushKind.fountainPen)
BrushKind brush, BrushKind brush,
}) = _EditorStroke; }) = _EditorStroke;
@@ -154,6 +164,11 @@ abstract class EditorStroke with _$EditorStroke {
filled: stroke.filled, filled: stroke.filled,
textContent: stroke.textContent, textContent: stroke.textContent,
fontSize: stroke.fontSize, 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 /// Lossless adapter to the freezed/JSON [InkStroke] model. Null superset

View File

@@ -303,12 +303,17 @@ mixin _$EditorStroke {
bool get filled => throw _privateConstructorUsedError; bool get filled => throw _privateConstructorUsedError;
String? get textContent => throw _privateConstructorUsedError; String? get textContent => throw _privateConstructorUsedError;
double get fontSize => double get fontSize =>
throw _privateConstructorUsedError; // Brush the stroke was drawn with. NOT serialized (increment 1 keeps brush throw _privateConstructorUsedError; // Brush the stroke was drawn with — drives the committed render path's
// out of persistence / InkStroke round-trip — see TODO(brush-persist)); it // perfect_freehand geometry + opacity/blend (resolveStrokePaint). Persisted
// is an IN-MEMORY render hint only, so the committed render path can resolve // as the stable `BrushKind` @JsonValue name (e.g. "ballpoint") so a
// each stroke's perfect_freehand geometry. Defaults to fountainPen so loaded // ballpoint/highlighter/pencil stroke keeps its look across close/reopen.
// (deserialized) strokes keep the legacy pen visual. // BACK-COMPAT: sidecars written before this field existed have no `brush`
@JsonKey(includeFromJson: false, includeToJson: false) // 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; BrushKind get brush => throw _privateConstructorUsedError;
/// Serializes this EditorStroke to a JSON map. /// Serializes this EditorStroke to a JSON map.
@@ -337,7 +342,11 @@ abstract class $EditorStrokeCopyWith<$Res> {
bool filled, bool filled,
String? textContent, String? textContent,
double fontSize, 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, bool filled,
String? textContent, String? textContent,
double fontSize, 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.filled = false,
this.textContent, this.textContent,
this.fontSize = 14.0, this.fontSize = 14.0,
@JsonKey(includeFromJson: false, includeToJson: false) @JsonKey(
defaultValue: BrushKind.fountainPen,
unknownEnumValue: BrushKind.fountainPen,
)
this.brush = BrushKind.fountainPen, this.brush = BrushKind.fountainPen,
}) : _points = points, }) : _points = points,
super._(); super._();
@@ -546,13 +562,18 @@ class _$EditorStrokeImpl extends _EditorStroke {
@override @override
@JsonKey() @JsonKey()
final double fontSize; final double fontSize;
// Brush the stroke was drawn with. NOT serialized (increment 1 keeps brush // Brush the stroke was drawn with — drives the committed render path's
// out of persistence / InkStroke round-trip — see TODO(brush-persist)); it // perfect_freehand geometry + opacity/blend (resolveStrokePaint). Persisted
// is an IN-MEMORY render hint only, so the committed render path can resolve // as the stable `BrushKind` @JsonValue name (e.g. "ballpoint") so a
// each stroke's perfect_freehand geometry. Defaults to fountainPen so loaded // ballpoint/highlighter/pencil stroke keeps its look across close/reopen.
// (deserialized) strokes keep the legacy pen visual. // 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 @override
@JsonKey(includeFromJson: false, includeToJson: false) @JsonKey(
defaultValue: BrushKind.fountainPen,
unknownEnumValue: BrushKind.fountainPen,
)
final BrushKind brush; final BrushKind brush;
@override @override
@@ -617,7 +638,10 @@ abstract class _EditorStroke extends EditorStroke {
final bool filled, final bool filled,
final String? textContent, final String? textContent,
final double fontSize, final double fontSize,
@JsonKey(includeFromJson: false, includeToJson: false) @JsonKey(
defaultValue: BrushKind.fountainPen,
unknownEnumValue: BrushKind.fountainPen,
)
final BrushKind brush, final BrushKind brush,
}) = _$EditorStrokeImpl; }) = _$EditorStrokeImpl;
_EditorStroke._() : super._(); _EditorStroke._() : super._();
@@ -640,13 +664,18 @@ abstract class _EditorStroke extends EditorStroke {
@override @override
String? get textContent; String? get textContent;
@override @override
double get fontSize; // Brush the stroke was drawn with. NOT serialized (increment 1 keeps brush double get fontSize; // Brush the stroke was drawn with — drives the committed render path's
// out of persistence / InkStroke round-trip — see TODO(brush-persist)); it // perfect_freehand geometry + opacity/blend (resolveStrokePaint). Persisted
// is an IN-MEMORY render hint only, so the committed render path can resolve // as the stable `BrushKind` @JsonValue name (e.g. "ballpoint") so a
// each stroke's perfect_freehand geometry. Defaults to fountainPen so loaded // ballpoint/highlighter/pencil stroke keeps its look across close/reopen.
// (deserialized) strokes keep the legacy pen visual. // 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 @override
@JsonKey(includeFromJson: false, includeToJson: false) @JsonKey(
defaultValue: BrushKind.fountainPen,
unknownEnumValue: BrushKind.fountainPen,
)
BrushKind get brush; BrushKind get brush;
/// Create a copy of EditorStroke /// Create a copy of EditorStroke

View File

@@ -52,6 +52,13 @@ _$EditorStrokeImpl _$$EditorStrokeImplFromJson(Map<String, dynamic> json) =>
filled: json['filled'] as bool? ?? false, filled: json['filled'] as bool? ?? false,
textContent: json['textContent'] as String?, textContent: json['textContent'] as String?,
fontSize: (json['fontSize'] as num?)?.toDouble() ?? 14.0, fontSize: (json['fontSize'] as num?)?.toDouble() ?? 14.0,
brush:
$enumDecodeNullable(
_$BrushKindEnumMap,
json['brush'],
unknownValue: BrushKind.fountainPen,
) ??
BrushKind.fountainPen,
); );
Map<String, dynamic> _$$EditorStrokeImplToJson(_$EditorStrokeImpl instance) => Map<String, dynamic> _$$EditorStrokeImplToJson(_$EditorStrokeImpl instance) =>
@@ -64,6 +71,7 @@ Map<String, dynamic> _$$EditorStrokeImplToJson(_$EditorStrokeImpl instance) =>
'filled': instance.filled, 'filled': instance.filled,
'textContent': instance.textContent, 'textContent': instance.textContent,
'fontSize': instance.fontSize, 'fontSize': instance.fontSize,
'brush': _$BrushKindEnumMap[instance.brush]!,
}; };
const _$EditorToolEnumMap = { const _$EditorToolEnumMap = {
@@ -71,3 +79,10 @@ const _$EditorToolEnumMap = {
EditorTool.highlighter: 'highlighter', EditorTool.highlighter: 'highlighter',
EditorTool.eraser: 'eraser', EditorTool.eraser: 'eraser',
}; };
const _$BrushKindEnumMap = {
BrushKind.fountainPen: 'fountainPen',
BrushKind.ballpoint: 'ballpoint',
BrushKind.highlighter: 'highlighter',
BrushKind.pencil: 'pencil',
};

View File

@@ -10,6 +10,7 @@ import 'dart:convert';
import 'package:flutter_test/flutter_test.dart'; 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/editor/engine/stroke_model.dart';
import 'package:badnote/models/bookmark.dart'; import 'package:badnote/models/bookmark.dart';
import 'package:badnote/models/ink_point.dart'; import 'package:badnote/models/ink_point.dart';
@@ -141,6 +142,43 @@ void main() {
expect(sp.strokes, original.scratchLinks.single.scratchpad.strokes); 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', () { test('strokes JSON is byte-compatible with EditorStroke.toJson', () {
final stroke = _editorStroke('s0', EditorTool.pen); final stroke = _editorStroke('s0', EditorTool.pen);
final sidecar = BadnoteSidecar(strokes: {2: [stroke]}); final sidecar = BadnoteSidecar(strokes: {2: [stroke]});

View File

@@ -10,6 +10,7 @@
import 'dart:convert'; import 'dart:convert';
import 'dart:ui'; import 'dart:ui';
import 'package:badnote/editor/engine/brush.dart';
import 'package:badnote/editor/engine/stroke_geometry.dart'; import 'package:badnote/editor/engine/stroke_geometry.dart';
import 'package:badnote/editor/engine/stroke_model.dart'; import 'package:badnote/editor/engine/stroke_model.dart';
import 'package:badnote/models/ink_point.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<String, dynamic>,
);
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<String, dynamic>;
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<String, dynamic>
..['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<String, dynamic>,
);
expect(decoded.brush, BrushKind.pencil);
});
});
group('EditorStroke <-> InkStroke round-trip', () { group('EditorStroke <-> InkStroke round-trip', () {
test('InkStroke -> EditorStroke -> InkStroke is lossless', () { test('InkStroke -> EditorStroke -> InkStroke is lossless', () {
final ink = InkStroke( final ink = InkStroke(