From fae877c41495aac351a572c69c9a2fef63ea7309 Mon Sep 17 00:00:00 2001 From: David Albert Date: Mon, 18 Mar 2024 13:14:01 -0400 Subject: [PATCH 01/73] Get rid of deferred layout --- Watt.xcodeproj/project.pbxproj | 4 + Watt/ClipView.swift | 8 +- Watt/LayoutManager/LayoutManager.swift | 6 +- Watt/LineNumberView/LineNumberView.swift | 13 +++ Watt/TextView/TextView+Input.swift | 6 +- Watt/TextView/TextView+Layout.swift | 119 ++++------------------- Watt/TextView/TextView+Scrolling.swift | 80 +++++++++++++++ Watt/TextView/TextView.swift | 36 +++---- 8 files changed, 143 insertions(+), 129 deletions(-) create mode 100644 Watt/TextView/TextView+Scrolling.swift diff --git a/Watt.xcodeproj/project.pbxproj b/Watt.xcodeproj/project.pbxproj index fcb35d4b..cbf1980b 100644 --- a/Watt.xcodeproj/project.pbxproj +++ b/Watt.xcodeproj/project.pbxproj @@ -11,6 +11,7 @@ 6315D3002A9B769100B8A077 /* AttributedRope.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6315D2FF2A9B769100B8A077 /* AttributedRope.swift */; }; 6315D3022A9BD00F00B8A077 /* AttributedRopeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6315D3012A9BD00F00B8A077 /* AttributedRopeTests.swift */; }; 631FD2652A696D27005FA854 /* Heights.swift in Sources */ = {isa = PBXBuildFile; fileRef = 631FD2642A696D27005FA854 /* Heights.swift */; }; + 63206D2A2BA74AF600CAFBC4 /* TextView+Scrolling.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63206D292BA74AF600CAFBC4 /* TextView+Scrolling.swift */; }; 63267C292B97BF0B00F6E968 /* BTreeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63267C282B97BF0B00F6E968 /* BTreeTests.swift */; }; 63286AE22A13DF5D00839B25 /* ClipView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63286AE12A13DF5D00839B25 /* ClipView.swift */; }; 632A73722B87C69F009F38C3 /* Range+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 632A73712B87C69F009F38C3 /* Range+Extensions.swift */; }; @@ -130,6 +131,7 @@ 6315D2FF2A9B769100B8A077 /* AttributedRope.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AttributedRope.swift; sourceTree = ""; }; 6315D3012A9BD00F00B8A077 /* AttributedRopeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = AttributedRopeTests.swift; path = WattTests/AttributedRopeTests.swift; sourceTree = SOURCE_ROOT; }; 631FD2642A696D27005FA854 /* Heights.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Heights.swift; sourceTree = ""; }; + 63206D292BA74AF600CAFBC4 /* TextView+Scrolling.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TextView+Scrolling.swift"; sourceTree = ""; }; 63267C282B97BF0B00F6E968 /* BTreeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTreeTests.swift; sourceTree = ""; }; 63286AE12A13DF5D00839B25 /* ClipView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ClipView.swift; sourceTree = ""; }; 632A73712B87C69F009F38C3 /* Range+Extensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Range+Extensions.swift"; sourceTree = ""; }; @@ -341,6 +343,7 @@ 63A618752A0FD61B00E70B0E /* TextView+LineNumbers.swift */, 637867722A1BD1E10039960E /* TextView+Mouse.swift */, 6396DD2D2B45A5DE000F85D1 /* TextView+Pasteboard.swift */, + 63206D292BA74AF600CAFBC4 /* TextView+Scrolling.swift */, 637867702A17F7460039960E /* TextView+Selection.swift */, 635B36B02AAFCD95002FAE70 /* Theme.swift */, 635F815C2AA9E51900553E23 /* TreeSitter.swift */, @@ -646,6 +649,7 @@ 6315D3002A9B769100B8A077 /* AttributedRope.swift in Sources */, 63D280892B5B275900CDCF04 /* DirentTextField.swift in Sources */, 63286AE22A13DF5D00839B25 /* ClipView.swift in Sources */, + 63206D2A2BA74AF600CAFBC4 /* TextView+Scrolling.swift in Sources */, 63A95D842B5462D60032E8B9 /* FSEvents.swift in Sources */, 63DD20D92B71A53200E55747 /* Window.swift in Sources */, 637BC1422A156B620089A34D /* CGRect+Extensions.swift in Sources */, diff --git a/Watt/ClipView.swift b/Watt/ClipView.swift index 013eb376..b5c13431 100644 --- a/Watt/ClipView.swift +++ b/Watt/ClipView.swift @@ -8,17 +8,19 @@ import Cocoa protocol ClipViewDelegate: AnyObject { - func viewDidMoveToClipView() + func viewDidMoveToClipView(_ clipView: ClipView) } extension ClipViewDelegate { - func viewDidMoveToClipView() {} + func viewDidMoveToClipView(_ clipView: ClipView) {} } class ClipView: NSClipView { + weak var delegate: ClipViewDelegate? + override var documentView: NSView? { didSet { - (documentView as? ClipViewDelegate)?.viewDidMoveToClipView() + delegate?.viewDidMoveToClipView(self) } } } diff --git a/Watt/LayoutManager/LayoutManager.swift b/Watt/LayoutManager/LayoutManager.swift index c00e60d0..f92e85d8 100644 --- a/Watt/LayoutManager/LayoutManager.swift +++ b/Watt/LayoutManager/LayoutManager.swift @@ -10,10 +10,8 @@ import CoreText import StandardKeyBindingResponder protocol LayoutManagerDelegate: AnyObject { - // Text container coordinates. Includes overdraw. + // Text container coordinates. func viewportBounds(for layoutManager: LayoutManager) -> CGRect - // Text container coordinates. No overdraw. - func visibleRect(for layoutManager: LayoutManager) -> CGRect func didInvalidateLayout(for layoutManager: LayoutManager) func defaultAttributes(for layoutManager: LayoutManager) -> AttributedRope.Attributes @@ -853,7 +851,7 @@ extension LayoutManager: TextLayoutDataSource { } var viewportSize: CGSize { - delegate?.visibleRect(for: self).size ?? .zero + delegate?.viewportBounds(for: self).size ?? .zero } // Enumerating over the first line fragment of each string: diff --git a/Watt/LineNumberView/LineNumberView.swift b/Watt/LineNumberView/LineNumberView.swift index e393926c..9a7347c9 100644 --- a/Watt/LineNumberView/LineNumberView.swift +++ b/Watt/LineNumberView/LineNumberView.swift @@ -109,6 +109,19 @@ class LineNumberView: NSView, CALayerDelegate, NSViewLayerContentScaleDelegate { } } + override func viewDidMoveToWindow() { + // TextView's .viewDidMoveToWindow gets called before LineNumberView's, so we've already done our first + // layout pass, including creating the first set of LineNumberLayers with a contentsScale of 1.0. + // + // At this point, draw(in:) hasn't been called for any layers, so this doesn't trigger any + // unnecessary drawing. + if let window { + for layer in lineNumberLayers { + layer.contentsScale = window.backingScaleFactor + } + } + } + func layer(_ layer: CALayer, shouldInheritContentsScale newScale: CGFloat, from window: NSWindow) -> Bool { true } diff --git a/Watt/TextView/TextView+Input.swift b/Watt/TextView/TextView+Input.swift index 6cdce528..f0d28549 100644 --- a/Watt/TextView/TextView+Input.swift +++ b/Watt/TextView/TextView+Input.swift @@ -86,9 +86,9 @@ extension TextView: NSTextInputClient { // TODO: if we're the only one who calls unmarkText(), we can remove // these layout calls, because we already do layout in didInvalidateLayout(for layoutManager: LayoutManager) - textLayer.setNeedsLayout() - selectionLayer.setNeedsLayout() - insertionPointLayer.setNeedsLayout() + layoutTextLayer() + layoutSelectionLayer() + layoutInsertionPointLayer() } func selectedRange() -> NSRange { diff --git a/Watt/TextView/TextView+Layout.swift b/Watt/TextView/TextView+Layout.swift index 0c580ae5..5744c5d3 100644 --- a/Watt/TextView/TextView+Layout.swift +++ b/Watt/TextView/TextView+Layout.swift @@ -34,19 +34,6 @@ extension TextView: CALayerDelegate, NSViewLayerContentScaleDelegate { } } - func layoutSublayers(of layer: CALayer) { - switch layer { - case textLayer: - layoutTextLayer() - case selectionLayer: - layoutSelectionLayer() - case insertionPointLayer: - layoutInsertionPointLayer() - default: - break - } - } - func layer(_ layer: CALayer, shouldInheritContentsScale newScale: CGFloat, from window: NSWindow) -> Bool { true } @@ -63,22 +50,9 @@ extension TextView: CALayerDelegate, NSViewLayerContentScaleDelegate { if layoutManager.textContainer.size.width != width { layoutManager.textContainer.size = CGSize(width: width, height: .greatestFiniteMagnitude) - // This isn't needed when this function is called from - // setFrameSize, but it is needed when the line number - // view is added, removed, or resized due to the number - // of lines in the document changing. - // - // In the former case, AppKit will resize the view's - // layer, which will trigger the resizing of these layers - // due to their autoresizing masks. - // - // In the latter case, because the line number view floats - // above the text view, the text view's frame size doesn't - // change when the line number view's size changes, but we - // do need to re-layout our text. - selectionLayer.setNeedsLayout() - textLayer.setNeedsLayout() - insertionPointLayer.setNeedsLayout() + layoutTextLayer() + layoutSelectionLayer() + layoutInsertionPointLayer() } } @@ -145,88 +119,19 @@ extension TextView: CALayerDelegate, NSViewLayerContentScaleDelegate { return CGRect(x: x, y: y, width: maxX - x, height: maxY - y) } -} - -// MARK: - Scrolling - -extension TextView { - override func prepareContent(in rect: NSRect) { - super.prepareContent(in: rect) - - selectionLayer.setNeedsLayout() - textLayer.setNeedsLayout() - insertionPointLayer.setNeedsLayout() - } - - var scrollView: NSScrollView? { - if let enclosingScrollView, enclosingScrollView.documentView == self { - return enclosingScrollView - } - - return nil - } - - var scrollOffset: CGPoint { - guard let scrollView else { - return .zero - } - - return scrollView.contentView.bounds.origin - } - - var textContainerScrollOffset: CGPoint { - convertToTextContainer(scrollOffset) - } var textContainerVisibleRect: CGRect { convertToTextContainer(clampToTextContainer(visibleRect)) } - - func scrollIndexToVisible(_ index: Buffer.Index) { - guard let rect = layoutManager.caretRect(for: index, affinity: index == buffer.endIndex ? .upstream : .downstream) else { - return - } - - let viewRect = convertFromTextContainer(rect) - scrollToVisible(viewRect) - } - - func scrollIndexToCenter(_ index: Buffer.Index) { - guard let rect = layoutManager.caretRect(for: index, affinity: index == buffer.endIndex ? .upstream : .downstream) else { - return - } - - let viewRect = convertFromTextContainer(rect) - scrollToCenter(viewRect) - } - - func scrollToCenter(_ rect: NSRect) { - let dx = rect.midX - visibleRect.midX - let dy = rect.midY - visibleRect.midY - scroll(CGPoint(x: scrollOffset.x + dx, y: scrollOffset.y + dy)) - } } extension TextView: LayoutManagerDelegate { func viewportBounds(for layoutManager: LayoutManager) -> CGRect { - var bounds: CGRect - if preparedContentRect.intersects(visibleRect) { - bounds = preparedContentRect.union(visibleRect) - } else { - bounds = visibleRect - } - - return convertToTextContainer(clampToTextContainer(bounds)) - } - - func visibleRect(for layoutManager: LayoutManager) -> CGRect { textContainerVisibleRect } func didInvalidateLayout(for layoutManager: LayoutManager) { - textLayer.setNeedsLayout() - selectionLayer.setNeedsLayout() - insertionPointLayer.setNeedsLayout() + layoutTextLayer() updateInsertionPointTimer() inputContext?.invalidateCharacterCoordinates() @@ -292,6 +197,8 @@ extension TextView: LayoutManagerDelegate { func layoutManager(_ layoutManager: LayoutManager, createLayerForLine line: Line) -> LineLayer { let l = LineLayer(line: line) l.anchorPoint = .zero + // Bounds origin is always (0, 0), so setNeedsDisplay() will only be called when the + // layer's size changes due window resize. l.needsDisplayOnBoundsChange = true l.delegate = self // LineLayerDelegate + NSViewLayerContentScaleDelegate l.contentsScale = window?.backingScaleFactor ?? 1.0 @@ -308,6 +215,10 @@ extension TextView: LayoutManagerDelegate { extension TextView { func layoutTextLayer() { + if window == nil { + return + } + var scrollAdjustment: CGFloat = 0 let updateLineNumbers = lineNumberView.superview != nil if updateLineNumbers { @@ -384,6 +295,10 @@ extension TextView: LineLayerDelegate { extension TextView { func layoutSelectionLayer() { + if window == nil { + return + } + selectionLayer.sublayers = nil layoutManager.layoutSelections { rect in @@ -400,6 +315,7 @@ extension TextView { l.delegate = self // SelectionLayerDelegate + NSViewLayerContentScaleDelegate l.needsDisplayOnBoundsChange = true l.contentsScale = window?.backingScaleFactor ?? 1.0 + l.isOpaque = true l.bounds = CGRect(origin: .zero, size: rect.size) l.position = convertFromTextContainer(rect.origin) @@ -411,6 +327,10 @@ extension TextView { extension TextView { func layoutInsertionPointLayer() { + if window == nil { + return + } + insertionPointLayer.sublayers = nil layoutManager.layoutInsertionPoints { rect in @@ -427,6 +347,7 @@ extension TextView { l.delegate = self // InsertionPointLayerDelegate + NSViewLayerContentScaleDelegate l.needsDisplayOnBoundsChange = true l.contentsScale = window?.backingScaleFactor ?? 1.0 + l.isOpaque = true l.bounds = CGRect(origin: .zero, size: rect.size) l.position = convertFromTextContainer(rect.origin) diff --git a/Watt/TextView/TextView+Scrolling.swift b/Watt/TextView/TextView+Scrolling.swift new file mode 100644 index 00000000..9a41d8a1 --- /dev/null +++ b/Watt/TextView/TextView+Scrolling.swift @@ -0,0 +1,80 @@ +// +// TextView+Scrolling.swift +// Watt +// +// Created by David Albert on 3/17/24. +// + +import Cocoa + +extension TextView { + override class var isCompatibleWithResponsiveScrolling: Bool { + true + } + + class func scrollableTextView() -> NSScrollView { + let textView = Self() + + let clipView = ClipView() + clipView.delegate = textView + + let scrollView = NSScrollView() + scrollView.contentView = clipView + scrollView.hasVerticalScroller = true + scrollView.documentView = textView + + textView.autoresizingMask = [.width, .height] + + return scrollView + } + + var scrollView: NSScrollView? { + if let enclosingScrollView, enclosingScrollView.documentView == self { + return enclosingScrollView + } + + return nil + } + + var scrollOffset: CGPoint { + guard let scrollView else { + return .zero + } + + return scrollView.contentView.bounds.origin + } + + var textContainerScrollOffset: CGPoint { + convertToTextContainer(scrollOffset) + } + + func scrollIndexToVisible(_ index: Buffer.Index) { + guard let rect = layoutManager.caretRect(for: index, affinity: index == buffer.endIndex ? .upstream : .downstream) else { + return + } + + let viewRect = convertFromTextContainer(rect) + scrollToVisible(viewRect) + } + + func scrollIndexToCenter(_ index: Buffer.Index) { + guard let rect = layoutManager.caretRect(for: index, affinity: index == buffer.endIndex ? .upstream : .downstream) else { + return + } + + let viewRect = convertFromTextContainer(rect) + scrollToCenter(viewRect) + } + + func scrollToCenter(_ rect: NSRect) { + let dx = rect.midX - visibleRect.midX + let dy = rect.midY - visibleRect.midY + scroll(CGPoint(x: scrollOffset.x + dx, y: scrollOffset.y + dy)) + } + + @objc func didScroll(_ notification: Notification) { + layoutTextLayer() + layoutSelectionLayer() + layoutInsertionPointLayer() + } +} diff --git a/Watt/TextView/TextView.swift b/Watt/TextView/TextView.swift index 5420a181..09dd5e42 100644 --- a/Watt/TextView/TextView.swift +++ b/Watt/TextView/TextView.swift @@ -8,19 +8,6 @@ import Cocoa class TextView: NSView, ClipViewDelegate { - class func scrollableTextView() -> NSScrollView { - let textView = Self() - - let scrollView = NSScrollView() - scrollView.contentView = ClipView() - scrollView.hasVerticalScroller = true - scrollView.documentView = textView - - textView.autoresizingMask = [.width, .height] - - return scrollView - } - override var isFlipped: Bool { true } @@ -112,8 +99,8 @@ class TextView: NSView, ClipViewDelegate { didSet { setTypingAttributes() - selectionLayer.setNeedsLayout() - insertionPointLayer.setNeedsLayout() + layoutSelectionLayer() + layoutInsertionPointLayer() updateInsertionPointTimer() } } @@ -173,6 +160,8 @@ class TextView: NSView, ClipViewDelegate { } func commonInit() { + layerContentsRedrawPolicy = .never + layoutManager.buffer = buffer layoutManager.delegate = self @@ -223,19 +212,26 @@ class TextView: NSView, ClipViewDelegate { // has had a chance to update its geometry. // // If we don't do this, the LineNumberView is flipped upside down. - func viewDidMoveToClipView() { + func viewDidMoveToClipView(_ clipView: ClipView) { addLineNumberView() } override func viewDidMoveToWindow() { + NotificationCenter.default.removeObserver(self, name: NSView.boundsDidChangeNotification, object: nil) NotificationCenter.default.removeObserver(self, name: NSWindow.didBecomeKeyNotification, object: nil) NotificationCenter.default.removeObserver(self, name: NSWindow.didResignKeyNotification, object: nil) - guard let window else { - return + if let scrollView { + NotificationCenter.default.addObserver(self, selector: #selector(didScroll(_:)), name: NSView.boundsDidChangeNotification, object: scrollView.contentView) } - NotificationCenter.default.addObserver(self, selector: #selector(windowDidBecomeKey(_:)), name: NSWindow.didBecomeKeyNotification, object: window) - NotificationCenter.default.addObserver(self, selector: #selector(windowDidResignKey(_:)), name: NSWindow.didResignKeyNotification, object: window) + if let window { + NotificationCenter.default.addObserver(self, selector: #selector(windowDidBecomeKey(_:)), name: NSWindow.didBecomeKeyNotification, object: window) + NotificationCenter.default.addObserver(self, selector: #selector(windowDidResignKey(_:)), name: NSWindow.didResignKeyNotification, object: window) + + layoutTextLayer() + layoutSelectionLayer() + layoutInsertionPointLayer() + } } } From d4f069cb7f6bf22d22d3af4b5f249bd7715e5c1f Mon Sep 17 00:00:00 2001 From: David Albert Date: Mon, 18 Mar 2024 17:25:43 -0400 Subject: [PATCH 02/73] Organize Utilities --- Watt.xcodeproj/project.pbxproj | 32 ++++++++++++------- Watt/{ => TextView}/ClipView.swift | 0 .../CheckedContinuationReference.swift | 0 Watt/{ => Utilities}/DragAndDrop.swift | 0 Watt/{ => Utilities}/FSEvents.swift | 0 .../OutlineViewDiffableDataSource.swift | 0 Watt/{ => Utilities}/SimpleProxy.h | 0 Watt/{ => Utilities}/SimpleProxy.m | 0 Watt/{ => Utilities}/Utilities.swift | 0 Watt/{ => Utilities}/Weak.swift | 0 Watt/Watt-Bridging-Header.h | 2 +- .../ContainerViewController.swift | 0 12 files changed, 21 insertions(+), 13 deletions(-) rename Watt/{ => TextView}/ClipView.swift (100%) rename Watt/{ => Utilities}/CheckedContinuationReference.swift (100%) rename Watt/{ => Utilities}/DragAndDrop.swift (100%) rename Watt/{ => Utilities}/FSEvents.swift (100%) rename Watt/{ => Utilities}/OutlineViewDiffableDataSource.swift (100%) rename Watt/{ => Utilities}/SimpleProxy.h (100%) rename Watt/{ => Utilities}/SimpleProxy.m (100%) rename Watt/{ => Utilities}/Utilities.swift (100%) rename Watt/{ => Utilities}/Weak.swift (100%) rename Watt/{ => Workspace}/ContainerViewController.swift (100%) diff --git a/Watt.xcodeproj/project.pbxproj b/Watt.xcodeproj/project.pbxproj index cbf1980b..8aebed4d 100644 --- a/Watt.xcodeproj/project.pbxproj +++ b/Watt.xcodeproj/project.pbxproj @@ -272,6 +272,21 @@ /* End PBXFrameworksBuildPhase section */ /* Begin PBXGroup section */ + 63206D2B2BA8E73000CAFBC4 /* Utilities */ = { + isa = PBXGroup; + children = ( + 63C2C0452B4DF34D00BFB976 /* SimpleProxy.h */, + 63C2C0462B4DF34D00BFB976 /* SimpleProxy.m */, + 63DD20DA2B7278E400E55747 /* CheckedContinuationReference.swift */, + 637E58532B8415A400C498EE /* DragAndDrop.swift */, + 63A95D832B5462D60032E8B9 /* FSEvents.swift */, + 63C2C03E2B4D95EB00BFB976 /* OutlineViewDiffableDataSource.swift */, + 63B8B1752A9F890100F32C2C /* Utilities.swift */, + 63A618712A0EAF2000E70B0E /* Weak.swift */, + ); + path = Utilities; + sourceTree = ""; + }; 63359A392AA9081B00082762 /* Frameworks */ = { isa = PBXGroup; children = ( @@ -330,6 +345,7 @@ 636E850F2B6AAD7C00772ECE /* TextView */ = { isa = PBXGroup; children = ( + 63286AE12A13DF5D00839B25 /* ClipView.swift */, 635B36AE2AAE5424002FAE70 /* Highlighter.swift */, 637867802A1BEA960039960E /* InsertionPointLayer.swift */, 635B36AC2AAE4D7F002FAE70 /* Language.swift */, @@ -412,21 +428,10 @@ children = ( 6356588F2ACD9D5400F1DC63 /* Themes */, 6372889829FD8E1A005B95E5 /* Watt.entitlements */, - 63C2C0452B4DF34D00BFB976 /* SimpleProxy.h */, 63C2C04A2B4DF86100BFB976 /* Watt-Bridging-Header.h */, - 63C2C0462B4DF34D00BFB976 /* SimpleProxy.m */, 6372889729FD8E1A005B95E5 /* Info.plist */, 6372888B29FD8E19005B95E5 /* AppDelegate.swift */, - 63DD20DA2B7278E400E55747 /* CheckedContinuationReference.swift */, - 63286AE12A13DF5D00839B25 /* ClipView.swift */, - 6364DDA02B6C403400886DE3 /* ContainerViewController.swift */, - 637E58532B8415A400C498EE /* DragAndDrop.swift */, - 63A95D832B5462D60032E8B9 /* FSEvents.swift */, - 63C2C03E2B4D95EB00BFB976 /* OutlineViewDiffableDataSource.swift */, 63A95D872B55B9D70032E8B9 /* UserDefaults+Keys.swift */, - 63B8B1752A9F890100F32C2C /* Utilities.swift */, - 63B8B1772A9F8A3A00F32C2C /* UtilitiesTest.swift */, - 63A618712A0EAF2000E70B0E /* Weak.swift */, 63A618612A0C3AE200E70B0E /* Moby Dick.txt */, 6372889229FD8E1A005B95E5 /* Assets.xcassets */, 636E84662B683F3A00772ECE /* Localizable.xcstrings */, @@ -437,6 +442,7 @@ 6372889429FD8E1A005B95E5 /* MainMenu.xib */, 636E85102B6AADC900772ECE /* Rope */, 636E850F2B6AAD7C00772ECE /* TextView */, + 63206D2B2BA8E73000CAFBC4 /* Utilities */, 63C2C04E2B4F3B8E00BFB976 /* Workspace */, ); path = Watt; @@ -452,6 +458,7 @@ 63D5463F2AFC304B00C72CDE /* LayoutManagerTests.swift */, 63F0468E2A65D417001B8DA8 /* RopeTests.swift */, 6315D2FD2A96630F00B8A077 /* SpansTests.swift */, + 63B8B1772A9F8A3A00F32C2C /* UtilitiesTest.swift */, 634A70A32B1F7A0A00BDD0D3 /* Support */, ); path = WattTests; @@ -478,9 +485,11 @@ 63C2C04E2B4F3B8E00BFB976 /* Workspace */ = { isa = PBXGroup; children = ( + 6364DDA02B6C403400886DE3 /* ContainerViewController.swift */, 63C2C04F2B4F3BB700BFB976 /* Dirent.swift */, 63D280882B5B275900CDCF04 /* DirentTextField.swift */, 6364DDA22B6C408700886DE3 /* DocumentPaneViewController.swift */, + 63DD20D82B71A53200E55747 /* Window.swift */, 6364DD9E2B6C3BB000886DE3 /* WindowController.swift */, 63C541EA2B486210001C370B /* Workspace.swift */, 636DD5302B599E150064C989 /* Workspace+LoadRequest.swift */, @@ -488,7 +497,6 @@ 63D280AA2B605A2A00CDCF04 /* WorkspacePasteboardWriter.swift */, 63DC933C2B473BB5001018FE /* WorkspaceViewController.swift */, 6364DD962B6C05C300886DE3 /* WorkspaceWindowController.swift */, - 63DD20D82B71A53200E55747 /* Window.swift */, ); path = Workspace; sourceTree = ""; diff --git a/Watt/ClipView.swift b/Watt/TextView/ClipView.swift similarity index 100% rename from Watt/ClipView.swift rename to Watt/TextView/ClipView.swift diff --git a/Watt/CheckedContinuationReference.swift b/Watt/Utilities/CheckedContinuationReference.swift similarity index 100% rename from Watt/CheckedContinuationReference.swift rename to Watt/Utilities/CheckedContinuationReference.swift diff --git a/Watt/DragAndDrop.swift b/Watt/Utilities/DragAndDrop.swift similarity index 100% rename from Watt/DragAndDrop.swift rename to Watt/Utilities/DragAndDrop.swift diff --git a/Watt/FSEvents.swift b/Watt/Utilities/FSEvents.swift similarity index 100% rename from Watt/FSEvents.swift rename to Watt/Utilities/FSEvents.swift diff --git a/Watt/OutlineViewDiffableDataSource.swift b/Watt/Utilities/OutlineViewDiffableDataSource.swift similarity index 100% rename from Watt/OutlineViewDiffableDataSource.swift rename to Watt/Utilities/OutlineViewDiffableDataSource.swift diff --git a/Watt/SimpleProxy.h b/Watt/Utilities/SimpleProxy.h similarity index 100% rename from Watt/SimpleProxy.h rename to Watt/Utilities/SimpleProxy.h diff --git a/Watt/SimpleProxy.m b/Watt/Utilities/SimpleProxy.m similarity index 100% rename from Watt/SimpleProxy.m rename to Watt/Utilities/SimpleProxy.m diff --git a/Watt/Utilities.swift b/Watt/Utilities/Utilities.swift similarity index 100% rename from Watt/Utilities.swift rename to Watt/Utilities/Utilities.swift diff --git a/Watt/Weak.swift b/Watt/Utilities/Weak.swift similarity index 100% rename from Watt/Weak.swift rename to Watt/Utilities/Weak.swift diff --git a/Watt/Watt-Bridging-Header.h b/Watt/Watt-Bridging-Header.h index 141f8370..861c44b6 100644 --- a/Watt/Watt-Bridging-Header.h +++ b/Watt/Watt-Bridging-Header.h @@ -2,5 +2,5 @@ // Use this file to import your target's public headers that you would like to expose to Swift. // -#import "SimpleProxy.h" +#import "Utilities/SimpleProxy.h" #import "Extensions/NSObject+Extensions.h" diff --git a/Watt/ContainerViewController.swift b/Watt/Workspace/ContainerViewController.swift similarity index 100% rename from Watt/ContainerViewController.swift rename to Watt/Workspace/ContainerViewController.swift From e52e0965b2a9432d215f043761ce26262b8a9cea Mon Sep 17 00:00:00 2001 From: David Albert Date: Mon, 18 Mar 2024 21:23:52 -0400 Subject: [PATCH 03/73] WIP smooth autoscroll --- Watt.xcodeproj/project.pbxproj | 4 + Watt/TextView/TextView+Mouse.swift | 46 ++-------- Watt/TextView/TextView.swift | 2 + Watt/Utilities/Autoscroller.swift | 131 +++++++++++++++++++++++++++++ 4 files changed, 146 insertions(+), 37 deletions(-) create mode 100644 Watt/Utilities/Autoscroller.swift diff --git a/Watt.xcodeproj/project.pbxproj b/Watt.xcodeproj/project.pbxproj index 8aebed4d..ffe74feb 100644 --- a/Watt.xcodeproj/project.pbxproj +++ b/Watt.xcodeproj/project.pbxproj @@ -12,6 +12,7 @@ 6315D3022A9BD00F00B8A077 /* AttributedRopeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6315D3012A9BD00F00B8A077 /* AttributedRopeTests.swift */; }; 631FD2652A696D27005FA854 /* Heights.swift in Sources */ = {isa = PBXBuildFile; fileRef = 631FD2642A696D27005FA854 /* Heights.swift */; }; 63206D2A2BA74AF600CAFBC4 /* TextView+Scrolling.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63206D292BA74AF600CAFBC4 /* TextView+Scrolling.swift */; }; + 63206D2D2BA8E8F400CAFBC4 /* Autoscroller.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63206D2C2BA8E8F400CAFBC4 /* Autoscroller.swift */; }; 63267C292B97BF0B00F6E968 /* BTreeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63267C282B97BF0B00F6E968 /* BTreeTests.swift */; }; 63286AE22A13DF5D00839B25 /* ClipView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63286AE12A13DF5D00839B25 /* ClipView.swift */; }; 632A73722B87C69F009F38C3 /* Range+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 632A73712B87C69F009F38C3 /* Range+Extensions.swift */; }; @@ -132,6 +133,7 @@ 6315D3012A9BD00F00B8A077 /* AttributedRopeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = AttributedRopeTests.swift; path = WattTests/AttributedRopeTests.swift; sourceTree = SOURCE_ROOT; }; 631FD2642A696D27005FA854 /* Heights.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Heights.swift; sourceTree = ""; }; 63206D292BA74AF600CAFBC4 /* TextView+Scrolling.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TextView+Scrolling.swift"; sourceTree = ""; }; + 63206D2C2BA8E8F400CAFBC4 /* Autoscroller.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Autoscroller.swift; sourceTree = ""; }; 63267C282B97BF0B00F6E968 /* BTreeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTreeTests.swift; sourceTree = ""; }; 63286AE12A13DF5D00839B25 /* ClipView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ClipView.swift; sourceTree = ""; }; 632A73712B87C69F009F38C3 /* Range+Extensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Range+Extensions.swift"; sourceTree = ""; }; @@ -283,6 +285,7 @@ 63C2C03E2B4D95EB00BFB976 /* OutlineViewDiffableDataSource.swift */, 63B8B1752A9F890100F32C2C /* Utilities.swift */, 63A618712A0EAF2000E70B0E /* Weak.swift */, + 63206D2C2BA8E8F400CAFBC4 /* Autoscroller.swift */, ); path = Utilities; sourceTree = ""; @@ -666,6 +669,7 @@ 6364DDA82B6C49C700886DE3 /* DocumentViewController.swift in Sources */, 6346D1952A06EC2900CFB7EE /* TextContainer.swift in Sources */, 6396DD262B433781000F85D1 /* Comparable+Extensions.swift in Sources */, + 63206D2D2BA8E8F400CAFBC4 /* Autoscroller.swift in Sources */, 63B8B1762A9F890100F32C2C /* Utilities.swift in Sources */, 63F25BE12A689D2400DEACCF /* Line.swift in Sources */, 637867872A1FB2C50039960E /* NSRange+Extensions.swift in Sources */, diff --git a/Watt/TextView/TextView+Mouse.swift b/Watt/TextView/TextView+Mouse.swift index ead81ca6..1786b5df 100644 --- a/Watt/TextView/TextView+Mouse.swift +++ b/Watt/TextView/TextView+Mouse.swift @@ -29,54 +29,26 @@ extension TextView { selection = SelectionNavigator(selection).selection(for: .paragraph, enclosing: point, dataSource: layoutManager) } - var mouseEvent = event - var done = false - NSEvent.startPeriodicEvents(afterDelay: 0.1, withPeriod: 0.05) - - while !done { - guard let nextEvent = NSApp.nextEvent(matching: [.leftMouseUp, .leftMouseDragged, .periodic], until: .distantFuture, inMode: .eventTracking, dequeue: true) else { - print("Unexpected nil event, should not expire") - continue - } - - switch nextEvent.type { - case .periodic: - autoscroll(with: mouseEvent) - extendSelection(with: mouseEvent) - // Don't dispatch periodic events to NSApp. Doesn't really matter in practice, but - // NSApp doesn't normally receive periodic events, so let's not rock the boat. - continue - case .leftMouseUp: - done = true - case .leftMouseDragged: - mouseEvent = nextEvent - default: - print("Unexpected event type \(nextEvent.type)") - } - - NSApp.sendEvent(nextEvent) + autoscroller = Autoscroller(self, event: event) { [weak self] locationInView in + guard let self else { return } + let clamped = locationInView.clamped(to: visibleRect) + let point = convertToTextContainer(clamped) + selection = SelectionNavigator(selection).selection(extendingTo: point, dataSource: layoutManager) } - - NSEvent.stopPeriodicEvents() + autoscroller?.start() } override func mouseDragged(with event: NSEvent) { if inputContext?.handleEvent(event) ?? false { return } - - extendSelection(with: event) - } - - func extendSelection(with event: NSEvent) { - let locationInView = convert(event.locationInWindow, from: nil) - let clamped = locationInView.clamped(to: visibleRect) - let point = convertToTextContainer(clamped) - selection = SelectionNavigator(selection).selection(extendingTo: point, dataSource: layoutManager) + autoscroller?.update(with: event) } override func mouseUp(with event: NSEvent) { inputContext?.handleEvent(event) + autoscroller?.stop() + autoscroller = nil } override func cursorUpdate(with event: NSEvent) { diff --git a/Watt/TextView/TextView.swift b/Watt/TextView/TextView.swift index 09dd5e42..ca68d803 100644 --- a/Watt/TextView/TextView.swift +++ b/Watt/TextView/TextView.swift @@ -140,6 +140,8 @@ class TextView: NSView, ClipViewDelegate { var insertionPointLayerCache: WeakDictionary = WeakDictionary() var insertionPointTimer: Timer? + var autoscroller: Autoscroller? + // HACK: See layoutTextLayer() for context. var previousVisibleRect: CGRect = .zero diff --git a/Watt/Utilities/Autoscroller.swift b/Watt/Utilities/Autoscroller.swift new file mode 100644 index 00000000..ef17a323 --- /dev/null +++ b/Watt/Utilities/Autoscroller.swift @@ -0,0 +1,131 @@ +// +// Autoscroller.swift +// Watt +// +// Created by David Albert on 3/18/24. +// + +import Cocoa + +fileprivate let mouseEvents: [NSEvent.EventType] = [ + .leftMouseDown, + .leftMouseUp, + .leftMouseDragged, + .rightMouseDown, + .rightMouseUp, + .rightMouseDragged, + .otherMouseUp, + .otherMouseDown, + .otherMouseDragged, + .mouseMoved +] + +class Autoscroller { + private weak var view: NSView? + private let runloop: RunLoop + private let mode: RunLoop.Mode + private let callback: (NSPoint) -> Void + + private (set) var locationInWindow: NSPoint + + private(set) var running: Bool + private var displayLink: CADisplayLink! + + init(_ view: NSView, event: NSEvent, runloop: RunLoop = .main, mode: RunLoop.Mode = .common, using block: @escaping (CGPoint) -> Void) { + self.view = view + self.runloop = runloop + self.mode = mode + self.callback = block + self.locationInWindow = event.locationInWindow + + self.running = false + self.displayLink = view.displayLink(target: self, selector: #selector(tick(_:))) + } + + deinit { + stop() + } + + func update(with event: NSEvent) { + precondition(mouseEvents.contains(event.type), "Expected mouse event but got \(event.type).") + locationInWindow = event.locationInWindow + } + + func start() { + if running || view == nil { + return + } + displayLink.add(to: runloop, forMode: mode) + running = true + } + + func stop() { + if !running { + return + } + displayLink.remove(from: runloop, forMode: mode) + running = false + } + + @objc func tick(_ sender: CADisplayLink) { + guard let view else { + stop() + return + } + + let locationInView = view.convert(locationInWindow, from: nil) + + guard let scrollView = view.enclosingScrollView, scrollView.documentView == view else { + callback(locationInView) + return + } + + let viewportBounds = scrollView.contentView.bounds + + let slowdown: CGFloat = 16 + let maxScroll: CGFloat = 256 + + let dx: CGFloat + if view.bounds.width <= viewportBounds.width { + dx = 0 + } else if locationInView.x <= viewportBounds.minX { + let d = (viewportBounds.minX - locationInView.x) / slowdown + let scaled = min(maxScroll, d*d) + dx = -scaled + } else if locationInView.x >= viewportBounds.maxX { + let d = (locationInView.x - viewportBounds.maxX) / slowdown + let scaled = min(maxScroll, d*d) + dx = scaled + } else { + dx = 0 + } + + let dy: CGFloat + if view.bounds.height <= viewportBounds.height { + dy = 0 + } else if locationInView.y <= viewportBounds.minY { + let d = (viewportBounds.minY - locationInView.y) / slowdown + let scaled = min(maxScroll, d*d) + dy = -scaled + } else if locationInView.y >= viewportBounds.maxY { + let d = (locationInView.y - viewportBounds.maxY) / slowdown + let scaled = min(maxScroll, d*d) + dy = scaled + } else { + dy = 0 + } + + if dx == 0 && dy == 0 { + callback(locationInView) + return + } + + let scrollOffset = viewportBounds.origin + let x = (scrollOffset.x + dx).clamped(to: 0...(view.bounds.width - viewportBounds.width)) + let y = (scrollOffset.y + dy).clamped(to: 0...(view.bounds.height - viewportBounds.height)) + + view.scroll(NSPoint(x: x, y: y)) + + callback(locationInView) + } +} From 0f0bc6d85ebc6d24db890e9c1a7664ed039e9a11 Mon Sep 17 00:00:00 2001 From: David Albert Date: Tue, 19 Mar 2024 10:47:47 -0400 Subject: [PATCH 04/73] Tweaks to autoscroll --- Watt/Utilities/Autoscroller.swift | 34 +++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/Watt/Utilities/Autoscroller.swift b/Watt/Utilities/Autoscroller.swift index ef17a323..40ffaf3b 100644 --- a/Watt/Utilities/Autoscroller.swift +++ b/Watt/Utilities/Autoscroller.swift @@ -20,26 +20,42 @@ fileprivate let mouseEvents: [NSEvent.EventType] = [ .mouseMoved ] +// A helper class for smooth autoscrolling. class Autoscroller { + // TODO: for whatever reason, I'm not actually able to get 120 Hz working on my M1 MacBook Pro. Investigate. + static var preferredFrameRateRange: CAFrameRateRange { + // Apple recommends min=80 max=120 preferred=120 for 120 Hz displays. https://developer.apple.com/documentation/quartzcore/optimizing_promotion_refresh_rates_for_iphone_13_pro_and_ipad_pro + + let max: Float = Float(NSScreen.screens.map(\.maximumFramesPerSecond).max() ?? 60) + let min: Float = max >= 80.0 ? 80.0 : 60.0 + + return CAFrameRateRange(minimum: min, maximum: max, preferred: max) + } + private weak var view: NSView? - private let runloop: RunLoop - private let mode: RunLoop.Mode private let callback: (NSPoint) -> Void - private (set) var locationInWindow: NSPoint + private var locationInWindow: NSPoint + + private var dxRemainder: CGFloat + private var dyRemainder: CGFloat private(set) var running: Bool private var displayLink: CADisplayLink! - init(_ view: NSView, event: NSEvent, runloop: RunLoop = .main, mode: RunLoop.Mode = .common, using block: @escaping (CGPoint) -> Void) { + + init(_ view: NSView, event: NSEvent, using block: @escaping (CGPoint) -> Void) { self.view = view - self.runloop = runloop - self.mode = mode self.callback = block self.locationInWindow = event.locationInWindow + self.dxRemainder = 0 + self.dyRemainder = 0 + self.running = false self.displayLink = view.displayLink(target: self, selector: #selector(tick(_:))) + + displayLink.preferredFrameRateRange = Self.preferredFrameRateRange } deinit { @@ -55,7 +71,7 @@ class Autoscroller { if running || view == nil { return } - displayLink.add(to: runloop, forMode: mode) + displayLink.add(to: .main, forMode: .common) running = true } @@ -63,7 +79,7 @@ class Autoscroller { if !running { return } - displayLink.remove(from: runloop, forMode: mode) + displayLink.remove(from: .main, forMode: .common) running = false } @@ -126,6 +142,6 @@ class Autoscroller { view.scroll(NSPoint(x: x, y: y)) - callback(locationInView) + callback(NSPoint(x: locationInView.x + dx, y: locationInView.y + dy)) } } From 6ed5a43ccbc696bb4da025336bb6c5ac6d4bd8e2 Mon Sep 17 00:00:00 2001 From: David Albert Date: Tue, 19 Mar 2024 12:35:15 -0400 Subject: [PATCH 05/73] Autoscroll the same regardless of refresh rate --- Watt/Utilities/Autoscroller.swift | 52 +++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/Watt/Utilities/Autoscroller.swift b/Watt/Utilities/Autoscroller.swift index 40ffaf3b..fb0206ce 100644 --- a/Watt/Utilities/Autoscroller.swift +++ b/Watt/Utilities/Autoscroller.swift @@ -20,7 +20,7 @@ fileprivate let mouseEvents: [NSEvent.EventType] = [ .mouseMoved ] -// A helper class for smooth autoscrolling. +// A helper for smooth autoscrolling. class Autoscroller { // TODO: for whatever reason, I'm not actually able to get 120 Hz working on my M1 MacBook Pro. Investigate. static var preferredFrameRateRange: CAFrameRateRange { @@ -37,6 +37,9 @@ class Autoscroller { private var locationInWindow: NSPoint + // For small enough dx and dy, the scroll view doesn't end up scrolling. When that happens, accumulate + // the dx and dy each frame so that we can still scroll when the pointer is very close to the edge of + // the view. private var dxRemainder: CGFloat private var dyRemainder: CGFloat @@ -97,21 +100,29 @@ class Autoscroller { } let viewportBounds = scrollView.contentView.bounds - - let slowdown: CGFloat = 16 - let maxScroll: CGFloat = 256 + let dt = sender.targetTimestamp - sender.timestamp + let hz = (1/dt).rounded() + + // minScroll is set up so that we end up scrolling at least one point per second, or 0.5 points per + // 0.5 seconds, which seems to be the granularity of an NSScrollView on a 2x display. + // + // maxScroll=256 and slowdown=(1/16) feel right for 60 Hz. To keep the same feel for other refresh + // rates, we scale these values. Slowdown ends up squared, so we have to scale it by the square root. + let minScroll: CGFloat = dt + let maxScroll: CGFloat = 256.0 * (60.0/hz) + let slowdown: CGFloat = (1.0/16.0) * sqrt(60/hz) let dx: CGFloat if view.bounds.width <= viewportBounds.width { dx = 0 } else if locationInView.x <= viewportBounds.minX { - let d = (viewportBounds.minX - locationInView.x) / slowdown - let scaled = min(maxScroll, d*d) - dx = -scaled + let d = (viewportBounds.minX - locationInView.x) * slowdown + let scaled = (d*d).clamped(to: minScroll...maxScroll) + dx = -scaled + min(dxRemainder, 0) } else if locationInView.x >= viewportBounds.maxX { - let d = (locationInView.x - viewportBounds.maxX) / slowdown - let scaled = min(maxScroll, d*d) - dx = scaled + let d = (locationInView.x - viewportBounds.maxX) * slowdown + let scaled = (d*d).clamped(to: minScroll...maxScroll) + dx = scaled + max(dxRemainder, 0) } else { dx = 0 } @@ -120,13 +131,13 @@ class Autoscroller { if view.bounds.height <= viewportBounds.height { dy = 0 } else if locationInView.y <= viewportBounds.minY { - let d = (viewportBounds.minY - locationInView.y) / slowdown - let scaled = min(maxScroll, d*d) - dy = -scaled + let d = (viewportBounds.minY - locationInView.y) * slowdown + let scaled = (d*d).clamped(to: minScroll...maxScroll) + dy = -scaled + min(dyRemainder, 0) } else if locationInView.y >= viewportBounds.maxY { - let d = (locationInView.y - viewportBounds.maxY) / slowdown - let scaled = min(maxScroll, d*d) - dy = scaled + let d = (locationInView.y - viewportBounds.maxY) * slowdown + let scaled = (d*d).clamped(to: minScroll...maxScroll) + dy = scaled + max(dyRemainder, 0) } else { dy = 0 } @@ -142,6 +153,15 @@ class Autoscroller { view.scroll(NSPoint(x: x, y: y)) + // NSClipView rounds its bounds origin and only scrolls when abs(newX-oldX) >= a threshold. Experimentally + // on a 2x display, that threshold is 0.5. But rather than hardcoding, just check to see if we've actually + // scrolled, and if we haven't save dy and dx to accumulate on the next frame. + let newScrollOffset = scrollView.contentView.bounds.origin + let xScrolled = abs(newScrollOffset.x - scrollOffset.x) > 1e-10 + let yScrolled = abs(newScrollOffset.y - scrollOffset.y) > 1e-10 + dxRemainder = xScrolled ? 0 : dx + dyRemainder = yScrolled ? 0 : dy + callback(NSPoint(x: locationInView.x + dx, y: locationInView.y + dy)) } } From 1a396bd052e815e51d4a6825c5bb16bb250849a2 Mon Sep 17 00:00:00 2001 From: David Albert Date: Tue, 19 Mar 2024 12:38:17 -0400 Subject: [PATCH 06/73] Clarify comment --- Watt/Utilities/Autoscroller.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Watt/Utilities/Autoscroller.swift b/Watt/Utilities/Autoscroller.swift index fb0206ce..4350f172 100644 --- a/Watt/Utilities/Autoscroller.swift +++ b/Watt/Utilities/Autoscroller.swift @@ -103,8 +103,9 @@ class Autoscroller { let dt = sender.targetTimestamp - sender.timestamp let hz = (1/dt).rounded() - // minScroll is set up so that we end up scrolling at least one point per second, or 0.5 points per - // 0.5 seconds, which seems to be the granularity of an NSScrollView on a 2x display. + // minScroll is set up so that we end up scrolling at least one point per second. On a 2x display + // where NSScrollView's minimum scroll is 0.5 points (1 physical pixel), this means we scroll a + // minimum of 0.5 points every half second. // // maxScroll=256 and slowdown=(1/16) feel right for 60 Hz. To keep the same feel for other refresh // rates, we scale these values. Slowdown ends up squared, so we have to scale it by the square root. From e96cc9a84649c7624ca95490ccc6051b8154e638 Mon Sep 17 00:00:00 2001 From: David Albert Date: Tue, 19 Mar 2024 12:54:54 -0400 Subject: [PATCH 07/73] Just move things around --- Watt/TextView/TextView+Layout.swift | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Watt/TextView/TextView+Layout.swift b/Watt/TextView/TextView+Layout.swift index 5744c5d3..81210a85 100644 --- a/Watt/TextView/TextView+Layout.swift +++ b/Watt/TextView/TextView+Layout.swift @@ -7,7 +7,7 @@ import Cocoa -extension TextView: CALayerDelegate, NSViewLayerContentScaleDelegate { +extension TextView { override func layout() { // If we need to call setNeedsLayout on our subviews, do it here, // before calling super.layout() @@ -34,10 +34,6 @@ extension TextView: CALayerDelegate, NSViewLayerContentScaleDelegate { } } - func layer(_ layer: CALayer, shouldInheritContentsScale newScale: CGFloat, from window: NSWindow) -> Bool { - true - } - override func setFrameSize(_ newSize: NSSize) { super.setFrameSize(newSize) updateTextContainerSizeIfNeeded() @@ -284,6 +280,12 @@ extension TextView { } } +extension TextView: NSViewLayerContentScaleDelegate { + func layer(_ layer: CALayer, shouldInheritContentsScale newScale: CGFloat, from window: NSWindow) -> Bool { + true + } +} + extension TextView: LineLayerDelegate { func effectiveAppearance(for lineLayer: LineLayer) -> NSAppearance { effectiveAppearance From 242c7d73dc3aeb97a9e620724dde961e390897ef Mon Sep 17 00:00:00 2001 From: David Albert Date: Tue, 19 Mar 2024 16:06:35 -0400 Subject: [PATCH 08/73] Re-layout text when scroll view resizes --- Watt.xcodeproj/project.pbxproj | 4 ++ Watt/TextView/ClipView.swift | 8 +++ Watt/TextView/TextView+Input.swift | 8 +-- Watt/TextView/TextView+Layout.swift | 10 ++-- Watt/TextView/TextView+Scrolling.swift | 10 ++-- Watt/TextView/TextView+Transactions.swift | 66 +++++++++++++++++++++++ Watt/TextView/TextView.swift | 36 ++++++++++--- 7 files changed, 123 insertions(+), 19 deletions(-) create mode 100644 Watt/TextView/TextView+Transactions.swift diff --git a/Watt.xcodeproj/project.pbxproj b/Watt.xcodeproj/project.pbxproj index ffe74feb..b0278b4c 100644 --- a/Watt.xcodeproj/project.pbxproj +++ b/Watt.xcodeproj/project.pbxproj @@ -13,6 +13,7 @@ 631FD2652A696D27005FA854 /* Heights.swift in Sources */ = {isa = PBXBuildFile; fileRef = 631FD2642A696D27005FA854 /* Heights.swift */; }; 63206D2A2BA74AF600CAFBC4 /* TextView+Scrolling.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63206D292BA74AF600CAFBC4 /* TextView+Scrolling.swift */; }; 63206D2D2BA8E8F400CAFBC4 /* Autoscroller.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63206D2C2BA8E8F400CAFBC4 /* Autoscroller.swift */; }; + 63206D312BA9FFD300CAFBC4 /* TextView+Transactions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63206D302BA9FFD300CAFBC4 /* TextView+Transactions.swift */; }; 63267C292B97BF0B00F6E968 /* BTreeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63267C282B97BF0B00F6E968 /* BTreeTests.swift */; }; 63286AE22A13DF5D00839B25 /* ClipView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63286AE12A13DF5D00839B25 /* ClipView.swift */; }; 632A73722B87C69F009F38C3 /* Range+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 632A73712B87C69F009F38C3 /* Range+Extensions.swift */; }; @@ -134,6 +135,7 @@ 631FD2642A696D27005FA854 /* Heights.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Heights.swift; sourceTree = ""; }; 63206D292BA74AF600CAFBC4 /* TextView+Scrolling.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TextView+Scrolling.swift"; sourceTree = ""; }; 63206D2C2BA8E8F400CAFBC4 /* Autoscroller.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Autoscroller.swift; sourceTree = ""; }; + 63206D302BA9FFD300CAFBC4 /* TextView+Transactions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TextView+Transactions.swift"; sourceTree = ""; }; 63267C282B97BF0B00F6E968 /* BTreeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTreeTests.swift; sourceTree = ""; }; 63286AE12A13DF5D00839B25 /* ClipView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ClipView.swift; sourceTree = ""; }; 632A73712B87C69F009F38C3 /* Range+Extensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Range+Extensions.swift"; sourceTree = ""; }; @@ -364,6 +366,7 @@ 6396DD2D2B45A5DE000F85D1 /* TextView+Pasteboard.swift */, 63206D292BA74AF600CAFBC4 /* TextView+Scrolling.swift */, 637867702A17F7460039960E /* TextView+Selection.swift */, + 63206D302BA9FFD300CAFBC4 /* TextView+Transactions.swift */, 635B36B02AAFCD95002FAE70 /* Theme.swift */, 635F815C2AA9E51900553E23 /* TreeSitter.swift */, ); @@ -693,6 +696,7 @@ 636DD5322B599E150064C989 /* Workspace+LoadRequest.swift in Sources */, 637867832A1EC5B50039960E /* TextView+Input.swift in Sources */, 6364DDA32B6C408700886DE3 /* DocumentPaneViewController.swift in Sources */, + 63206D312BA9FFD300CAFBC4 /* TextView+Transactions.swift in Sources */, 6396DD2E2B45A5DE000F85D1 /* TextView+Pasteboard.swift in Sources */, 63A618722A0EAF2000E70B0E /* Weak.swift in Sources */, 63D280922B5ECDF600CDCF04 /* NSFileCoordinator+Extensions.swift in Sources */, diff --git a/Watt/TextView/ClipView.swift b/Watt/TextView/ClipView.swift index b5c13431..da3a9053 100644 --- a/Watt/TextView/ClipView.swift +++ b/Watt/TextView/ClipView.swift @@ -9,10 +9,12 @@ import Cocoa protocol ClipViewDelegate: AnyObject { func viewDidMoveToClipView(_ clipView: ClipView) + func clipView(_ clipView: ClipView, frameSizeDidChangeFrom oldSize: NSSize) } extension ClipViewDelegate { func viewDidMoveToClipView(_ clipView: ClipView) {} + func clipView(_ clipView: ClipView, frameSizeDidChangeFrom oldSize: NSSize) {} } class ClipView: NSClipView { @@ -23,5 +25,11 @@ class ClipView: NSClipView { delegate?.viewDidMoveToClipView(self) } } + + override func setFrameSize(_ newSize: NSSize) { + let old = frame.size + super.setFrameSize(newSize) + delegate?.clipView(self, frameSizeDidChangeFrom: old) + } } diff --git a/Watt/TextView/TextView+Input.swift b/Watt/TextView/TextView+Input.swift index f0d28549..eee7e9c2 100644 --- a/Watt/TextView/TextView+Input.swift +++ b/Watt/TextView/TextView+Input.swift @@ -86,9 +86,11 @@ extension TextView: NSTextInputClient { // TODO: if we're the only one who calls unmarkText(), we can remove // these layout calls, because we already do layout in didInvalidateLayout(for layoutManager: LayoutManager) - layoutTextLayer() - layoutSelectionLayer() - layoutInsertionPointLayer() + transaction { + schedule(.textLayout) + schedule(.selectionLayout) + schedule(.insertionPointLayout) + } } func selectedRange() -> NSRange { diff --git a/Watt/TextView/TextView+Layout.swift b/Watt/TextView/TextView+Layout.swift index 81210a85..2bc1be06 100644 --- a/Watt/TextView/TextView+Layout.swift +++ b/Watt/TextView/TextView+Layout.swift @@ -45,10 +45,6 @@ extension TextView { if layoutManager.textContainer.size.width != width { layoutManager.textContainer.size = CGSize(width: width, height: .greatestFiniteMagnitude) - - layoutTextLayer() - layoutSelectionLayer() - layoutInsertionPointLayer() } } @@ -127,7 +123,11 @@ extension TextView: LayoutManagerDelegate { } func didInvalidateLayout(for layoutManager: LayoutManager) { - layoutTextLayer() + transaction { + schedule(.textLayout) + schedule(.insertionPointLayout) + schedule(.selectionLayout) + } updateInsertionPointTimer() inputContext?.invalidateCharacterCoordinates() diff --git a/Watt/TextView/TextView+Scrolling.swift b/Watt/TextView/TextView+Scrolling.swift index 9a41d8a1..d281d316 100644 --- a/Watt/TextView/TextView+Scrolling.swift +++ b/Watt/TextView/TextView+Scrolling.swift @@ -72,9 +72,11 @@ extension TextView { scroll(CGPoint(x: scrollOffset.x + dx, y: scrollOffset.y + dy)) } - @objc func didScroll(_ notification: Notification) { - layoutTextLayer() - layoutSelectionLayer() - layoutInsertionPointLayer() + @objc func clipViewDidScroll(_ notification: Notification) { + transaction { + schedule(.textLayout) + schedule(.selectionLayout) + schedule(.insertionPointLayout) + } } } diff --git a/Watt/TextView/TextView+Transactions.swift b/Watt/TextView/TextView+Transactions.swift new file mode 100644 index 00000000..96140ec6 --- /dev/null +++ b/Watt/TextView/TextView+Transactions.swift @@ -0,0 +1,66 @@ +// +// TextView+Transactions.swift +// Watt +// +// Created by David Albert on 3/19/24. +// + +import Foundation +import OrderedCollections + +extension TextView { + enum Action { + case textLayout + case selectionLayout + case insertionPointLayout + } + + struct Transaction { + var count: Int + var actions: OrderedSet + + init() { + count = 0 + actions = [] + } + } + + func beginTransaction() { + transaction.count += 1 + } + + func endTransaction() { + precondition(transaction.count >= 1) + transaction.count -= 1 + + guard transaction.count == 0 else { + return + } + + for action in transaction.actions { + switch action { + case .textLayout: + layoutTextLayer() + case .selectionLayout: + layoutSelectionLayer() + case .insertionPointLayout: + layoutInsertionPointLayer() + } + } + + transaction.actions.removeAll(keepingCapacity: true) + } + + @discardableResult + func transaction(block: () -> T) -> T { + beginTransaction() + defer { endTransaction() } + return block() + } + + func schedule(_ action: Action) { + transaction { + transaction.actions.append(action) + } + } +} diff --git a/Watt/TextView/TextView.swift b/Watt/TextView/TextView.swift index ca68d803..2cb7e633 100644 --- a/Watt/TextView/TextView.swift +++ b/Watt/TextView/TextView.swift @@ -89,7 +89,9 @@ class TextView: NSView, ClipViewDelegate { layoutManager.buffer } set { - layoutManager.buffer = newValue + transaction { + layoutManager.buffer = newValue + } } } @@ -99,8 +101,10 @@ class TextView: NSView, ClipViewDelegate { didSet { setTypingAttributes() - layoutSelectionLayer() - layoutInsertionPointLayer() + transaction { + schedule(.selectionLayout) + schedule(.insertionPointLayout) + } updateInsertionPointTimer() } } @@ -142,6 +146,8 @@ class TextView: NSView, ClipViewDelegate { var autoscroller: Autoscroller? + var transaction: Transaction = Transaction() + // HACK: See layoutTextLayer() for context. var previousVisibleRect: CGRect = .zero @@ -218,22 +224,38 @@ class TextView: NSView, ClipViewDelegate { addLineNumberView() } + func clipView(_ clipView: ClipView, frameSizeDidChangeFrom oldSize: NSSize) { + let heightChanged = oldSize.height != clipView.frame.height + let widthChanged = oldSize.width != clipView.frame.width + + if heightChanged || (widthChanged && textContainer.width < .greatestFiniteMagnitude) { + transaction { + schedule(.textLayout) + schedule(.selectionLayout) + schedule(.insertionPointLayout) + } + } + } + override func viewDidMoveToWindow() { NotificationCenter.default.removeObserver(self, name: NSView.boundsDidChangeNotification, object: nil) NotificationCenter.default.removeObserver(self, name: NSWindow.didBecomeKeyNotification, object: nil) NotificationCenter.default.removeObserver(self, name: NSWindow.didResignKeyNotification, object: nil) if let scrollView { - NotificationCenter.default.addObserver(self, selector: #selector(didScroll(_:)), name: NSView.boundsDidChangeNotification, object: scrollView.contentView) + NotificationCenter.default.addObserver(self, selector: #selector(clipViewDidScroll(_:)), name: NSView.boundsDidChangeNotification, object: scrollView.contentView) + } if let window { NotificationCenter.default.addObserver(self, selector: #selector(windowDidBecomeKey(_:)), name: NSWindow.didBecomeKeyNotification, object: window) NotificationCenter.default.addObserver(self, selector: #selector(windowDidResignKey(_:)), name: NSWindow.didResignKeyNotification, object: window) - layoutTextLayer() - layoutSelectionLayer() - layoutInsertionPointLayer() + transaction { + schedule(.textLayout) + schedule(.selectionLayout) + schedule(.insertionPointLayout) + } } } } From 5d6a1335a24cdf4303787e6473e41a7fddb24620 Mon Sep 17 00:00:00 2001 From: David Albert Date: Tue, 19 Mar 2024 16:14:33 -0400 Subject: [PATCH 09/73] =?UTF-8?q?Don=E2=80=99t=20lay=20out=20extraneous=20?= =?UTF-8?q?lines=20if=20our=20initial=20height=20estimate=20was=20too=20sm?= =?UTF-8?q?all?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Watt/LayoutManager/LayoutManager.swift | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Watt/LayoutManager/LayoutManager.swift b/Watt/LayoutManager/LayoutManager.swift index f92e85d8..4e4db462 100644 --- a/Watt/LayoutManager/LayoutManager.swift +++ b/Watt/LayoutManager/LayoutManager.swift @@ -87,7 +87,17 @@ class LayoutManager { delegate.layoutManager(self, positionLineLayer: layer) layers.append(layer) block(layer, prevAlignmentFrame) - return true + + // viewportRange is derived from heights, and performing layout can cause heights to change. + // If heights are growing, e.g. from an estimates to an actual heights, it's possible for + // viewportRange to be too big, and we'd end up laying out lines that we don't need. + // + // To prevent this, bail as soon as the viewport is full. + // + // TODO: what about when line height shrinks. Then viewportRange will be too short, and we'll + // miss paragraphs. Probably what we want is enumerateLines(startingAt:), which is what + // NSTextLayoutManager has, and then bail when we're ready. + return line.alignmentFrame.maxY <= viewportBounds.maxY } lineLayers = layers } From 30510dc44d9ddc6457a5fd4204a0f5a36850d001 Mon Sep 17 00:00:00 2001 From: David Albert Date: Tue, 19 Mar 2024 16:26:20 -0400 Subject: [PATCH 10/73] Fix bug where changing a theme would not change the background color --- Watt/TextView/TextView.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Watt/TextView/TextView.swift b/Watt/TextView/TextView.swift index 2cb7e633..3cc15788 100644 --- a/Watt/TextView/TextView.swift +++ b/Watt/TextView/TextView.swift @@ -168,7 +168,7 @@ class TextView: NSView, ClipViewDelegate { } func commonInit() { - layerContentsRedrawPolicy = .never + layerContentsRedrawPolicy = .onSetNeedsDisplay layoutManager.buffer = buffer From 2b6aaf178345a0f2cb97068b37e4b9fe8601a863 Mon Sep 17 00:00:00 2001 From: David Albert Date: Tue, 19 Mar 2024 16:26:48 -0400 Subject: [PATCH 11/73] Simplify layout invalidation on buffer reloading --- Watt/LayoutManager/LayoutManager.swift | 1 - Watt/TextView/TextView+Layout.swift | 12 +++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/Watt/LayoutManager/LayoutManager.swift b/Watt/LayoutManager/LayoutManager.swift index 4e4db462..bd5c238f 100644 --- a/Watt/LayoutManager/LayoutManager.swift +++ b/Watt/LayoutManager/LayoutManager.swift @@ -725,7 +725,6 @@ class LayoutManager { heights = Heights(rope: buffer.text) lineLayers = [] delegate?.layoutManager(self, bufferDidReload: buffer) - delegate?.didInvalidateLayout(for: self) } // MARK: - Converting coordinates diff --git a/Watt/TextView/TextView+Layout.swift b/Watt/TextView/TextView+Layout.swift index 2bc1be06..a686d048 100644 --- a/Watt/TextView/TextView+Layout.swift +++ b/Watt/TextView/TextView+Layout.swift @@ -174,7 +174,17 @@ extension TextView: LayoutManagerDelegate { func layoutManager(_ layoutManager: LayoutManager, bufferDidReload buffer: Buffer) { lineNumberView.lineCount = buffer.lines.count - selection = Selection(atStartOf: buffer) + + transaction { + schedule(.textLayout) + schedule(.insertionPointLayout) + schedule(.selectionLayout) + + selection = Selection(atStartOf: buffer) + } + + inputContext?.invalidateCharacterCoordinates() + updateFrameHeightIfNeeded() } // TODO: once we're showing the same Buffer in more than one TextView, editing the text in one TextView From 80909fef6270334fcb1519aa2ddc77d03f96db9c Mon Sep 17 00:00:00 2001 From: David Albert Date: Tue, 19 Mar 2024 16:37:25 -0400 Subject: [PATCH 12/73] =?UTF-8?q?Fix=20bug=20where=20forground=20color=20d?= =?UTF-8?q?idn=E2=80=99t=20change=20when=20theme=20changed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Watt/TextView/TextView.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Watt/TextView/TextView.swift b/Watt/TextView/TextView.swift index 3cc15788..b9ba3c14 100644 --- a/Watt/TextView/TextView.swift +++ b/Watt/TextView/TextView.swift @@ -37,8 +37,10 @@ class TextView: NSView, ClipViewDelegate { var theme: Theme = .system { didSet { + defaultAttributes.foregroundColor = theme.foregroundColor layoutManager.invalidateLayout() needsDisplay = true + lineNumberView.textColor = theme.lineNumberColor lineNumberView.backgroundColor = theme.backgroundColor } From 2c0ed275bd7e9c3142b73e27bc013f0f6e44da3d Mon Sep 17 00:00:00 2001 From: David Albert Date: Tue, 19 Mar 2024 16:41:35 -0400 Subject: [PATCH 13/73] Fix a bug where inserting text was broken --- Watt/TextView/TextView+Input.swift | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Watt/TextView/TextView+Input.swift b/Watt/TextView/TextView+Input.swift index eee7e9c2..8f97d6ef 100644 --- a/Watt/TextView/TextView+Input.swift +++ b/Watt/TextView/TextView+Input.swift @@ -204,7 +204,10 @@ extension TextView { let undoContents = AttributedRope(buffer[r]) - buffer.replaceSubrange(r, with: s) + transaction { + buffer.replaceSubrange(r, with: s) + updateStateAfterReplacingSubrange(r, withStringOfCount: s.count) + } let undoRange = r.lowerBound.position..<(r.lowerBound.position + s.count) @@ -212,8 +215,6 @@ extension TextView { let u = Range(undoRange, in: target.buffer) target.replaceSubrange(u, with: undoContents) } - - updateStateAfterReplacingSubrange(r, withStringOfCount: s.count) } func replaceSubrange(_ subrange: Range, with s: String) { From a9eb29f39a6d1a53918e213c3bc7062f8c70f480 Mon Sep 17 00:00:00 2001 From: David Albert Date: Tue, 19 Mar 2024 17:01:07 -0400 Subject: [PATCH 14/73] Fix creating NSRange from Subrope --- Watt/Rope/Rope.swift | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Watt/Rope/Rope.swift b/Watt/Rope/Rope.swift index 1cc4634b..d7f45e8f 100644 --- a/Watt/Rope/Rope.swift +++ b/Watt/Rope/Rope.swift @@ -1881,6 +1881,9 @@ extension NSRange { // Don't use for user provided ranges. init(unvalidatedRange range: Range, in rope: Rope) { + range.lowerBound.assertValid(for: rope.base) + range.upperBound.assertValid(for: rope.base) + let i = rope.root.count(.utf16, upThrough: range.lowerBound.position, edge: .trailing) let j = rope.root.count(.utf16, upThrough: range.upperBound.position, edge: .trailing) @@ -1888,8 +1891,14 @@ extension NSRange { } init(unvalidatedRange range: Range, in subrope: Subrope) { + range.lowerBound.assertValid(for: subrope.base) + range.upperBound.assertValid(for: subrope.base) + assert(range.lowerBound.position >= subrope.startIndex.position && range.lowerBound.position <= subrope.endIndex.position) + assert(range.upperBound.position >= subrope.startIndex.position && range.upperBound.position <= subrope.endIndex.position) + let nsRange = NSRange(unvalidatedRange: range, in: subrope.base) - self.init(location: nsRange.location - subrope.bounds.lowerBound.position, length: nsRange.length) + let start = subrope.root.count(.utf16, upThrough: subrope.startIndex.position, edge: .trailing) + self.init(location: nsRange.location - start, length: nsRange.length) } } From 6868616b7ecf9a4579956538e18559afadd9c808 Mon Sep 17 00:00:00 2001 From: David Albert Date: Tue, 19 Mar 2024 17:01:35 -0400 Subject: [PATCH 15/73] Transaction in insertText --- Watt/TextView/TextView+Input.swift | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Watt/TextView/TextView+Input.swift b/Watt/TextView/TextView+Input.swift index 8f97d6ef..a02c350c 100644 --- a/Watt/TextView/TextView+Input.swift +++ b/Watt/TextView/TextView+Input.swift @@ -32,10 +32,11 @@ extension TextView: NSTextInputClient { attrRope = AttributedRope(string as! String, attributes: typingAttributes) } - replaceSubrange(range, with: attrRope) - - print("insertText - ", terminator: "") - unmarkText() + transaction { + replaceSubrange(range, with: attrRope) + print("insertText - ", terminator: "") + unmarkText() + } } override func doCommand(by selector: Selector) { From 80101beada7707c96fdc6308e88b57e5e30ba6c3 Mon Sep 17 00:00:00 2001 From: David Albert Date: Tue, 19 Mar 2024 17:05:39 -0400 Subject: [PATCH 16/73] Add missing transactions (this is annoying) --- Watt/TextView/TextView+Input.swift | 39 ++++++----- Watt/TextView/TextView+KeyBinding.swift | 88 ++++++++++++++++--------- 2 files changed, 78 insertions(+), 49 deletions(-) diff --git a/Watt/TextView/TextView+Input.swift b/Watt/TextView/TextView+Input.swift index a02c350c..f5730e3a 100644 --- a/Watt/TextView/TextView+Input.swift +++ b/Watt/TextView/TextView+Input.swift @@ -59,24 +59,26 @@ extension TextView: NSTextInputClient { attrRope = AttributedRope(string as! String, attributes: typingAttributes.merging(markedTextAttributes)) } - replaceSubrange(range, with: attrRope) + transaction { + replaceSubrange(range, with: attrRope) - let start = buffer.index(fromOldIndex: range.lowerBound) - let anchor = buffer.utf16.index(start, offsetBy: selectedRange.lowerBound) - let head = buffer.utf16.index(anchor, offsetBy: selectedRange.length) + let start = buffer.index(fromOldIndex: range.lowerBound) + let anchor = buffer.utf16.index(start, offsetBy: selectedRange.lowerBound) + let head = buffer.utf16.index(anchor, offsetBy: selectedRange.length) - let markedRange: Range? - if attrRope.count == 0 { - markedRange = nil - } else { - let end = buffer.index(start, offsetBy: attrRope.count) - markedRange = start..? + if attrRope.count == 0 { + markedRange = nil + } else { + let end = buffer.index(start, offsetBy: attrRope.count) + markedRange = start.., withStringOfCount count: Int) { diff --git a/Watt/TextView/TextView+KeyBinding.swift b/Watt/TextView/TextView+KeyBinding.swift index bcd7acb7..9d67bb73 100644 --- a/Watt/TextView/TextView+KeyBinding.swift +++ b/Watt/TextView/TextView+KeyBinding.swift @@ -397,11 +397,13 @@ extension TextView { return } - replaceSubrange((i...j).relative(to: buffer.text), with: String(buffer[j]) + String(buffer[i])) - - let anchor = buffer.index(fromOldIndex: i) - let head = buffer.index(anchor, offsetBy: 2) - selection = Selection(anchor: anchor, head: head, granularity: .character) + transaction { + replaceSubrange((i...j).relative(to: buffer.text), with: String(buffer[j]) + String(buffer[i])) + + let anchor = buffer.index(fromOldIndex: i) + let head = buffer.index(anchor, offsetBy: 2) + selection = Selection(anchor: anchor, head: head, granularity: .character) + } } override func transposeWords(_ sender: Any?) { @@ -414,12 +416,14 @@ extension TextView { b.push(buffer[word1.upperBound.. Date: Tue, 19 Mar 2024 17:19:34 -0400 Subject: [PATCH 17/73] =?UTF-8?q?Autoscroll:=20don=E2=80=99t=20update=20se?= =?UTF-8?q?lection=20unless=20location=20in=20view=20changes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Watt/TextView/TextView+Mouse.swift | 7 +++++++ Watt/Utilities/Autoscroller.swift | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/Watt/TextView/TextView+Mouse.swift b/Watt/TextView/TextView+Mouse.swift index 1786b5df..32042c4d 100644 --- a/Watt/TextView/TextView+Mouse.swift +++ b/Watt/TextView/TextView+Mouse.swift @@ -29,8 +29,15 @@ extension TextView { selection = SelectionNavigator(selection).selection(for: .paragraph, enclosing: point, dataSource: layoutManager) } + var lastLocationInView: CGPoint? autoscroller = Autoscroller(self, event: event) { [weak self] locationInView in guard let self else { return } + defer { lastLocationInView = locationInView } + + if lastLocationInView == locationInView { + return + } + let clamped = locationInView.clamped(to: visibleRect) let point = convertToTextContainer(clamped) selection = SelectionNavigator(selection).selection(extendingTo: point, dataSource: layoutManager) diff --git a/Watt/Utilities/Autoscroller.swift b/Watt/Utilities/Autoscroller.swift index 4350f172..34e55891 100644 --- a/Watt/Utilities/Autoscroller.swift +++ b/Watt/Utilities/Autoscroller.swift @@ -47,7 +47,7 @@ class Autoscroller { private var displayLink: CADisplayLink! - init(_ view: NSView, event: NSEvent, using block: @escaping (CGPoint) -> Void) { + init(_ view: NSView, event: NSEvent, using block: @escaping (NSPoint) -> Void) { self.view = view self.callback = block self.locationInWindow = event.locationInWindow From 2aab3bc3a7d8e74c140937cfd984731e5a9185fa Mon Sep 17 00:00:00 2001 From: David Albert Date: Tue, 19 Mar 2024 17:44:31 -0400 Subject: [PATCH 18/73] Autoscroll: disable CADisplayLink when mouse is inside the viewport --- Watt/Utilities/Autoscroller.swift | 39 +++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/Watt/Utilities/Autoscroller.swift b/Watt/Utilities/Autoscroller.swift index 34e55891..03ab1e87 100644 --- a/Watt/Utilities/Autoscroller.swift +++ b/Watt/Utilities/Autoscroller.swift @@ -43,10 +43,9 @@ class Autoscroller { private var dxRemainder: CGFloat private var dyRemainder: CGFloat - private(set) var running: Bool + private(set) var enabled: Bool private var displayLink: CADisplayLink! - init(_ view: NSView, event: NSEvent, using block: @escaping (NSPoint) -> Void) { self.view = view self.callback = block @@ -55,7 +54,7 @@ class Autoscroller { self.dxRemainder = 0 self.dyRemainder = 0 - self.running = false + self.enabled = false self.displayLink = view.displayLink(target: self, selector: #selector(tick(_:))) displayLink.preferredFrameRateRange = Self.preferredFrameRateRange @@ -68,22 +67,48 @@ class Autoscroller { func update(with event: NSEvent) { precondition(mouseEvents.contains(event.type), "Expected mouse event but got \(event.type).") locationInWindow = event.locationInWindow + + if displayLink.isPaused, let view { + let locationInView = view.convert(locationInWindow, from: nil) + callback(locationInView) + } + + if displayLink.isPaused != shouldPauseDisplayLink() { + displayLink.isPaused.toggle() + } + } + + private func shouldPauseDisplayLink() -> Bool { + guard let view else { + return true + } + + guard let scrollView = view.enclosingScrollView, scrollView.documentView == view else { + return true + } + + let locationInView = view.convert(locationInWindow, from: nil) + let viewportBounds = scrollView.contentView.bounds + + return locationInView.x > viewportBounds.minX && locationInView.x < viewportBounds.maxX && + locationInView.y > viewportBounds.minY && locationInView.y < viewportBounds.maxY } func start() { - if running || view == nil { + if enabled || view == nil { return } + enabled = true + displayLink.isPaused = shouldPauseDisplayLink() displayLink.add(to: .main, forMode: .common) - running = true } func stop() { - if !running { + if !enabled { return } + enabled = false displayLink.remove(from: .main, forMode: .common) - running = false } @objc func tick(_ sender: CADisplayLink) { From 61490be5dd9cb617441aa4ae391d45a1bff49ca3 Mon Sep 17 00:00:00 2001 From: David Albert Date: Tue, 19 Mar 2024 17:53:08 -0400 Subject: [PATCH 19/73] Round to the point boundary before deciding whether to change selection while autoscrolling --- Watt/Extensions/CGPoint+Extensions.swift | 4 ++++ Watt/TextView/TextView+Mouse.swift | 7 ++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/Watt/Extensions/CGPoint+Extensions.swift b/Watt/Extensions/CGPoint+Extensions.swift index 7e84b04f..53190bed 100644 --- a/Watt/Extensions/CGPoint+Extensions.swift +++ b/Watt/Extensions/CGPoint+Extensions.swift @@ -18,4 +18,8 @@ extension CGPoint { func clamped(to rect: CGRect) -> CGPoint { CGPoint(x: x.clamped(to: rect.minX...rect.maxX), y: y.clamped(to: rect.minY...rect.maxY)) } + + func rounded() -> CGPoint { + CGPoint(x: x.rounded(), y: y.rounded()) + } } diff --git a/Watt/TextView/TextView+Mouse.swift b/Watt/TextView/TextView+Mouse.swift index 32042c4d..904114b4 100644 --- a/Watt/TextView/TextView+Mouse.swift +++ b/Watt/TextView/TextView+Mouse.swift @@ -29,12 +29,13 @@ extension TextView { selection = SelectionNavigator(selection).selection(for: .paragraph, enclosing: point, dataSource: layoutManager) } - var lastLocationInView: CGPoint? + var lastRoundedLocation: CGPoint? autoscroller = Autoscroller(self, event: event) { [weak self] locationInView in guard let self else { return } - defer { lastLocationInView = locationInView } + let roundedLocation = locationInView.rounded() + defer { lastRoundedLocation = roundedLocation } - if lastLocationInView == locationInView { + if lastRoundedLocation == roundedLocation { return } From 21e04adfdb47eee4ab65c956aac902b812442891 Mon Sep 17 00:00:00 2001 From: David Albert Date: Thu, 21 Mar 2024 16:43:54 -0400 Subject: [PATCH 20/73] Dragging to the bottom always ends up at the bottom of the document --- Watt/TextView/TextView+Layout.swift | 25 +++++-------------------- Watt/TextView/TextView+Scrolling.swift | 12 ++++++++++++ Watt/TextView/TextView.swift | 7 ++++++- 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/Watt/TextView/TextView+Layout.swift b/Watt/TextView/TextView+Layout.swift index a686d048..5d8d5815 100644 --- a/Watt/TextView/TextView+Layout.swift +++ b/Watt/TextView/TextView+Layout.swift @@ -233,6 +233,8 @@ extension TextView { var lineno: Int? + let scrollingUp = visibleRect.minY < previousVisibleRect.minY + var layers: [CALayer] = [] layoutManager.layoutText { layer, prevAlignmentFrame in layers.append(layer) @@ -240,19 +242,8 @@ extension TextView { let oldHeight = prevAlignmentFrame.height let newHeight = layer.line.alignmentFrame.height let delta = newHeight - oldHeight - let oldMaxY = layer.line.origin.y + oldHeight - - // TODO: I don't know why I have to use the previous frame's - // visible rect here. My best guess is that it has something - // to do with the fact that I'm doing deferred layout of my - // sublayers (e.g. textLayer.setNeedsLayout(), etc.). I tried - // changing the deferred layout calls in prepareContent(in:) - // to immediate layout calls, but it didn't seem to fix the - // problem. On the other hand, I'm not sure if I've totally - // gotten scroll correction right here anyways (there are - // sometimes things that look like jumps during scrolling). - // I'll come back to this later. - if oldMaxY <= previousVisibleRect.minY && delta != 0 { + + if scrollingUp || isDraggingScroller { scrollAdjustment += delta } @@ -275,18 +266,12 @@ extension TextView { } previousVisibleRect = visibleRect + updateFrameHeightIfNeeded() - // Adjust scroll offset. - // TODO: is it possible to move this into prepareContent(in:) directly? - // That way it would only happen when we scroll. It's also possible - // that would let us get rid of previousVisibleRect, but according to - // the comment below, I tried that, so I'm doubtful. if scrollAdjustment != 0 { let current = scrollOffset scroll(CGPoint(x: current.x, y: current.y + scrollAdjustment)) } - - updateFrameHeightIfNeeded() } } diff --git a/Watt/TextView/TextView+Scrolling.swift b/Watt/TextView/TextView+Scrolling.swift index d281d316..19e9c599 100644 --- a/Watt/TextView/TextView+Scrolling.swift +++ b/Watt/TextView/TextView+Scrolling.swift @@ -79,4 +79,16 @@ extension TextView { schedule(.insertionPointLayout) } } + + @objc func willStartLiveScroll(_ notification: Notification) { + guard let scrollView = notification.object as? NSScrollView else { + return + } + + isDraggingScroller = scrollView.verticalScroller?.hitPart == .knob + } + + @objc func didEndLiveScroll(_ notification: Notification) { + isDraggingScroller = false + } } diff --git a/Watt/TextView/TextView.swift b/Watt/TextView/TextView.swift index b9ba3c14..3bee1404 100644 --- a/Watt/TextView/TextView.swift +++ b/Watt/TextView/TextView.swift @@ -152,6 +152,7 @@ class TextView: NSView, ClipViewDelegate { // HACK: See layoutTextLayer() for context. var previousVisibleRect: CGRect = .zero + var isDraggingScroller: Bool = false override init(frame frameRect: NSRect) { layoutManager = LayoutManager() @@ -241,12 +242,16 @@ class TextView: NSView, ClipViewDelegate { override func viewDidMoveToWindow() { NotificationCenter.default.removeObserver(self, name: NSView.boundsDidChangeNotification, object: nil) + NotificationCenter.default.removeObserver(self, name: NSScrollView.willStartLiveScrollNotification, object: nil) + NotificationCenter.default.removeObserver(self, name: NSScrollView.didEndLiveScrollNotification, object: nil) + NotificationCenter.default.removeObserver(self, name: NSWindow.didBecomeKeyNotification, object: nil) NotificationCenter.default.removeObserver(self, name: NSWindow.didResignKeyNotification, object: nil) if let scrollView { NotificationCenter.default.addObserver(self, selector: #selector(clipViewDidScroll(_:)), name: NSView.boundsDidChangeNotification, object: scrollView.contentView) - + NotificationCenter.default.addObserver(self, selector: #selector(willStartLiveScroll(_:)), name: NSScrollView.willStartLiveScrollNotification, object: scrollView) + NotificationCenter.default.addObserver(self, selector: #selector(didEndLiveScroll(_:)), name: NSScrollView.didEndLiveScrollNotification, object: scrollView) } if let window { From 8a3ceca948e3df6c81e7af25bb720df80f674395 Mon Sep 17 00:00:00 2001 From: David Albert Date: Thu, 21 Mar 2024 17:13:43 -0400 Subject: [PATCH 21/73] A bit of cleanup --- Watt/TextView/TextView+Layout.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Watt/TextView/TextView+Layout.swift b/Watt/TextView/TextView+Layout.swift index 5d8d5815..666f1e0c 100644 --- a/Watt/TextView/TextView+Layout.swift +++ b/Watt/TextView/TextView+Layout.swift @@ -225,7 +225,9 @@ extension TextView { return } + let scrollingUp = visibleRect.minY < previousVisibleRect.minY var scrollAdjustment: CGFloat = 0 + let updateLineNumbers = lineNumberView.superview != nil if updateLineNumbers { lineNumberView.beginUpdates() @@ -233,8 +235,6 @@ extension TextView { var lineno: Int? - let scrollingUp = visibleRect.minY < previousVisibleRect.minY - var layers: [CALayer] = [] layoutManager.layoutText { layer, prevAlignmentFrame in layers.append(layer) From 2cf1b4bd50dd7e7bdba70a246d5f12c67442e819 Mon Sep 17 00:00:00 2001 From: David Albert Date: Thu, 21 Mar 2024 19:04:31 -0400 Subject: [PATCH 22/73] Disallow starting a new transaction while committing an existing one Also make LayoutManager.layoutText(using:) take into account a moving viewport due to scroll correction --- Watt/LayoutManager/LayoutManager.swift | 38 ++++++++++++++--------- Watt/TextView/TextView+Layout.swift | 9 ++++++ Watt/TextView/TextView+Scrolling.swift | 7 +++++ Watt/TextView/TextView+Transactions.swift | 6 ++++ Watt/TextView/TextView.swift | 1 + 5 files changed, 46 insertions(+), 15 deletions(-) diff --git a/Watt/LayoutManager/LayoutManager.swift b/Watt/LayoutManager/LayoutManager.swift index bd5c238f..16d446c0 100644 --- a/Watt/LayoutManager/LayoutManager.swift +++ b/Watt/LayoutManager/LayoutManager.swift @@ -12,6 +12,8 @@ import StandardKeyBindingResponder protocol LayoutManagerDelegate: AnyObject { // Text container coordinates. func viewportBounds(for layoutManager: LayoutManager) -> CGRect + func willPerformScrollCorrection(for layoutManager: LayoutManager) -> Bool + func didInvalidateLayout(for layoutManager: LayoutManager) func defaultAttributes(for layoutManager: LayoutManager) -> AttributedRope.Attributes @@ -78,16 +80,23 @@ class LayoutManager { return } - let viewportBounds = delegate.viewportBounds(for: self) - let viewportRange = characterRange(intersecting: viewportBounds) + let willPerformScrollCorrection = delegate.willPerformScrollCorrection(for: self) + var viewportBounds = delegate.viewportBounds(for: self) + let start = startOfFirstLine(intersecting: viewportBounds) var layers: [LineLayer] = [] - enumerateLines(in: viewportRange) { line, existingLayer, prevAlignmentFrame in + enumerateLines(startingAt: start) { line, existingLayer, prevAlignmentFrame in let layer = existingLayer ?? delegate.layoutManager(self, createLayerForLine: line) delegate.layoutManager(self, positionLineLayer: layer) layers.append(layer) block(layer, prevAlignmentFrame) + if willPerformScrollCorrection { + let oldHeight = prevAlignmentFrame.height + let newHeight = line.alignmentFrame.height + viewportBounds.origin.y += newHeight - oldHeight + } + // viewportRange is derived from heights, and performing layout can cause heights to change. // If heights are growing, e.g. from an estimates to an actual heights, it's possible for // viewportRange to be too big, and we'd end up laying out lines that we don't need. @@ -194,7 +203,7 @@ class LayoutManager { } func enumerateTextSegments(in range: Range, type: SegmentType, using block: (Range, CGRect) -> Bool) { - enumerateLines(in: range) { line, _, _ in + enumerateLines(startingAt: range.lowerBound) { line, _, _ in for frag in line.lineFragments { let rangesOverlap = range.overlaps(frag.range) || range.isEmpty && frag.range.contains(range.lowerBound) let atEndOfDocument = frag.range.upperBound == buffer.endIndex && frag.range.upperBound == range.lowerBound @@ -249,17 +258,9 @@ class LayoutManager { // range - the range of the line in buffer // line - the line // previousBounds - The (possibly estimated) bounds of the line before layout was performed. If the line was already laid out, this is equal to line.typographicBounds. - func enumerateLines(in range: Range, using block: (_ line: Line, _ layer: LineLayer?, _ prevAlignmentFrame: CGRect) -> Bool) { - var i = buffer.lines.index(roundingDown: range.lowerBound) - - let end: Buffer.Index - if range.upperBound == buffer.endIndex { - end = buffer.endIndex - } else if buffer.lines.isBoundary(range.upperBound) && range.upperBound > i { - end = range.upperBound - } else { - end = buffer.lines.index(after: range.upperBound) - } + func enumerateLines(startingAt index: Buffer.Index, using block: (_ line: Line, _ layer: LineLayer?, _ prevAlignmentFrame: CGRect) -> Bool) { + var i = buffer.lines.index(roundingDown: index) + let end = buffer.endIndex var y = heights.yOffset(upThroughPosition: i.position) @@ -691,6 +692,13 @@ class LayoutManager { return (glyphOrigin, typographicBounds) } + func startOfFirstLine(intersecting rect: CGRect) -> Buffer.Index { + let byteOffset = heights.position(upThroughYOffset: rect.minY) + let i = buffer.utf8.index(at: byteOffset) + assert(buffer.lines.isBoundary(i)) + return i + } + // Returns the range of the buffer contained by rect. The start // and end of the range are rounded down and up to the nearest line // boundary respectively, so that if you were to lay out those lines, diff --git a/Watt/TextView/TextView+Layout.swift b/Watt/TextView/TextView+Layout.swift index 666f1e0c..9ef347e6 100644 --- a/Watt/TextView/TextView+Layout.swift +++ b/Watt/TextView/TextView+Layout.swift @@ -122,6 +122,12 @@ extension TextView: LayoutManagerDelegate { textContainerVisibleRect } + func willPerformScrollCorrection(for layoutManager: LayoutManager) -> Bool { + // TODO: currently duplicated between here and layoutTextLayer(). Remove duplication once LayoutManager becomes Editor. + let scrollingUp = visibleRect.minY < previousVisibleRect.minY + return scrollingUp || isDraggingScroller + } + func didInvalidateLayout(for layoutManager: LayoutManager) { transaction { schedule(.textLayout) @@ -243,6 +249,7 @@ extension TextView { let newHeight = layer.line.alignmentFrame.height let delta = newHeight - oldHeight + // TODO: currently duplicated in willPerformScrollCorrection(for:). Remove duplication once LayoutManager becomes Editor. if scrollingUp || isDraggingScroller { scrollAdjustment += delta } @@ -270,7 +277,9 @@ extension TextView { if scrollAdjustment != 0 { let current = scrollOffset + performingScrollCorrection = true scroll(CGPoint(x: current.x, y: current.y + scrollAdjustment)) + performingScrollCorrection = false } } } diff --git a/Watt/TextView/TextView+Scrolling.swift b/Watt/TextView/TextView+Scrolling.swift index 19e9c599..15ef53c0 100644 --- a/Watt/TextView/TextView+Scrolling.swift +++ b/Watt/TextView/TextView+Scrolling.swift @@ -73,6 +73,13 @@ extension TextView { } @objc func clipViewDidScroll(_ notification: Notification) { + // As part of textLayout, we may end up calling NSView.scroll(_:) in order to change our + // scroll offset in response to a change in document height. In that case, we're in the + // middle of committing a transaction, and starting a new one would panic. + if performingScrollCorrection { + return + } + transaction { schedule(.textLayout) schedule(.selectionLayout) diff --git a/Watt/TextView/TextView+Transactions.swift b/Watt/TextView/TextView+Transactions.swift index 96140ec6..f8370adc 100644 --- a/Watt/TextView/TextView+Transactions.swift +++ b/Watt/TextView/TextView+Transactions.swift @@ -18,14 +18,17 @@ extension TextView { struct Transaction { var count: Int var actions: OrderedSet + var committing: Bool init() { count = 0 actions = [] + committing = false } } func beginTransaction() { + precondition(!transaction.committing, "can't start a transaction while committing an existing one") transaction.count += 1 } @@ -37,6 +40,9 @@ extension TextView { return } + transaction.committing = true + defer { transaction.committing = false } + for action in transaction.actions { switch action { case .textLayout: diff --git a/Watt/TextView/TextView.swift b/Watt/TextView/TextView.swift index 3bee1404..ab36269d 100644 --- a/Watt/TextView/TextView.swift +++ b/Watt/TextView/TextView.swift @@ -153,6 +153,7 @@ class TextView: NSView, ClipViewDelegate { // HACK: See layoutTextLayer() for context. var previousVisibleRect: CGRect = .zero var isDraggingScroller: Bool = false + var performingScrollCorrection: Bool = false override init(frame frameRect: NSRect) { layoutManager = LayoutManager() From bf4b6cba1fd7c2989b61f67c61e915c8565fbcdb Mon Sep 17 00:00:00 2001 From: David Albert Date: Mon, 25 Mar 2024 14:07:06 -0400 Subject: [PATCH 23/73] clipViewDidScroll -> viewDidScroll --- Watt/TextView/TextView+Scrolling.swift | 2 +- Watt/TextView/TextView.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Watt/TextView/TextView+Scrolling.swift b/Watt/TextView/TextView+Scrolling.swift index 15ef53c0..2469bc16 100644 --- a/Watt/TextView/TextView+Scrolling.swift +++ b/Watt/TextView/TextView+Scrolling.swift @@ -72,7 +72,7 @@ extension TextView { scroll(CGPoint(x: scrollOffset.x + dx, y: scrollOffset.y + dy)) } - @objc func clipViewDidScroll(_ notification: Notification) { + @objc func viewDidScroll(_ notification: Notification) { // As part of textLayout, we may end up calling NSView.scroll(_:) in order to change our // scroll offset in response to a change in document height. In that case, we're in the // middle of committing a transaction, and starting a new one would panic. diff --git a/Watt/TextView/TextView.swift b/Watt/TextView/TextView.swift index ab36269d..5d324092 100644 --- a/Watt/TextView/TextView.swift +++ b/Watt/TextView/TextView.swift @@ -250,7 +250,7 @@ class TextView: NSView, ClipViewDelegate { NotificationCenter.default.removeObserver(self, name: NSWindow.didResignKeyNotification, object: nil) if let scrollView { - NotificationCenter.default.addObserver(self, selector: #selector(clipViewDidScroll(_:)), name: NSView.boundsDidChangeNotification, object: scrollView.contentView) + NotificationCenter.default.addObserver(self, selector: #selector(viewDidScroll(_:)), name: NSView.boundsDidChangeNotification, object: scrollView.contentView) NotificationCenter.default.addObserver(self, selector: #selector(willStartLiveScroll(_:)), name: NSScrollView.willStartLiveScrollNotification, object: scrollView) NotificationCenter.default.addObserver(self, selector: #selector(didEndLiveScroll(_:)), name: NSScrollView.didEndLiveScrollNotification, object: scrollView) } From c6abb2f4a6fe9b656b8585bd045144e342b79d49 Mon Sep 17 00:00:00 2001 From: David Albert Date: Mon, 25 Mar 2024 14:31:55 -0400 Subject: [PATCH 24/73] WIP: smooth scroll animation --- Watt.xcodeproj/project.pbxproj | 23 +++++- .../xcshareddata/swiftpm/Package.resolved | 18 +++++ Watt/TextView/TextView+KeyBinding.swift | 12 +-- Watt/TextView/TextView+Layout.swift | 2 + Watt/TextView/TextView.swift | 3 + Watt/Utilities/ScrollAnimator.swift | 74 +++++++++++++++++++ 6 files changed, 125 insertions(+), 7 deletions(-) create mode 100644 Watt/Utilities/ScrollAnimator.swift diff --git a/Watt.xcodeproj/project.pbxproj b/Watt.xcodeproj/project.pbxproj index b0278b4c..fd0daf75 100644 --- a/Watt.xcodeproj/project.pbxproj +++ b/Watt.xcodeproj/project.pbxproj @@ -14,6 +14,8 @@ 63206D2A2BA74AF600CAFBC4 /* TextView+Scrolling.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63206D292BA74AF600CAFBC4 /* TextView+Scrolling.swift */; }; 63206D2D2BA8E8F400CAFBC4 /* Autoscroller.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63206D2C2BA8E8F400CAFBC4 /* Autoscroller.swift */; }; 63206D312BA9FFD300CAFBC4 /* TextView+Transactions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63206D302BA9FFD300CAFBC4 /* TextView+Transactions.swift */; }; + 63206D362BAB635500CAFBC4 /* Motion in Frameworks */ = {isa = PBXBuildFile; productRef = 63206D352BAB635500CAFBC4 /* Motion */; }; + 63206D382BAB707E00CAFBC4 /* ScrollAnimator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63206D372BAB707E00CAFBC4 /* ScrollAnimator.swift */; }; 63267C292B97BF0B00F6E968 /* BTreeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63267C282B97BF0B00F6E968 /* BTreeTests.swift */; }; 63286AE22A13DF5D00839B25 /* ClipView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63286AE12A13DF5D00839B25 /* ClipView.swift */; }; 632A73722B87C69F009F38C3 /* Range+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 632A73712B87C69F009F38C3 /* Range+Extensions.swift */; }; @@ -136,6 +138,7 @@ 63206D292BA74AF600CAFBC4 /* TextView+Scrolling.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TextView+Scrolling.swift"; sourceTree = ""; }; 63206D2C2BA8E8F400CAFBC4 /* Autoscroller.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Autoscroller.swift; sourceTree = ""; }; 63206D302BA9FFD300CAFBC4 /* TextView+Transactions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TextView+Transactions.swift"; sourceTree = ""; }; + 63206D372BAB707E00CAFBC4 /* ScrollAnimator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ScrollAnimator.swift; sourceTree = ""; }; 63267C282B97BF0B00F6E968 /* BTreeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTreeTests.swift; sourceTree = ""; }; 63286AE12A13DF5D00839B25 /* ClipView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ClipView.swift; sourceTree = ""; }; 632A73712B87C69F009F38C3 /* Range+Extensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Range+Extensions.swift"; sourceTree = ""; }; @@ -253,6 +256,7 @@ 63F30E672AA90012009929F7 /* TreeSitter in Frameworks */, 63DD20E72B76BBE500E55747 /* OrderedCollections in Frameworks */, 63359A3E2AA9115100082762 /* TreeSitterC in Frameworks */, + 63206D362BAB635500CAFBC4 /* Motion in Frameworks */, 63E32EB22AF3F51600EDB062 /* StandardKeyBindingResponder in Frameworks */, 635F815B2AA9198200553E23 /* TreeSitterObjC in Frameworks */, ); @@ -281,13 +285,14 @@ children = ( 63C2C0452B4DF34D00BFB976 /* SimpleProxy.h */, 63C2C0462B4DF34D00BFB976 /* SimpleProxy.m */, + 63206D2C2BA8E8F400CAFBC4 /* Autoscroller.swift */, 63DD20DA2B7278E400E55747 /* CheckedContinuationReference.swift */, 637E58532B8415A400C498EE /* DragAndDrop.swift */, 63A95D832B5462D60032E8B9 /* FSEvents.swift */, 63C2C03E2B4D95EB00BFB976 /* OutlineViewDiffableDataSource.swift */, + 63206D372BAB707E00CAFBC4 /* ScrollAnimator.swift */, 63B8B1752A9F890100F32C2C /* Utilities.swift */, 63A618712A0EAF2000E70B0E /* Weak.swift */, - 63206D2C2BA8E8F400CAFBC4 /* Autoscroller.swift */, ); path = Utilities; sourceTree = ""; @@ -530,6 +535,7 @@ 63E32EB12AF3F51600EDB062 /* StandardKeyBindingResponder */, 63C2C0412B4D97E700BFB976 /* Tree */, 63DD20E62B76BBE500E55747 /* OrderedCollections */, + 63206D352BAB635500CAFBC4 /* Motion */, ); productName = Watt; productReference = 6372888829FD8E19005B95E5 /* Watt.app */; @@ -613,6 +619,7 @@ 634A70A02B1F79CB00BDD0D3 /* XCRemoteSwiftPackageReference "CwlPreconditionTesting" */, 63C2C0402B4D97E700BFB976 /* XCRemoteSwiftPackageReference "Tree" */, 63DD20E52B76BBE500E55747 /* XCRemoteSwiftPackageReference "swift-collections" */, + 63206D342BAB635500CAFBC4 /* XCRemoteSwiftPackageReference "Motion" */, ); productRefGroup = 6372888929FD8E19005B95E5 /* Products */; projectDirPath = ""; @@ -706,6 +713,7 @@ 6378677D2A1BE3FE0039960E /* LineNumberLayer.swift in Sources */, 632A73722B87C69F009F38C3 /* Range+Extensions.swift in Sources */, 63F0468A2A65D3EA001B8DA8 /* BTree.swift in Sources */, + 63206D382BAB707E00CAFBC4 /* ScrollAnimator.swift in Sources */, 63DD20DB2B7278E400E55747 /* CheckedContinuationReference.swift in Sources */, 637867812A1BEA960039960E /* InsertionPointLayer.swift in Sources */, 63F25BE52A68A27600DEACCF /* LineFragment.swift in Sources */, @@ -1094,6 +1102,14 @@ /* End XCConfigurationList section */ /* Begin XCRemoteSwiftPackageReference section */ + 63206D342BAB635500CAFBC4 /* XCRemoteSwiftPackageReference "Motion" */ = { + isa = XCRemoteSwiftPackageReference; + repositoryURL = "https://github.com/b3ll/Motion"; + requirement = { + branch = main; + kind = branch; + }; + }; 63359A3C2AA9115100082762 /* XCRemoteSwiftPackageReference "tree-sitter-c" */ = { isa = XCRemoteSwiftPackageReference; repositoryURL = "https://github.com/tree-sitter/tree-sitter-c"; @@ -1137,6 +1153,11 @@ /* End XCRemoteSwiftPackageReference section */ /* Begin XCSwiftPackageProductDependency section */ + 63206D352BAB635500CAFBC4 /* Motion */ = { + isa = XCSwiftPackageProductDependency; + package = 63206D342BAB635500CAFBC4 /* XCRemoteSwiftPackageReference "Motion" */; + productName = Motion; + }; 63359A3D2AA9115100082762 /* TreeSitterC */ = { isa = XCSwiftPackageProductDependency; package = 63359A3C2AA9115100082762 /* XCRemoteSwiftPackageReference "tree-sitter-c" */; diff --git a/Watt.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/Watt.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index bbf293d2..a7ccee58 100644 --- a/Watt.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/Watt.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -18,6 +18,15 @@ "version" : "2.2.0" } }, + { + "identity" : "motion", + "kind" : "remoteSourceControl", + "location" : "https://github.com/b3ll/Motion", + "state" : { + "branch" : "main", + "revision" : "533e148d5f239cbdc5ad6b91b7c32b94bc005981" + } + }, { "identity" : "swift-collections", "kind" : "remoteSourceControl", @@ -27,6 +36,15 @@ "version" : "1.1.0" } }, + { + "identity" : "swift-numerics", + "kind" : "remoteSourceControl", + "location" : "https://github.com/apple/swift-numerics", + "state" : { + "revision" : "0a5bc04095a675662cf24757cc0640aa2204253b", + "version" : "1.0.2" + } + }, { "identity" : "tree", "kind" : "remoteSourceControl", diff --git a/Watt/TextView/TextView+KeyBinding.swift b/Watt/TextView/TextView+KeyBinding.swift index 9d67bb73..2af3150d 100644 --- a/Watt/TextView/TextView+KeyBinding.swift +++ b/Watt/TextView/TextView+KeyBinding.swift @@ -372,21 +372,21 @@ extension TextView { // I wonder if there's another way around this. override func scrollToBeginningOfDocument(_ sender: Any?) { let point = CGPoint( - x: textContainerScrollOffset.x, + x: scrollOffset.x, y: 0 ) - animator().scroll(convertFromTextContainer(point)) + scrollAnimator.scroll(to: point) } override func scrollToEndOfDocument(_ sender: Any?) { - let viewport = textContainerVisibleRect + let viewport = visibleRect let point = CGPoint( - x: textContainerScrollOffset.x, - y: layoutManager.contentHeight - viewport.height + x: scrollOffset.x, + y: frame.height - viewport.height ) - animator().scroll(convertFromTextContainer(point)) + scrollAnimator.scroll(to: point) } diff --git a/Watt/TextView/TextView+Layout.swift b/Watt/TextView/TextView+Layout.swift index 9ef347e6..65d6dc89 100644 --- a/Watt/TextView/TextView+Layout.swift +++ b/Watt/TextView/TextView+Layout.swift @@ -280,6 +280,8 @@ extension TextView { performingScrollCorrection = true scroll(CGPoint(x: current.x, y: current.y + scrollAdjustment)) performingScrollCorrection = false + // TODO: do we need this? I think so. + // scrollAnimator.didCorrectScroll(by: CGVector(dx: 0, dy: scrollAdjustment)) } } } diff --git a/Watt/TextView/TextView.swift b/Watt/TextView/TextView.swift index 5d324092..dbe69d1f 100644 --- a/Watt/TextView/TextView.swift +++ b/Watt/TextView/TextView.swift @@ -146,6 +146,9 @@ class TextView: NSView, ClipViewDelegate { var insertionPointLayerCache: WeakDictionary = WeakDictionary() var insertionPointTimer: Timer? + lazy var scrollAnimator: ScrollAnimator = { + ScrollAnimator(self) + }() var autoscroller: Autoscroller? var transaction: Transaction = Transaction() diff --git a/Watt/Utilities/ScrollAnimator.swift b/Watt/Utilities/ScrollAnimator.swift new file mode 100644 index 00000000..f64699b7 --- /dev/null +++ b/Watt/Utilities/ScrollAnimator.swift @@ -0,0 +1,74 @@ +// +// ScrollAnimator.swift +// Watt +// +// Created by David Albert on 3/20/24. +// + +import Cocoa +import Motion + +class ScrollAnimator { + weak var view: NSView? + private var animation: SpringAnimation? + + var isAnimating: Bool { + animation != nil + } + + init(_ view: NSView) { + self.view = view + + if let scrollView = view.enclosingScrollView { + NotificationCenter.default.addObserver(self, selector: #selector(willStartLiveScroll(_:)), name: NSScrollView.willStartLiveScrollNotification, object: scrollView) + } + } + + func scroll(to point: CGPoint) { + guard let scrollView = view?.enclosingScrollView else { + return + } + + let velocity = animation?.velocity ?? .zero + animation?.stop() + + let spring = SpringAnimation( + initialValue: scrollView.contentView.bounds.origin, + response: 0.2, + dampingRatio: 1.0, + environment: scrollView + ) + + spring.toValue = point + spring.velocity = velocity + spring.resolvingEpsilon = 0.000001 + + spring.onValueChanged() { [weak self] value in + self?.view?.scroll(value) + } + + spring.completion = { [weak self] in + self?.animation = nil + } + + spring.start() + animation = spring + } + + func didCorrectScroll(by delta: CGVector) { + guard let animation else { + return + } + + let old = animation.toValue + let new = CGPoint(x: old.x + delta.dx, y: old.y + delta.dy) + + scroll(to: new) + } + + @objc func willStartLiveScroll(_ notification: Notification) { + // If the user interrupts the scroll for any reason, stop the animation. + animation?.stop() + animation = nil + } +} From d82c23b6cb44fa5844007e013b2fa26c1880c141 Mon Sep 17 00:00:00 2001 From: David Albert Date: Mon, 25 Mar 2024 14:38:39 -0400 Subject: [PATCH 25/73] WIP: correct scroll offset while scrolling down --- Watt/TextView/TextView+Layout.swift | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Watt/TextView/TextView+Layout.swift b/Watt/TextView/TextView+Layout.swift index 65d6dc89..bd7bc820 100644 --- a/Watt/TextView/TextView+Layout.swift +++ b/Watt/TextView/TextView+Layout.swift @@ -125,7 +125,7 @@ extension TextView: LayoutManagerDelegate { func willPerformScrollCorrection(for layoutManager: LayoutManager) -> Bool { // TODO: currently duplicated between here and layoutTextLayer(). Remove duplication once LayoutManager becomes Editor. let scrollingUp = visibleRect.minY < previousVisibleRect.minY - return scrollingUp || isDraggingScroller + return scrollingUp || isDraggingScroller || scrollAnimator.isAnimating } func didInvalidateLayout(for layoutManager: LayoutManager) { @@ -250,7 +250,7 @@ extension TextView { let delta = newHeight - oldHeight // TODO: currently duplicated in willPerformScrollCorrection(for:). Remove duplication once LayoutManager becomes Editor. - if scrollingUp || isDraggingScroller { + if scrollingUp || isDraggingScroller || scrollAnimator.isAnimating { scrollAdjustment += delta } @@ -280,8 +280,7 @@ extension TextView { performingScrollCorrection = true scroll(CGPoint(x: current.x, y: current.y + scrollAdjustment)) performingScrollCorrection = false - // TODO: do we need this? I think so. - // scrollAnimator.didCorrectScroll(by: CGVector(dx: 0, dy: scrollAdjustment)) + scrollAnimator.didCorrectScroll(by: CGVector(dx: 0, dy: scrollAdjustment)) } } } From 156e34876d223a8e90bc7587b60cf792e87e3bc6 Mon Sep 17 00:00:00 2001 From: David Albert Date: Mon, 25 Mar 2024 14:53:09 -0400 Subject: [PATCH 26/73] Add TODO for smoother scrolling to document end --- Watt/TextView/TextView+KeyBinding.swift | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Watt/TextView/TextView+KeyBinding.swift b/Watt/TextView/TextView+KeyBinding.swift index 2af3150d..b694207d 100644 --- a/Watt/TextView/TextView+KeyBinding.swift +++ b/Watt/TextView/TextView+KeyBinding.swift @@ -380,6 +380,17 @@ extension TextView { } override func scrollToEndOfDocument(_ sender: Any?) { + // TODO: if the scroll animation is slow enough, and the heights for the lines at the bottom of + // the document haven't yet been laid out, you can see a jump at the final frame as the 2nd + // to last line of Moby Dick is laid out. + // + // The solution is to make sure we've laid out the text in the viewport at the bottom of the + // document at least once. I want to find a general solution to this problem so we're not just + // hard coding things here. + // + // N.b. after forcing layout for the end of the document, frame.height probably won't be correct + // yet, so we'll have to either use layoutManager.contentHeight + inset.top + inset.bottom, or + // structure things in another way so that frame.height is correct. let viewport = visibleRect let point = CGPoint( x: scrollOffset.x, From d81018d1a21c0b4685ce995c422d874303e62385 Mon Sep 17 00:00:00 2001 From: David Albert Date: Mon, 25 Mar 2024 14:53:59 -0400 Subject: [PATCH 27/73] Remove outdated comment --- Watt/LayoutManager/LayoutManager.swift | 9 --------- 1 file changed, 9 deletions(-) diff --git a/Watt/LayoutManager/LayoutManager.swift b/Watt/LayoutManager/LayoutManager.swift index 16d446c0..3f8d3624 100644 --- a/Watt/LayoutManager/LayoutManager.swift +++ b/Watt/LayoutManager/LayoutManager.swift @@ -97,15 +97,6 @@ class LayoutManager { viewportBounds.origin.y += newHeight - oldHeight } - // viewportRange is derived from heights, and performing layout can cause heights to change. - // If heights are growing, e.g. from an estimates to an actual heights, it's possible for - // viewportRange to be too big, and we'd end up laying out lines that we don't need. - // - // To prevent this, bail as soon as the viewport is full. - // - // TODO: what about when line height shrinks. Then viewportRange will be too short, and we'll - // miss paragraphs. Probably what we want is enumerateLines(startingAt:), which is what - // NSTextLayoutManager has, and then bail when we're ready. return line.alignmentFrame.maxY <= viewportBounds.maxY } lineLayers = layers From cf1b02b72dea8689568a89b01a9d9cc3d5d8a683 Mon Sep 17 00:00:00 2001 From: David Albert Date: Mon, 25 Mar 2024 15:17:37 -0400 Subject: [PATCH 28/73] Add comment --- Watt/LayoutManager/LayoutManager.swift | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Watt/LayoutManager/LayoutManager.swift b/Watt/LayoutManager/LayoutManager.swift index 3f8d3624..e8da82bd 100644 --- a/Watt/LayoutManager/LayoutManager.swift +++ b/Watt/LayoutManager/LayoutManager.swift @@ -91,6 +91,14 @@ class LayoutManager { layers.append(layer) block(layer, prevAlignmentFrame) + // TODO: This isn't quite right. We're trying to account for the situation where + // when we're scrolling up, scroll correction moves the viewport down to account + // for growing lines above us, but that's not the only time we perform scroll + // correction. We also do it when scrolling to the beginning or end of the + // document. Try setting the initial estimate of each line to 5000, and then + // scrolling to the end of the document. We don't get what we want. We need + // something more principled. This will probably be easier once LayoutManager + // becomes Editor. if willPerformScrollCorrection { let oldHeight = prevAlignmentFrame.height let newHeight = line.alignmentFrame.height From 642e12251ce5855e5eb3a8bdb374a4240170067c Mon Sep 17 00:00:00 2001 From: David Albert Date: Tue, 26 Mar 2024 11:42:45 -0400 Subject: [PATCH 29/73] More comments about programatic scroll animation --- Watt/TextView/TextView+KeyBinding.swift | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Watt/TextView/TextView+KeyBinding.swift b/Watt/TextView/TextView+KeyBinding.swift index b694207d..d9a144ba 100644 --- a/Watt/TextView/TextView+KeyBinding.swift +++ b/Watt/TextView/TextView+KeyBinding.swift @@ -391,6 +391,16 @@ extension TextView { // N.b. after forcing layout for the end of the document, frame.height probably won't be correct // yet, so we'll have to either use layoutManager.contentHeight + inset.top + inset.bottom, or // structure things in another way so that frame.height is correct. + // + // A different way to solve this would be to smear height changes over time. Rather than having + // a discontinuous height change, store a delta, and either in each frame of the scroll animation, + // or if we're not scrolling, each tick of a timer, update the height of the document by a + // of the delta. I think this would require some sort of offsetting of the document contents by + // the same delta (negative? positive?) so that you don't end up with things like the last paragraph + // cut off past the end of the document and slowly moving up into view. This is probably tricky to + // get right. + // + // It's also possible that with good enough height estimtes this just won't be a problem. let viewport = visibleRect let point = CGPoint( x: scrollOffset.x, From f61020eaef203cf6c7ff6350b6023937d3c2c81e Mon Sep 17 00:00:00 2001 From: David Albert Date: Tue, 26 Mar 2024 18:44:52 -0400 Subject: [PATCH 30/73] Partially working scroll correction in ScrollAnimator Works: - Scrolling with gestures - Scrolling by grabbing the scroller - Scrolling programatically Broken: - Editing --- Watt/LayoutManager/Heights.swift | 7 + Watt/LayoutManager/LayoutManager.swift | 24 ++- Watt/TextView/TextView+KeyBinding.swift | 4 +- Watt/TextView/TextView+Layout.swift | 40 ++--- Watt/TextView/TextView+Scrolling.swift | 19 --- Watt/TextView/TextView.swift | 14 +- Watt/Utilities/ScrollAnimator.swift | 204 ++++++++++++++++++++++-- 7 files changed, 225 insertions(+), 87 deletions(-) diff --git a/Watt/LayoutManager/Heights.swift b/Watt/LayoutManager/Heights.swift index 5ceae954..ad0bf9f2 100644 --- a/Watt/LayoutManager/Heights.swift +++ b/Watt/LayoutManager/Heights.swift @@ -415,6 +415,8 @@ extension Heights { func yOffset(upThroughPosition offset: Int) -> CGFloat { precondition(offset >= 0 && offset <= root.count, "Position out of bounds") + // I forget exactly why we special case offset == root.count, but I think it's probably + // to deal with empty last lines. if offset == root.count { let i = endIndex let (leaf, _) = i.read()! @@ -426,6 +428,11 @@ extension Heights { return root.count(.heights, upThrough: offset, edge: .leading) } + func height(upThroughPosition offset: Int) -> CGFloat { + precondition(offset >= 0 && offset <= root.count, "Position out of bounds") + return root.count(.heights, upThrough: offset, edge: .trailing) + } + func position(upThroughYOffset yOffset: CGFloat) -> Int { if yOffset < 0 { return 0 diff --git a/Watt/LayoutManager/LayoutManager.swift b/Watt/LayoutManager/LayoutManager.swift index e8da82bd..ef234a0b 100644 --- a/Watt/LayoutManager/LayoutManager.swift +++ b/Watt/LayoutManager/LayoutManager.swift @@ -12,7 +12,6 @@ import StandardKeyBindingResponder protocol LayoutManagerDelegate: AnyObject { // Text container coordinates. func viewportBounds(for layoutManager: LayoutManager) -> CGRect - func willPerformScrollCorrection(for layoutManager: LayoutManager) -> Bool func didInvalidateLayout(for layoutManager: LayoutManager) func defaultAttributes(for layoutManager: LayoutManager) -> AttributedRope.Attributes @@ -24,6 +23,7 @@ protocol LayoutManagerDelegate: AnyObject { func layoutManager(_ layoutManager: LayoutManager, bufferDidReload buffer: Buffer) func layoutManager(_ layoutManager: LayoutManager, buffer: Buffer, contentsDidChangeFrom old: Rope, to new: Rope, withDelta delta: BTreeDelta) + func layoutManager(_ layoutManager: LayoutManager, rectDidResizeFrom old: CGRect, to new: CGRect) func layoutManager(_ layoutManager: LayoutManager, createLayerForLine line: Line) -> LineLayer func layoutManager(_ layoutManager: LayoutManager, positionLineLayer layer: LineLayer) @@ -80,7 +80,6 @@ class LayoutManager { return } - let willPerformScrollCorrection = delegate.willPerformScrollCorrection(for: self) var viewportBounds = delegate.viewportBounds(for: self) let start = startOfFirstLine(intersecting: viewportBounds) @@ -91,19 +90,9 @@ class LayoutManager { layers.append(layer) block(layer, prevAlignmentFrame) - // TODO: This isn't quite right. We're trying to account for the situation where - // when we're scrolling up, scroll correction moves the viewport down to account - // for growing lines above us, but that's not the only time we perform scroll - // correction. We also do it when scrolling to the beginning or end of the - // document. Try setting the initial estimate of each line to 5000, and then - // scrolling to the end of the document. We don't get what we want. We need - // something more principled. This will probably be easier once LayoutManager - // becomes Editor. - if willPerformScrollCorrection { let oldHeight = prevAlignmentFrame.height let newHeight = line.alignmentFrame.height viewportBounds.origin.y += newHeight - oldHeight - } return line.alignmentFrame.maxY <= viewportBounds.maxY } @@ -531,13 +520,14 @@ class LayoutManager { let oldHeight = heights[hi] let newHeight = line.alignmentFrame.height + var old = line.alignmentFrame + old.size.height = oldHeight + if oldHeight != newHeight { heights[hi] = newHeight + delegate?.layoutManager(self, rectDidResizeFrom: old, to: line.alignmentFrame) } - var old = line.alignmentFrame - old.size.height = oldHeight - return (line, nil, old) } } @@ -829,7 +819,11 @@ extension LayoutManager: BufferDelegate { assert(start == newLineRange.upperBound) } + let oldRect = CGRect(x: 0, y: minY, width: textContainer.width, height: oldMaxY - minY) + let newRect = CGRect(x: 0, y: minY, width: textContainer.width, height: newMaxY - minY) + delegate?.layoutManager(self, buffer: buffer, contentsDidChangeFrom: old, to: new, withDelta: delta) + delegate?.layoutManager(self, rectDidResizeFrom: oldRect, to: newRect) delegate?.didInvalidateLayout(for: self) } diff --git a/Watt/TextView/TextView+KeyBinding.swift b/Watt/TextView/TextView+KeyBinding.swift index d9a144ba..90fca648 100644 --- a/Watt/TextView/TextView+KeyBinding.swift +++ b/Watt/TextView/TextView+KeyBinding.swift @@ -376,7 +376,7 @@ extension TextView { y: 0 ) - scrollAnimator.scroll(to: point) + scrollAnimator.animateScroll(to: point) } override func scrollToEndOfDocument(_ sender: Any?) { @@ -407,7 +407,7 @@ extension TextView { y: frame.height - viewport.height ) - scrollAnimator.scroll(to: point) + scrollAnimator.animateScroll(to: point) } diff --git a/Watt/TextView/TextView+Layout.swift b/Watt/TextView/TextView+Layout.swift index bd7bc820..4a6e4219 100644 --- a/Watt/TextView/TextView+Layout.swift +++ b/Watt/TextView/TextView+Layout.swift @@ -54,9 +54,9 @@ extension TextView { } let currentHeight = frame.height - let clipviewHeight = scrollView.contentSize.height + let clipViewHeight = scrollView.contentSize.height let inset = computedTextContainerInset - let newHeight = round(max(clipviewHeight, layoutManager.contentHeight + inset.top + inset.bottom)) + let newHeight = round(max(clipViewHeight, layoutManager.contentHeight + inset.top + inset.bottom)) if abs(currentHeight - newHeight) > 1e-10 { setFrameSize(CGSize(width: frame.width, height: newHeight)) @@ -122,12 +122,6 @@ extension TextView: LayoutManagerDelegate { textContainerVisibleRect } - func willPerformScrollCorrection(for layoutManager: LayoutManager) -> Bool { - // TODO: currently duplicated between here and layoutTextLayer(). Remove duplication once LayoutManager becomes Editor. - let scrollingUp = visibleRect.minY < previousVisibleRect.minY - return scrollingUp || isDraggingScroller || scrollAnimator.isAnimating - } - func didInvalidateLayout(for layoutManager: LayoutManager) { transaction { schedule(.textLayout) @@ -206,6 +200,13 @@ extension TextView: LayoutManagerDelegate { lineNumberView.lineCount = new.lines.count } + func layoutManager(_ layoutManager: LayoutManager, rectDidResizeFrom old: CGRect, to new: CGRect) { + let old = convertFromTextContainer(old) + let new = convertFromTextContainer(new) + + scrollAnimator.rectInDocumentViewDidChange(from: old, to: new) + } + func layoutManager(_ layoutManager: LayoutManager, createLayerForLine line: Line) -> LineLayer { let l = LineLayer(line: line) l.anchorPoint = .zero @@ -231,9 +232,6 @@ extension TextView { return } - let scrollingUp = visibleRect.minY < previousVisibleRect.minY - var scrollAdjustment: CGFloat = 0 - let updateLineNumbers = lineNumberView.superview != nil if updateLineNumbers { lineNumberView.beginUpdates() @@ -242,18 +240,9 @@ extension TextView { var lineno: Int? var layers: [CALayer] = [] - layoutManager.layoutText { layer, prevAlignmentFrame in + layoutManager.layoutText { layer, _ in layers.append(layer) - let oldHeight = prevAlignmentFrame.height - let newHeight = layer.line.alignmentFrame.height - let delta = newHeight - oldHeight - - // TODO: currently duplicated in willPerformScrollCorrection(for:). Remove duplication once LayoutManager becomes Editor. - if scrollingUp || isDraggingScroller || scrollAnimator.isAnimating { - scrollAdjustment += delta - } - if updateLineNumbers { let n = lineno ?? buffer.lines.distance(from: buffer.startIndex, to: layer.line.range.lowerBound) let inset = computedTextContainerInset @@ -272,16 +261,7 @@ extension TextView { lineNumberView.endUpdates() } - previousVisibleRect = visibleRect updateFrameHeightIfNeeded() - - if scrollAdjustment != 0 { - let current = scrollOffset - performingScrollCorrection = true - scroll(CGPoint(x: current.x, y: current.y + scrollAdjustment)) - performingScrollCorrection = false - scrollAnimator.didCorrectScroll(by: CGVector(dx: 0, dy: scrollAdjustment)) - } } } diff --git a/Watt/TextView/TextView+Scrolling.swift b/Watt/TextView/TextView+Scrolling.swift index 2469bc16..aeea5c74 100644 --- a/Watt/TextView/TextView+Scrolling.swift +++ b/Watt/TextView/TextView+Scrolling.swift @@ -73,29 +73,10 @@ extension TextView { } @objc func viewDidScroll(_ notification: Notification) { - // As part of textLayout, we may end up calling NSView.scroll(_:) in order to change our - // scroll offset in response to a change in document height. In that case, we're in the - // middle of committing a transaction, and starting a new one would panic. - if performingScrollCorrection { - return - } - transaction { schedule(.textLayout) schedule(.selectionLayout) schedule(.insertionPointLayout) } } - - @objc func willStartLiveScroll(_ notification: Notification) { - guard let scrollView = notification.object as? NSScrollView else { - return - } - - isDraggingScroller = scrollView.verticalScroller?.hitPart == .knob - } - - @objc func didEndLiveScroll(_ notification: Notification) { - isDraggingScroller = false - } } diff --git a/Watt/TextView/TextView.swift b/Watt/TextView/TextView.swift index dbe69d1f..b1927472 100644 --- a/Watt/TextView/TextView.swift +++ b/Watt/TextView/TextView.swift @@ -153,11 +153,6 @@ class TextView: NSView, ClipViewDelegate { var transaction: Transaction = Transaction() - // HACK: See layoutTextLayer() for context. - var previousVisibleRect: CGRect = .zero - var isDraggingScroller: Bool = false - var performingScrollCorrection: Bool = false - override init(frame frameRect: NSRect) { layoutManager = LayoutManager() lineNumberView = LineNumberView() @@ -218,6 +213,10 @@ class TextView: NSView, ClipViewDelegate { removeLineNumberView() } + override func viewDidMoveToSuperview() { + scrollAnimator.viewDidMoveToSuperview() + } + // This is a custom method on ClipViewDelegate. Normally we'd use // viewDidMoveToSuperview, but when we're added to a clip view, // that's called too early – specifically, it's called before the @@ -246,16 +245,11 @@ class TextView: NSView, ClipViewDelegate { override func viewDidMoveToWindow() { NotificationCenter.default.removeObserver(self, name: NSView.boundsDidChangeNotification, object: nil) - NotificationCenter.default.removeObserver(self, name: NSScrollView.willStartLiveScrollNotification, object: nil) - NotificationCenter.default.removeObserver(self, name: NSScrollView.didEndLiveScrollNotification, object: nil) - NotificationCenter.default.removeObserver(self, name: NSWindow.didBecomeKeyNotification, object: nil) NotificationCenter.default.removeObserver(self, name: NSWindow.didResignKeyNotification, object: nil) if let scrollView { NotificationCenter.default.addObserver(self, selector: #selector(viewDidScroll(_:)), name: NSView.boundsDidChangeNotification, object: scrollView.contentView) - NotificationCenter.default.addObserver(self, selector: #selector(willStartLiveScroll(_:)), name: NSScrollView.willStartLiveScrollNotification, object: scrollView) - NotificationCenter.default.addObserver(self, selector: #selector(didEndLiveScroll(_:)), name: NSScrollView.didEndLiveScrollNotification, object: scrollView) } if let window { diff --git a/Watt/Utilities/ScrollAnimator.swift b/Watt/Utilities/ScrollAnimator.swift index f64699b7..5729f238 100644 --- a/Watt/Utilities/ScrollAnimator.swift +++ b/Watt/Utilities/ScrollAnimator.swift @@ -8,8 +8,24 @@ import Cocoa import Motion +@MainActor class ScrollAnimator { - weak var view: NSView? + private(set) weak var view: NSView? + private(set) var isDraggingScroller: Bool + + // Think model and presentation layers in Core Animation + struct PresentationProperties { + var scrollOffset: CGPoint + } + private(set) var scrollOffset: CGPoint + + private(set) var presentation: PresentationProperties + + // A unit point representing the position of the dragged scroller knobs. + var requestedUnitScrollOffset: CGPoint? + + private var isScrollCorrectionScheduled: Bool + private var animation: SpringAnimation? var isAnimating: Bool { @@ -18,33 +34,66 @@ class ScrollAnimator { init(_ view: NSView) { self.view = view + self.isDraggingScroller = false + self.scrollOffset = .zero + self.presentation = PresentationProperties(scrollOffset: .zero) + self.isScrollCorrectionScheduled = false + + if view.superview != nil { + viewDidMoveToSuperview() + } + } + + func viewDidMoveToSuperview() { + NotificationCenter.default.removeObserver(self, name: NSView.boundsDidChangeNotification, object: nil) + NotificationCenter.default.removeObserver(self, name: NSScrollView.willStartLiveScrollNotification, object: nil) + NotificationCenter.default.removeObserver(self, name: NSScrollView.didLiveScrollNotification, object: nil) + NotificationCenter.default.removeObserver(self, name: NSScrollView.didEndLiveScrollNotification, object: nil) + + animation?.stop() + animation = nil + + isDraggingScroller = false + isScrollCorrectionScheduled = false + + requestedUnitScrollOffset = nil + + let offset = view?.enclosingScrollView?.contentView.bounds.origin ?? .zero + scrollOffset = offset + presentation.scrollOffset = offset - if let scrollView = view.enclosingScrollView { + if let scrollView = view?.enclosingScrollView { + NotificationCenter.default.addObserver(self, selector: #selector(viewDidScroll(_:)), name: NSView.boundsDidChangeNotification, object: scrollView.contentView) NotificationCenter.default.addObserver(self, selector: #selector(willStartLiveScroll(_:)), name: NSScrollView.willStartLiveScrollNotification, object: scrollView) + NotificationCenter.default.addObserver(self, selector: #selector(didLiveScroll(_:)), name: NSScrollView.didLiveScrollNotification, object: scrollView) + NotificationCenter.default.addObserver(self, selector: #selector(didEndLiveScroll(_:)), name: NSScrollView.didEndLiveScrollNotification, object: scrollView) } } - func scroll(to point: CGPoint) { + func animateScroll(to point: NSPoint) { guard let scrollView = view?.enclosingScrollView else { return } + scrollOffset = point + let velocity = animation?.velocity ?? .zero animation?.stop() + assert(presentation.scrollOffset == scrollView.contentView.bounds.origin) let spring = SpringAnimation( - initialValue: scrollView.contentView.bounds.origin, + initialValue: presentation.scrollOffset, response: 0.2, dampingRatio: 1.0, environment: scrollView ) - spring.toValue = point + spring.toValue = scrollOffset spring.velocity = velocity spring.resolvingEpsilon = 0.000001 spring.onValueChanged() { [weak self] value in - self?.view?.scroll(value) + self?.view?.enclosingScrollView?.documentView?.scroll(value) } spring.completion = { [weak self] in @@ -55,20 +104,153 @@ class ScrollAnimator { animation = spring } - func didCorrectScroll(by delta: CGVector) { - guard let animation else { + func rectInDocumentViewDidChange(from old: NSRect, to new: NSRect) { + precondition(old.origin == new.origin, "old and new rects must share an origin") + + guard let scrollView = view?.enclosingScrollView else { return } - let old = animation.toValue - let new = CGPoint(x: old.x + delta.dx, y: old.y + delta.dy) + assert(presentation.scrollOffset == scrollView.contentView.bounds.origin) - scroll(to: new) + let dx = old.minX >= scrollOffset.x ? 0 : new.width - old.width + let dy = old.minY >= scrollOffset.y ? 0 : new.height - old.height + + if dx == 0 && dy == 0 { + return + } + + scrollOffset = CGPoint(x: scrollOffset.x + dx, y: scrollOffset.y + dy) + scheduleScrollCorrection() + } + + @objc func viewDidScroll(_ notification: Notification) { + guard let scrollView = view?.enclosingScrollView else { + return + } + + assert(scrollView.contentView == (notification.object as? NSView)) + + let offset = scrollView.contentView.bounds.origin + if isAnimating { + presentation.scrollOffset = offset + } else { + scrollOffset = offset + presentation.scrollOffset = offset + } } @objc func willStartLiveScroll(_ notification: Notification) { // If the user interrupts the scroll for any reason, stop the animation. animation?.stop() animation = nil + + scrollOffset = presentation.scrollOffset + + guard let scrollView = notification.object as? NSScrollView else { + return + } + + isDraggingScroller = scrollView.horizontalScroller?.hitPart == .knob || scrollView.verticalScroller?.hitPart == .knob + } + + @objc func didLiveScroll(_ notification: Notification) { + guard let scrollView = notification.object as? NSScrollView else { + return + } + + assert(scrollView == view?.enclosingScrollView) + assert(!isAnimating) +// assert(scrollView.contentView.bounds.origin == scrollOffset) + + if !isDraggingScroller { + return + } + + // scrollView encloses view, so it must have a document view + assert(scrollView.documentView != nil) + guard let documentSize = scrollView.documentView?.frame.size else { + return + } + + let viewport = scrollView.contentView.bounds + + let unitx: CGFloat + if let v = scrollView.horizontalScroller?.floatValue { + unitx = CGFloat(v) + } else { + let maxX = max(documentSize.width - viewport.width, 0) + assert(maxX > 0 || scrollOffset.x == 0) + unitx = maxX == 0 ? 0 : scrollOffset.x / maxX + } + + let unity: CGFloat + if let v = scrollView.verticalScroller?.floatValue { + unity = CGFloat(v) + } else { + let maxY = max(documentSize.height - viewport.height, 0) + assert(maxY > 0 || scrollOffset.y == 0) + unity = maxY == 0 ? 0 : scrollOffset.y / maxY + } + + requestedUnitScrollOffset = CGPoint(x: unitx, y: unity) + scheduleScrollCorrection() + } + + @objc func didEndLiveScroll(_ notification: Notification) { + isDraggingScroller = false + } + + private func scheduleScrollCorrection() { + if isScrollCorrectionScheduled { + return + } + + isScrollCorrectionScheduled = true + DispatchQueue.main.async { [weak self] in + self?.performScrollCorrection() + } + } + + private func performScrollCorrection() { + if !isScrollCorrectionScheduled { + return + } + + isScrollCorrectionScheduled = false + guard let scrollView = view?.enclosingScrollView else { + return + } + + if let requestedUnitScrollOffset { + defer { self.requestedUnitScrollOffset = nil } + + assert(!isAnimating) + assert(scrollView.documentView != nil) + + guard let documentSize = scrollView.documentView?.frame.size else { + return + } + + let viewport = scrollView.contentView.bounds + let offset = NSPoint( + x: requestedUnitScrollOffset.x * (documentSize.width - viewport.width), + y: requestedUnitScrollOffset.y * (documentSize.height - viewport.height) + ) + + if viewport.origin != offset { + scrollView.documentView?.scroll(offset) + } + return + } + + let offset = scrollView.contentView.bounds.origin + if offset != scrollOffset { + if isAnimating { + animateScroll(to: scrollOffset) + } else { + scrollView.documentView?.scroll(scrollOffset) + } + } } } From adc9655392d4d6a4cc9b884e73623a0f673201e0 Mon Sep 17 00:00:00 2001 From: David Albert Date: Tue, 26 Mar 2024 18:49:30 -0400 Subject: [PATCH 31/73] Whoops --- Watt/LayoutManager/LayoutManager.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Watt/LayoutManager/LayoutManager.swift b/Watt/LayoutManager/LayoutManager.swift index ef234a0b..d3991369 100644 --- a/Watt/LayoutManager/LayoutManager.swift +++ b/Watt/LayoutManager/LayoutManager.swift @@ -788,8 +788,13 @@ extension LayoutManager: BufferDelegate { let oldRange = Range(r, in: old) let newRange = Range(r.lowerBound..<(r.lowerBound + count), in: new) + let minY = heights.yOffset(upThroughPosition: r.lowerBound) + let oldMaxY = heights.height(upThroughPosition: r.upperBound) + heights.replaceSubrange(r, with: new[newRange]) + let newMaxY = heights.height(upThroughPosition: r.lowerBound + count) + removeLineLayers(touching: oldRange) // Offset line ranges that fall after the edit From 601997385d2b511d7e611793996a7e15228e3e22 Mon Sep 17 00:00:00 2001 From: David Albert Date: Tue, 26 Mar 2024 21:35:30 -0400 Subject: [PATCH 32/73] Cleanup --- Watt/Utilities/ScrollAnimator.swift | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/Watt/Utilities/ScrollAnimator.swift b/Watt/Utilities/ScrollAnimator.swift index 5729f238..d869199a 100644 --- a/Watt/Utilities/ScrollAnimator.swift +++ b/Watt/Utilities/ScrollAnimator.swift @@ -18,11 +18,10 @@ class ScrollAnimator { var scrollOffset: CGPoint } private(set) var scrollOffset: CGPoint - private(set) var presentation: PresentationProperties // A unit point representing the position of the dragged scroller knobs. - var requestedUnitScrollOffset: CGPoint? + private var absoluteUnitOffset: CGPoint? private var isScrollCorrectionScheduled: Bool @@ -56,7 +55,7 @@ class ScrollAnimator { isDraggingScroller = false isScrollCorrectionScheduled = false - requestedUnitScrollOffset = nil + absoluteUnitOffset = nil let offset = view?.enclosingScrollView?.contentView.bounds.origin ?? .zero scrollOffset = offset @@ -193,7 +192,7 @@ class ScrollAnimator { unity = maxY == 0 ? 0 : scrollOffset.y / maxY } - requestedUnitScrollOffset = CGPoint(x: unitx, y: unity) + absoluteUnitOffset = CGPoint(x: unitx, y: unity) scheduleScrollCorrection() } @@ -222,8 +221,8 @@ class ScrollAnimator { return } - if let requestedUnitScrollOffset { - defer { self.requestedUnitScrollOffset = nil } + if let absoluteUnitOffset { + defer { self.absoluteUnitOffset = nil } assert(!isAnimating) assert(scrollView.documentView != nil) @@ -234,8 +233,8 @@ class ScrollAnimator { let viewport = scrollView.contentView.bounds let offset = NSPoint( - x: requestedUnitScrollOffset.x * (documentSize.width - viewport.width), - y: requestedUnitScrollOffset.y * (documentSize.height - viewport.height) + x: absoluteUnitOffset.x * (documentSize.width - viewport.width), + y: absoluteUnitOffset.y * (documentSize.height - viewport.height) ) if viewport.origin != offset { From bb26b89219082d383ef975b5b210ef7260435982 Mon Sep 17 00:00:00 2001 From: David Albert Date: Wed, 27 Mar 2024 11:19:51 -0400 Subject: [PATCH 33/73] Scroll to beginning/end of document kind of works MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issues: - Scrolling to the end with correction slows down - You can’t interrupt a liveScroll with a keyboard command --- Watt/Utilities/ScrollAnimator.swift | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/Watt/Utilities/ScrollAnimator.swift b/Watt/Utilities/ScrollAnimator.swift index d869199a..dbca4cfd 100644 --- a/Watt/Utilities/ScrollAnimator.swift +++ b/Watt/Utilities/ScrollAnimator.swift @@ -42,7 +42,7 @@ class ScrollAnimator { viewDidMoveToSuperview() } } - + func viewDidMoveToSuperview() { NotificationCenter.default.removeObserver(self, name: NSView.boundsDidChangeNotification, object: nil) NotificationCenter.default.removeObserver(self, name: NSScrollView.willStartLiveScrollNotification, object: nil) @@ -112,8 +112,15 @@ class ScrollAnimator { assert(presentation.scrollOffset == scrollView.contentView.bounds.origin) - let dx = old.minX >= scrollOffset.x ? 0 : new.width - old.width - let dy = old.minY >= scrollOffset.y ? 0 : new.height - old.height + // When animating upwards, any size change with an origin above the viewport contributes + // to scroll correction. When animating downwards, size changes with origins in the viewport + // also contribute. + let viewport = scrollView.contentView.bounds + let anchorX = scrollOffset.x > presentation.scrollOffset.x ? 1.0 : 0.0 + let anchorY = scrollOffset.y > presentation.scrollOffset.y ? 1.0 : 0.0 + + let dx = old.minX >= scrollOffset.x + (anchorX*viewport.width) ? 0 : new.width - old.width + let dy = old.minY >= scrollOffset.y + (anchorY*viewport.height) ? 0 : new.height - old.height if dx == 0 && dy == 0 { return @@ -158,9 +165,12 @@ class ScrollAnimator { return } + // From the didLiveScrollNotification docs: "Some user-initiated scrolls (for example, scrolling + // using legacy mice) are not bracketed by a "willStart/didEnd” notification pair." + animation?.stop() + animation = nil + assert(scrollView == view?.enclosingScrollView) - assert(!isAnimating) -// assert(scrollView.contentView.bounds.origin == scrollOffset) if !isDraggingScroller { return @@ -247,6 +257,7 @@ class ScrollAnimator { if offset != scrollOffset { if isAnimating { animateScroll(to: scrollOffset) + assert(offset == presentation.scrollOffset) } else { scrollView.documentView?.scroll(scrollOffset) } From 37831a5727f0fbcdda686754614e2dfebf4b73b4 Mon Sep 17 00:00:00 2001 From: David Albert Date: Wed, 27 Mar 2024 15:24:08 -0400 Subject: [PATCH 34/73] Change scroll correction signature to take a size rather than a second rect --- Watt/LayoutManager/LayoutManager.swift | 10 +++++----- Watt/TextView/TextView+Layout.swift | 8 +++----- Watt/Utilities/ScrollAnimator.swift | 8 +++----- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/Watt/LayoutManager/LayoutManager.swift b/Watt/LayoutManager/LayoutManager.swift index d3991369..3ea2aeb2 100644 --- a/Watt/LayoutManager/LayoutManager.swift +++ b/Watt/LayoutManager/LayoutManager.swift @@ -23,7 +23,7 @@ protocol LayoutManagerDelegate: AnyObject { func layoutManager(_ layoutManager: LayoutManager, bufferDidReload buffer: Buffer) func layoutManager(_ layoutManager: LayoutManager, buffer: Buffer, contentsDidChangeFrom old: Rope, to new: Rope, withDelta delta: BTreeDelta) - func layoutManager(_ layoutManager: LayoutManager, rectDidResizeFrom old: CGRect, to new: CGRect) + func layoutManager(_ layoutManager: LayoutManager, rect: CGRect, didResizeTo new: CGSize) func layoutManager(_ layoutManager: LayoutManager, createLayerForLine line: Line) -> LineLayer func layoutManager(_ layoutManager: LayoutManager, positionLineLayer layer: LineLayer) @@ -525,7 +525,7 @@ class LayoutManager { if oldHeight != newHeight { heights[hi] = newHeight - delegate?.layoutManager(self, rectDidResizeFrom: old, to: line.alignmentFrame) + delegate?.layoutManager(self, rect: old, didResizeTo: line.alignmentFrame.size) } return (line, nil, old) @@ -824,11 +824,11 @@ extension LayoutManager: BufferDelegate { assert(start == newLineRange.upperBound) } - let oldRect = CGRect(x: 0, y: minY, width: textContainer.width, height: oldMaxY - minY) - let newRect = CGRect(x: 0, y: minY, width: textContainer.width, height: newMaxY - minY) + let rect = CGRect(x: 0, y: minY, width: textContainer.width, height: oldMaxY - minY) + let newSize = CGSize(width: textContainer.width, height: newMaxY - minY) delegate?.layoutManager(self, buffer: buffer, contentsDidChangeFrom: old, to: new, withDelta: delta) - delegate?.layoutManager(self, rectDidResizeFrom: oldRect, to: newRect) + delegate?.layoutManager(self, rect: rect, didResizeTo: newSize) delegate?.didInvalidateLayout(for: self) } diff --git a/Watt/TextView/TextView+Layout.swift b/Watt/TextView/TextView+Layout.swift index 4a6e4219..074f7610 100644 --- a/Watt/TextView/TextView+Layout.swift +++ b/Watt/TextView/TextView+Layout.swift @@ -200,11 +200,9 @@ extension TextView: LayoutManagerDelegate { lineNumberView.lineCount = new.lines.count } - func layoutManager(_ layoutManager: LayoutManager, rectDidResizeFrom old: CGRect, to new: CGRect) { - let old = convertFromTextContainer(old) - let new = convertFromTextContainer(new) - - scrollAnimator.rectInDocumentViewDidChange(from: old, to: new) + func layoutManager(_ layoutManager: LayoutManager, rect: CGRect, didResizeTo newSize: CGSize) { + let rect = convertFromTextContainer(rect) + scrollAnimator.documentRect(rect, didResizeTo: newSize) } func layoutManager(_ layoutManager: LayoutManager, createLayerForLine line: Line) -> LineLayer { diff --git a/Watt/Utilities/ScrollAnimator.swift b/Watt/Utilities/ScrollAnimator.swift index dbca4cfd..e110c249 100644 --- a/Watt/Utilities/ScrollAnimator.swift +++ b/Watt/Utilities/ScrollAnimator.swift @@ -103,9 +103,7 @@ class ScrollAnimator { animation = spring } - func rectInDocumentViewDidChange(from old: NSRect, to new: NSRect) { - precondition(old.origin == new.origin, "old and new rects must share an origin") - + func documentRect(_ rect: NSRect, didResizeTo newSize: NSSize) { guard let scrollView = view?.enclosingScrollView else { return } @@ -119,8 +117,8 @@ class ScrollAnimator { let anchorX = scrollOffset.x > presentation.scrollOffset.x ? 1.0 : 0.0 let anchorY = scrollOffset.y > presentation.scrollOffset.y ? 1.0 : 0.0 - let dx = old.minX >= scrollOffset.x + (anchorX*viewport.width) ? 0 : new.width - old.width - let dy = old.minY >= scrollOffset.y + (anchorY*viewport.height) ? 0 : new.height - old.height + let dx = rect.minX >= scrollOffset.x + (anchorX*viewport.width) ? 0 : newSize.width - rect.width + let dy = rect.minY >= scrollOffset.y + (anchorY*viewport.height) ? 0 : newSize.height - rect.height if dx == 0 && dy == 0 { return From e15cb6591aab8f05dbafda3796fa7bf95a1951c0 Mon Sep 17 00:00:00 2001 From: David Albert Date: Wed, 27 Mar 2024 15:37:56 -0400 Subject: [PATCH 35/73] Simpler correction when dragging the scroller --- Watt/Utilities/ScrollAnimator.swift | 50 ++++++----------------------- 1 file changed, 9 insertions(+), 41 deletions(-) diff --git a/Watt/Utilities/ScrollAnimator.swift b/Watt/Utilities/ScrollAnimator.swift index e110c249..60e4b05c 100644 --- a/Watt/Utilities/ScrollAnimator.swift +++ b/Watt/Utilities/ScrollAnimator.swift @@ -20,9 +20,6 @@ class ScrollAnimator { private(set) var scrollOffset: CGPoint private(set) var presentation: PresentationProperties - // A unit point representing the position of the dragged scroller knobs. - private var absoluteUnitOffset: CGPoint? - private var isScrollCorrectionScheduled: Bool private var animation: SpringAnimation? @@ -55,8 +52,6 @@ class ScrollAnimator { isDraggingScroller = false isScrollCorrectionScheduled = false - absoluteUnitOffset = nil - let offset = view?.enclosingScrollView?.contentView.bounds.origin ?? .zero scrollOffset = offset presentation.scrollOffset = offset @@ -170,39 +165,10 @@ class ScrollAnimator { assert(scrollView == view?.enclosingScrollView) - if !isDraggingScroller { - return - } - - // scrollView encloses view, so it must have a document view - assert(scrollView.documentView != nil) - guard let documentSize = scrollView.documentView?.frame.size else { - return - } - - let viewport = scrollView.contentView.bounds - - let unitx: CGFloat - if let v = scrollView.horizontalScroller?.floatValue { - unitx = CGFloat(v) - } else { - let maxX = max(documentSize.width - viewport.width, 0) - assert(maxX > 0 || scrollOffset.x == 0) - unitx = maxX == 0 ? 0 : scrollOffset.x / maxX - } - - let unity: CGFloat - if let v = scrollView.verticalScroller?.floatValue { - unity = CGFloat(v) - } else { - let maxY = max(documentSize.height - viewport.height, 0) - assert(maxY > 0 || scrollOffset.y == 0) - unity = maxY == 0 ? 0 : scrollOffset.y / maxY - } - - absoluteUnitOffset = CGPoint(x: unitx, y: unity) + if isDraggingScroller { scheduleScrollCorrection() } + } @objc func didEndLiveScroll(_ notification: Notification) { isDraggingScroller = false @@ -229,9 +195,11 @@ class ScrollAnimator { return } - if let absoluteUnitOffset { - defer { self.absoluteUnitOffset = nil } - + // Dragging a scroller is equivalent to telling the scroll view "please set your offset to this + // fixed percentage through the document." In this case, scroll correction just consists of + // reading the percentage out of the scrollers and setting the new origin if the document size + // has changed. + if isDraggingScroller { assert(!isAnimating) assert(scrollView.documentView != nil) @@ -241,8 +209,8 @@ class ScrollAnimator { let viewport = scrollView.contentView.bounds let offset = NSPoint( - x: absoluteUnitOffset.x * (documentSize.width - viewport.width), - y: absoluteUnitOffset.y * (documentSize.height - viewport.height) + x: CGFloat(scrollView.horizontalScroller?.floatValue ?? 0) * (documentSize.width - viewport.width), + y: CGFloat(scrollView.verticalScroller?.floatValue ?? 0) * (documentSize.height - viewport.height) ) if viewport.origin != offset { From a7c31758e9d0bbd288f0447a1911e6aefa363a41 Mon Sep 17 00:00:00 2001 From: David Albert Date: Wed, 27 Mar 2024 16:21:43 -0400 Subject: [PATCH 36/73] Get rid of prevAlignmentFrame --- Watt/LayoutManager/LayoutManager.swift | 36 ++++++++++++-------------- Watt/TextView/TextView+Layout.swift | 2 +- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/Watt/LayoutManager/LayoutManager.swift b/Watt/LayoutManager/LayoutManager.swift index 3ea2aeb2..c50b8b25 100644 --- a/Watt/LayoutManager/LayoutManager.swift +++ b/Watt/LayoutManager/LayoutManager.swift @@ -75,26 +75,24 @@ class LayoutManager { heights.contentHeight } - func layoutText(using block: (_ layer: LineLayer, _ prevAlignmentFrame: CGRect) -> Void) { + func layoutText(using block: (LineLayer) -> Void) { guard let delegate else { return } - var viewportBounds = delegate.viewportBounds(for: self) + let viewportBounds = delegate.viewportBounds(for: self) let start = startOfFirstLine(intersecting: viewportBounds) var layers: [LineLayer] = [] - enumerateLines(startingAt: start) { line, existingLayer, prevAlignmentFrame in + var height: CGFloat = 0 + enumerateLines(startingAt: start) { line, existingLayer in let layer = existingLayer ?? delegate.layoutManager(self, createLayerForLine: line) delegate.layoutManager(self, positionLineLayer: layer) layers.append(layer) - block(layer, prevAlignmentFrame) - - let oldHeight = prevAlignmentFrame.height - let newHeight = line.alignmentFrame.height - viewportBounds.origin.y += newHeight - oldHeight + block(layer) - return line.alignmentFrame.maxY <= viewportBounds.maxY + height += line.alignmentFrame.height - max(viewportBounds.minY - line.alignmentFrame.minY, 0) + return height <= viewportBounds.height } lineLayers = layers } @@ -191,7 +189,7 @@ class LayoutManager { } func enumerateTextSegments(in range: Range, type: SegmentType, using block: (Range, CGRect) -> Bool) { - enumerateLines(startingAt: range.lowerBound) { line, _, _ in + enumerateLines(startingAt: range.lowerBound) { line, _ in for frag in line.lineFragments { let rangesOverlap = range.overlaps(frag.range) || range.isEmpty && frag.range.contains(range.lowerBound) let atEndOfDocument = frag.range.upperBound == buffer.endIndex && frag.range.upperBound == range.lowerBound @@ -246,7 +244,7 @@ class LayoutManager { // range - the range of the line in buffer // line - the line // previousBounds - The (possibly estimated) bounds of the line before layout was performed. If the line was already laid out, this is equal to line.typographicBounds. - func enumerateLines(startingAt index: Buffer.Index, using block: (_ line: Line, _ layer: LineLayer?, _ prevAlignmentFrame: CGRect) -> Bool) { + func enumerateLines(startingAt index: Buffer.Index, using block: (_ line: Line, _ layer: LineLayer?) -> Bool) { var i = buffer.lines.index(roundingDown: index) let end = buffer.endIndex @@ -254,9 +252,9 @@ class LayoutManager { while i < end { let next = buffer.lines.index(after: i) - let (line, layer, prevAlignmentFrame) = layoutLineIfNecessary(from: buffer, inRange: i.., atPoint point: CGPoint) -> (line: Line, layer: LineLayer?, prevAlignmentFrame: CGRect) { + func layoutLineIfNecessary(from buffer: Buffer, inRange range: Range, atPoint point: CGPoint) -> (Line, LineLayer?) { assert(range.lowerBound == buffer.lines.index(roundingDown: range.lowerBound)) assert(range.upperBound == buffer.endIndex || range.upperBound == buffer.lines.index(roundingDown: range.upperBound)) @@ -510,7 +508,7 @@ class LayoutManager { assert(Range(unvalidatedRange: layer.line.range) == intRange) layer.line.origin.y = point.y - return (layer.line, layer, layer.line.alignmentFrame) + return (layer.line, layer) } else { assert(lineLayers.allSatisfy { !$0.line.range.overlaps(range) }) @@ -528,7 +526,7 @@ class LayoutManager { delegate?.layoutManager(self, rect: old, didResizeTo: line.alignmentFrame.size) } - return (line, nil, old) + return (line, nil) } } diff --git a/Watt/TextView/TextView+Layout.swift b/Watt/TextView/TextView+Layout.swift index 074f7610..38e907aa 100644 --- a/Watt/TextView/TextView+Layout.swift +++ b/Watt/TextView/TextView+Layout.swift @@ -238,7 +238,7 @@ extension TextView { var lineno: Int? var layers: [CALayer] = [] - layoutManager.layoutText { layer, _ in + layoutManager.layoutText { layer in layers.append(layer) if updateLineNumbers { From 2f241cb6c52c19b2e1163c5ebb87341b01280346 Mon Sep 17 00:00:00 2001 From: David Albert Date: Wed, 27 Mar 2024 18:45:28 -0400 Subject: [PATCH 37/73] Remove transaction system in favor of deferred layout --- Watt.xcodeproj/project.pbxproj | 4 -- Watt/Base.lproj/MainMenu.xib | 4 +- Watt/TextView/TextView+Input.swift | 64 +++++++---------- Watt/TextView/TextView+KeyBinding.swift | 88 ++++++++--------------- Watt/TextView/TextView+Layout.swift | 35 ++++++--- Watt/TextView/TextView+Scrolling.swift | 8 +-- Watt/TextView/TextView+Transactions.swift | 72 ------------------- Watt/TextView/TextView.swift | 52 +++++++++----- 8 files changed, 120 insertions(+), 207 deletions(-) delete mode 100644 Watt/TextView/TextView+Transactions.swift diff --git a/Watt.xcodeproj/project.pbxproj b/Watt.xcodeproj/project.pbxproj index fd0daf75..fbb15191 100644 --- a/Watt.xcodeproj/project.pbxproj +++ b/Watt.xcodeproj/project.pbxproj @@ -13,7 +13,6 @@ 631FD2652A696D27005FA854 /* Heights.swift in Sources */ = {isa = PBXBuildFile; fileRef = 631FD2642A696D27005FA854 /* Heights.swift */; }; 63206D2A2BA74AF600CAFBC4 /* TextView+Scrolling.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63206D292BA74AF600CAFBC4 /* TextView+Scrolling.swift */; }; 63206D2D2BA8E8F400CAFBC4 /* Autoscroller.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63206D2C2BA8E8F400CAFBC4 /* Autoscroller.swift */; }; - 63206D312BA9FFD300CAFBC4 /* TextView+Transactions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63206D302BA9FFD300CAFBC4 /* TextView+Transactions.swift */; }; 63206D362BAB635500CAFBC4 /* Motion in Frameworks */ = {isa = PBXBuildFile; productRef = 63206D352BAB635500CAFBC4 /* Motion */; }; 63206D382BAB707E00CAFBC4 /* ScrollAnimator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63206D372BAB707E00CAFBC4 /* ScrollAnimator.swift */; }; 63267C292B97BF0B00F6E968 /* BTreeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63267C282B97BF0B00F6E968 /* BTreeTests.swift */; }; @@ -137,7 +136,6 @@ 631FD2642A696D27005FA854 /* Heights.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Heights.swift; sourceTree = ""; }; 63206D292BA74AF600CAFBC4 /* TextView+Scrolling.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TextView+Scrolling.swift"; sourceTree = ""; }; 63206D2C2BA8E8F400CAFBC4 /* Autoscroller.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Autoscroller.swift; sourceTree = ""; }; - 63206D302BA9FFD300CAFBC4 /* TextView+Transactions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TextView+Transactions.swift"; sourceTree = ""; }; 63206D372BAB707E00CAFBC4 /* ScrollAnimator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ScrollAnimator.swift; sourceTree = ""; }; 63267C282B97BF0B00F6E968 /* BTreeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTreeTests.swift; sourceTree = ""; }; 63286AE12A13DF5D00839B25 /* ClipView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ClipView.swift; sourceTree = ""; }; @@ -371,7 +369,6 @@ 6396DD2D2B45A5DE000F85D1 /* TextView+Pasteboard.swift */, 63206D292BA74AF600CAFBC4 /* TextView+Scrolling.swift */, 637867702A17F7460039960E /* TextView+Selection.swift */, - 63206D302BA9FFD300CAFBC4 /* TextView+Transactions.swift */, 635B36B02AAFCD95002FAE70 /* Theme.swift */, 635F815C2AA9E51900553E23 /* TreeSitter.swift */, ); @@ -703,7 +700,6 @@ 636DD5322B599E150064C989 /* Workspace+LoadRequest.swift in Sources */, 637867832A1EC5B50039960E /* TextView+Input.swift in Sources */, 6364DDA32B6C408700886DE3 /* DocumentPaneViewController.swift in Sources */, - 63206D312BA9FFD300CAFBC4 /* TextView+Transactions.swift in Sources */, 6396DD2E2B45A5DE000F85D1 /* TextView+Pasteboard.swift in Sources */, 63A618722A0EAF2000E70B0E /* Weak.swift in Sources */, 63D280922B5ECDF600CDCF04 /* NSFileCoordinator+Extensions.swift in Sources */, diff --git a/Watt/Base.lproj/MainMenu.xib b/Watt/Base.lproj/MainMenu.xib index b1dbee65..d4808320 100644 --- a/Watt/Base.lproj/MainMenu.xib +++ b/Watt/Base.lproj/MainMenu.xib @@ -1,7 +1,7 @@ - + - + diff --git a/Watt/TextView/TextView+Input.swift b/Watt/TextView/TextView+Input.swift index f5730e3a..f74efd8c 100644 --- a/Watt/TextView/TextView+Input.swift +++ b/Watt/TextView/TextView+Input.swift @@ -32,11 +32,9 @@ extension TextView: NSTextInputClient { attrRope = AttributedRope(string as! String, attributes: typingAttributes) } - transaction { - replaceSubrange(range, with: attrRope) - print("insertText - ", terminator: "") - unmarkText() - } + replaceSubrange(range, with: attrRope) + print("insertText - ", terminator: "") + unmarkText() } override func doCommand(by selector: Selector) { @@ -59,26 +57,24 @@ extension TextView: NSTextInputClient { attrRope = AttributedRope(string as! String, attributes: typingAttributes.merging(markedTextAttributes)) } - transaction { - replaceSubrange(range, with: attrRope) + replaceSubrange(range, with: attrRope) - let start = buffer.index(fromOldIndex: range.lowerBound) - let anchor = buffer.utf16.index(start, offsetBy: selectedRange.lowerBound) - let head = buffer.utf16.index(anchor, offsetBy: selectedRange.length) + let start = buffer.index(fromOldIndex: range.lowerBound) + let anchor = buffer.utf16.index(start, offsetBy: selectedRange.lowerBound) + let head = buffer.utf16.index(anchor, offsetBy: selectedRange.length) - let markedRange: Range? - if attrRope.count == 0 { - markedRange = nil - } else { - let end = buffer.index(start, offsetBy: attrRope.count) - markedRange = start..? + if attrRope.count == 0 { + markedRange = nil + } else { + let end = buffer.index(start, offsetBy: attrRope.count) + markedRange = start.. NSRange { @@ -207,10 +201,8 @@ extension TextView { let undoContents = AttributedRope(buffer[r]) - transaction { - buffer.replaceSubrange(r, with: s) - updateStateAfterReplacingSubrange(r, withStringOfCount: s.count) - } + buffer.replaceSubrange(r, with: s) + updateStateAfterReplacingSubrange(r, withStringOfCount: s.count) let undoRange = r.lowerBound.position..<(r.lowerBound.position + s.count) @@ -227,10 +219,8 @@ extension TextView { let undoContents = String(buffer[r]) - transaction { - buffer.replaceSubrange(r, with: s) - updateStateAfterReplacingSubrange(r, withStringOfCount: s.count) - } + buffer.replaceSubrange(r, with: s) + updateStateAfterReplacingSubrange(r, withStringOfCount: s.count) let undoRange = r.lowerBound.position..<(r.lowerBound.position + s.count) diff --git a/Watt/TextView/TextView+KeyBinding.swift b/Watt/TextView/TextView+KeyBinding.swift index 90fca648..96434f4b 100644 --- a/Watt/TextView/TextView+KeyBinding.swift +++ b/Watt/TextView/TextView+KeyBinding.swift @@ -418,13 +418,11 @@ extension TextView { return } - transaction { - replaceSubrange((i...j).relative(to: buffer.text), with: String(buffer[j]) + String(buffer[i])) - - let anchor = buffer.index(fromOldIndex: i) - let head = buffer.index(anchor, offsetBy: 2) - selection = Selection(anchor: anchor, head: head, granularity: .character) - } + replaceSubrange((i...j).relative(to: buffer.text), with: String(buffer[j]) + String(buffer[i])) + + let anchor = buffer.index(fromOldIndex: i) + let head = buffer.index(anchor, offsetBy: 2) + selection = Selection(anchor: anchor, head: head, granularity: .character) } override func transposeWords(_ sender: Any?) { @@ -437,14 +435,12 @@ extension TextView { b.push(buffer[word1.upperBound.. - var committing: Bool - - init() { - count = 0 - actions = [] - committing = false - } - } - - func beginTransaction() { - precondition(!transaction.committing, "can't start a transaction while committing an existing one") - transaction.count += 1 - } - - func endTransaction() { - precondition(transaction.count >= 1) - transaction.count -= 1 - - guard transaction.count == 0 else { - return - } - - transaction.committing = true - defer { transaction.committing = false } - - for action in transaction.actions { - switch action { - case .textLayout: - layoutTextLayer() - case .selectionLayout: - layoutSelectionLayer() - case .insertionPointLayout: - layoutInsertionPointLayer() - } - } - - transaction.actions.removeAll(keepingCapacity: true) - } - - @discardableResult - func transaction(block: () -> T) -> T { - beginTransaction() - defer { endTransaction() } - return block() - } - - func schedule(_ action: Action) { - transaction { - transaction.actions.append(action) - } - } -} diff --git a/Watt/TextView/TextView.swift b/Watt/TextView/TextView.swift index b1927472..800d24fb 100644 --- a/Watt/TextView/TextView.swift +++ b/Watt/TextView/TextView.swift @@ -91,9 +91,7 @@ class TextView: NSView, ClipViewDelegate { layoutManager.buffer } set { - transaction { - layoutManager.buffer = newValue - } + layoutManager.buffer = newValue } } @@ -103,10 +101,8 @@ class TextView: NSView, ClipViewDelegate { didSet { setTypingAttributes() - transaction { - schedule(.selectionLayout) - schedule(.insertionPointLayout) - } + needsSelectionLayout = true + needsInsertionPointLayout = true updateInsertionPointTimer() } } @@ -142,6 +138,30 @@ class TextView: NSView, ClipViewDelegate { let textLayer: CALayer = CALayer() let insertionPointLayer: CALayer = CALayer() + var needsTextLayout: Bool = false { + didSet { + if needsTextLayout { + needsLayout = true + } + } + } + var needsSelectionLayout: Bool = false { + didSet { + if needsSelectionLayout { + needsLayout = true + } + } + } + var needsInsertionPointLayout: Bool = false { + didSet { + if needsInsertionPointLayout { + needsLayout = true + } + } + } + + var performingLayout: Bool = false + var selectionLayerCache: WeakDictionary = WeakDictionary() var insertionPointLayerCache: WeakDictionary = WeakDictionary() var insertionPointTimer: Timer? @@ -151,8 +171,6 @@ class TextView: NSView, ClipViewDelegate { }() var autoscroller: Autoscroller? - var transaction: Transaction = Transaction() - override init(frame frameRect: NSRect) { layoutManager = LayoutManager() lineNumberView = LineNumberView() @@ -235,11 +253,9 @@ class TextView: NSView, ClipViewDelegate { let widthChanged = oldSize.width != clipView.frame.width if heightChanged || (widthChanged && textContainer.width < .greatestFiniteMagnitude) { - transaction { - schedule(.textLayout) - schedule(.selectionLayout) - schedule(.insertionPointLayout) - } + needsTextLayout = true + needsSelectionLayout = true + needsInsertionPointLayout = true } } @@ -256,11 +272,9 @@ class TextView: NSView, ClipViewDelegate { NotificationCenter.default.addObserver(self, selector: #selector(windowDidBecomeKey(_:)), name: NSWindow.didBecomeKeyNotification, object: window) NotificationCenter.default.addObserver(self, selector: #selector(windowDidResignKey(_:)), name: NSWindow.didResignKeyNotification, object: window) - transaction { - schedule(.textLayout) - schedule(.selectionLayout) - schedule(.insertionPointLayout) - } + needsTextLayout = true + needsSelectionLayout = true + needsInsertionPointLayout = true } } } From c5f2e69fc1339ed35c868774e69606db6521647e Mon Sep 17 00:00:00 2001 From: David Albert Date: Thu, 28 Mar 2024 11:18:07 -0400 Subject: [PATCH 38/73] ScrollAnimator -> ScrollManager --- Watt.xcodeproj/project.pbxproj | 8 ++++---- Watt/TextView/TextView+KeyBinding.swift | 4 ++-- Watt/TextView/TextView+Layout.swift | 2 +- Watt/TextView/TextView.swift | 6 +++--- .../{ScrollAnimator.swift => ScrollManager.swift} | 8 ++++---- 5 files changed, 14 insertions(+), 14 deletions(-) rename Watt/Utilities/{ScrollAnimator.swift => ScrollManager.swift} (98%) diff --git a/Watt.xcodeproj/project.pbxproj b/Watt.xcodeproj/project.pbxproj index fbb15191..afa64d99 100644 --- a/Watt.xcodeproj/project.pbxproj +++ b/Watt.xcodeproj/project.pbxproj @@ -14,7 +14,7 @@ 63206D2A2BA74AF600CAFBC4 /* TextView+Scrolling.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63206D292BA74AF600CAFBC4 /* TextView+Scrolling.swift */; }; 63206D2D2BA8E8F400CAFBC4 /* Autoscroller.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63206D2C2BA8E8F400CAFBC4 /* Autoscroller.swift */; }; 63206D362BAB635500CAFBC4 /* Motion in Frameworks */ = {isa = PBXBuildFile; productRef = 63206D352BAB635500CAFBC4 /* Motion */; }; - 63206D382BAB707E00CAFBC4 /* ScrollAnimator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63206D372BAB707E00CAFBC4 /* ScrollAnimator.swift */; }; + 63206D382BAB707E00CAFBC4 /* ScrollManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63206D372BAB707E00CAFBC4 /* ScrollManager.swift */; }; 63267C292B97BF0B00F6E968 /* BTreeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63267C282B97BF0B00F6E968 /* BTreeTests.swift */; }; 63286AE22A13DF5D00839B25 /* ClipView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63286AE12A13DF5D00839B25 /* ClipView.swift */; }; 632A73722B87C69F009F38C3 /* Range+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 632A73712B87C69F009F38C3 /* Range+Extensions.swift */; }; @@ -136,7 +136,7 @@ 631FD2642A696D27005FA854 /* Heights.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Heights.swift; sourceTree = ""; }; 63206D292BA74AF600CAFBC4 /* TextView+Scrolling.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TextView+Scrolling.swift"; sourceTree = ""; }; 63206D2C2BA8E8F400CAFBC4 /* Autoscroller.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Autoscroller.swift; sourceTree = ""; }; - 63206D372BAB707E00CAFBC4 /* ScrollAnimator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ScrollAnimator.swift; sourceTree = ""; }; + 63206D372BAB707E00CAFBC4 /* ScrollManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ScrollManager.swift; sourceTree = ""; }; 63267C282B97BF0B00F6E968 /* BTreeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTreeTests.swift; sourceTree = ""; }; 63286AE12A13DF5D00839B25 /* ClipView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ClipView.swift; sourceTree = ""; }; 632A73712B87C69F009F38C3 /* Range+Extensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Range+Extensions.swift"; sourceTree = ""; }; @@ -288,7 +288,7 @@ 637E58532B8415A400C498EE /* DragAndDrop.swift */, 63A95D832B5462D60032E8B9 /* FSEvents.swift */, 63C2C03E2B4D95EB00BFB976 /* OutlineViewDiffableDataSource.swift */, - 63206D372BAB707E00CAFBC4 /* ScrollAnimator.swift */, + 63206D372BAB707E00CAFBC4 /* ScrollManager.swift */, 63B8B1752A9F890100F32C2C /* Utilities.swift */, 63A618712A0EAF2000E70B0E /* Weak.swift */, ); @@ -709,7 +709,7 @@ 6378677D2A1BE3FE0039960E /* LineNumberLayer.swift in Sources */, 632A73722B87C69F009F38C3 /* Range+Extensions.swift in Sources */, 63F0468A2A65D3EA001B8DA8 /* BTree.swift in Sources */, - 63206D382BAB707E00CAFBC4 /* ScrollAnimator.swift in Sources */, + 63206D382BAB707E00CAFBC4 /* ScrollManager.swift in Sources */, 63DD20DB2B7278E400E55747 /* CheckedContinuationReference.swift in Sources */, 637867812A1BEA960039960E /* InsertionPointLayer.swift in Sources */, 63F25BE52A68A27600DEACCF /* LineFragment.swift in Sources */, diff --git a/Watt/TextView/TextView+KeyBinding.swift b/Watt/TextView/TextView+KeyBinding.swift index 96434f4b..8c9d9e2b 100644 --- a/Watt/TextView/TextView+KeyBinding.swift +++ b/Watt/TextView/TextView+KeyBinding.swift @@ -376,7 +376,7 @@ extension TextView { y: 0 ) - scrollAnimator.animateScroll(to: point) + scrollManager.animateScroll(to: point) } override func scrollToEndOfDocument(_ sender: Any?) { @@ -407,7 +407,7 @@ extension TextView { y: frame.height - viewport.height ) - scrollAnimator.animateScroll(to: point) + scrollManager.animateScroll(to: point) } diff --git a/Watt/TextView/TextView+Layout.swift b/Watt/TextView/TextView+Layout.swift index e0b43cba..2a18c14f 100644 --- a/Watt/TextView/TextView+Layout.swift +++ b/Watt/TextView/TextView+Layout.swift @@ -215,7 +215,7 @@ extension TextView: LayoutManagerDelegate { func layoutManager(_ layoutManager: LayoutManager, rect: CGRect, didResizeTo newSize: CGSize) { let rect = convertFromTextContainer(rect) - scrollAnimator.documentRect(rect, didResizeTo: newSize) + scrollManager.documentRect(rect, didResizeTo: newSize) } func layoutManager(_ layoutManager: LayoutManager, createLayerForLine line: Line) -> LineLayer { diff --git a/Watt/TextView/TextView.swift b/Watt/TextView/TextView.swift index 800d24fb..334b01ab 100644 --- a/Watt/TextView/TextView.swift +++ b/Watt/TextView/TextView.swift @@ -166,8 +166,8 @@ class TextView: NSView, ClipViewDelegate { var insertionPointLayerCache: WeakDictionary = WeakDictionary() var insertionPointTimer: Timer? - lazy var scrollAnimator: ScrollAnimator = { - ScrollAnimator(self) + lazy var scrollManager: ScrollManager = { + ScrollManager(self) }() var autoscroller: Autoscroller? @@ -232,7 +232,7 @@ class TextView: NSView, ClipViewDelegate { } override func viewDidMoveToSuperview() { - scrollAnimator.viewDidMoveToSuperview() + scrollManager.viewDidMoveToSuperview() } // This is a custom method on ClipViewDelegate. Normally we'd use diff --git a/Watt/Utilities/ScrollAnimator.swift b/Watt/Utilities/ScrollManager.swift similarity index 98% rename from Watt/Utilities/ScrollAnimator.swift rename to Watt/Utilities/ScrollManager.swift index 60e4b05c..1254f6f0 100644 --- a/Watt/Utilities/ScrollAnimator.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -1,5 +1,5 @@ // -// ScrollAnimator.swift +// ScrollManager.swift // Watt // // Created by David Albert on 3/20/24. @@ -9,7 +9,7 @@ import Cocoa import Motion @MainActor -class ScrollAnimator { +class ScrollManager { private(set) weak var view: NSView? private(set) var isDraggingScroller: Bool @@ -166,8 +166,8 @@ class ScrollAnimator { assert(scrollView == view?.enclosingScrollView) if isDraggingScroller { - scheduleScrollCorrection() - } + scheduleScrollCorrection() + } } @objc func didEndLiveScroll(_ notification: Notification) { From 746548976d056c92503e05ca8574685d9f4f69fb Mon Sep 17 00:00:00 2001 From: David Albert Date: Thu, 28 Mar 2024 12:58:33 -0400 Subject: [PATCH 39/73] Attempt to fix dragging scroller --- Watt.xcodeproj/project.pbxproj | 4 +++ Watt/Extensions/CGPoint+Extensions.swift | 6 ++++ Watt/Extensions/CGVector+Extensions.swift | 18 +++++++++++ Watt/LayoutManager/LayoutManager.swift | 39 +++++++++++++++++------ Watt/TextView/TextView.swift | 3 +- Watt/Utilities/ScrollManager.swift | 33 +++++++++++++++++-- 6 files changed, 89 insertions(+), 14 deletions(-) create mode 100644 Watt/Extensions/CGVector+Extensions.swift diff --git a/Watt.xcodeproj/project.pbxproj b/Watt.xcodeproj/project.pbxproj index afa64d99..99bb55ae 100644 --- a/Watt.xcodeproj/project.pbxproj +++ b/Watt.xcodeproj/project.pbxproj @@ -15,6 +15,7 @@ 63206D2D2BA8E8F400CAFBC4 /* Autoscroller.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63206D2C2BA8E8F400CAFBC4 /* Autoscroller.swift */; }; 63206D362BAB635500CAFBC4 /* Motion in Frameworks */ = {isa = PBXBuildFile; productRef = 63206D352BAB635500CAFBC4 /* Motion */; }; 63206D382BAB707E00CAFBC4 /* ScrollManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63206D372BAB707E00CAFBC4 /* ScrollManager.swift */; }; + 63206D3A2BB5C65500CAFBC4 /* CGVector+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63206D392BB5C65500CAFBC4 /* CGVector+Extensions.swift */; }; 63267C292B97BF0B00F6E968 /* BTreeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63267C282B97BF0B00F6E968 /* BTreeTests.swift */; }; 63286AE22A13DF5D00839B25 /* ClipView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63286AE12A13DF5D00839B25 /* ClipView.swift */; }; 632A73722B87C69F009F38C3 /* Range+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 632A73712B87C69F009F38C3 /* Range+Extensions.swift */; }; @@ -137,6 +138,7 @@ 63206D292BA74AF600CAFBC4 /* TextView+Scrolling.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TextView+Scrolling.swift"; sourceTree = ""; }; 63206D2C2BA8E8F400CAFBC4 /* Autoscroller.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Autoscroller.swift; sourceTree = ""; }; 63206D372BAB707E00CAFBC4 /* ScrollManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ScrollManager.swift; sourceTree = ""; }; + 63206D392BB5C65500CAFBC4 /* CGVector+Extensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CGVector+Extensions.swift"; sourceTree = ""; }; 63267C282B97BF0B00F6E968 /* BTreeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTreeTests.swift; sourceTree = ""; }; 63286AE12A13DF5D00839B25 /* ClipView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ClipView.swift; sourceTree = ""; }; 632A73712B87C69F009F38C3 /* Range+Extensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Range+Extensions.swift"; sourceTree = ""; }; @@ -335,6 +337,7 @@ 637BC1432A156B820089A34D /* CGPoint+Extensions.swift */, 637BC1412A156B620089A34D /* CGRect+Extensions.swift */, 637BC1452A156B9B0089A34D /* CGSize+Extensions.swift */, + 63206D392BB5C65500CAFBC4 /* CGVector+Extensions.swift */, 63820B8C2A7164A500CC7458 /* Collection+Extensions.swift */, 6396DD252B433775000F85D1 /* Comparable+Extensions.swift */, 63DD20E12B76B59300E55747 /* Logger+Extensions.swift */, @@ -723,6 +726,7 @@ 63F25BD92A683CC500DEACCF /* Buffer.swift in Sources */, 63DD20DF2B73CFF200E55747 /* NSObject+Extensions.m in Sources */, 63DD20E22B76B59300E55747 /* Logger+Extensions.swift in Sources */, + 63206D3A2BB5C65500CAFBC4 /* CGVector+Extensions.swift in Sources */, 637867712A17F7460039960E /* TextView+Selection.swift in Sources */, 6364DD972B6C05C300886DE3 /* WorkspaceWindowController.swift in Sources */, 63D280902B5EC14D00CDCF04 /* NSFilePromiseReceiver+Extensions.swift in Sources */, diff --git a/Watt/Extensions/CGPoint+Extensions.swift b/Watt/Extensions/CGPoint+Extensions.swift index 53190bed..31ac35ce 100644 --- a/Watt/Extensions/CGPoint+Extensions.swift +++ b/Watt/Extensions/CGPoint+Extensions.swift @@ -14,6 +14,12 @@ extension CGPoint: Hashable { } } +extension CGPoint { + static func + (lhs: CGPoint, rhs: CGVector) -> CGPoint { + CGPoint(x: lhs.x + rhs.dx, y: lhs.y + rhs.dy) + } +} + extension CGPoint { func clamped(to rect: CGRect) -> CGPoint { CGPoint(x: x.clamped(to: rect.minX...rect.maxX), y: y.clamped(to: rect.minY...rect.maxY)) diff --git a/Watt/Extensions/CGVector+Extensions.swift b/Watt/Extensions/CGVector+Extensions.swift new file mode 100644 index 00000000..3da48bef --- /dev/null +++ b/Watt/Extensions/CGVector+Extensions.swift @@ -0,0 +1,18 @@ +// +// CGVector+Extensions.swift +// Watt +// +// Created by David Albert on 3/28/24. +// + +import CoreGraphics + +extension CGVector: AdditiveArithmetic { + public static func + (lhs: CGVector, rhs: CGVector) -> CGVector { + .init(dx: lhs.dx + rhs.dx, dy: lhs.dy + rhs.dy) + } + + public static func - (lhs: CGVector, rhs: CGVector) -> CGVector { + .init(dx: lhs.dx - rhs.dx, dy: lhs.dy - rhs.dy) + } +} diff --git a/Watt/LayoutManager/LayoutManager.swift b/Watt/LayoutManager/LayoutManager.swift index c50b8b25..54158ee2 100644 --- a/Watt/LayoutManager/LayoutManager.swift +++ b/Watt/LayoutManager/LayoutManager.swift @@ -35,6 +35,9 @@ extension LayoutManagerDelegate { } } +var n = 0.0 +var m = 0.0 + class LayoutManager { weak var delegate: LayoutManagerDelegate? @@ -51,9 +54,6 @@ class LayoutManager { var heights: Heights - // invariant: sorted by line.range.lowerBound - var lineLayers: [LineLayer] - var textContainer: TextContainer { didSet { if textContainer != oldValue { @@ -62,11 +62,17 @@ class LayoutManager { } } + // invariant: sorted by line.range.lowerBound + var lineLayers: [LineLayer] + + var scrollCorrection: CGVector + init() { + buffer = Buffer() heights = Heights() - lineLayers = [] textContainer = TextContainer() - buffer = Buffer() + lineLayers = [] + scrollCorrection = .zero buffer.addDelegate(self) } @@ -80,20 +86,21 @@ class LayoutManager { return } - let viewportBounds = delegate.viewportBounds(for: self) - let start = startOfFirstLine(intersecting: viewportBounds) + let viewport = delegate.viewportBounds(for: self) + let start = startOfFirstLine(intersecting: viewport) var layers: [LineLayer] = [] - var height: CGFloat = 0 enumerateLines(startingAt: start) { line, existingLayer in + n += 1 let layer = existingLayer ?? delegate.layoutManager(self, createLayerForLine: line) delegate.layoutManager(self, positionLineLayer: layer) layers.append(layer) block(layer) - height += line.alignmentFrame.height - max(viewportBounds.minY - line.alignmentFrame.minY, 0) - return height <= viewportBounds.height + return line.alignmentFrame.maxY <= viewport.minY + scrollCorrection.dy + viewport.height } + m += 1 + print("!", n, n/m) lineLayers = layers } @@ -769,6 +776,18 @@ class LayoutManager { } } +// MARK: - ScrollManagerDelegate + +extension LayoutManager: ScrollManagerDelegate { + func scrollManager(_ scrollManager: ScrollManager, willCorrectScrollBy delta: CGVector) { + scrollCorrection += delta + } + + func scrollManager(_ scrollManager: ScrollManager, didCorrectScrollBy delta: CGVector) { + scrollCorrection = .zero + } +} + // MARK: - BufferDelegate extension LayoutManager: BufferDelegate { diff --git a/Watt/TextView/TextView.swift b/Watt/TextView/TextView.swift index 334b01ab..5c7df57f 100644 --- a/Watt/TextView/TextView.swift +++ b/Watt/TextView/TextView.swift @@ -191,9 +191,10 @@ class TextView: NSView, ClipViewDelegate { layerContentsRedrawPolicy = .onSetNeedsDisplay layoutManager.buffer = buffer - layoutManager.delegate = self + scrollManager.delegate = layoutManager + lineNumberView.lineCount = buffer.lines.count lineNumberView.font = font lineNumberView.textColor = theme.lineNumberColor diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index 1254f6f0..1fa40004 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -8,6 +8,21 @@ import Cocoa import Motion +protocol ScrollManagerDelegate: AnyObject { + // Called once for every call to documentRect(_:didResizeTo:) that performs a scroll correction. + // Can be called multiple times for each call to scrollmanager(_:didCorrectScrollBy:). + func scrollManager(_ scrollManager: ScrollManager, willCorrectScrollBy delta: CGVector) + + // Called once per run loop tick where scroll correction was performed. One call to this method + // can correspond to multiple calls to scrollManager(_:willCorrectScrollBy:). + func scrollManager(_ scrollManager: ScrollManager, didCorrectScrollBy delta: CGVector) +} + +extension ScrollManagerDelegate { + func scrollManager(_ scrollManager: ScrollManager, willCorrectScrollBy delta: CGVector) {} + func scrollManager(_ scrollManager: ScrollManager, didCorrectScrollBy delta: CGVector) {} +} + @MainActor class ScrollManager { private(set) weak var view: NSView? @@ -28,6 +43,8 @@ class ScrollManager { animation != nil } + weak var delegate: ScrollManagerDelegate? + init(_ view: NSView) { self.view = view self.isDraggingScroller = false @@ -119,6 +136,8 @@ class ScrollManager { return } + delegate?.scrollManager(self, willCorrectScrollBy: CGVector(dx: dx, dy: dy)) + scrollOffset = CGPoint(x: scrollOffset.x + dx, y: scrollOffset.y + dy) scheduleScrollCorrection() } @@ -215,17 +234,25 @@ class ScrollManager { if viewport.origin != offset { scrollView.documentView?.scroll(offset) + + let delta = CGVector(dx: offset.x - viewport.origin.x, dy: offset.y - viewport.origin.y) + delegate?.scrollManager(self, didCorrectScrollBy: delta) } return } - let offset = scrollView.contentView.bounds.origin - if offset != scrollOffset { + let viewport = scrollView.contentView.bounds + if viewport.origin != scrollOffset { if isAnimating { animateScroll(to: scrollOffset) - assert(offset == presentation.scrollOffset) + assert(viewport.origin == presentation.scrollOffset) + + let delta = CGVector(dx: presentation.scrollOffset.x - viewport.origin.x, dy: presentation.scrollOffset.y - viewport.origin.y) + delegate?.scrollManager(self, didCorrectScrollBy: delta) } else { scrollView.documentView?.scroll(scrollOffset) + let delta = CGVector(dx: scrollOffset.x - viewport.origin.x, dy: scrollOffset.y - viewport.origin.y) + delegate?.scrollManager(self, didCorrectScrollBy: delta) } } } From 2a0e2e26690fbe7566b0bf29e6490efc82217d2c Mon Sep 17 00:00:00 2001 From: David Albert Date: Thu, 28 Mar 2024 13:03:30 -0400 Subject: [PATCH 40/73] Make scrollToEndOfDocument work --- Watt/Utilities/ScrollManager.swift | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index 1fa40004..7e9cb128 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -35,6 +35,8 @@ class ScrollManager { private(set) var scrollOffset: CGPoint private(set) var presentation: PresentationProperties + private var delta: CGVector + private var isScrollCorrectionScheduled: Bool private var animation: SpringAnimation? @@ -50,6 +52,7 @@ class ScrollManager { self.isDraggingScroller = false self.scrollOffset = .zero self.presentation = PresentationProperties(scrollOffset: .zero) + self.delta = .zero self.isScrollCorrectionScheduled = false if view.superview != nil { @@ -72,6 +75,7 @@ class ScrollManager { let offset = view?.enclosingScrollView?.contentView.bounds.origin ?? .zero scrollOffset = offset presentation.scrollOffset = offset + delta = .zero if let scrollView = view?.enclosingScrollView { NotificationCenter.default.addObserver(self, selector: #selector(viewDidScroll(_:)), name: NSView.boundsDidChangeNotification, object: scrollView.contentView) @@ -136,6 +140,8 @@ class ScrollManager { return } + delta += CGVector(dx: dx, dy: dy) + delegate?.scrollManager(self, willCorrectScrollBy: CGVector(dx: dx, dy: dy)) scrollOffset = CGPoint(x: scrollOffset.x + dx, y: scrollOffset.y + dy) @@ -209,6 +215,8 @@ class ScrollManager { return } + defer { delta = .zero } + isScrollCorrectionScheduled = false guard let scrollView = view?.enclosingScrollView else { return @@ -244,10 +252,8 @@ class ScrollManager { let viewport = scrollView.contentView.bounds if viewport.origin != scrollOffset { if isAnimating { + scrollView.documentView?.scroll(presentation.scrollOffset + delta) animateScroll(to: scrollOffset) - assert(viewport.origin == presentation.scrollOffset) - - let delta = CGVector(dx: presentation.scrollOffset.x - viewport.origin.x, dy: presentation.scrollOffset.y - viewport.origin.y) delegate?.scrollManager(self, didCorrectScrollBy: delta) } else { scrollView.documentView?.scroll(scrollOffset) From ee23af2233dc39130b49318146eb0d4eccbde4b3 Mon Sep 17 00:00:00 2001 From: David Albert Date: Thu, 28 Mar 2024 15:38:14 -0400 Subject: [PATCH 41/73] Simplify ScrollManagerDelegate --- Watt/LayoutManager/LayoutManager.swift | 1 + Watt/Utilities/ScrollManager.swift | 13 +++++-------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/Watt/LayoutManager/LayoutManager.swift b/Watt/LayoutManager/LayoutManager.swift index 54158ee2..43afa7de 100644 --- a/Watt/LayoutManager/LayoutManager.swift +++ b/Watt/LayoutManager/LayoutManager.swift @@ -784,6 +784,7 @@ extension LayoutManager: ScrollManagerDelegate { } func scrollManager(_ scrollManager: ScrollManager, didCorrectScrollBy delta: CGVector) { + func scrollManagerDidCorrectScroll(_ scrollManager: ScrollManager) { scrollCorrection = .zero } } diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index 7e9cb128..8501ad3e 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -15,12 +15,12 @@ protocol ScrollManagerDelegate: AnyObject { // Called once per run loop tick where scroll correction was performed. One call to this method // can correspond to multiple calls to scrollManager(_:willCorrectScrollBy:). - func scrollManager(_ scrollManager: ScrollManager, didCorrectScrollBy delta: CGVector) + func scrollManagerDidCorrectScroll(_ scrollManager: ScrollManager) } extension ScrollManagerDelegate { func scrollManager(_ scrollManager: ScrollManager, willCorrectScrollBy delta: CGVector) {} - func scrollManager(_ scrollManager: ScrollManager, didCorrectScrollBy delta: CGVector) {} + func scrollManagerDidCorrectScroll(_ scrollManager: ScrollManager) {} } @MainActor @@ -242,9 +242,7 @@ class ScrollManager { if viewport.origin != offset { scrollView.documentView?.scroll(offset) - - let delta = CGVector(dx: offset.x - viewport.origin.x, dy: offset.y - viewport.origin.y) - delegate?.scrollManager(self, didCorrectScrollBy: delta) + delegate?.scrollManagerDidCorrectScroll(self) } return } @@ -254,11 +252,10 @@ class ScrollManager { if isAnimating { scrollView.documentView?.scroll(presentation.scrollOffset + delta) animateScroll(to: scrollOffset) - delegate?.scrollManager(self, didCorrectScrollBy: delta) + delegate?.scrollManagerDidCorrectScroll(self) } else { scrollView.documentView?.scroll(scrollOffset) - let delta = CGVector(dx: scrollOffset.x - viewport.origin.x, dy: scrollOffset.y - viewport.origin.y) - delegate?.scrollManager(self, didCorrectScrollBy: delta) + delegate?.scrollManagerDidCorrectScroll(self) } } } From 358b7462777cc66122b8f5318afb4808db4e2562 Mon Sep 17 00:00:00 2001 From: David Albert Date: Thu, 28 Mar 2024 16:18:37 -0400 Subject: [PATCH 42/73] Working (I think) scroller drag --- Watt/LayoutManager/LayoutManager.swift | 8 -------- Watt/Utilities/ScrollManager.swift | 14 ++++++++++++-- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/Watt/LayoutManager/LayoutManager.swift b/Watt/LayoutManager/LayoutManager.swift index 43afa7de..a2777f90 100644 --- a/Watt/LayoutManager/LayoutManager.swift +++ b/Watt/LayoutManager/LayoutManager.swift @@ -35,9 +35,6 @@ extension LayoutManagerDelegate { } } -var n = 0.0 -var m = 0.0 - class LayoutManager { weak var delegate: LayoutManagerDelegate? @@ -91,16 +88,12 @@ class LayoutManager { var layers: [LineLayer] = [] enumerateLines(startingAt: start) { line, existingLayer in - n += 1 let layer = existingLayer ?? delegate.layoutManager(self, createLayerForLine: line) delegate.layoutManager(self, positionLineLayer: layer) layers.append(layer) block(layer) - return line.alignmentFrame.maxY <= viewport.minY + scrollCorrection.dy + viewport.height } - m += 1 - print("!", n, n/m) lineLayers = layers } @@ -783,7 +776,6 @@ extension LayoutManager: ScrollManagerDelegate { scrollCorrection += delta } - func scrollManager(_ scrollManager: ScrollManager, didCorrectScrollBy delta: CGVector) { func scrollManagerDidCorrectScroll(_ scrollManager: ScrollManager) { scrollCorrection = .zero } diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index 8501ad3e..f98f74c0 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -35,6 +35,8 @@ class ScrollManager { private(set) var scrollOffset: CGPoint private(set) var presentation: PresentationProperties + private var prevPresentationScrollOffset: CGPoint + private var delta: CGVector private var isScrollCorrectionScheduled: Bool @@ -52,6 +54,7 @@ class ScrollManager { self.isDraggingScroller = false self.scrollOffset = .zero self.presentation = PresentationProperties(scrollOffset: .zero) + self.prevPresentationScrollOffset = .zero self.delta = .zero self.isScrollCorrectionScheduled = false @@ -75,6 +78,7 @@ class ScrollManager { let offset = view?.enclosingScrollView?.contentView.bounds.origin ?? .zero scrollOffset = offset presentation.scrollOffset = offset + prevPresentationScrollOffset = offset delta = .zero if let scrollView = view?.enclosingScrollView { @@ -129,10 +133,14 @@ class ScrollManager { // When animating upwards, any size change with an origin above the viewport contributes // to scroll correction. When animating downwards, size changes with origins in the viewport // also contribute. + let movingRight = presentation.scrollOffset.x > prevPresentationScrollOffset.x + let movingDown = presentation.scrollOffset.y > prevPresentationScrollOffset.y + let viewport = scrollView.contentView.bounds - let anchorX = scrollOffset.x > presentation.scrollOffset.x ? 1.0 : 0.0 - let anchorY = scrollOffset.y > presentation.scrollOffset.y ? 1.0 : 0.0 + let anchorX = movingRight ? 1.0 : 0.0 + let anchorY = movingDown ? 1.0 : 0.0 + // TODO: I think this should actually be maxX and maxY, but that jumps when scrolling up. See https://dave.is/worklog/2023/07/24/adjusting-scroll-offset-when-document-height-changes/ let dx = rect.minX >= scrollOffset.x + (anchorX*viewport.width) ? 0 : newSize.width - rect.width let dy = rect.minY >= scrollOffset.y + (anchorY*viewport.height) ? 0 : newSize.height - rect.height @@ -155,6 +163,8 @@ class ScrollManager { assert(scrollView.contentView == (notification.object as? NSView)) + prevPresentationScrollOffset = presentation.scrollOffset + let offset = scrollView.contentView.bounds.origin if isAnimating { presentation.scrollOffset = offset From 5dfa2d6abfff4ea8db0701b4eb41c7219261ec54 Mon Sep 17 00:00:00 2001 From: David Albert Date: Fri, 29 Mar 2024 10:55:53 -0400 Subject: [PATCH 43/73] WIP more correct scroll correction code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scrolling up and dragging works. Animating doesn’t. --- Watt/LayoutManager/LayoutManager.swift | 1 + Watt/Utilities/ScrollManager.swift | 129 ++++++++++++------------- 2 files changed, 65 insertions(+), 65 deletions(-) diff --git a/Watt/LayoutManager/LayoutManager.swift b/Watt/LayoutManager/LayoutManager.swift index a2777f90..1a13aaca 100644 --- a/Watt/LayoutManager/LayoutManager.swift +++ b/Watt/LayoutManager/LayoutManager.swift @@ -93,6 +93,7 @@ class LayoutManager { layers.append(layer) block(layer) return line.alignmentFrame.maxY <= viewport.minY + scrollCorrection.dy + viewport.height + return line.alignmentFrame.maxY <= viewport.maxY + scrollCorrection.dy } lineLayers = layers } diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index f98f74c0..e8324f90 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -15,12 +15,12 @@ protocol ScrollManagerDelegate: AnyObject { // Called once per run loop tick where scroll correction was performed. One call to this method // can correspond to multiple calls to scrollManager(_:willCorrectScrollBy:). - func scrollManagerDidCorrectScroll(_ scrollManager: ScrollManager) + func scrollManagerDidPerformScrollCorrection(_ scrollManager: ScrollManager) } extension ScrollManagerDelegate { func scrollManager(_ scrollManager: ScrollManager, willCorrectScrollBy delta: CGVector) {} - func scrollManagerDidCorrectScroll(_ scrollManager: ScrollManager) {} + func scrollManagerDidPerformScrollCorrection(_ scrollManager: ScrollManager) {} } @MainActor @@ -28,18 +28,11 @@ class ScrollManager { private(set) weak var view: NSView? private(set) var isDraggingScroller: Bool - // Think model and presentation layers in Core Animation - struct PresentationProperties { - var scrollOffset: CGPoint - } private(set) var scrollOffset: CGPoint - private(set) var presentation: PresentationProperties - - private var prevPresentationScrollOffset: CGPoint - + private var prevScrollOffset: CGPoint private var delta: CGVector - private var isScrollCorrectionScheduled: Bool + private var needsScrollCorrection: Bool private var animation: SpringAnimation? @@ -47,22 +40,42 @@ class ScrollManager { animation != nil } + var animationDestination: CGPoint? { + animation?.toValue + } + weak var delegate: ScrollManagerDelegate? + private var observer: CFRunLoopObserver? + init(_ view: NSView) { self.view = view self.isDraggingScroller = false self.scrollOffset = .zero - self.presentation = PresentationProperties(scrollOffset: .zero) - self.prevPresentationScrollOffset = .zero + self.prevScrollOffset = .zero self.delta = .zero - self.isScrollCorrectionScheduled = false + self.needsScrollCorrection = false + + var context = CFRunLoopObserverContext(version: 0, info: Unmanaged.passUnretained(self).toOpaque(), retain: nil, release: nil, copyDescription: nil) + let observer = CFRunLoopObserverCreate(kCFAllocatorDefault, CFRunLoopActivity.beforeSources.rawValue, true, 0, { observer, activity, info in + let manager = Unmanaged.fromOpaque(info!).takeUnretainedValue() + manager.performScrollCorrection() + }, &context) + + CFRunLoopAddObserver(CFRunLoopGetMain(), observer, .commonModes) + self.observer = observer if view.superview != nil { viewDidMoveToSuperview() } } + deinit { + if let observer { + CFRunLoopRemoveObserver(CFRunLoopGetMain(), observer, .commonModes) + } + } + func viewDidMoveToSuperview() { NotificationCenter.default.removeObserver(self, name: NSView.boundsDidChangeNotification, object: nil) NotificationCenter.default.removeObserver(self, name: NSScrollView.willStartLiveScrollNotification, object: nil) @@ -73,12 +86,11 @@ class ScrollManager { animation = nil isDraggingScroller = false - isScrollCorrectionScheduled = false + needsScrollCorrection = false let offset = view?.enclosingScrollView?.contentView.bounds.origin ?? .zero scrollOffset = offset - presentation.scrollOffset = offset - prevPresentationScrollOffset = offset + prevScrollOffset = offset delta = .zero if let scrollView = view?.enclosingScrollView { @@ -94,20 +106,18 @@ class ScrollManager { return } - scrollOffset = point - let velocity = animation?.velocity ?? .zero animation?.stop() - assert(presentation.scrollOffset == scrollView.contentView.bounds.origin) + assert(scrollOffset == scrollView.contentView.bounds.origin) let spring = SpringAnimation( - initialValue: presentation.scrollOffset, + initialValue: scrollOffset, response: 0.2, dampingRatio: 1.0, environment: scrollView ) - spring.toValue = scrollOffset + spring.toValue = point spring.velocity = velocity spring.resolvingEpsilon = 0.000001 @@ -128,21 +138,28 @@ class ScrollManager { return } - assert(presentation.scrollOffset == scrollView.contentView.bounds.origin) + // If we're dragging the scroller, we're requesting an absolute percent through the document. + // If layout changed at all while dragging, we're going to be in the wrong place, and need + // to correct. + if isDraggingScroller { + needsScrollCorrection = true + return + } // When animating upwards, any size change with an origin above the viewport contributes // to scroll correction. When animating downwards, size changes with origins in the viewport // also contribute. - let movingRight = presentation.scrollOffset.x > prevPresentationScrollOffset.x - let movingDown = presentation.scrollOffset.y > prevPresentationScrollOffset.y + let movingRight = scrollOffset.x > prevScrollOffset.x + let movingDown = scrollOffset.y > prevScrollOffset.y - let viewport = scrollView.contentView.bounds let anchorX = movingRight ? 1.0 : 0.0 let anchorY = movingDown ? 1.0 : 0.0 - // TODO: I think this should actually be maxX and maxY, but that jumps when scrolling up. See https://dave.is/worklog/2023/07/24/adjusting-scroll-offset-when-document-height-changes/ - let dx = rect.minX >= scrollOffset.x + (anchorX*viewport.width) ? 0 : newSize.width - rect.width - let dy = rect.minY >= scrollOffset.y + (anchorY*viewport.height) ? 0 : newSize.height - rect.height + assert(scrollOffset == scrollView.contentView.bounds.origin) + let viewport = scrollView.contentView.bounds + + let dx = rect.maxX <= (animationDestination ?? prevScrollOffset).x + (anchorX*viewport.width) ? newSize.width - rect.width : 0 + let dy = rect.maxY <= (animationDestination ?? prevScrollOffset).y + (anchorY*viewport.height) ? newSize.height - rect.height : 0 if dx == 0 && dy == 0 { return @@ -151,9 +168,7 @@ class ScrollManager { delta += CGVector(dx: dx, dy: dy) delegate?.scrollManager(self, willCorrectScrollBy: CGVector(dx: dx, dy: dy)) - - scrollOffset = CGPoint(x: scrollOffset.x + dx, y: scrollOffset.y + dy) - scheduleScrollCorrection() + needsScrollCorrection = true } @objc func viewDidScroll(_ notification: Notification) { @@ -163,15 +178,8 @@ class ScrollManager { assert(scrollView.contentView == (notification.object as? NSView)) - prevPresentationScrollOffset = presentation.scrollOffset - - let offset = scrollView.contentView.bounds.origin - if isAnimating { - presentation.scrollOffset = offset - } else { - scrollOffset = offset - presentation.scrollOffset = offset - } + prevScrollOffset = scrollOffset + scrollOffset = scrollView.contentView.bounds.origin } @objc func willStartLiveScroll(_ notification: Notification) { @@ -179,8 +187,6 @@ class ScrollManager { animation?.stop() animation = nil - scrollOffset = presentation.scrollOffset - guard let scrollView = notification.object as? NSScrollView else { return } @@ -201,7 +207,7 @@ class ScrollManager { assert(scrollView == view?.enclosingScrollView) if isDraggingScroller { - scheduleScrollCorrection() + needsScrollCorrection = true } } @@ -209,29 +215,28 @@ class ScrollManager { isDraggingScroller = false } - private func scheduleScrollCorrection() { - if isScrollCorrectionScheduled { + private func performScrollCorrection() { + if !needsScrollCorrection { return } - isScrollCorrectionScheduled = true - DispatchQueue.main.async { [weak self] in - self?.performScrollCorrection() - } - } - - private func performScrollCorrection() { - if !isScrollCorrectionScheduled { + // We always want scroll correction to run after layout so that we can take into account any resizing + // that happens during layout. Layout happens in a .beforeTimers observer, and we run in a .beforeSources + // observer, which happens later. But there may be any number of iterations through run loop between when + // needsScrollCorrection is set and layout is run. So if layout hasn't happened yet, bail early. + if let view, view.needsLayout { return } defer { delta = .zero } + needsScrollCorrection = false - isScrollCorrectionScheduled = false guard let scrollView = view?.enclosingScrollView else { return } + defer { delegate?.scrollManagerDidPerformScrollCorrection(self) } + // Dragging a scroller is equivalent to telling the scroll view "please set your offset to this // fixed percentage through the document." In this case, scroll correction just consists of // reading the percentage out of the scrollers and setting the new origin if the document size @@ -252,20 +257,14 @@ class ScrollManager { if viewport.origin != offset { scrollView.documentView?.scroll(offset) - delegate?.scrollManagerDidCorrectScroll(self) } return } - let viewport = scrollView.contentView.bounds - if viewport.origin != scrollOffset { - if isAnimating { - scrollView.documentView?.scroll(presentation.scrollOffset + delta) - animateScroll(to: scrollOffset) - delegate?.scrollManagerDidCorrectScroll(self) - } else { - scrollView.documentView?.scroll(scrollOffset) - delegate?.scrollManagerDidCorrectScroll(self) + if delta != .zero { + scrollView.documentView?.scroll(scrollOffset + delta) + if let animation { + animateScroll(to: animation.toValue + delta) } } } From 4bba8d6f99e6daeadf4672d9e6e92176fa400645 Mon Sep 17 00:00:00 2001 From: David Albert Date: Fri, 29 Mar 2024 11:11:12 -0400 Subject: [PATCH 44/73] =?UTF-8?q?Explain=20why=20we=E2=80=99re=20using=20a?= =?UTF-8?q?=20run=20loop=20observer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Watt/Utilities/ScrollManager.swift | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index e8324f90..116e85b8 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -46,6 +46,10 @@ class ScrollManager { weak var delegate: ScrollManagerDelegate? + // We use a run loop observer, rather than DispatchQueue.main.async because we want to be able to wait + // until after layout has been performed to do scroll correction. While this is possible to do with + // libdispatch – we'd just have to reschedule performScrollCorrection() if layout hasn't been performed + // yet – it's a bit clearer if we just check every tick of the run loop. private var observer: CFRunLoopObserver? init(_ view: NSView) { @@ -57,6 +61,9 @@ class ScrollManager { self.needsScrollCorrection = false var context = CFRunLoopObserverContext(version: 0, info: Unmanaged.passUnretained(self).toOpaque(), retain: nil, release: nil, copyDescription: nil) + + // .beforeSources is after .beforeTimers, and layout is run on .beforeTimers, so we use .beforeSources so that we + // can run as early as possible after layout occurs. let observer = CFRunLoopObserverCreate(kCFAllocatorDefault, CFRunLoopActivity.beforeSources.rawValue, true, 0, { observer, activity, info in let manager = Unmanaged.fromOpaque(info!).takeUnretainedValue() manager.performScrollCorrection() From 5760fd3264a149f9cbd7cdf68f65e3b305aef59a Mon Sep 17 00:00:00 2001 From: David Albert Date: Fri, 29 Mar 2024 15:11:36 -0400 Subject: [PATCH 45/73] Better scroll correction, but animation is still broken --- Watt/LayoutManager/LayoutManager.swift | 3 +- Watt/TextView/TextView.swift | 15 +-- Watt/Utilities/ScrollManager.swift | 127 ++++++++++++++++++------- 3 files changed, 103 insertions(+), 42 deletions(-) diff --git a/Watt/LayoutManager/LayoutManager.swift b/Watt/LayoutManager/LayoutManager.swift index 1a13aaca..216539e2 100644 --- a/Watt/LayoutManager/LayoutManager.swift +++ b/Watt/LayoutManager/LayoutManager.swift @@ -777,7 +777,8 @@ extension LayoutManager: ScrollManagerDelegate { scrollCorrection += delta } - func scrollManagerDidCorrectScroll(_ scrollManager: ScrollManager) { + // Called after layout + func scrollManagerDidCommitScrollCorrection(_ scrollManager: ScrollManager) { scrollCorrection = .zero } } diff --git a/Watt/TextView/TextView.swift b/Watt/TextView/TextView.swift index 5c7df57f..f2791f45 100644 --- a/Watt/TextView/TextView.swift +++ b/Watt/TextView/TextView.swift @@ -233,6 +233,12 @@ class TextView: NSView, ClipViewDelegate { } override func viewDidMoveToSuperview() { + NotificationCenter.default.removeObserver(self, name: NSView.boundsDidChangeNotification, object: nil) + + if let scrollView { + NotificationCenter.default.addObserver(self, selector: #selector(viewDidScroll), name: NSView.boundsDidChangeNotification, object: scrollView.contentView) + } + scrollManager.viewDidMoveToSuperview() } @@ -261,17 +267,12 @@ class TextView: NSView, ClipViewDelegate { } override func viewDidMoveToWindow() { - NotificationCenter.default.removeObserver(self, name: NSView.boundsDidChangeNotification, object: nil) NotificationCenter.default.removeObserver(self, name: NSWindow.didBecomeKeyNotification, object: nil) NotificationCenter.default.removeObserver(self, name: NSWindow.didResignKeyNotification, object: nil) - if let scrollView { - NotificationCenter.default.addObserver(self, selector: #selector(viewDidScroll(_:)), name: NSView.boundsDidChangeNotification, object: scrollView.contentView) - } - if let window { - NotificationCenter.default.addObserver(self, selector: #selector(windowDidBecomeKey(_:)), name: NSWindow.didBecomeKeyNotification, object: window) - NotificationCenter.default.addObserver(self, selector: #selector(windowDidResignKey(_:)), name: NSWindow.didResignKeyNotification, object: window) + NotificationCenter.default.addObserver(self, selector: #selector(windowDidBecomeKey), name: NSWindow.didBecomeKeyNotification, object: window) + NotificationCenter.default.addObserver(self, selector: #selector(windowDidResignKey), name: NSWindow.didResignKeyNotification, object: window) needsTextLayout = true needsSelectionLayout = true diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index 116e85b8..0f51e18e 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -9,18 +9,24 @@ import Cocoa import Motion protocol ScrollManagerDelegate: AnyObject { - // Called once for every call to documentRect(_:didResizeTo:) that performs a scroll correction. - // Can be called multiple times for each call to scrollmanager(_:didCorrectScrollBy:). + // Called once for every call to documentRect(_:didResizeTo:) that queues up a scroll correction. func scrollManager(_ scrollManager: ScrollManager, willCorrectScrollBy delta: CGVector) - // Called once per run loop tick where scroll correction was performed. One call to this method - // can correspond to multiple calls to scrollManager(_:willCorrectScrollBy:). - func scrollManagerDidPerformScrollCorrection(_ scrollManager: ScrollManager) + // Called once per run loop tick directly before scroll correction is performed – i.e. before + // NSView.scroll(_:) is called on the documentView. This method is called regardless of whether + // NSView.scroll(_:) is actually called, and can correspond to multiple calls to + // scrollManager(_:willCorrectScrollBy:). + func scrollManagerWillCommitScrollCorrection(_ scrollManager: ScrollManager) + + // Same semantics as scrollManagerWillCommitScrollCorrection(_:), but called after a possible call + // to NSView.scroll(_:) rather than before. + func scrollManagerDidCommitScrollCorrection(_ scrollManager: ScrollManager) } extension ScrollManagerDelegate { func scrollManager(_ scrollManager: ScrollManager, willCorrectScrollBy delta: CGVector) {} - func scrollManagerDidPerformScrollCorrection(_ scrollManager: ScrollManager) {} + func scrollManagerWillCommitScrollCorrection(_ scrollManager: ScrollManager) {} + func scrollManagerDidCommitScrollCorrection(_ scrollManager: ScrollManager) {} } @MainActor @@ -34,6 +40,9 @@ class ScrollManager { private var needsScrollCorrection: Bool + var isLiveScrolling: Bool + var didLiveScroll: Bool + private var animation: SpringAnimation? var isAnimating: Bool { @@ -48,7 +57,7 @@ class ScrollManager { // We use a run loop observer, rather than DispatchQueue.main.async because we want to be able to wait // until after layout has been performed to do scroll correction. While this is possible to do with - // libdispatch – we'd just have to reschedule performScrollCorrection() if layout hasn't been performed + // DispatchQueue – we'd just have to reschedule performScrollCorrection() if layout hasn't been performed // yet – it's a bit clearer if we just check every tick of the run loop. private var observer: CFRunLoopObserver? @@ -60,13 +69,16 @@ class ScrollManager { self.delta = .zero self.needsScrollCorrection = false + self.isLiveScrolling = false + self.didLiveScroll = false + var context = CFRunLoopObserverContext(version: 0, info: Unmanaged.passUnretained(self).toOpaque(), retain: nil, release: nil, copyDescription: nil) // .beforeSources is after .beforeTimers, and layout is run on .beforeTimers, so we use .beforeSources so that we // can run as early as possible after layout occurs. let observer = CFRunLoopObserverCreate(kCFAllocatorDefault, CFRunLoopActivity.beforeSources.rawValue, true, 0, { observer, activity, info in - let manager = Unmanaged.fromOpaque(info!).takeUnretainedValue() - manager.performScrollCorrection() + let scrollManager = Unmanaged.fromOpaque(info!).takeUnretainedValue() + scrollManager.observe() }, &context) CFRunLoopAddObserver(CFRunLoopGetMain(), observer, .commonModes) @@ -95,6 +107,9 @@ class ScrollManager { isDraggingScroller = false needsScrollCorrection = false + isLiveScrolling = false + didLiveScroll = false + let offset = view?.enclosingScrollView?.contentView.bounds.origin ?? .zero scrollOffset = offset prevScrollOffset = offset @@ -153,29 +168,39 @@ class ScrollManager { return } + if let animation { // When animating upwards, any size change with an origin above the viewport contributes // to scroll correction. When animating downwards, size changes with origins in the viewport // also contribute. - let movingRight = scrollOffset.x > prevScrollOffset.x - let movingDown = scrollOffset.y > prevScrollOffset.y + let movingRight = animation.toValue.x > scrollOffset.x + let movingDown = animation.toValue.y > scrollOffset.y - let anchorX = movingRight ? 1.0 : 0.0 - let anchorY = movingDown ? 1.0 : 0.0 - - assert(scrollOffset == scrollView.contentView.bounds.origin) let viewport = scrollView.contentView.bounds + let cutoffX = movingRight ? viewport.maxX : viewport.minX + let cutoffY = movingDown ? viewport.maxY : viewport.minY - let dx = rect.maxX <= (animationDestination ?? prevScrollOffset).x + (anchorX*viewport.width) ? newSize.width - rect.width : 0 - let dy = rect.maxY <= (animationDestination ?? prevScrollOffset).y + (anchorY*viewport.height) ? newSize.height - rect.height : 0 + let dx = rect.maxX <= cutoffX ? newSize.width - rect.width : 0 + let dy = rect.maxY < cutoffY ? newSize.height - rect.height : 0 if dx == 0 && dy == 0 { return } + needsScrollCorrection = true delta += CGVector(dx: dx, dy: dy) - delegate?.scrollManager(self, willCorrectScrollBy: CGVector(dx: dx, dy: dy)) + } + + let dx = rect.maxX <= prevScrollOffset.x ? newSize.width - rect.width : 0 + let dy = rect.maxY <= prevScrollOffset.y ? newSize.height - rect.height : 0 + + if dx == 0 && dy == 0 { + return + } + needsScrollCorrection = true + delta += CGVector(dx: dx, dy: dy) + delegate?.scrollManager(self, willCorrectScrollBy: CGVector(dx: dx, dy: dy)) } @objc func viewDidScroll(_ notification: Notification) { @@ -198,19 +223,23 @@ class ScrollManager { return } - isDraggingScroller = scrollView.horizontalScroller?.hitPart == .knob || scrollView.verticalScroller?.hitPart == .knob - } + isLiveScrolling = true - @objc func didLiveScroll(_ notification: Notification) { - guard let scrollView = notification.object as? NSScrollView else { - return + isDraggingScroller = scrollView.horizontalScroller?.hitPart == .knob || scrollView.verticalScroller?.hitPart == .knob } + @objc func didLiveScroll(_ notification: Notification) { // From the didLiveScrollNotification docs: "Some user-initiated scrolls (for example, scrolling // using legacy mice) are not bracketed by a "willStart/didEnd” notification pair." animation?.stop() animation = nil + guard let scrollView = notification.object as? NSScrollView else { + return + } + + didLiveScroll = true + assert(scrollView == view?.enclosingScrollView) if isDraggingScroller { @@ -219,14 +248,15 @@ class ScrollManager { } @objc func didEndLiveScroll(_ notification: Notification) { + + isLiveScrolling = false + didLiveScroll = false isDraggingScroller = false - } - private func performScrollCorrection() { - if !needsScrollCorrection { - return + prevScrollOffset = scrollOffset } + private func observe() { // We always want scroll correction to run after layout so that we can take into account any resizing // that happens during layout. Layout happens in a .beforeTimers observer, and we run in a .beforeSources // observer, which happens later. But there may be any number of iterations through run loop between when @@ -235,23 +265,41 @@ class ScrollManager { return } - defer { delta = .zero } + performScrollCorrectionIfNecessary() + + // We used a scroll wheel and didn't get willBeginLiveScroll. Reset scrolling here. + if didLiveScroll && !isLiveScrolling { + didLiveScroll = false + prevScrollOffset = scrollOffset + assert(!isDraggingScroller) + } + } + + private func performScrollCorrectionIfNecessary() { + if !needsScrollCorrection { + return + } + needsScrollCorrection = false guard let scrollView = view?.enclosingScrollView else { return } - defer { delegate?.scrollManagerDidPerformScrollCorrection(self) } - // Dragging a scroller is equivalent to telling the scroll view "please set your offset to this // fixed percentage through the document." In this case, scroll correction just consists of // reading the percentage out of the scrollers and setting the new origin if the document size // has changed. if isDraggingScroller { assert(!isAnimating) - assert(scrollView.documentView != nil) + // Ignore delta because we're scrolling to an absolute position. + delta = .zero + + delegate?.scrollManagerWillCommitScrollCorrection(self) + defer { delegate?.scrollManagerDidCommitScrollCorrection(self) } + + assert(scrollView.documentView != nil) guard let documentSize = scrollView.documentView?.frame.size else { return } @@ -268,11 +316,22 @@ class ScrollManager { return } - if delta != .zero { - scrollView.documentView?.scroll(scrollOffset + delta) + + delegate?.scrollManagerWillCommitScrollCorrection(self) + + // documentView?.scroll(_:) can cause additional calls to documentRect(_:didResizeTo), which can request + // further scroll correction and increment delta. If we do `delta = .zero` after calling scroll(_:), we'd + // throw out those extra deltas. + let d = delta + delta = .zero + + if d != .zero { + scrollView.documentView?.scroll(scrollOffset + d) if let animation { - animateScroll(to: animation.toValue + delta) + animateScroll(to: animation.toValue + d) } } + + delegate?.scrollManagerDidCommitScrollCorrection(self) } } From 19d58d87a8d76bfa4a2d420e8cc0594981198e45 Mon Sep 17 00:00:00 2001 From: David Albert Date: Fri, 29 Mar 2024 15:44:47 -0400 Subject: [PATCH 46/73] =?UTF-8?q?Don=E2=80=99t=20use=20previous=20offset?= =?UTF-8?q?=20if=20we=E2=80=99re=20not=20live=20scrolling?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Watt/Utilities/ScrollManager.swift | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index 0f51e18e..2002b62c 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -36,6 +36,7 @@ class ScrollManager { private(set) var scrollOffset: CGPoint private var prevScrollOffset: CGPoint + private var prevLiveScrollOffset: CGPoint? private var delta: CGVector private var needsScrollCorrection: Bool @@ -95,6 +96,10 @@ class ScrollManager { } } + // If you're changing your view's frame directly in one of the below notifications, make sure to call + // ScrollManager.viewDidMoveToSuperview() before attaching your own observers. ScrollManager needs its + // observers to be called first so that its state can be correctly set up for any calls + // to documentRect(_:didResizeTo:) func viewDidMoveToSuperview() { NotificationCenter.default.removeObserver(self, name: NSView.boundsDidChangeNotification, object: nil) NotificationCenter.default.removeObserver(self, name: NSScrollView.willStartLiveScrollNotification, object: nil) @@ -113,6 +118,7 @@ class ScrollManager { let offset = view?.enclosingScrollView?.contentView.bounds.origin ?? .zero scrollOffset = offset prevScrollOffset = offset + prevLiveScrollOffset = nil delta = .zero if let scrollView = view?.enclosingScrollView { @@ -191,8 +197,9 @@ class ScrollManager { delegate?.scrollManager(self, willCorrectScrollBy: CGVector(dx: dx, dy: dy)) } - let dx = rect.maxX <= prevScrollOffset.x ? newSize.width - rect.width : 0 - let dy = rect.maxY <= prevScrollOffset.y ? newSize.height - rect.height : 0 + + let dx = rect.maxX <= (prevLiveScrollOffset ?? scrollOffset).x ? newSize.width - rect.width : 0 + let dy = rect.maxY <= (prevLiveScrollOffset ?? scrollOffset).y ? newSize.height - rect.height : 0 if dx == 0 && dy == 0 { return @@ -210,6 +217,7 @@ class ScrollManager { assert(scrollView.contentView == (notification.object as? NSView)) + prevLiveScrollOffset = nil prevScrollOffset = scrollOffset scrollOffset = scrollView.contentView.bounds.origin } @@ -239,6 +247,7 @@ class ScrollManager { } didLiveScroll = true + prevLiveScrollOffset = prevScrollOffset assert(scrollView == view?.enclosingScrollView) @@ -253,7 +262,7 @@ class ScrollManager { didLiveScroll = false isDraggingScroller = false - prevScrollOffset = scrollOffset + prevLiveScrollOffset = nil } private func observe() { @@ -270,7 +279,7 @@ class ScrollManager { // We used a scroll wheel and didn't get willBeginLiveScroll. Reset scrolling here. if didLiveScroll && !isLiveScrolling { didLiveScroll = false - prevScrollOffset = scrollOffset + prevLiveScrollOffset = nil assert(!isDraggingScroller) } } From 78fd1cfd040391440b2def41145336864aab6870 Mon Sep 17 00:00:00 2001 From: David Albert Date: Fri, 29 Mar 2024 15:45:58 -0400 Subject: [PATCH 47/73] Fix whitespace --- Watt/Utilities/ScrollManager.swift | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index 2002b62c..7f1a7bc9 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -175,26 +175,26 @@ class ScrollManager { } if let animation { - // When animating upwards, any size change with an origin above the viewport contributes - // to scroll correction. When animating downwards, size changes with origins in the viewport - // also contribute. + // When animating upwards, any size change with an origin above the viewport contributes + // to scroll correction. When animating downwards, size changes with origins in the viewport + // also contribute. let movingRight = animation.toValue.x > scrollOffset.x let movingDown = animation.toValue.y > scrollOffset.y - let viewport = scrollView.contentView.bounds + let viewport = scrollView.contentView.bounds let cutoffX = movingRight ? viewport.maxX : viewport.minX let cutoffY = movingDown ? viewport.maxY : viewport.minY let dx = rect.maxX <= cutoffX ? newSize.width - rect.width : 0 let dy = rect.maxY < cutoffY ? newSize.height - rect.height : 0 - if dx == 0 && dy == 0 { - return - } + if dx == 0 && dy == 0 { + return + } needsScrollCorrection = true - delta += CGVector(dx: dx, dy: dy) - delegate?.scrollManager(self, willCorrectScrollBy: CGVector(dx: dx, dy: dy)) + delta += CGVector(dx: dx, dy: dy) + delegate?.scrollManager(self, willCorrectScrollBy: CGVector(dx: dx, dy: dy)) } @@ -234,7 +234,7 @@ class ScrollManager { isLiveScrolling = true isDraggingScroller = scrollView.horizontalScroller?.hitPart == .knob || scrollView.verticalScroller?.hitPart == .knob - } + } @objc func didLiveScroll(_ notification: Notification) { // From the didLiveScrollNotification docs: "Some user-initiated scrolls (for example, scrolling @@ -263,7 +263,7 @@ class ScrollManager { isDraggingScroller = false prevLiveScrollOffset = nil - } + } private func observe() { // We always want scroll correction to run after layout so that we can take into account any resizing From 84fd3d7d115bf7f8c649dd3c67abe1667214e171 Mon Sep 17 00:00:00 2001 From: David Albert Date: Fri, 29 Mar 2024 15:46:23 -0400 Subject: [PATCH 48/73] Whoops --- Watt/Utilities/ScrollManager.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index 7f1a7bc9..48809ab6 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -166,6 +166,7 @@ class ScrollManager { return } + assert(scrollOffset == scrollView.contentView.bounds.origin) // If we're dragging the scroller, we're requesting an absolute percent through the document. // If layout changed at all while dragging, we're going to be in the wrong place, and need // to correct. From b61c5769cac6ca0e255f382ec7a8ae857dd45a8c Mon Sep 17 00:00:00 2001 From: David Albert Date: Fri, 29 Mar 2024 16:46:01 -0400 Subject: [PATCH 49/73] Animation is working! --- Watt/Utilities/ScrollManager.swift | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index 48809ab6..7a993568 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -50,10 +50,6 @@ class ScrollManager { animation != nil } - var animationDestination: CGPoint? { - animation?.toValue - } - weak var delegate: ScrollManagerDelegate? // We use a run loop observer, rather than DispatchQueue.main.async because we want to be able to wait @@ -179,15 +175,15 @@ class ScrollManager { // When animating upwards, any size change with an origin above the viewport contributes // to scroll correction. When animating downwards, size changes with origins in the viewport // also contribute. - let movingRight = animation.toValue.x > scrollOffset.x - let movingDown = animation.toValue.y > scrollOffset.y + let animatingRight = animation.toValue.x >= scrollOffset.x + let animatingDown = animation.toValue.y >= scrollOffset.y let viewport = scrollView.contentView.bounds - let cutoffX = movingRight ? viewport.maxX : viewport.minX - let cutoffY = movingDown ? viewport.maxY : viewport.minY + let cutoffX = animatingRight ? (animation.toValue.x + viewport.width + delta.dx) : viewport.minX + let cutoffY = animatingDown ? (animation.toValue.y + viewport.height + delta.dy) : viewport.minY let dx = rect.maxX <= cutoffX ? newSize.width - rect.width : 0 - let dy = rect.maxY < cutoffY ? newSize.height - rect.height : 0 + let dy = rect.maxY <= cutoffY ? newSize.height - rect.height : 0 if dx == 0 && dy == 0 { return @@ -196,6 +192,8 @@ class ScrollManager { needsScrollCorrection = true delta += CGVector(dx: dx, dy: dy) delegate?.scrollManager(self, willCorrectScrollBy: CGVector(dx: dx, dy: dy)) + + return } From cfcbf719b22fff5ad455ec45cf409f5ba15a7a3a Mon Sep 17 00:00:00 2001 From: David Albert Date: Fri, 29 Mar 2024 16:59:23 -0400 Subject: [PATCH 50/73] Add comment explaining why we use prevLiveScrollOffset when scrolling --- Watt/Utilities/ScrollManager.swift | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index 7a993568..0eb5fa65 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -197,6 +197,22 @@ class ScrollManager { } + // If we're scrolling, the goal is to have no jump between the previous frame and this frame. + // If we're scrolling up (which is the case where this generally matters), rects that will + // cause a jump are those that are fully above the viewport. But if we're forcing layout because + // we're scrolling up, all rects that are being laid out are by definition in the viewport this + // frame. So we have to look at the previous frame to see if we should apply the correction. + // + // If we're not scrolling and the document changed size, e.g. because of an edit, just use the + // current frame. + // + // I'm less sure about using prevLiveScrollOffset when we're scrolling down. In general, scrolling + // down will only cause layout below the viewport, and that shouldn't cause a scroll correction + // while we're not animating. So this means when scrolling down, the only corrections we should get + // are from edits in the document. When we're scrolling down, prevLiveScrollOffset.y will be + // less than scrollOffset.y, which means we're being more restrictive in when we'll do a scroll + // correction than we otherwise would be if we weren't scrolling. Not sure whether this will + // cause any issues. let dx = rect.maxX <= (prevLiveScrollOffset ?? scrollOffset).x ? newSize.width - rect.width : 0 let dy = rect.maxY <= (prevLiveScrollOffset ?? scrollOffset).y ? newSize.height - rect.height : 0 From 2e95fa28c93c3d158a39e87b9469c2bb879fd8e0 Mon Sep 17 00:00:00 2001 From: David Albert Date: Sat, 30 Mar 2024 10:06:41 -0400 Subject: [PATCH 51/73] Probable fix for scrolling up (untested) --- Watt/Utilities/ScrollManager.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index 0eb5fa65..eefd410f 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -179,8 +179,9 @@ class ScrollManager { let animatingDown = animation.toValue.y >= scrollOffset.y let viewport = scrollView.contentView.bounds - let cutoffX = animatingRight ? (animation.toValue.x + viewport.width + delta.dx) : viewport.minX - let cutoffY = animatingDown ? (animation.toValue.y + viewport.height + delta.dy) : viewport.minY + + let cutoffX = animatingRight ? (animation.toValue.x + viewport.width + delta.dx) : (viewport.minX + delta.dx) + let cutoffY = animatingDown ? (animation.toValue.y + viewport.height + delta.dy) : (viewport.minY + delta.dy) let dx = rect.maxX <= cutoffX ? newSize.width - rect.width : 0 let dy = rect.maxY <= cutoffY ? newSize.height - rect.height : 0 From cbcb2ec4a8eadcb0b46e8e0c6bafd716525888e1 Mon Sep 17 00:00:00 2001 From: David Albert Date: Sat, 30 Mar 2024 10:43:20 -0400 Subject: [PATCH 52/73] Animate scroll using anchors --- Watt/TextView/TextView+KeyBinding.swift | 6 +- Watt/Utilities/ScrollManager.swift | 82 +++++++++++++++++++------ 2 files changed, 67 insertions(+), 21 deletions(-) diff --git a/Watt/TextView/TextView+KeyBinding.swift b/Watt/TextView/TextView+KeyBinding.swift index 8c9d9e2b..45eaa3d6 100644 --- a/Watt/TextView/TextView+KeyBinding.swift +++ b/Watt/TextView/TextView+KeyBinding.swift @@ -376,7 +376,7 @@ extension TextView { y: 0 ) - scrollManager.animateScroll(to: point) + scrollManager.animateScroll(to: point, anchor: .topLeading) } override func scrollToEndOfDocument(_ sender: Any?) { @@ -404,10 +404,10 @@ extension TextView { let viewport = visibleRect let point = CGPoint( x: scrollOffset.x, - y: frame.height - viewport.height + y: frame.height ) - scrollManager.animateScroll(to: point) + scrollManager.animateScroll(to: point, anchor: .bottomLeading) } diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index eefd410f..e01058ad 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -31,6 +31,44 @@ extension ScrollManagerDelegate { @MainActor class ScrollManager { + // A unit point for determining the part of the viewport anchored for an animated scroll + struct Anchor { + let x: CGFloat + let y: CGFloat + + static let topLeading = Anchor(x: 0, y: 0) + static let top = Anchor(x: 0.5, y: 0) + static let topTrailing = Anchor(x: 1, y: 0) + + static let leading = Anchor(x: 0, y: 0.5) + static let center = Anchor(x: 0.5, y: 0.5) + static let trailing = Anchor(x: 1, y: 0.5) + + static let bottomLeading = Anchor(x: 0, y: 1) + static let bottom = Anchor(x: 0.5, y: 1) + static let bottomTrailing = Anchor(x: 1, y: 1) + } + + struct Animation { + var spring: SpringAnimation + var anchor: Anchor + + var velocity: CGPoint { + spring.velocity + } + + func scrollDestination(in viewport: CGRect) -> CGPoint { + CGPoint( + x: spring.toValue.x - (viewport.width * anchor.x), + y: spring.toValue.y - (viewport.height * anchor.y) + ) + } + + func stop() { + spring.stop() + } + } + private(set) weak var view: NSView? private(set) var isDraggingScroller: Bool @@ -44,7 +82,7 @@ class ScrollManager { var isLiveScrolling: Bool var didLiveScroll: Bool - private var animation: SpringAnimation? + private var animation: Animation? var isAnimating: Bool { animation != nil @@ -54,8 +92,8 @@ class ScrollManager { // We use a run loop observer, rather than DispatchQueue.main.async because we want to be able to wait // until after layout has been performed to do scroll correction. While this is possible to do with - // DispatchQueue – we'd just have to reschedule performScrollCorrection() if layout hasn't been performed - // yet – it's a bit clearer if we just check every tick of the run loop. + // DispatchQueue – we'd just have to reschedule performScrollCorrectionIfNecessary() if layout hasn't + // been performed yet – it's a bit clearer if we just check every tick of the run loop. private var observer: CFRunLoopObserver? init(_ view: NSView) { @@ -125,7 +163,7 @@ class ScrollManager { } } - func animateScroll(to point: NSPoint) { + func animateScroll(to point: NSPoint, anchor: Anchor) { guard let scrollView = view?.enclosingScrollView else { return } @@ -141,12 +179,25 @@ class ScrollManager { environment: scrollView ) - spring.toValue = point + let viewport = scrollView.contentView.bounds + + spring.toValue = CGPoint( + x: point.x + (viewport.width * anchor.x), + y: point.y + (viewport.height * anchor.y) + ) spring.velocity = velocity spring.resolvingEpsilon = 0.000001 spring.onValueChanged() { [weak self] value in - self?.view?.enclosingScrollView?.documentView?.scroll(value) + guard let scrollView = self?.view?.enclosingScrollView else { + return + } + let viewport = scrollView.contentView.bounds + let offset = CGPoint( + x: value.x - (viewport.width * anchor.x), + y: value.y - (viewport.height * anchor.y) + ) + scrollView.documentView?.scroll(offset) } spring.completion = { [weak self] in @@ -154,9 +205,10 @@ class ScrollManager { } spring.start() - animation = spring + animation = Animation(spring: spring, anchor: anchor) } + // Assumes the new rect has the same origin. In other words, rect always grows down and right. func documentRect(_ rect: NSRect, didResizeTo newSize: NSSize) { guard let scrollView = view?.enclosingScrollView else { return @@ -172,19 +224,12 @@ class ScrollManager { } if let animation { - // When animating upwards, any size change with an origin above the viewport contributes - // to scroll correction. When animating downwards, size changes with origins in the viewport - // also contribute. - let animatingRight = animation.toValue.x >= scrollOffset.x - let animatingDown = animation.toValue.y >= scrollOffset.y - let viewport = scrollView.contentView.bounds + let destination = animation.scrollDestination(in: viewport) - let cutoffX = animatingRight ? (animation.toValue.x + viewport.width + delta.dx) : (viewport.minX + delta.dx) - let cutoffY = animatingDown ? (animation.toValue.y + viewport.height + delta.dy) : (viewport.minY + delta.dy) + let dx = rect.maxX <= destination.x + delta.dx ? newSize.width - rect.width : 0 + let dy = rect.maxY <= destination.y + delta.dy ? newSize.height - rect.height : 0 - let dx = rect.maxX <= cutoffX ? newSize.width - rect.width : 0 - let dy = rect.maxY <= cutoffY ? newSize.height - rect.height : 0 if dx == 0 && dy == 0 { return @@ -353,7 +398,8 @@ class ScrollManager { if d != .zero { scrollView.documentView?.scroll(scrollOffset + d) if let animation { - animateScroll(to: animation.toValue + d) + let viewport = scrollView.contentView.bounds + animateScroll(to: animation.scrollDestination(in: viewport) + d, anchor: animation.anchor) } } From 6de0293c430ca364af964a935235ddbfe2c32701 Mon Sep 17 00:00:00 2001 From: David Albert Date: Sat, 30 Mar 2024 10:45:56 -0400 Subject: [PATCH 53/73] anchor -> viewportAnchor --- Watt/TextView/TextView+KeyBinding.swift | 4 ++-- Watt/Utilities/ScrollManager.swift | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Watt/TextView/TextView+KeyBinding.swift b/Watt/TextView/TextView+KeyBinding.swift index 45eaa3d6..35f84580 100644 --- a/Watt/TextView/TextView+KeyBinding.swift +++ b/Watt/TextView/TextView+KeyBinding.swift @@ -376,7 +376,7 @@ extension TextView { y: 0 ) - scrollManager.animateScroll(to: point, anchor: .topLeading) + scrollManager.animateScroll(to: point, viewportAnchor: .topLeading) } override func scrollToEndOfDocument(_ sender: Any?) { @@ -407,7 +407,7 @@ extension TextView { y: frame.height ) - scrollManager.animateScroll(to: point, anchor: .bottomLeading) + scrollManager.animateScroll(to: point, viewportAnchor: .bottomLeading) } diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index e01058ad..7e6c07d7 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -163,7 +163,7 @@ class ScrollManager { } } - func animateScroll(to point: NSPoint, anchor: Anchor) { + func animateScroll(to point: NSPoint, viewportAnchor anchor: Anchor) { guard let scrollView = view?.enclosingScrollView else { return } From ae25ee31f59d34aedbe026628c5b4922822a7f4c Mon Sep 17 00:00:00 2001 From: David Albert Date: Sat, 30 Mar 2024 10:49:47 -0400 Subject: [PATCH 54/73] Refactor documentRect(_:didResizeTo:) --- Watt/Utilities/ScrollManager.swift | 59 ++++++++++++++---------------- 1 file changed, 27 insertions(+), 32 deletions(-) diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index 7e6c07d7..8accbde3 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -223,44 +223,39 @@ class ScrollManager { return } + let cutoff: CGPoint if let animation { let viewport = scrollView.contentView.bounds let destination = animation.scrollDestination(in: viewport) - - let dx = rect.maxX <= destination.x + delta.dx ? newSize.width - rect.width : 0 - let dy = rect.maxY <= destination.y + delta.dy ? newSize.height - rect.height : 0 - - - if dx == 0 && dy == 0 { - return - } - - needsScrollCorrection = true - delta += CGVector(dx: dx, dy: dy) - delegate?.scrollManager(self, willCorrectScrollBy: CGVector(dx: dx, dy: dy)) - - return + cutoff = CGPoint( + x: destination.x + delta.dx, + y: destination.y + delta.dy + ) + } else { + // If we're scrolling, the goal is to have no jump between the previous frame and this frame. + // If we're scrolling up (which is the case where this generally matters), rects that will + // cause a jump are those that are fully above the viewport. But if we're forcing layout because + // we're scrolling up, all rects that are being laid out are by definition in the viewport this + // frame. So we have to look at the previous frame to see if we should apply the correction. + // + // If we're not scrolling and the document changed size, e.g. because of an edit, just use the + // current frame. + // + // I'm less sure about using prevLiveScrollOffset when we're scrolling down. In general, scrolling + // down will only cause layout below the viewport, and that shouldn't cause a scroll correction + // while we're not animating. So this means when scrolling down, the only corrections we should get + // are from edits in the document. When we're scrolling down, prevLiveScrollOffset.y will be + // less than scrollOffset.y, which means we're being more restrictive in when we'll do a scroll + // correction than we otherwise would be if we weren't scrolling. Not sure whether this will + // cause any issues. + + // TODO: get rid of prevLiveScrollOffset and do scrollOffset + delta? + cutoff = prevLiveScrollOffset ?? scrollOffset } - // If we're scrolling, the goal is to have no jump between the previous frame and this frame. - // If we're scrolling up (which is the case where this generally matters), rects that will - // cause a jump are those that are fully above the viewport. But if we're forcing layout because - // we're scrolling up, all rects that are being laid out are by definition in the viewport this - // frame. So we have to look at the previous frame to see if we should apply the correction. - // - // If we're not scrolling and the document changed size, e.g. because of an edit, just use the - // current frame. - // - // I'm less sure about using prevLiveScrollOffset when we're scrolling down. In general, scrolling - // down will only cause layout below the viewport, and that shouldn't cause a scroll correction - // while we're not animating. So this means when scrolling down, the only corrections we should get - // are from edits in the document. When we're scrolling down, prevLiveScrollOffset.y will be - // less than scrollOffset.y, which means we're being more restrictive in when we'll do a scroll - // correction than we otherwise would be if we weren't scrolling. Not sure whether this will - // cause any issues. - let dx = rect.maxX <= (prevLiveScrollOffset ?? scrollOffset).x ? newSize.width - rect.width : 0 - let dy = rect.maxY <= (prevLiveScrollOffset ?? scrollOffset).y ? newSize.height - rect.height : 0 + let dx = rect.maxX <= cutoff.x ? newSize.width - rect.width : 0 + let dy = rect.maxY <= cutoff.y ? newSize.height - rect.height : 0 if dx == 0 && dy == 0 { return From a734ba0cb52d359803c725dedd21c9f466ecf00b Mon Sep 17 00:00:00 2001 From: David Albert Date: Sat, 30 Mar 2024 10:53:45 -0400 Subject: [PATCH 55/73] Get rid of bad todo --- Watt/Utilities/ScrollManager.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index 8accbde3..059379f2 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -248,8 +248,6 @@ class ScrollManager { // less than scrollOffset.y, which means we're being more restrictive in when we'll do a scroll // correction than we otherwise would be if we weren't scrolling. Not sure whether this will // cause any issues. - - // TODO: get rid of prevLiveScrollOffset and do scrollOffset + delta? cutoff = prevLiveScrollOffset ?? scrollOffset } From e9cde3c41d1b7da786ff4c69cea489abc3402628 Mon Sep 17 00:00:00 2001 From: David Albert Date: Sat, 30 Mar 2024 11:17:37 -0400 Subject: [PATCH 56/73] Whoops --- Watt/Utilities/ScrollManager.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index 059379f2..c8a92c1e 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -392,7 +392,7 @@ class ScrollManager { scrollView.documentView?.scroll(scrollOffset + d) if let animation { let viewport = scrollView.contentView.bounds - animateScroll(to: animation.scrollDestination(in: viewport) + d, anchor: animation.anchor) + animateScroll(to: animation.scrollDestination(in: viewport) + d, viewportAnchor: animation.anchor) } } From 58f59cb455cb44bd5850ffa5c8f874833b4c8a65 Mon Sep 17 00:00:00 2001 From: David Albert Date: Sat, 30 Mar 2024 15:29:03 -0400 Subject: [PATCH 57/73] Add TODO for animation --- Watt/Utilities/ScrollManager.swift | 100 ++++++++++++++++++++++++++++- 1 file changed, 99 insertions(+), 1 deletion(-) diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index c8a92c1e..0072e08a 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -8,6 +8,103 @@ import Cocoa import Motion +// Animation notes: +// +// Doing scroll correction while animating turns out to be difficult. Here's what you want: +// +// 1. The animation takes the same amount of time, no matter how much the document's size +// changes during animation. +// 2. No contents jumping in the viewport. +// +// Right now, we prioritize #1 over #2. We do this by making sure the distance between the +// current viewport and the animation's destination viewport always remains the same during +// scroll correction. We apply the same delta to both viewports and only make a rect +// contribute to the delta if the rect's maxY is <= to the destination viewport, ignoring +// the current viewport entirely. +// +// For rects that change size above both the current and destination viewports, this is not +// a problem. But if a rect changes size between the two viewports, the contents of the +// viewport will appear to jump. +// +// This isn't a problem for long distances, but for short animations – i.e. scrolling by a +// line or a paragraph, it can be easily detectable. I'm not sure how much this will matter +// in practice. +// +// To illustrate, here are some diagrams. In each, the current viewport has a solid outline +// and the destination viewport has a dashed outline. +// +// In examples a, b, and c, a correction is applied to both viewports because +// rect.maxY <= targetViewport.minY. In example d, no correction is applied because the new +// rect is below the target viewport. In examples b and c this causes the contents within +// the current viewport to move undesirably. +// +// Animating down +// +// A. New rect above both viewports ✅ b. New rect in between viewports ❌ +// ┌───────────────┐ ┌───────────────┐ ┌───────────────┐ ┌───────────────┐ +// │ │ ├───────────────┤ │ │ │ │ +// │ │ │ New rect │ │ │ │ │ +// │ ┌─────────┐ │ ├───────────────┤ │ ┌─────────┐ │ │ │ +// │ │x │ │ │ │ │ │x │ │ │ x │ ◀── Viewport jumps +// │ │ │ │ │ ┌─────────┐ │ │ │ │ │ │ ┌─────────┐ │ unnecessarily +// │ │ │ │ │ │x │ │ │ │ │ │ │ │ │ │ +// │ └─────────┘ │ │ │ │ │ │ └─────────┘ │ │ │ │ │ +// │ │ │ │ │ │ │ │ │ │ │ │ │ +// │ │ │ │ └─────────┘ │ │ │ │ └─────────┘ │ +// ▼ │ ┌ ─ ─ ─ ─ ┐ │ │ │ │ ┌ ─ ─ ─ ─ ┐ │ ├───────────────┤ +// │ │ │ │ │ │ │ New rect │ +// │ │ │ │ │ ┌ ─ ─ ─ ─ ┐ │ │ │ │ │ ├───────────────┤ +// │ │ │ │ │ │ │ ┌ ─ ─ ─ ─ ┐ │ +// │ └ ─ ─ ─ ─ ┘ │ │ │ │ │ │ └ ─ ─ ─ ─ ┘ │ │ │ +// │ │ │ │ │ │ │ │ │ │ +// │ │ │ └ ─ ─ ─ ─ ┘ │ │ │ │ │ +// └───────────────┘ │ │ └───────────────┘ │ └ ─ ─ ─ ─ ┘ │ +// │ │ │ │ +// └───────────────┘ │ │ +// └───────────────┘ +// Animating up +// +// c. New rect above both viewports ✅ d. New rect in between viewports ❌ +// ┌───────────────┐ ┌───────────────┐ ┌───────────────┐ ┌───────────────┐ +// │ │ ├───────────────┤ │ │ │ │ +// │ │ │ New rect │ │ │ │ │ +// │ ┌ ─ ─ ─ ─ ┐ │ ├───────────────┤ │ ┌ ─ ─ ─ ─ ┐ │ │ ┌ ─ ─ ─ ─ ┐ │ +// │ │ │ │ │ │ │ │ +// │ │ │ │ │ ┌ ─ ─ ─ ─ ┐ │ │ │ │ │ │ │ │ │ +// │ │ │ │ │ │ │ │ +// │ └ ─ ─ ─ ─ ┘ │ │ │ │ │ │ └ ─ ─ ─ ─ ┘ │ │ └ ─ ─ ─ ─ ┘ │ +// ▲ │ │ │ │ │ │ ├───────────────┤ +// │ │ │ │ └ ─ ─ ─ ─ ┘ │ │ │ │ New rect │ +// │ │ ┌─────────┐ │ │ │ │ │ ├───────────────┤ +// │ │x │ │ │ │ │ ┌─────────┐ │ │ ┌─────────┐ │ +// │ │ │ │ │ ┌─────────┐ │ │ │x │ │ │ │ │ │ +// │ │ │ │ │ │x │ │ │ │ │ │ │ │x │ │ ◀──Content jumps in +// │ └─────────┘ │ │ │ │ │ │ │ │ │ │ │ │ │ viewport +// │ │ │ │ │ │ │ └─────────┘ │ │ └─────────┘ │ +// │ │ │ └─────────┘ │ │ │ │ │ +// └───────────────┘ │ │ └───────────────┘ │ │ +// │ │ │ │ +// └───────────────┘ └───────────────┘ +// +// +// A possible solution: +// +// 1. Have two deltas: delta and animDelta. The former applies to the current viewport, and +// the latter applies to the target. +// 2. When a rect changes, it contributes to delta if rect.maxY <= currentViewport.minY and +// contributes to animDelta if rect.maxY <= targetViewport.minY. +// 3. Instead of just copying over velocity from the existing animation, use +// Motion.SpringFunction to compute the velocity as if the distance from the original +// viewport (at the start of the animation) and the target viewport had been incremented +// by delta: +// +// let spring = SpringFunction(response: 0.2, dampingRatio: 1.0) +// spring.solve( +// dt: currentTime - startTime, +// x0: (targetViewport + animDelta) - (originalViewport + delta), +// velocity: &velocity +// ) + protocol ScrollManagerDelegate: AnyObject { // Called once for every call to documentRect(_:didResizeTo:) that queues up a scroll correction. func scrollManager(_ scrollManager: ScrollManager, willCorrectScrollBy delta: CGVector) @@ -225,6 +322,8 @@ class ScrollManager { let cutoff: CGPoint if let animation { + // TODO: make it so the contents of the viewport never jump in short animations. See notes at + // the top of the file for more info. let viewport = scrollView.contentView.bounds let destination = animation.scrollDestination(in: viewport) cutoff = CGPoint( @@ -251,7 +350,6 @@ class ScrollManager { cutoff = prevLiveScrollOffset ?? scrollOffset } - let dx = rect.maxX <= cutoff.x ? newSize.width - rect.width : 0 let dy = rect.maxY <= cutoff.y ? newSize.height - rect.height : 0 From 672281fff70a9a11cff9062cb6af328f4875b5a1 Mon Sep 17 00:00:00 2001 From: David Albert Date: Sat, 30 Mar 2024 15:55:33 -0400 Subject: [PATCH 58/73] Fix spring behavior --- Watt/Utilities/ScrollManager.swift | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index 0072e08a..9149e59c 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -154,11 +154,8 @@ class ScrollManager { spring.velocity } - func scrollDestination(in viewport: CGRect) -> CGPoint { - CGPoint( - x: spring.toValue.x - (viewport.width * anchor.x), - y: spring.toValue.y - (viewport.height * anchor.y) - ) + var toValue: CGPoint { + spring.toValue } func stop() { @@ -269,19 +266,21 @@ class ScrollManager { animation?.stop() assert(scrollOffset == scrollView.contentView.bounds.origin) + + let viewport = scrollView.contentView.bounds + let start = CGPoint( + x: scrollOffset.x + (viewport.width * anchor.x), + y: scrollOffset.y + (viewport.height * anchor.y) + ) + let spring = SpringAnimation( - initialValue: scrollOffset, + initialValue: start, response: 0.2, dampingRatio: 1.0, environment: scrollView ) - let viewport = scrollView.contentView.bounds - - spring.toValue = CGPoint( - x: point.x + (viewport.width * anchor.x), - y: point.y + (viewport.height * anchor.y) - ) + spring.toValue = point spring.velocity = velocity spring.resolvingEpsilon = 0.000001 @@ -324,8 +323,7 @@ class ScrollManager { if let animation { // TODO: make it so the contents of the viewport never jump in short animations. See notes at // the top of the file for more info. - let viewport = scrollView.contentView.bounds - let destination = animation.scrollDestination(in: viewport) + let destination = animation.toValue cutoff = CGPoint( x: destination.x + delta.dx, y: destination.y + delta.dy @@ -489,8 +487,7 @@ class ScrollManager { if d != .zero { scrollView.documentView?.scroll(scrollOffset + d) if let animation { - let viewport = scrollView.contentView.bounds - animateScroll(to: animation.scrollDestination(in: viewport) + d, viewportAnchor: animation.anchor) + animateScroll(to: animation.toValue + d, viewportAnchor: animation.anchor) } } From 4051bf548e0b039081047e89a25be6226c101fdc Mon Sep 17 00:00:00 2001 From: David Albert Date: Sat, 30 Mar 2024 16:03:49 -0400 Subject: [PATCH 59/73] Let programmatic animations interrupt live scroll --- Watt/Utilities/ScrollManager.swift | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index 9149e59c..b9b7744f 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -173,6 +173,7 @@ class ScrollManager { private var needsScrollCorrection: Bool + // "Live scrolling" is when the user scrolls using an input device like a mouse, trackpad, etc. var isLiveScrolling: Bool var didLiveScroll: Bool @@ -389,8 +390,13 @@ class ScrollManager { @objc func didLiveScroll(_ notification: Notification) { // From the didLiveScrollNotification docs: "Some user-initiated scrolls (for example, scrolling // using legacy mice) are not bracketed by a "willStart/didEnd” notification pair." - animation?.stop() - animation = nil + // + // isLiveScrolling will be false if the user scrolls with a scroll wheel or other input methods that + // don't trigger willStart/didEnd. + if !isLiveScrolling { + animation?.stop() + animation = nil + } guard let scrollView = notification.object as? NSScrollView else { return From a253e4b9d7a472aac5e54ea1a1c8ef3a12cff46c Mon Sep 17 00:00:00 2001 From: David Albert Date: Sat, 30 Mar 2024 16:06:02 -0400 Subject: [PATCH 60/73] Whoops --- Watt/LayoutManager/LayoutManager.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Watt/LayoutManager/LayoutManager.swift b/Watt/LayoutManager/LayoutManager.swift index 216539e2..8c21df31 100644 --- a/Watt/LayoutManager/LayoutManager.swift +++ b/Watt/LayoutManager/LayoutManager.swift @@ -92,7 +92,6 @@ class LayoutManager { delegate.layoutManager(self, positionLineLayer: layer) layers.append(layer) block(layer) - return line.alignmentFrame.maxY <= viewport.minY + scrollCorrection.dy + viewport.height return line.alignmentFrame.maxY <= viewport.maxY + scrollCorrection.dy } lineLayers = layers From a5949930dbb4f14a1b766465e73f5abd094f9b04 Mon Sep 17 00:00:00 2001 From: David Albert Date: Sat, 30 Mar 2024 17:14:03 -0400 Subject: [PATCH 61/73] Clarify comment --- Watt/Utilities/ScrollManager.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index b9b7744f..4b486fe1 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -432,7 +432,8 @@ class ScrollManager { performScrollCorrectionIfNecessary() - // We used a scroll wheel and didn't get willBeginLiveScroll. Reset scrolling here. + // If we scrolled using a scroll wheel or another "legacy" input method that doesn't cause + // willStartLiveScroll and didEndLiveScroll to be called, reset scrolling state here. if didLiveScroll && !isLiveScrolling { didLiveScroll = false prevLiveScrollOffset = nil From 971ec65ee879c7472bcb721c11a7912c0afdb179 Mon Sep 17 00:00:00 2001 From: David Albert Date: Sat, 30 Mar 2024 17:28:17 -0400 Subject: [PATCH 62/73] Fix scroll animation with large estimates that shrink --- Watt/LayoutManager/LayoutManager.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Watt/LayoutManager/LayoutManager.swift b/Watt/LayoutManager/LayoutManager.swift index 8c21df31..b5f5d8fb 100644 --- a/Watt/LayoutManager/LayoutManager.swift +++ b/Watt/LayoutManager/LayoutManager.swift @@ -92,7 +92,7 @@ class LayoutManager { delegate.layoutManager(self, positionLineLayer: layer) layers.append(layer) block(layer) - return line.alignmentFrame.maxY <= viewport.maxY + scrollCorrection.dy + return line.alignmentFrame.maxY <= viewport.maxY + max(0, scrollCorrection.dy) } lineLayers = layers } From 07d8935772b01694d0ab45ceaed08c36d604a5cb Mon Sep 17 00:00:00 2001 From: David Albert Date: Sat, 30 Mar 2024 17:51:39 -0400 Subject: [PATCH 63/73] Make scrollPageUp/scrollPageDown animate with ScrollManager --- Watt/TextView/TextView+KeyBinding.swift | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/Watt/TextView/TextView+KeyBinding.swift b/Watt/TextView/TextView+KeyBinding.swift index 35f84580..7a2e2b90 100644 --- a/Watt/TextView/TextView+KeyBinding.swift +++ b/Watt/TextView/TextView+KeyBinding.swift @@ -275,23 +275,29 @@ extension TextView { override func scrollPageUp(_ sender: Any?) { - let viewport = textContainerVisibleRect + guard let viewport = scrollView?.contentView.bounds else { + return + } + let point = CGPoint( - x: textContainerScrollOffset.x, + x: scrollOffset.x, y: viewport.minY - viewport.height ) - animator().scroll(convertFromTextContainer(point)) + scrollManager.animateScroll(to: point, viewportAnchor: .topLeading) } override func scrollPageDown(_ sender: Any?) { - let viewport = textContainerVisibleRect + guard let viewport = scrollView?.contentView.bounds else { + return + } + let point = CGPoint( - x: textContainerScrollOffset.x, + x: scrollOffset.x, y: viewport.maxY ) - animator().scroll(convertFromTextContainer(point)) + scrollManager.animateScroll(to: point, viewportAnchor: .topLeading) } override func scrollLineUp(_ sender: Any?) { @@ -365,11 +371,6 @@ extension TextView { - // TODO: without better height estimates, or a full asyncronous layout pass, - // scrollToBeginningOfDocument and scrollToEndOfDocument often don't actually - // put you at the beginning or the end. - // - // I wonder if there's another way around this. override func scrollToBeginningOfDocument(_ sender: Any?) { let point = CGPoint( x: scrollOffset.x, From 02d303e67f8ef7cbdc4bc147598955f90960976a Mon Sep 17 00:00:00 2001 From: David Albert Date: Sat, 30 Mar 2024 18:50:09 -0400 Subject: [PATCH 64/73] Animate scrollLineUp and scrollLineDown --- Watt/TextView/TextView+KeyBinding.swift | 20 +++++++++----------- Watt/TextView/TextView+Scrolling.swift | 4 ---- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/Watt/TextView/TextView+KeyBinding.swift b/Watt/TextView/TextView+KeyBinding.swift index 7a2e2b90..ee47c52c 100644 --- a/Watt/TextView/TextView+KeyBinding.swift +++ b/Watt/TextView/TextView+KeyBinding.swift @@ -280,7 +280,7 @@ extension TextView { } let point = CGPoint( - x: scrollOffset.x, + x: viewport.minX, y: viewport.minY - viewport.height ) @@ -293,7 +293,7 @@ extension TextView { } let point = CGPoint( - x: scrollOffset.x, + x: viewport.minX, y: viewport.maxY ) @@ -332,17 +332,16 @@ extension TextView { let frame = layoutManager.convert(targetFrag.alignmentFrame, from: targetLine) let target = CGPoint( - x: textContainerScrollOffset.x, + x: viewport.minX, y: frame.minY ) - // Not sure why this isn't animating. I thought it might have been due to only - // scrolling by a short distance, but I tried adding longer distances and it - // still didn't scroll. But other calls in this file are definitely animating, - // so something else is going on. I'll have to look into it. - animator().scroll(convertFromTextContainer(target)) + scrollManager.animateScroll(to: convertFromTextContainer(target), viewportAnchor: .topLeading) } + // TODO: this doesn't correctly take into account text container insets, so when you invoke this from the + // top of the scroll view, the first line you get to can be cut off. I'm not sure why this is. scrollLineUp + // doesn't have the same problem when invoking it from the bottom of the scroll view. override func scrollLineDown(_ sender: Any?) { let viewport = textContainerVisibleRect @@ -361,12 +360,11 @@ extension TextView { let frame = layoutManager.convert(targetFrag.alignmentFrame, from: targetLine) let target = CGPoint( - x: textContainerScrollOffset.x, + x: viewport.minX, y: frame.maxY - viewport.height ) - // not sure why this isn't animating? Maybe it doesn't animate for short changes? - animator().scroll(convertFromTextContainer(target)) + scrollManager.animateScroll(to: convertFromTextContainer(target), viewportAnchor: .topLeading) } diff --git a/Watt/TextView/TextView+Scrolling.swift b/Watt/TextView/TextView+Scrolling.swift index c82ff69b..f0dd04d3 100644 --- a/Watt/TextView/TextView+Scrolling.swift +++ b/Watt/TextView/TextView+Scrolling.swift @@ -44,10 +44,6 @@ extension TextView { return scrollView.contentView.bounds.origin } - var textContainerScrollOffset: CGPoint { - convertToTextContainer(scrollOffset) - } - func scrollIndexToVisible(_ index: Buffer.Index) { guard let rect = layoutManager.caretRect(for: index, affinity: index == buffer.endIndex ? .upstream : .downstream) else { return From d1b26e1f1eca32b0aecbd6ffd233e8d1da99eaf7 Mon Sep 17 00:00:00 2001 From: David Albert Date: Sat, 30 Mar 2024 18:57:51 -0400 Subject: [PATCH 65/73] Whitespace --- Watt/Utilities/ScrollManager.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index 4b486fe1..24195423 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -413,7 +413,6 @@ class ScrollManager { } @objc func didEndLiveScroll(_ notification: Notification) { - isLiveScrolling = false didLiveScroll = false isDraggingScroller = false From 0a97bb294528fd779de465b7976740a529eb6eea Mon Sep 17 00:00:00 2001 From: David Albert Date: Sun, 31 Mar 2024 14:32:46 -0400 Subject: [PATCH 66/73] Only perform scroll correction in layoutText MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes an issue where editing would cause wierd jumps. I don’t know the root cause of why this is necessary. I think this will become clearer once we switch to line fragment by line fragment rendering. We’ll see. --- Watt/LayoutManager/LayoutManager.swift | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/Watt/LayoutManager/LayoutManager.swift b/Watt/LayoutManager/LayoutManager.swift index b5f5d8fb..7515533c 100644 --- a/Watt/LayoutManager/LayoutManager.swift +++ b/Watt/LayoutManager/LayoutManager.swift @@ -252,7 +252,7 @@ class LayoutManager { while i < end { let next = buffer.lines.index(after: i) - let (line, layer) = layoutLineIfNecessary(from: buffer, inRange: i.., atPoint point: CGPoint) -> (Line, LineLayer?) { + func layoutLineIfNecessary(from buffer: Buffer, inRange range: Range, atPoint point: CGPoint, performScrollCorrection: Bool) -> (Line, LineLayer?) { assert(range.lowerBound == buffer.lines.index(roundingDown: range.lowerBound)) assert(range.upperBound == buffer.endIndex || range.upperBound == buffer.lines.index(roundingDown: range.upperBound)) @@ -523,7 +523,9 @@ class LayoutManager { if oldHeight != newHeight { heights[hi] = newHeight - delegate?.layoutManager(self, rect: old, didResizeTo: line.alignmentFrame.size) + if performScrollCorrection { + delegate?.layoutManager(self, rect: old, didResizeTo: line.alignmentFrame.size) + } } return (line, nil) @@ -799,13 +801,8 @@ extension LayoutManager: BufferDelegate { let oldRange = Range(r, in: old) let newRange = Range(r.lowerBound..<(r.lowerBound + count), in: new) - let minY = heights.yOffset(upThroughPosition: r.lowerBound) - let oldMaxY = heights.height(upThroughPosition: r.upperBound) - heights.replaceSubrange(r, with: new[newRange]) - let newMaxY = heights.height(upThroughPosition: r.lowerBound + count) - removeLineLayers(touching: oldRange) // Offset line ranges that fall after the edit @@ -835,11 +832,7 @@ extension LayoutManager: BufferDelegate { assert(start == newLineRange.upperBound) } - let rect = CGRect(x: 0, y: minY, width: textContainer.width, height: oldMaxY - minY) - let newSize = CGSize(width: textContainer.width, height: newMaxY - minY) - delegate?.layoutManager(self, buffer: buffer, contentsDidChangeFrom: old, to: new, withDelta: delta) - delegate?.layoutManager(self, rect: rect, didResizeTo: newSize) delegate?.didInvalidateLayout(for: self) } From bf45222c9978a8663995489e31a7e9174ba3c1e1 Mon Sep 17 00:00:00 2001 From: David Albert Date: Sun, 31 Mar 2024 14:50:01 -0400 Subject: [PATCH 67/73] Get rid of ScrollManagerDelegate. It seems unnecessary --- Watt/LayoutManager/LayoutManager.swift | 23 +++++------------- Watt/TextView/TextView.swift | 2 -- Watt/Utilities/ScrollManager.swift | 32 -------------------------- 3 files changed, 6 insertions(+), 51 deletions(-) diff --git a/Watt/LayoutManager/LayoutManager.swift b/Watt/LayoutManager/LayoutManager.swift index 7515533c..c0103eb2 100644 --- a/Watt/LayoutManager/LayoutManager.swift +++ b/Watt/LayoutManager/LayoutManager.swift @@ -62,14 +62,11 @@ class LayoutManager { // invariant: sorted by line.range.lowerBound var lineLayers: [LineLayer] - var scrollCorrection: CGVector - init() { buffer = Buffer() heights = Heights() textContainer = TextContainer() lineLayers = [] - scrollCorrection = .zero buffer.addDelegate(self) } @@ -92,7 +89,12 @@ class LayoutManager { delegate.layoutManager(self, positionLineLayer: layer) layers.append(layer) block(layer) - return line.alignmentFrame.maxY <= viewport.maxY + max(0, scrollCorrection.dy) + + // I used to have `<= viewport.maxY + max(0, scrollCorrection.dy)` here. I thought it was necessary + // in case we performed scroll correction and the viewport moved down (and thus we had more space + // we needed to layout and fill). But it seems like it doesn't make a difference. I'm sure it did at + // some point. I'm not sure why it doesn't make a difference now. + return line.alignmentFrame.maxY <= viewport.maxY } lineLayers = layers } @@ -771,19 +773,6 @@ class LayoutManager { } } -// MARK: - ScrollManagerDelegate - -extension LayoutManager: ScrollManagerDelegate { - func scrollManager(_ scrollManager: ScrollManager, willCorrectScrollBy delta: CGVector) { - scrollCorrection += delta - } - - // Called after layout - func scrollManagerDidCommitScrollCorrection(_ scrollManager: ScrollManager) { - scrollCorrection = .zero - } -} - // MARK: - BufferDelegate extension LayoutManager: BufferDelegate { diff --git a/Watt/TextView/TextView.swift b/Watt/TextView/TextView.swift index f2791f45..4f015e65 100644 --- a/Watt/TextView/TextView.swift +++ b/Watt/TextView/TextView.swift @@ -193,8 +193,6 @@ class TextView: NSView, ClipViewDelegate { layoutManager.buffer = buffer layoutManager.delegate = self - scrollManager.delegate = layoutManager - lineNumberView.lineCount = buffer.lines.count lineNumberView.font = font lineNumberView.textColor = theme.lineNumberColor diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index 24195423..188b03a8 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -105,27 +105,6 @@ import Motion // velocity: &velocity // ) -protocol ScrollManagerDelegate: AnyObject { - // Called once for every call to documentRect(_:didResizeTo:) that queues up a scroll correction. - func scrollManager(_ scrollManager: ScrollManager, willCorrectScrollBy delta: CGVector) - - // Called once per run loop tick directly before scroll correction is performed – i.e. before - // NSView.scroll(_:) is called on the documentView. This method is called regardless of whether - // NSView.scroll(_:) is actually called, and can correspond to multiple calls to - // scrollManager(_:willCorrectScrollBy:). - func scrollManagerWillCommitScrollCorrection(_ scrollManager: ScrollManager) - - // Same semantics as scrollManagerWillCommitScrollCorrection(_:), but called after a possible call - // to NSView.scroll(_:) rather than before. - func scrollManagerDidCommitScrollCorrection(_ scrollManager: ScrollManager) -} - -extension ScrollManagerDelegate { - func scrollManager(_ scrollManager: ScrollManager, willCorrectScrollBy delta: CGVector) {} - func scrollManagerWillCommitScrollCorrection(_ scrollManager: ScrollManager) {} - func scrollManagerDidCommitScrollCorrection(_ scrollManager: ScrollManager) {} -} - @MainActor class ScrollManager { // A unit point for determining the part of the viewport anchored for an animated scroll @@ -183,8 +162,6 @@ class ScrollManager { animation != nil } - weak var delegate: ScrollManagerDelegate? - // We use a run loop observer, rather than DispatchQueue.main.async because we want to be able to wait // until after layout has been performed to do scroll correction. While this is possible to do with // DispatchQueue – we'd just have to reschedule performScrollCorrectionIfNecessary() if layout hasn't @@ -358,7 +335,6 @@ class ScrollManager { needsScrollCorrection = true delta += CGVector(dx: dx, dy: dy) - delegate?.scrollManager(self, willCorrectScrollBy: CGVector(dx: dx, dy: dy)) } @objc func viewDidScroll(_ notification: Notification) { @@ -461,9 +437,6 @@ class ScrollManager { // Ignore delta because we're scrolling to an absolute position. delta = .zero - delegate?.scrollManagerWillCommitScrollCorrection(self) - defer { delegate?.scrollManagerDidCommitScrollCorrection(self) } - assert(scrollView.documentView != nil) guard let documentSize = scrollView.documentView?.frame.size else { return @@ -481,9 +454,6 @@ class ScrollManager { return } - - delegate?.scrollManagerWillCommitScrollCorrection(self) - // documentView?.scroll(_:) can cause additional calls to documentRect(_:didResizeTo), which can request // further scroll correction and increment delta. If we do `delta = .zero` after calling scroll(_:), we'd // throw out those extra deltas. @@ -496,7 +466,5 @@ class ScrollManager { animateScroll(to: animation.toValue + d, viewportAnchor: animation.anchor) } } - - delegate?.scrollManagerDidCommitScrollCorrection(self) } } From 5ca0bca1d5e0e3c15f55e2829e37fe9188ff5399 Mon Sep 17 00:00:00 2001 From: David Albert Date: Sun, 31 Mar 2024 16:01:16 -0400 Subject: [PATCH 68/73] Add (somewhat confusing?) comment explaining viewportAnchor --- Watt/Utilities/ScrollManager.swift | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index 188b03a8..2a3d22a5 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -235,6 +235,24 @@ class ScrollManager { } } + // Scrolls point to coincide with the point in the viewport specified by viewportAnchor, compensating + // for changes in document size during the scroll. viewportAnchor is a unit point. + // + // N.b. scrolling to ((x, y), .topLeading) is not the same as scrolling to ((x, y + viewport.height), .bottomLeading). + // The difference lies in how they handle vertical scroll correction: the former will offset point.y for any vertical + // changes in document size that are fully above y. The latter will do the same, but for any size changes above + // y + viewport.height. + // + // Put another way, .topLeading will offset point for any layout that happens above the viewport, while .bottomLeading + // will offset points for any layout that happens above **or within** the viewport. + // + // Most of the time you want .topLeading. For other positions, consider scrolling to the bottom of the document. With + // If you animate .topLeading, the viewport origin will advance until scrollOrigin.y == documentHeight - viewportHeight, + // but once that happens, laying out the final viewport location might cause documentHeight to grow, and then the + // viewport will no longer be at the bottom. If you use animate .bottomLeading, when the viewport's origin reaches the + // same location, any layout of the contents of the viewport will contribute to further scroll correction (possibly + // triggering extra layout passes), until the last line is laid out. The result is that at the end of the animation, + // the bottom of the viewport will be at the bottom of the document. func animateScroll(to point: NSPoint, viewportAnchor anchor: Anchor) { guard let scrollView = view?.enclosingScrollView else { return From 1b39a6ed879db71f3e8a67414649d2b25c9430f1 Mon Sep 17 00:00:00 2001 From: David Albert Date: Sun, 31 Mar 2024 16:02:44 -0400 Subject: [PATCH 69/73] Remove unused variable --- Watt/TextView/TextView+KeyBinding.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Watt/TextView/TextView+KeyBinding.swift b/Watt/TextView/TextView+KeyBinding.swift index ee47c52c..0c341ac3 100644 --- a/Watt/TextView/TextView+KeyBinding.swift +++ b/Watt/TextView/TextView+KeyBinding.swift @@ -400,7 +400,6 @@ extension TextView { // get right. // // It's also possible that with good enough height estimtes this just won't be a problem. - let viewport = visibleRect let point = CGPoint( x: scrollOffset.x, y: frame.height From 296807e6071a9f435d00a1b99b6f3bb8c30f7cea Mon Sep 17 00:00:00 2001 From: David Albert Date: Sun, 31 Mar 2024 16:06:19 -0400 Subject: [PATCH 70/73] Remove dead code --- Watt/LineNumberView/LineNumberView.swift | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/Watt/LineNumberView/LineNumberView.swift b/Watt/LineNumberView/LineNumberView.swift index 9a7347c9..e393926c 100644 --- a/Watt/LineNumberView/LineNumberView.swift +++ b/Watt/LineNumberView/LineNumberView.swift @@ -109,19 +109,6 @@ class LineNumberView: NSView, CALayerDelegate, NSViewLayerContentScaleDelegate { } } - override func viewDidMoveToWindow() { - // TextView's .viewDidMoveToWindow gets called before LineNumberView's, so we've already done our first - // layout pass, including creating the first set of LineNumberLayers with a contentsScale of 1.0. - // - // At this point, draw(in:) hasn't been called for any layers, so this doesn't trigger any - // unnecessary drawing. - if let window { - for layer in lineNumberLayers { - layer.contentsScale = window.backingScaleFactor - } - } - } - func layer(_ layer: CALayer, shouldInheritContentsScale newScale: CGFloat, from window: NSWindow) -> Bool { true } From ad21d624c3889dff79bc478986e474ddb86bf729 Mon Sep 17 00:00:00 2001 From: David Albert Date: Sun, 31 Mar 2024 16:08:12 -0400 Subject: [PATCH 71/73] Comment --- Watt/TextView/TextView.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Watt/TextView/TextView.swift b/Watt/TextView/TextView.swift index 4f015e65..edac7910 100644 --- a/Watt/TextView/TextView.swift +++ b/Watt/TextView/TextView.swift @@ -253,6 +253,8 @@ class TextView: NSView, ClipViewDelegate { addLineNumberView() } + // Necessary (as opposed to observing boundsDidChangeNotification) because + // we need to know both the old size and new size of the clip view. func clipView(_ clipView: ClipView, frameSizeDidChangeFrom oldSize: NSSize) { let heightChanged = oldSize.height != clipView.frame.height let widthChanged = oldSize.width != clipView.frame.width From eb345294492104064f3f2622f573c10fcc5a3774 Mon Sep 17 00:00:00 2001 From: David Albert Date: Sun, 31 Mar 2024 16:15:53 -0400 Subject: [PATCH 72/73] Update comments --- Watt/TextView/TextView+Input.swift | 2 -- Watt/Utilities/ScrollManager.swift | 8 ++++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/Watt/TextView/TextView+Input.swift b/Watt/TextView/TextView+Input.swift index f74efd8c..fe7311f5 100644 --- a/Watt/TextView/TextView+Input.swift +++ b/Watt/TextView/TextView+Input.swift @@ -83,8 +83,6 @@ extension TextView: NSTextInputClient { selection = selection.unmarked - // TODO: if we're the only one who calls unmarkText(), we can remove these layout calls because - // we already schedule layout in didInvalidateLayout(for layoutManager: LayoutManager). needsTextLayout = true needsSelectionLayout = true needsInsertionPointLayout = true diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index 2a3d22a5..36381569 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -202,10 +202,10 @@ class ScrollManager { } } - // If you're changing your view's frame directly in one of the below notifications, make sure to call - // ScrollManager.viewDidMoveToSuperview() before attaching your own observers. ScrollManager needs its - // observers to be called first so that its state can be correctly set up for any calls - // to documentRect(_:didResizeTo:) + // If you're changing your view's frame directly in an observer for one of the below notifications, + // make sure to call ScrollManager.viewDidMoveToSuperview() before attaching your own observers. + // ScrollManager needs its observers to be called first so that its state can be correctly set up + // for any calls to documentRect(_:didResizeTo:) func viewDidMoveToSuperview() { NotificationCenter.default.removeObserver(self, name: NSView.boundsDidChangeNotification, object: nil) NotificationCenter.default.removeObserver(self, name: NSScrollView.willStartLiveScrollNotification, object: nil) From b462b81c579182f6a268a8431f2e3f084d67291b Mon Sep 17 00:00:00 2001 From: David Albert Date: Sun, 31 Mar 2024 16:20:11 -0400 Subject: [PATCH 73/73] Comment --- Watt/Utilities/ScrollManager.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Watt/Utilities/ScrollManager.swift b/Watt/Utilities/ScrollManager.swift index 36381569..b493ac98 100644 --- a/Watt/Utilities/ScrollManager.swift +++ b/Watt/Utilities/ScrollManager.swift @@ -301,6 +301,11 @@ class ScrollManager { } // Assumes the new rect has the same origin. In other words, rect always grows down and right. + // + // Scroll correction is only performed if rect is fully above the viewport origin (or the origin of + // the animation's destination viewport if an animation is in progress). This means that you may + // have unexpected results if rect is partially within the viewport and the size change occurred + // fully above the viewport. func documentRect(_ rect: NSRect, didResizeTo newSize: NSSize) { guard let scrollView = view?.enclosingScrollView else { return