diff --git a/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager+Edits.swift b/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager+Edits.swift index 8219bf162..1c3d97240 100644 --- a/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager+Edits.swift +++ b/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager+Edits.swift @@ -10,17 +10,50 @@ import AppKit // MARK: - Edits extension TextLayoutManager: NSTextStorageDelegate { - /// Notifies the layout manager of an edit. + /// Receives edit notifications from the text storage and updates internal data structures to stay in sync with + /// text content. /// - /// Used by the `TextView` to tell the layout manager about any edits that will happen. - /// Use this to keep the layout manager's line storage in sync with the text storage. + /// If the changes are only attribute changes, this method invalidates layout for the edited range and returns. /// - /// - Parameters: - /// - range: The range of the edit. - /// - string: The string to replace in the given range. - public func willReplaceCharactersInRange(range: NSRange, with string: String) { + /// Otherwise, any lines that were removed or replaced by the edit are first removed from the text line layout + /// storage. Then, any new lines are inserted into the same storage. + /// + /// For instance, if inserting a newline this method will: + /// - Remove no lines (none were replaced) + /// - Update the current line's range to contain the newline character. + /// - Insert a new line after the current line. + /// + /// If a selection containing a newline is deleted and replaced with two more newlines this method will: + /// - Delete the original line. + /// - Insert two lines. + /// + /// - Note: This method *does not* cause a layout calculation. If a method is finding `NaN` values for line + /// fragments, ensure `layout` or `ensureLayoutUntil` are called on the subject ranges. + public func textStorage( + _ textStorage: NSTextStorage, + didProcessEditing editedMask: NSTextStorageEditActions, + range editedRange: NSRange, + changeInLength delta: Int + ) { + guard editedMask.contains(.editedCharacters) else { + if editedMask.contains(.editedAttributes) && delta == 0 { + invalidateLayoutForRange(editedRange) + } + return + } + + let insertedStringRange = NSRange(location: editedRange.location, length: editedRange.length - delta) + removeLayoutLinesIn(range: insertedStringRange) + insertNewLines(for: editedRange) + invalidateLayoutForRange(editedRange) + } + + /// Removes all lines in the range, as if they were deleted. This is a setup for inserting the lines back in on an + /// edit. + /// - Parameter range: The range that was deleted. + private func removeLayoutLinesIn(range: NSRange) { // Loop through each line being replaced in reverse, updating and removing where necessary. - for linePosition in lineStorage.linesInRange(range).reversed() { + for linePosition in lineStorage.linesInRange(range).reversed() { // Two cases: Updated line, deleted line entirely guard let intersection = linePosition.range.intersection(range), !intersection.isEmpty else { continue } if intersection == linePosition.range && linePosition.range.max != lineStorage.length { @@ -38,25 +71,24 @@ extension TextLayoutManager: NSTextStorageDelegate { lineStorage.update(atIndex: linePosition.range.location, delta: -intersection.length, deltaHeight: 0) } } + } + /// Inserts any newly inserted lines into the line layout storage. Exits early if the range is empty. + /// - Parameter range: The range of the string that was inserted into the text storage. + private func insertNewLines(for range: NSRange) { + guard !range.isEmpty, let string = textStorage?.substring(from: range) as? NSString else { return } // Loop through each line being inserted, inserting & splitting where necessary - if !string.isEmpty { - var index = 0 - while let nextLine = (string as NSString).getNextLine(startingAt: index) { - let lineRange = NSRange(start: index, end: nextLine.max) - applyLineInsert((string as NSString).substring(with: lineRange) as NSString, at: range.location + index) - index = nextLine.max - } + var index = 0 + while let nextLine = string.getNextLine(startingAt: index) { + let lineRange = NSRange(start: index, end: nextLine.max) + applyLineInsert(string.substring(with: lineRange) as NSString, at: range.location + index) + index = nextLine.max + } - if index < (string as NSString).length { - // Get the last line. - applyLineInsert( - (string as NSString).substring(from: index) as NSString, - at: range.location + index - ) - } + if index < string.length { + // Get the last line. + applyLineInsert(string.substring(from: index) as NSString, at: range.location + index) } - setNeedsLayout() } /// Applies a line insert to the internal line storage tree. @@ -65,7 +97,7 @@ extension TextLayoutManager: NSTextStorageDelegate { /// - location: The location the string is being inserted into. private func applyLineInsert(_ insertedString: NSString, at location: Int) { if LineEnding(line: insertedString as String) != nil { - if location == textStorage?.length ?? 0 { + if location == lineStorage.length { // Insert a new line at the end of the document, need to insert a new line 'cause there's nothing to // split. Also, append the new text to the last line. lineStorage.update(atIndex: location, delta: insertedString.length, deltaHeight: 0.0) @@ -96,18 +128,4 @@ extension TextLayoutManager: NSTextStorageDelegate { lineStorage.update(atIndex: location, delta: insertedString.length, deltaHeight: 0.0) } } - - /// This method is to simplify keeping the layout manager in sync with attribute changes in the storage object. - /// This does not handle cases where characters have been inserted or removed from the storage. - /// For that, see the `willPerformEdit` method. - public func textStorage( - _ textStorage: NSTextStorage, - didProcessEditing editedMask: NSTextStorageEditActions, - range editedRange: NSRange, - changeInLength delta: Int - ) { - if editedMask.contains(.editedAttributes) && delta == 0 { - invalidateLayoutForRange(editedRange) - } - } } diff --git a/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager+Public.swift b/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager+Public.swift index f53e14f85..6fa1d9c9e 100644 --- a/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager+Public.swift +++ b/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager+Public.swift @@ -121,7 +121,7 @@ extension TextLayoutManager { return nil } if linePosition.data.lineFragments.isEmpty { - let newHeight = ensureLayoutFor(position: linePosition) + let newHeight = preparePositionForDisplay(linePosition) if linePosition.height != newHeight { delegate?.layoutManagerHeightDidUpdate(newHeight: lineStorage.height) } @@ -165,7 +165,8 @@ extension TextLayoutManager { /// - line: The line to calculate rects for. /// - Returns: Multiple bounding rects. Will return one rect for each line fragment that overlaps the given range. public func rectsFor(range: NSRange) -> [CGRect] { - lineStorage.linesInRange(range).flatMap { self.rectsFor(range: range, in: $0) } + ensureLayoutUntil(range.max) + return lineStorage.linesInRange(range).flatMap { self.rectsFor(range: range, in: $0) } } /// Calculates all text bounding rects that intersect with a given range, with a given line position. @@ -190,6 +191,7 @@ extension TextLayoutManager { for fragmentPosition in line.data.lineFragments.linesInRange(relativeRange) { guard let intersectingRange = fragmentPosition.range.intersection(relativeRange) else { continue } let fragmentRect = fragmentPosition.data.rectFor(range: intersectingRange) + guard fragmentRect.width > 0 else { continue } rects.append( CGRect( x: fragmentRect.minX + edgeInsets.left, @@ -239,6 +241,8 @@ extension TextLayoutManager { // Combine the points in clockwise order let points = leftSidePoints + rightSidePoints + guard points.allSatisfy({ $0.x.isFinite && $0.y.isFinite }) else { return nil } + // Close the path if let firstPoint = points.first { return NSBezierPath.smoothPath(points + [firstPoint], radius: cornerRadius) @@ -286,7 +290,7 @@ extension TextLayoutManager { for linePosition in lineStorage.linesInRange( NSRange(start: startingLinePosition.range.location, end: linePosition.range.max) ) { - let height = ensureLayoutFor(position: linePosition) + let height = preparePositionForDisplay(linePosition) if height != linePosition.height { lineStorage.update( atIndex: linePosition.range.location, diff --git a/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager+Transaction.swift b/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager+Transaction.swift deleted file mode 100644 index c160bfd57..000000000 --- a/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager+Transaction.swift +++ /dev/null @@ -1,38 +0,0 @@ -// -// TextLayoutManager+Transaction.swift -// CodeEditTextView -// -// Created by Khan Winter on 2/24/24. -// - -import Foundation - -extension TextLayoutManager { - /// Begins a transaction, preventing the layout manager from performing layout until the `endTransaction` is called. - /// Useful for grouping attribute modifications into one layout pass rather than laying out every update. - /// - /// You can nest transaction start/end calls, the layout manager will not cause layout until the last transaction - /// group is ended. - /// - /// Ensure there is a balanced number of begin/end calls. If there is a missing endTranscaction call, the layout - /// manager will never lay out text. If there is a end call without matching a start call an assertionFailure - /// will occur. - public func beginTransaction() { - transactionCounter += 1 - } - - /// Ends a transaction. When called, the layout manager will layout any necessary lines. - public func endTransaction(forceLayout: Bool = false) { - transactionCounter -= 1 - if transactionCounter == 0 { - if forceLayout { - setNeedsLayout() - } - layoutLines() - } else if transactionCounter < 0 { - assertionFailure( - "TextLayoutManager.endTransaction called without a matching TextLayoutManager.beginTransaction call" - ) - } - } -} diff --git a/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager+ensureLayout.swift b/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager+ensureLayout.swift index e0e5fa07d..e2cf08d15 100644 --- a/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager+ensureLayout.swift +++ b/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager+ensureLayout.swift @@ -8,9 +8,10 @@ import Foundation extension TextLayoutManager { - /// Forces layout calculation for all lines up to and including the given offset. - /// - Parameter offset: The offset to ensure layout until. - package func ensureLayoutFor(position: TextLineStorage.TextLinePosition) -> CGFloat { + /// Invalidates and prepares a line position for display. + /// - Parameter position: The line position to prepare. + /// - Returns: The height of the newly laid out line and all it's fragments. + package func preparePositionForDisplay(_ position: TextLineStorage.TextLinePosition) -> CGFloat { guard let textStorage else { return 0 } let displayData = TextLine.DisplayData( maxWidth: maxLineLayoutWidth, diff --git a/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager.swift b/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager.swift index 3e042225c..880c28748 100644 --- a/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager.swift +++ b/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager.swift @@ -199,8 +199,6 @@ public class TextLayoutManager: NSObject { /// ``TextLayoutManager/estimateLineHeight()`` is called. private var _estimateLineHeight: CGFloat? - // MARK: - Layout - /// Asserts that the caller is not in an active layout pass. /// See docs on ``isInLayout`` for more details. private func assertNotInLayout() { @@ -209,6 +207,8 @@ public class TextLayoutManager: NSObject { #endif } + // MARK: - Layout + /// Lays out all visible lines func layoutLines(in rect: NSRect? = nil) { // swiftlint:disable:this function_body_length assertNotInLayout() @@ -217,9 +217,15 @@ public class TextLayoutManager: NSObject { let textStorage else { return } + + // The macOS may call `layout` on the textView while we're laying out fragment views. This ensures the view + // tree modifications caused by this method are atomic, so macOS won't call `layout` while we're already doing + // that + CATransaction.begin() #if DEBUG isInLayout = true #endif + let minY = max(visibleRect.minY - verticalLayoutPadding, 0) let maxY = max(visibleRect.maxY + verticalLayoutPadding, 0) let originalHeight = lineStorage.height @@ -265,16 +271,17 @@ public class TextLayoutManager: NSObject { newVisibleLines.insert(linePosition.data.id) } + #if DEBUG + isInLayout = false + #endif + CATransaction.commit() + // Enqueue any lines not used in this layout pass. viewReuseQueue.enqueueViews(notInSet: usedFragmentIDs) // Update the visible lines with the new set. visibleLineIds = newVisibleLines - #if DEBUG - isInLayout = false - #endif - // These are fine to update outside of `isInLayout` as our internal data structures are finalized at this point // so laying out again won't break our line storage or visible line. diff --git a/Sources/CodeEditTextView/TextSelectionManager/TextSelectionManager.swift b/Sources/CodeEditTextView/TextSelectionManager/TextSelectionManager.swift index 453fcac87..a56c73a68 100644 --- a/Sources/CodeEditTextView/TextSelectionManager/TextSelectionManager.swift +++ b/Sources/CodeEditTextView/TextSelectionManager/TextSelectionManager.swift @@ -86,6 +86,8 @@ public class TextSelectionManager: NSObject { /// Set the selected ranges to new ranges. Overrides any existing selections. /// - Parameter range: The selected ranges to set. public func setSelectedRanges(_ ranges: [NSRange]) { + let oldRanges = textSelections.map(\.range) + textSelections.forEach { $0.view?.removeFromSuperview() } // Remove duplicates, invalid ranges, update suggested X position. textSelections = Set(ranges) @@ -99,8 +101,11 @@ public class TextSelectionManager: NSObject { return selection } updateSelectionViews() - NotificationCenter.default.post(Notification(name: Self.selectionChangedNotification, object: self)) delegate?.setNeedsDisplay() + + if oldRanges != textSelections.map(\.range) { + NotificationCenter.default.post(Notification(name: Self.selectionChangedNotification, object: self)) + } } /// Append a new selected range to the existing ones. diff --git a/Sources/CodeEditTextView/TextView/TextView+ReplaceCharacters.swift b/Sources/CodeEditTextView/TextView/TextView+ReplaceCharacters.swift index 06949b21a..951e9977e 100644 --- a/Sources/CodeEditTextView/TextView/TextView+ReplaceCharacters.swift +++ b/Sources/CodeEditTextView/TextView/TextView+ReplaceCharacters.swift @@ -16,7 +16,6 @@ extension TextView { public func replaceCharacters(in ranges: [NSRange], with string: String) { guard isEditable else { return } NotificationCenter.default.post(name: Self.textWillChangeNotification, object: self) - layoutManager.beginTransaction() textStorage.beginEditing() // Can't insert an empty string into an empty range. One must be not empty @@ -25,7 +24,6 @@ extension TextView { (delegate?.textView(self, shouldReplaceContentsIn: range, with: string) ?? true) { delegate?.textView(self, willReplaceContentsIn: range, with: string) - layoutManager.willReplaceCharactersInRange(range: range, with: string) _undoManager?.registerMutation( TextMutation(string: string as String, range: range, limit: textStorage.length) ) @@ -39,7 +37,6 @@ extension TextView { } textStorage.endEditing() - layoutManager.endTransaction() selectionManager.notifyAfterEdit() NotificationCenter.default.post(name: Self.textDidChangeNotification, object: self) diff --git a/Sources/CodeEditTextView/TextView/TextView.swift b/Sources/CodeEditTextView/TextView/TextView.swift index 85b2e11b0..9b0337f9e 100644 --- a/Sources/CodeEditTextView/TextView/TextView.swift +++ b/Sources/CodeEditTextView/TextView/TextView.swift @@ -58,7 +58,6 @@ public class TextView: NSView, NSTextContent { textStorage.string } set { - layoutManager.willReplaceCharactersInRange(range: documentRange, with: newValue) textStorage.setAttributedString(NSAttributedString(string: newValue, attributes: typingAttributes)) } } @@ -339,6 +338,7 @@ public class TextView: NSView, NSTextContent { layoutManager = setUpLayoutManager(lineHeightMultiplier: lineHeightMultiplier, wrapLines: wrapLines) storageDelegate.addDelegate(layoutManager) + selectionManager = setUpSelectionManager() selectionManager.useSystemCursor = useSystemCursor diff --git a/Sources/CodeEditTextView/Utils/CEUndoManager.swift b/Sources/CodeEditTextView/Utils/CEUndoManager.swift index c716d49b4..0aaaf842f 100644 --- a/Sources/CodeEditTextView/Utils/CEUndoManager.swift +++ b/Sources/CodeEditTextView/Utils/CEUndoManager.swift @@ -173,19 +173,13 @@ public class CEUndoManager { /// Groups all incoming mutations. public func beginUndoGrouping() { - guard !isGrouping else { - assertionFailure("UndoManager already in a group. Call `beginUndoGrouping` before this can be called.") - return - } + guard !isGrouping else { return } isGrouping = true } /// Stops grouping all incoming mutations. public func endUndoGrouping() { - guard isGrouping else { - assertionFailure("UndoManager not in a group. Call `endUndoGrouping` before this can be called.") - return - } + guard isGrouping else { return } isGrouping = false } diff --git a/Tests/CodeEditTextViewTests/EmphasisManagerTests.swift b/Tests/CodeEditTextViewTests/EmphasisManagerTests.swift index a921f1051..4cdf8468b 100644 --- a/Tests/CodeEditTextViewTests/EmphasisManagerTests.swift +++ b/Tests/CodeEditTextViewTests/EmphasisManagerTests.swift @@ -4,7 +4,8 @@ import Foundation @Suite() struct EmphasisManagerTests { - @Test() @MainActor + @Test() + @MainActor func testFlashEmphasisLayersNotLeaked() { // Ensure layers are not leaked when switching from flash emphasis to any other emphasis type. let textView = TextView(string: "Lorem Ipsum") diff --git a/Tests/CodeEditTextViewTests/TextLayoutManagerTests.swift b/Tests/CodeEditTextViewTests/TextLayoutManagerTests.swift new file mode 100644 index 000000000..1dcc9a7dd --- /dev/null +++ b/Tests/CodeEditTextViewTests/TextLayoutManagerTests.swift @@ -0,0 +1,133 @@ +import Testing +import AppKit +@testable import CodeEditTextView + +extension TextLineStorage { + /// Validate that the internal tree is intact and correct. + /// + /// Ensures that: + /// - All lines can be queried by their index starting from `0`. + /// - All lines can be found by iterating `y` positions. + func validateInternalState() { + func validateLines(_ lines: [TextLineStorage.TextLinePosition]) { + var _lastLine: TextLineStorage.TextLinePosition? + for line in lines { + guard let lastLine = _lastLine else { + #expect(line.index == 0) + _lastLine = line + return + } + + #expect(line.index == lastLine.index + 1) + #expect(line.yPos >= lastLine.yPos + lastLine.height) + #expect(line.range.location == lastLine.range.max + 1) + _lastLine = line + } + } + + let linesUsingIndex = (0.. Bool)? + + func textView(_ textView: TextView, shouldReplaceContentsIn range: NSRange, with string: String) -> Bool { + shouldReplaceContents?(textView, range, string) ?? true + } + } + + let textView: TextView + let delegate: MockDelegate + + init() { + textView = TextView(string: "Lorem Ipsum") + delegate = MockDelegate() + textView.delegate = delegate + } + + @Test + func delegateChangesText() { + var hasReplaced = false + delegate.shouldReplaceContents = { textView, _, _ -> Bool in + if !hasReplaced { + hasReplaced.toggle() + textView.replaceCharacters(in: NSRange(location: 0, length: 0), with: " World ") + } + + return true + } + + textView.replaceCharacters(in: NSRange(location: 0, length: 0), with: "Hello") + + #expect(textView.string == "Hello World Lorem Ipsum") + // available in test module + textView.layoutManager.lineStorage.validateInternalState() + } +}