Skip to content

Commit

Permalink
[SuperEditor][SuperReader] - Reduce uses of node index queries in Doc…
Browse files Browse the repository at this point in the history 
…ument (Resolves #2434) (#2435)
  • Loading branch information
@matthew-carroll
matthew-carroll authored Dec 5, 2024
1 parent c1fa0cd commit bda71a4
Show file tree
Hide file tree
Showing 11 changed files with 141 additions and 176 deletions.
149 changes: 44 additions & 105 deletions super_editor/lib/src/core/document_selection.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,21 +150,8 @@ class DocumentSelection extends DocumentRange {
return this;
}

final baseNode = document.getNodeById(base.nodeId)!;
final extentNode = document.getNodeById(extent.nodeId)!;

if (baseNode == extentNode) {
// The selection is expanded, but it sits within a single node.
final upstreamNodePosition = extentNode.selectUpstreamPosition(
base.nodePosition,
extent.nodePosition,
);
return DocumentSelection.collapsed(
position: extent.copyWith(nodePosition: upstreamNodePosition),
);
}

return document.getNodeIndexById(baseNode.id) < document.getNodeIndexById(extentNode.id)
final selectionAffinity = document.getAffinityForSelection(this);
return selectionAffinity == TextAffinity.downstream //
? DocumentSelection.collapsed(position: base)
: DocumentSelection.collapsed(position: extent);
}
Expand All @@ -189,23 +176,10 @@ class DocumentSelection extends DocumentRange {
return this;
}

final baseNode = document.getNodeById(base.nodeId)!;
final extentNode = document.getNodeById(extent.nodeId)!;

if (baseNode == extentNode) {
// The selection is expanded, but it sits within a single node.
final downstreamNodePosition = extentNode.selectDownstreamPosition(
base.nodePosition,
extent.nodePosition,
);
return DocumentSelection.collapsed(
position: extent.copyWith(nodePosition: downstreamNodePosition),
);
}

return document.getNodeIndexById(baseNode.id) > document.getNodeIndexById(extentNode.id)
? DocumentSelection.collapsed(position: base)
: DocumentSelection.collapsed(position: extent);
final selectionAffinity = document.getAffinityForSelection(this);
return selectionAffinity == TextAffinity.downstream //
? DocumentSelection.collapsed(position: extent)
: DocumentSelection.collapsed(position: base);
}

@override
Expand Down Expand Up @@ -326,6 +300,21 @@ extension InspectDocumentAffinity on Document {
return getAffinityBetween(base: range.start, extent: range.end);
}

TextAffinity getAffinityBetweenNodes(DocumentNode base, DocumentNode extent) {
return getAffinityForSelection(
DocumentSelection(
base: DocumentPosition(
nodeId: base.id,
nodePosition: base.beginningPosition,
),
extent: DocumentPosition(
nodeId: extent.id,
nodePosition: extent.beginningPosition,
),
),
);
}

/// Returns the affinity direction implied by the given [base] and [extent].
// TODO: Replace TextAffinity with a DocumentAffinity to avoid confusion.
TextAffinity getAffinityBetween({
Expand All @@ -336,19 +325,17 @@ extension InspectDocumentAffinity on Document {
if (baseNode == null) {
throw Exception('No such position in document: $base');
}
final baseIndex = getNodeIndexById(baseNode.id);

final extentNode = getNode(extent);
if (extentNode == null) {
throw Exception('No such position in document: $extent');
}
final extentIndex = getNodeIndexById(extentNode.id);

late TextAffinity affinity;
if (extentIndex > baseIndex) {
affinity = TextAffinity.downstream;
} else if (extentIndex < baseIndex) {
affinity = TextAffinity.upstream;
if (base.nodeId != extent.nodeId) {
affinity = getNodeIndexById(base.nodeId) < getNodeIndexById(extent.nodeId)
? TextAffinity.downstream
: TextAffinity.upstream;
} else {
// The selection is within the same node. Ask the node which position
// comes first.
Expand Down Expand Up @@ -384,20 +371,16 @@ extension InspectDocumentSelection on Document {
/// Given [docPosition1] and [docPosition2], returns the `DocumentPosition` that
/// appears first in the document.
DocumentPosition selectUpstreamPosition(DocumentPosition docPosition1, DocumentPosition docPosition2) {
final docPosition1Node = getNodeById(docPosition1.nodeId)!;
final docPosition1NodeIndex = getNodeIndexById(docPosition1Node.id);
final docPosition2Node = getNodeById(docPosition2.nodeId)!;
final docPosition2NodeIndex = getNodeIndexById(docPosition2Node.id);

if (docPosition1NodeIndex < docPosition2NodeIndex) {
return docPosition1;
} else if (docPosition2NodeIndex < docPosition1NodeIndex) {
return docPosition2;
if (docPosition1.nodeId != docPosition2.nodeId) {
final affinity = getAffinityBetween(base: docPosition1, extent: docPosition2);
return affinity == TextAffinity.downstream //
? docPosition1
: docPosition2;
}

// Both document positions are in the same node. Figure out which
// node position comes first.
final theNode = docPosition1Node;
final theNode = getNodeById(docPosition1.nodeId)!;
return theNode.selectUpstreamPosition(docPosition1.nodePosition, docPosition2.nodePosition) ==
docPosition1.nodePosition
? docPosition1
Expand All @@ -418,62 +401,18 @@ extension InspectDocumentSelection on Document {
return false;
}

final baseNode = getNodeById(selection.base.nodeId)!;
final baseNodeIndex = getNodeIndexById(baseNode.id);
final extentNode = getNodeById(selection.extent.nodeId)!;
final extentNodeIndex = getNodeIndexById(extentNode.id);

final upstreamNode = baseNodeIndex < extentNodeIndex ? baseNode : extentNode;
final upstreamNodeIndex = baseNodeIndex < extentNodeIndex ? baseNodeIndex : extentNodeIndex;
final downstreamNode = baseNodeIndex < extentNodeIndex ? extentNode : baseNode;
final downstreamNodeIndex = baseNodeIndex < extentNodeIndex ? extentNodeIndex : baseNodeIndex;

final positionNodeIndex = getNodeIndexById(position.nodeId);

if (upstreamNodeIndex < positionNodeIndex && positionNodeIndex < downstreamNodeIndex) {
// The given position is sandwiched between two other nodes that form
// the bounds of the selection. Therefore, the position is definitely within
// the selection.
return true;
}

if (positionNodeIndex == upstreamNodeIndex) {
final upstreamPosition = selectUpstreamPosition(selection.base, selection.extent);
final downstreamPosition = upstreamPosition == selection.base ? selection.extent : selection.base;

// This is the furthest a position could sit in the upstream node
// and still contain the given position. Keep in mind that the
// upstream position, downstream position, and given position may
// all reside in the same node (in fact, they probably do).
final downstreamCap =
upstreamNodeIndex == downstreamNodeIndex ? downstreamPosition.nodePosition : upstreamNode.endPosition;

// If and only if the given position comes after the upstream position,
// and before the downstream cap, then the position is within the selection.
return upstreamNode.selectDownstreamPosition(upstreamPosition.nodePosition, position.nodePosition) ==
upstreamNode.selectUpstreamPosition(position.nodePosition, downstreamCap);
}

if (positionNodeIndex == downstreamNodeIndex) {
final upstreamPosition = selectUpstreamPosition(selection.base, selection.extent);
final downstreamPosition = upstreamPosition == selection.base ? selection.extent : selection.base;

// This is the furthest upstream that a position could sit in the
// downstream node and still contain the given position. Keep in
// mind that the upstream position, downstream position, and given
// position may all reside in the same node (in fact, they probably do).
final upstreamCap =
downstreamNodeIndex == upstreamNodeIndex ? upstreamPosition.nodePosition : downstreamNode.beginningPosition;

// If and only if the given position comes before the downstream position,
// and after the upstream cap, then the position is within the selection.
return downstreamNode.selectDownstreamPosition(upstreamCap, position.nodePosition) ==
downstreamNode.selectUpstreamPosition(position.nodePosition, downstreamPosition.nodePosition);
}

// If we got here, then the position is either before the upstream
// selection boundary, or after the downstream selection boundary.
// Either way, the position is not in the selection.
return false;
final selectionAffinity = getAffinityForSelection(selection);
final upstreamPosition = selectionAffinity == TextAffinity.downstream ? selection.base : selection.extent;
final downstreamPosition = selectionAffinity == TextAffinity.downstream ? selection.extent : selection.base;

// The selection contains the position if the ordering is as follows:
//
// selection start <= position <= selection end
//
// Another way of stating this relationship is that there's a downstream
// affinity from selection start to the position, and from the position to
// the selection end.
return getAffinityBetween(base: upstreamPosition, extent: position) == TextAffinity.downstream &&
getAffinityBetween(base: position, extent: downstreamPosition) == TextAffinity.downstream;
}
}
16 changes: 13 additions & 3 deletions super_editor/lib/src/core/editor.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1278,7 +1278,7 @@ class MutableDocument with Iterable<DocumentNode> implements Document, Editable
}
}

/// Returns [true] if the content of the [other] [Document] is equivalent
/// Returns `true` if the content of the [other] [Document] is equivalent
/// to the content of this [Document].
///
/// Content equivalency compares types of content nodes, and the content
Expand All @@ -1289,11 +1289,21 @@ class MutableDocument with Iterable<DocumentNode> implements Document, Editable
if (_nodes.length != other.nodeCount) {
return false;
}
if (isEmpty) {
// Both documents are empty, and therefore are equivalent.
return true;
}

DocumentNode? thisDocNode = first;
DocumentNode? otherDocNode = other.first;

for (int i = 0; i < _nodes.length; ++i) {
if (!_nodes[i].hasEquivalentContent(other.getNodeAt(i)!)) {
while (thisDocNode != null && otherDocNode != null) {
if (!thisDocNode.hasEquivalentContent(otherDocNode)) {
return false;
}

thisDocNode = getNodeAfter(thisDocNode);
otherDocNode = other.getNodeAfter(otherDocNode);
}

return true;
Expand Down
4 changes: 2 additions & 2 deletions super_editor/lib/src/core/styles.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ class _FirstBlockMatcher implements _BlockMatcher {

@override
bool matches(Document document, DocumentNode node) {
return document.getNodeIndexById(node.id) == 0;
return document.getNodeById(node.id) == document.firstOrNull;
}
}

Expand All @@ -216,7 +216,7 @@ class _LastBlockMatcher implements _BlockMatcher {

@override
bool matches(Document document, DocumentNode node) {
return document.getNodeIndexById(node.id) == document.nodeCount - 1;
return document.getNodeById(node.id) == document.lastOrNull;
}
}

Expand Down
24 changes: 13 additions & 11 deletions super_editor/lib/src/default_editor/common_editor_operations.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1404,31 +1404,33 @@ class CommonEditorOperations {
if (baseNode == null) {
throw Exception('Failed to _getDocumentPositionAfterDeletion because the base node no longer exists.');
}
final baseNodeIndex = document.getNodeIndexById(baseNode.id);

final extentPosition = selection.extent;
final extentNode = document.getNode(extentPosition);
if (extentNode == null) {
throw Exception('Failed to _getDocumentPositionAfterDeletion because the extent node no longer exists.');
}
final extentNodeIndex = document.getNodeIndexById(extentNode.id);

final topNodeIndex = min(baseNodeIndex, extentNodeIndex);
final topNode = document.getNodeAt(topNodeIndex)!;
final topNodePosition = baseNodeIndex < extentNodeIndex ? basePosition.nodePosition : extentPosition.nodePosition;
final selectionAffinity = document.getAffinityForSelection(selection);
final topPosition = selectionAffinity == TextAffinity.downstream //
? selection.base
: selection.extent;
final topNodePosition = topPosition.nodePosition;
final topNode = document.getNodeById(topPosition.nodeId)!;

final bottomNodeIndex = max(baseNodeIndex, extentNodeIndex);
final bottomNode = document.getNodeAt(bottomNodeIndex)!;
final bottomNodePosition =
baseNodeIndex < extentNodeIndex ? extentPosition.nodePosition : basePosition.nodePosition;
final bottomPosition = selectionAffinity == TextAffinity.downstream //
? selection.extent
: selection.base;
final bottomNodePosition = bottomPosition.nodePosition;
final bottomNode = document.getNodeById(bottomPosition.nodeId)!;

final normalizedRange = selection.normalize(document);
final nodes = document.getNodesInside(normalizedRange.start, normalizedRange.end);
final firstDeletableNodeId = nodes.firstWhereOrNull((node) => node.isDeletable)?.id;

DocumentPosition newSelectionPosition;

if (baseNodeIndex != extentNodeIndex) {
if (topPosition.nodeId != bottomPosition.nodeId) {
if (topNodePosition == topNode.beginningPosition && bottomNodePosition == bottomNode.endPosition) {
// All deletable nodes in the selection will be deleted. Assume that one of the
// nodes will be retained and converted into a paragraph, if it's not
Expand Down Expand Up @@ -1469,7 +1471,7 @@ class CommonEditorOperations {
// those nodes will remain.

// The caret should end up at the base position
newSelectionPosition = baseNodeIndex <= extentNodeIndex ? selection.base : selection.extent;
newSelectionPosition = selectionAffinity == TextAffinity.downstream ? selection.base : selection.extent;
}
} else {
// Selection is within a single node.
Expand Down
5 changes: 1 addition & 4 deletions super_editor/lib/src/default_editor/document_ime/document_delta_editing.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -388,17 +388,14 @@ class TextDeltasDocumentEditor {
editorImeLog.fine("Doc selection to delete: $docSelectionToDelete");

if (docSelectionToDelete == null) {
final selectedNodeIndex = document.getNodeIndexById(
selection.value!.extent.nodeId,
);
// The user is trying to delete upstream at the start of a node.
// This action requires intervention because the IME doesn't know
// that there's more content before this node. Instruct the editor
// to run a delete action upstream, which will take the desired
// "backspace" behavior at the start of this node.
editor.execute([
DeleteUpstreamAtBeginningOfNodeRequest(
document.getNodeAt(selectedNodeIndex)!,
document.getNodeById(selection.value!.extent.nodeId)!,
),
]);
return;
Expand Down
7 changes: 3 additions & 4 deletions super_editor/lib/src/default_editor/layout_single_column/_presenter.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,17 +164,16 @@ class SingleColumnLayoutPresenter {
// The document changed. All view models were invalidated. Create a
// new base document view model.
final viewModels = <SingleColumnLayoutComponentViewModel>[];
for (int i = 0; i < _document.nodeCount; i += 1) {
for (final node in _document) {
SingleColumnLayoutComponentViewModel? viewModel;
for (final builder in _componentBuilders) {
viewModel = builder.createViewModel(_document, _document.getNodeAt(i)!);
viewModel = builder.createViewModel(_document, node);
if (viewModel != null) {
break;
}
}
if (viewModel == null) {
throw Exception(
"Couldn't find styler to create component for document node: ${_document.getNodeAt(i)!.runtimeType}");
throw Exception("Couldn't find styler to create component for document node: ${node.runtimeType}");
}
viewModels.add(viewModel);
}
Expand Down
39 changes: 25 additions & 14 deletions super_editor/lib/src/default_editor/multi_node_editing.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -923,28 +923,39 @@ class DeleteContentCommand extends EditCommand {
required DocumentNode startNode,
required DocumentNode endNode,
}) {
if (startNode.id == endNode.id) {
// The start and end nodes are the same. Nothing to delete.
return [];
}

// Delete all nodes between the first node and the last node.
final startIndex = document.getNodeIndexById(startNode.id);
final endIndex = document.getNodeIndexById(endNode.id);
if (document.getAffinityBetweenNodes(startNode, endNode) != TextAffinity.downstream) {
throw Exception(
"Tried to delete the nodes between a start and end node, but the start node doesn't appear before the end node. Start: ${startNode.id}, End: ${endNode.id}.",
);
}

_log.log('_deleteNodesBetweenFirstAndLast', ' - start node index: $startIndex');
_log.log('_deleteNodesBetweenFirstAndLast', ' - end node index: $endIndex');
_log.log('_deleteNodesBetweenFirstAndLast', ' - start node: ${startNode.id}');
_log.log('_deleteNodesBetweenFirstAndLast', ' - end node: ${endNode.id}');
_log.log('_deleteNodesBetweenFirstAndLast', ' - initially ${document.nodeCount} nodes');

// Remove nodes from last to first so that indices don't get
// screwed up during removal.
final changes = <EditEvent>[];
for (int i = endIndex - 1; i > startIndex; --i) {
_log.log('_deleteNodesBetweenFirstAndLast', ' - deleting node $i: ${document.getNodeAt(i)?.id}');
final removedNode = document.getNodeAt(i)!;
if (!removedNode.isDeletable) {
// This node is not deletable. Ignore it.
continue;
var nodeToDelete = document.getNodeAfter(startNode);
while (nodeToDelete != null && nodeToDelete != endNode) {
_log.log('_deleteNodesBetweenFirstAndLast', ' - deleting node: ${nodeToDelete.id}');
final nextNode = document.getNodeAfter(nodeToDelete);
if (nodeToDelete.isDeletable) {
// This node is deletable, so delete it.
changes.add(DocumentEdit(
NodeRemovedEvent(nodeToDelete.id, nodeToDelete),
));
document.deleteNode(nodeToDelete);
}
changes.add(DocumentEdit(
NodeRemovedEvent(removedNode.id, removedNode),
));
document.deleteNodeAt(i);

// Move to the next node.
nodeToDelete = nextNode;
}
return changes;
}
Expand Down
Loading

0 comments on commit bda71a4

Please sign in to comment.