Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: don't create thumbnails when auto-saving #1175

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
17 changes: 14 additions & 3 deletions lib/components/canvas/save_indicator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,21 +28,26 @@ class SaveIndicator extends StatelessWidget {
icon: switch (savingState.value) {
SavingState.waitingToSave => const Icon(Icons.save),
SavingState.saving => const CircularProgressIndicator.adaptive(),
SavingState.saved => const Icon(Icons.arrow_back),
SavingState.savedWithoutThumbnail => const Icon(Icons.arrow_back),
SavingState.savedWithThumbnail => const Icon(Icons.arrow_back),
},
),
);
},
);
}

/// When the save/exit button is pressed
void _onPressed(BuildContext context) {
switch (savingState.value) {
case SavingState.waitingToSave:
triggerSave();
case SavingState.saving:
break;
case SavingState.saved:
case SavingState.savedWithoutThumbnail:
triggerSave(); // triggering save will be created thumbnail and then is finished editing
_back(context);
case SavingState.savedWithThumbnail:
_back(context);
}
}
Expand All @@ -62,5 +67,11 @@ class SaveIndicator extends StatelessWidget {
enum SavingState {
waitingToSave,
saving,
saved,

/// Saved, but the thumbnail still needs updating.
/// (Thumbnails aren't created when auto-saving to avoid lag.)
savedWithoutThumbnail,

/// Saved, and the thumbnail is up-to-date.
savedWithThumbnail,
}
5 changes: 4 additions & 1 deletion lib/components/home/preview_card.dart
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,11 @@ class _PreviewCardState extends State<PreviewCard> {
}

StreamSubscription? fileWriteSubscription;

/// Listen to changes of thumbnail file
void fileWriteListener(FileOperation event) {
if (event.filePath != widget.filePath) return;
if (event.filePath != '${widget.filePath}${Editor.extension}.p') return;
adil192 marked this conversation as resolved.
Show resolved Hide resolved

if (event.type == FileOperationType.delete) {
thumbnail.image = null;
} else if (event.type == FileOperationType.write) {
Expand Down
3 changes: 2 additions & 1 deletion lib/components/navbar/responsive_navbar.dart
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ class _ResponsiveNavbarState extends State<ResponsiveNavbar> {
final savingState = Whiteboard.savingState;
switch (savingState) {
case null:
case SavingState.saved:
case SavingState.savedWithoutThumbnail:
case SavingState.savedWithThumbnail:
break;
case SavingState.waitingToSave:
Whiteboard.triggerSave();
Expand Down
10 changes: 2 additions & 8 deletions lib/data/file_manager/file_manager.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ class FileManager {
/// Realistically, this value never changes.
static late String documentsDirectory;

/// A stream of [FileOperation]s. Note that file paths
/// include the file extension.
static final StreamController<FileOperation> fileWriteStream =
StreamController.broadcast(
onListen: () => _fileWriteStreamIsListening = true,
Expand Down Expand Up @@ -73,14 +75,6 @@ class FileManager {
@visibleForTesting
static void broadcastFileWrite(FileOperationType type, String path) async {
if (!_fileWriteStreamIsListening) return;

// remove extension
if (path.endsWith(Editor.extension)) {
path = path.substring(0, path.length - Editor.extension.length);
} else if (path.endsWith(Editor.extensionOldJson)) {
path = path.substring(0, path.length - Editor.extensionOldJson.length);
}

fileWriteStream.add(FileOperation(type, path));
}

Expand Down
119 changes: 76 additions & 43 deletions lib/pages/editor/editor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,9 @@ class EditorState extends State<Editor> {
Prefs.lastTool.value = tool.toolId;
}

ValueNotifier<SavingState> savingState = ValueNotifier(SavingState.saved);
Timer? _delayedSaveTimer;
final savingState = ValueNotifier(SavingState.savedWithThumbnail);
@visibleForTesting
Timer? delayedSaveTimer;

// used to prevent accidentally drawing when pinch zooming
int lastSeenPointerCount = 0;
Expand Down Expand Up @@ -258,7 +259,8 @@ class EditorState extends State<Editor> {
clearAllPages();

// save cleared whiteboard
await saveToFile();
// without thumbanil as whiteboard thumbnail is never used
await saveToFile(createThumbnail: false);
Whiteboard.needsToAutoClearWhiteboard = false;
} else {
setState(() {});
Expand Down Expand Up @@ -320,12 +322,12 @@ class EditorState extends State<Editor> {
late final topOfLastPage = -CanvasGestureDetector.getTopOfPage(
pageIndex: coreInfo.pages.length - 1,
pages: coreInfo.pages,
screenWidth: MediaQuery.sizeOf(context).width,
screenWidth: _mediaQuery.size.width,
);
final bottomOfLastPage = -CanvasGestureDetector.getTopOfPage(
pageIndex: coreInfo.pages.length,
pages: coreInfo.pages,
screenWidth: MediaQuery.sizeOf(context).width,
screenWidth: _mediaQuery.size.width,
);

if (scrollY < bottomOfLastPage) {
Expand Down Expand Up @@ -802,28 +804,34 @@ class EditorState extends State<Editor> {

void autosaveAfterDelay() {
savingState.value = SavingState.waitingToSave;
_delayedSaveTimer?.cancel();
delayedSaveTimer?.cancel();
if (Prefs.autosaveDelay.value < 0) return;
_delayedSaveTimer =
delayedSaveTimer =
Timer(Duration(milliseconds: Prefs.autosaveDelay.value), () {
saveToFile();
saveToFile(createThumbnail: false);
});
}

Future<void> saveToFile() async {
Future<void> saveToFile({required bool createThumbnail}) async {
// createThumbnail=false is used when called from autosave - to avoid lagging during thumbnail creation
if (coreInfo.readOnly) return;

switch (savingState.value) {
case SavingState.saved:
case SavingState.savedWithThumbnail:
// avoid saving if nothing has changed
return;
case SavingState.savedWithoutThumbnail:
// note is saved, but thumbnail need to be created
createThumbnailPreview();
savingState.value = SavingState.savedWithThumbnail;
return;
case SavingState.saving:
// avoid saving if already saving
log.warning('saveToFile() called while already saving');
return;
case SavingState.waitingToSave:
// continue
_delayedSaveTimer?.cancel();
delayedSaveTimer?.cancel();
savingState.value = SavingState.saving;
}

Expand Down Expand Up @@ -852,14 +860,26 @@ class EditorState extends State<Editor> {
numAssets: assets.length,
),
]);
savingState.value = SavingState.saved;
if (createThumbnail) {
savingState.value = SavingState.savedWithThumbnail;
} else {
savingState.value = SavingState.savedWithoutThumbnail;
}
} catch (e) {
log.severe('Failed to save file: $e', e);
savingState.value = SavingState.waitingToSave;
if (kDebugMode) rethrow;
return;
}

if (createThumbnail) await createThumbnailPreview();
}

/// create thumbnail of note
Future<void> createThumbnailPreview() async {
if (coreInfo.readOnly) return;
final filePath = coreInfo.filePath + Editor.extension;

if (!mounted) return;
final screenshotter = ScreenshotController();
final page = coreInfo.pages.first;
Expand All @@ -868,31 +888,37 @@ class EditorState extends State<Editor> {
);
final thumbnailSize = Size(720, 720 * previewHeight / page.size.width);
final thumbnail = await screenshotter.captureFromWidget(
Theme(
data: ThemeData(
brightness: Brightness.light,
colorScheme: const ColorScheme.light(
primary: EditorExporter.primaryColor,
secondary: EditorExporter.secondaryColor,
QubaB marked this conversation as resolved.
Show resolved Hide resolved
TranslationProvider(
child: MaterialApp(
theme: ThemeData(
brightness: Brightness.light,
colorScheme: const ColorScheme.light(
primary: EditorExporter.primaryColor,
secondary: EditorExporter.secondaryColor,
),
),
),
child: Localizations.override(
context: context,
child: SizedBox(
width: thumbnailSize.width,
height: thumbnailSize.height,
child: FittedBox(
child: pagePreviewBuilder(
context,
pageIndex: 0,
previewHeight: previewHeight,
home: MediaQuery(
data: MediaQueryData(
size: thumbnailSize,
devicePixelRatio: 1,
),
child: SizedBox(
width: thumbnailSize.width,
height: thumbnailSize.height,
child: FittedBox(
child: Builder(
builder: (context) => pagePreviewBuilder(
context,
pageIndex: 0,
previewHeight: previewHeight,
),
),
),
),
),
),
),
pixelRatio: 1,
context: context,
targetSize: thumbnailSize,
);
await FileManager.writeFile(
Expand Down Expand Up @@ -1280,6 +1306,14 @@ class EditorState extends State<Editor> {
));
}

late MediaQueryData _mediaQuery = const MediaQueryData();

@override
void didChangeDependencies() {
super.didChangeDependencies();
_mediaQuery = MediaQuery.of(context);
}

@override
Widget build(BuildContext context) {
final colorScheme = Theme.of(context).colorScheme;
Expand Down Expand Up @@ -1553,17 +1587,18 @@ class EditorState extends State<Editor> {
builder: (context, savingState, child) {
// don't allow user to go back until saving is done
return PopScope(
canPop: savingState == SavingState.saved,
canPop: savingState == SavingState.savedWithThumbnail,
onPopInvoked: (didPop) {
switch (savingState) {
case SavingState.waitingToSave:
assert(!didPop);
saveToFile(); // trigger save now
saveToFile(createThumbnail: true); // trigger save now
snackBarNeedsToSaveBeforeExiting();
case SavingState.saving:
assert(!didPop);
snackBarNeedsToSaveBeforeExiting();
case SavingState.saved:
case SavingState.savedWithoutThumbnail:
case SavingState.savedWithThumbnail:
break;
}
},
Expand Down Expand Up @@ -1592,7 +1627,7 @@ class EditorState extends State<Editor> {
),
leading: SaveIndicator(
savingState: savingState,
triggerSave: saveToFile,
triggerSave: () => saveToFile(createThumbnail: true),
),
actions: [
IconButton(
Expand All @@ -1607,7 +1642,7 @@ class EditorState extends State<Editor> {
CanvasGestureDetector.scrollToPage(
pageIndex: currentPageIndex + 1,
pages: coreInfo.pages,
screenWidth: MediaQuery.sizeOf(context).width,
screenWidth: _mediaQuery.size.width,
transformationController: _transformationController,
);
}),
Expand Down Expand Up @@ -1920,11 +1955,9 @@ class EditorState extends State<Editor> {
int get currentPageIndex {
if (!mounted) return _lastCurrentPageIndex;

final screenWidth = MediaQuery.sizeOf(context).width;

return _lastCurrentPageIndex = getPageIndexFromScrollPosition(
scrollY: -scrollY,
screenWidth: screenWidth,
screenWidth: _mediaQuery.size.width,
pages: coreInfo.pages,
);
}
Expand All @@ -1951,21 +1984,21 @@ class EditorState extends State<Editor> {
}

@override
void dispose() {
void dispose() async {
delayedSaveTimer?.cancel();
_lastSeenPointerCountTimer?.cancel();

(() async {
if (_renameTimer?.isActive ?? false) {
_renameTimer!.cancel();
await _renameFileNow();
filenameTextEditingController.dispose();
}
await saveToFile();
await saveToFile(createThumbnail: true);
})();

DynamicMaterialApp.removeFullscreenListener(_setState);

_delayedSaveTimer?.cancel();
_lastSeenPointerCountTimer?.cancel();

_removeKeybindings();

coreInfo.dispose();
Expand Down
2 changes: 1 addition & 1 deletion lib/pages/home/whiteboard.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class Whiteboard extends StatelessWidget {
final editorState = _whiteboardKey.currentState;
if (editorState == null) return;
assert(editorState.savingState.value == SavingState.waitingToSave);
editorState.saveToFile();
editorState.saveToFile(createThumbnail: false);
editorState.snackBarNeedsToSaveBeforeExiting();
}

Expand Down
13 changes: 5 additions & 8 deletions test/editor_undo_redo_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ void main() {
reason: 'Editor is still read-only');
printOnFailure('Editor core info is loaded');

addTearDown(() async {
editorState.delayedSaveTimer?.cancel();
await FileManager.deleteFile(filePath + Editor.extension);
});

IconButton getUndoBtn() => tester.widget<IconButton>(find.ancestor(
of: find.byIcon(Icons.undo),
matching: find.byType(IconButton),
Expand Down Expand Up @@ -108,14 +113,6 @@ void main() {
reason: 'Undo button should be enabled after undo and draw');
expect(getRedoBtn().onPressed, isNull,
reason: 'Redo button should be disabled after undo and draw');

// save file now to supersede the save timer (which would run after the test is finished)
printOnFailure('Saving file: $filePath${Editor.extension}');
await tester.runAsync(() async {
await editorState.saveToFile();
await Future.delayed(const Duration(milliseconds: 100));
await FileManager.deleteFile(filePath + Editor.extension);
});
});
}

Expand Down
Loading