diff --git a/CHANGELOG.md b/CHANGELOG.md index 314cdcafb..e88c9c394 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Improve `getDiff` perfomance for the editor [#2517](https://github.com/singerdmx/flutter-quill/pull/2517). + ## [11.2.0] - 2025-03-26 ### Added diff --git a/lib/src/delta/delta_diff.dart b/lib/src/delta/delta_diff.dart index acbe17d9a..084a8f9a3 100644 --- a/lib/src/delta/delta_diff.dart +++ b/lib/src/delta/delta_diff.dart @@ -1,7 +1,6 @@ import 'dart:math' as math; -import 'dart:ui' show TextDirection; -import 'package:flutter/foundation.dart' show immutable; +import 'package:flutter/material.dart'; import '../../quill_delta.dart'; import '../document/attribute.dart'; @@ -16,6 +15,33 @@ class Diff { required this.inserted, }); + const Diff.insert({ + required this.start, + required this.inserted, + }) : deleted = ''; + + const Diff.noDiff({ + this.start = 0, + }) : deleted = '', + inserted = ''; + + const Diff.delete({ + required this.start, + required this.deleted, + }) : inserted = ''; + + /// Checks if the diff is just a delete + bool get isDelete => inserted.isEmpty && deleted.isNotEmpty; + + /// Checks if the diff is just replace + bool get isReplace => inserted.isNotEmpty && deleted.isNotEmpty; + + /// Checks if the diff is just an isnertion + bool get isInsert => inserted.isNotEmpty && deleted.isEmpty; + + /// Checks if the diff has no changes + bool get hasNoDiff => inserted.isEmpty && deleted.isEmpty; + // Start index in old text at which changes begin. final int start; @@ -31,30 +57,222 @@ class Diff { } } -/* Get diff operation between old text and new text */ -Diff getDiff(String oldText, String newText, int cursorPosition) { - var end = oldText.length; - final delta = newText.length - end; - for (final limit = math.max(0, cursorPosition - delta); - end > limit && oldText[end - 1] == newText[end + delta - 1]; - end--) {} - var start = 0; - //TODO: we need to improve this part because this loop has a lot of unsafe index operations - for (final startLimit = cursorPosition - math.max(0, delta); - start < startLimit && - (start > oldText.length - 1 ? '' : oldText[start]) == - (start > newText.length - 1 ? '' : newText[start]); - start++) {} - final deleted = (start >= end) ? '' : oldText.substring(start, end); - // we need to make the check if the start is major than the end because if we directly get the - // new inserted text without checking first, this will always throw an error since this is an unsafe op - final inserted = - (start >= end + delta) ? '' : newText.substring(start, end + delta); - return Diff( - start: start, - deleted: deleted, - inserted: inserted, +/// Get text changes between two strings using [oldStr] and [newStr] +/// using selection as the base with [oldSelection] and [newSelection]. +/// +/// Performance: O([k]) where [k] == change size (not document length) +Diff getDiff( + String oldStr, + String newStr, + TextSelection oldSelection, + TextSelection newSelection, +) { + if (oldStr == newStr) return Diff.noDiff(start: newSelection.start); + + // 1. Calculate affected range based on selections + final affectedRange = + _getAffectedRange(oldStr, newStr, oldSelection, newSelection); + var start = affectedRange.start + .clamp(0, math.min(oldStr.length, newStr.length)) + .toInt(); + final end = affectedRange.end + .clamp(0, math.max(oldStr.length, newStr.length)) + .toInt(); + + // 2. Adjust bounds for length variations + final oldLen = oldStr.length; + final newLen = newStr.length; + final lengthDiff = newLen - oldLen; + + // 3. Forward search from range start + var hasForwardChange = false; + + while (start < end && start < oldStr.length && start < newStr.length) { + if (oldStr[start] != newStr[start]) { + hasForwardChange = true; + break; + } + // Force forward if the change comes only from the cursor position + if (start >= oldSelection.baseOffset && start >= newSelection.baseOffset) { + break; + } + start++; + } + + // 4. Backward search from range end + var oldEnd = math.min(end, oldLen); + var newEnd = math.min(end + lengthDiff, newLen); + + var hasBackwardChange = false; + + while (oldEnd > start && newEnd > start) { + if (oldStr[oldEnd - 1] != newStr[newEnd - 1]) { + hasBackwardChange = true; + break; + } + // Breaks if the cursor still into the same position + if (oldEnd - 1 <= oldSelection.baseOffset && + newEnd - 1 <= newSelection.baseOffset) { + break; + } + oldEnd--; + newEnd--; + } + + // This is a workaround that fixes an issue where, when the cursor + // is between two characters, that are the same ("s|s"), when you + // press backspace key, instead removes the "s" character before the cursor, + // it just moves to left without removing nothing + if (!hasForwardChange && !hasBackwardChange) { + if (oldStr.length > newStr.length) { + return Diff.delete( + start: oldSelection.baseOffset < newSelection.baseOffset + ? oldSelection.baseOffset + : newSelection.baseOffset, + deleted: ' ', + ); + } + } + + final safeOldEnd = oldEnd.clamp(start, oldStr.length); + final safeNewEnd = newEnd.clamp(start, newStr.length); + + // 5. Extract differences + final deleted = oldStr.substring(start, safeOldEnd); + final inserted = newStr.substring(start, safeNewEnd); + + // 6. Validate consistency + if (_isChangeConsistent( + deleted, inserted, oldStr, oldSelection, newSelection)) { + return _buildDiff(deleted, inserted, start); + } + + // Fallback for complex cases + return _fallbackDiff(oldStr, newStr, start, end); +} + +TextRange _getAffectedRange( + String oldStr, + String newStr, + TextSelection oldSel, + TextSelection newSel, +) { + // Calculate combined selection area + final start = math.min(oldSel.start, newSel.start); + final end = math.max(oldSel.end, newSel.end); + + // Expand by 20% to capture nearby changes + // + // We use this to avoid check all the string length + // unnecessarily when we can use the selection as a base + // to know where, and how was do it the change + final expansion = ((end - start) * 0.2).round(); + + return TextRange( + start: math.max(0, start - expansion), + end: math.min(math.max(oldStr.length, newStr.length), end + expansion), + ); +} + +bool _isChangeConsistent( + String deleted, + String inserted, + String oldText, + TextSelection oldSel, + TextSelection newSel, +) { + final isForwardDelete = _isForwardDelete( + deletedText: deleted, + oldText: oldText, + oldSelection: oldSel, + newSelection: newSel, ); + if (isForwardDelete) { + return newSel.start == oldSel.start && + deleted.length == (oldSel.end - oldSel.start); + } + final isInsert = newSel.start == newSel.end && inserted.isNotEmpty; + final isDelete = deleted.isNotEmpty && inserted.isEmpty; + + // Insert validation + if (isInsert) { + return newSel.start == oldSel.start + inserted.length; + } + + // Delete validation + if (isDelete) { + return oldSel.start - newSel.start == deleted.length; + } + + return true; +} + +/// Detect if the deletion was do it to forward +bool _isForwardDelete({ + required String deletedText, + required String oldText, + required TextSelection oldSelection, + required TextSelection newSelection, +}) { + // is forward delete if: + return + // 1. There's deleted text + deletedText.isNotEmpty && + + // 2. The original selection is collaped + oldSelection.isCollapsed && + + // 3. New and original selections has the same offset + newSelection.isCollapsed && + newSelection.baseOffset == oldSelection.baseOffset && + + // 4. The removed character if after the cursor position + oldText.startsWith(deletedText, oldSelection.baseOffset); +} + +Diff _fallbackDiff(String oldStr, String newStr, int start, [int? end]) { + end ??= math.min(oldStr.length, newStr.length); + + // 1. Find first divergence point + while (start < end && + start < oldStr.length && + start < newStr.length && + oldStr[start] == newStr[start]) { + start++; + } + + // 2. Find last divergence point + var oldEnd = oldStr.length; + var newEnd = newStr.length; + + while (oldEnd > start && + newEnd > start && + oldStr[oldEnd - 1] == newStr[newEnd - 1]) { + oldEnd--; + newEnd--; + } + + // 3. Extract differences + final deleted = oldStr.substring(start, oldEnd); + final inserted = newStr.substring(start, newEnd); + + return _buildDiff(deleted, inserted, start); +} + +Diff _buildDiff(String deleted, String inserted, int start) { + if (deleted.isEmpty && inserted.isEmpty) return const Diff.noDiff(); + + if (deleted.isNotEmpty && inserted.isNotEmpty) { + return Diff( + inserted: inserted, + start: start, + deleted: deleted, + ); + } else if (inserted.isNotEmpty) { + return Diff.insert(start: start, inserted: inserted); + } else { + return Diff.delete(start: start, deleted: deleted); + } } int getPositionDelta(Delta user, Delta actual) { diff --git a/lib/src/editor/raw_editor/raw_editor_state_selection_delegate_mixin.dart b/lib/src/editor/raw_editor/raw_editor_state_selection_delegate_mixin.dart index 9746711a5..6edaf7edb 100644 --- a/lib/src/editor/raw_editor/raw_editor_state_selection_delegate_mixin.dart +++ b/lib/src/editor/raw_editor/raw_editor_state_selection_delegate_mixin.dart @@ -15,18 +15,26 @@ mixin RawEditorStateSelectionDelegateMixin on EditorState } set textEditingValue(TextEditingValue value) { - final cursorPosition = value.selection.extentOffset; final oldText = widget.controller.document.toPlainText(); final newText = value.text; - final diff = getDiff(oldText, newText, cursorPosition); - if (diff.deleted == '' && diff.inserted == '') { + final diff = getDiff( + oldText, + newText, + widget.controller.selection, + value.selection, + ); + if (diff.hasNoDiff) { // Only changing selection range widget.controller.updateSelection(value.selection, ChangeSource.local); return; } widget.controller.replaceTextWithEmbeds( - diff.start, diff.deleted.length, diff.inserted, value.selection); + diff.start, + diff.deleted.length, + diff.inserted, + value.selection, + ); } @override diff --git a/lib/src/editor/raw_editor/raw_editor_state_text_input_client_mixin.dart b/lib/src/editor/raw_editor/raw_editor_state_text_input_client_mixin.dart index e35489b75..1e9779f61 100644 --- a/lib/src/editor/raw_editor/raw_editor_state_text_input_client_mixin.dart +++ b/lib/src/editor/raw_editor/raw_editor_state_text_input_client_mixin.dart @@ -228,9 +228,13 @@ mixin RawEditorStateTextInputClientMixin on EditorState _lastKnownRemoteTextEditingValue = value; final oldText = effectiveLastKnownValue.text; final text = value.text; - final cursorPosition = value.selection.extentOffset; - final diff = getDiff(oldText, text, cursorPosition); - if (diff.deleted.isEmpty && diff.inserted.isEmpty) { + final diff = getDiff( + oldText, + text, + widget.controller.selection, + value.selection, + ); + if (diff.hasNoDiff) { widget.controller.updateSelection(value.selection, ChangeSource.local); } else { widget.controller.replaceText( diff --git a/test/editor/perfomance_get_diff_test.dart b/test/editor/perfomance_get_diff_test.dart new file mode 100644 index 000000000..ceea30936 --- /dev/null +++ b/test/editor/perfomance_get_diff_test.dart @@ -0,0 +1,122 @@ +import 'package:flutter/services.dart' show TextSelection; +import 'package:flutter_quill/src/delta/delta_diff.dart'; +import 'package:test/test.dart'; + +void main() { + group('performance Tests', () { + late Stopwatch stopwatch; + const loremIpsum = + 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. ' + 'Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. '; + + setUp(() => stopwatch = Stopwatch()); + + test('simple insert (should complete <10ms)', () { + // Small text (50 chars) + final text = loremIpsum.substring(0, 50); + const selection = TextSelection.collapsed(offset: 10); + + stopwatch.start(); + final diff = getDiff( + text, + text.replaceRange(10, 10, 'X'), // Insert 'X' at position 10 + selection, + const TextSelection.collapsed(offset: 11), + ); + stopwatch.stop(); + + expect(diff.isInsert, isTrue); + expect(stopwatch.elapsedMilliseconds, lessThan(10)); + }); + + test('medium replace (should complete <50ms)', () { + // Medium text (1,000 chars) + final text = List.generate(20, (_) => loremIpsum).join(); + const selection = TextSelection(baseOffset: 100, extentOffset: 105); + + stopwatch.start(); + final diff = getDiff( + text, + text.replaceRange(100, 105, 'NEW'), // Replace 5 chars with "NEW" + selection, + const TextSelection.collapsed(offset: 103), + ); + stopwatch.stop(); + + expect(diff.isReplace, isTrue); + expect(stopwatch.elapsedMilliseconds, lessThan(50)); + }); + + test('large document edit (should complete <500ms)', () { + // Large text (100,000 chars) + final text = List.generate(2000, (_) => loremIpsum).join(); + const selection = TextSelection.collapsed(offset: 50000); + + stopwatch.start(); + final diff = getDiff( + text, + text.replaceRange(50000, 50000, 'INSERTION'), // Insert at position 50k + selection, + const TextSelection.collapsed(offset: 50008), + ); + stopwatch.stop(); + + expect(diff.isInsert, isTrue); + expect(stopwatch.elapsedMilliseconds, lessThan(500)); + }); + + test('complex multi-edit fallback (should complete <1000ms)', () { + final text = List.generate(1000, (_) => loremIpsum).join(); + const selection = TextSelection(baseOffset: 1000, extentOffset: 1005); + + // Simulate paste with multiple changes + stopwatch.start(); + getDiff( + text, + text + .replaceRange(1000, 1005, 'ABC') + .replaceRange(2000, 2001, 'X') // Second unrelated change + .replaceRange(3000, 3002, 'YZ'), // Third change + selection, + const TextSelection.collapsed(offset: 1003), + ); + stopwatch.stop(); + + expect(stopwatch.elapsedMilliseconds, lessThan(1000)); + }); + + test('worst-case full diff (should complete <2000ms)', () { + // Two completely different large texts + final text1 = List.generate(5000, (i) => 'Line $i: $loremIpsum\n').join(); + final text2 = + List.generate(5000, (i) => 'Modified ${i * 2}: $loremIpsum\n').join(); + + stopwatch.start(); + getDiff( + text1, + text2, + const TextSelection.collapsed(offset: 0), + const TextSelection.collapsed(offset: 0), + ); + stopwatch.stop(); + + expect(stopwatch.elapsedMilliseconds, lessThan(2000)); + }); + + test('simulates forward deletion (should complete <10ms)', () { + // A simple but large text + final text1 = List.generate(5000, (i) => 'Line $i: $loremIpsum\n').join(); + + stopwatch.start(); + getDiff( + text1, + text1.replaceRange(4500, 4501, ''), + const TextSelection.collapsed(offset: 4500), + const TextSelection.collapsed(offset: 4500), + ); + stopwatch.stop(); + + expect(stopwatch.elapsedMilliseconds, lessThan(10)); + }); + }); +}