diff --git a/macos/Sources/Ghostty/SurfaceScrollView.swift b/macos/Sources/Ghostty/SurfaceScrollView.swift index 6fdac9973..e455a32c8 100644 --- a/macos/Sources/Ghostty/SurfaceScrollView.swift +++ b/macos/Sources/Ghostty/SurfaceScrollView.swift @@ -19,7 +19,6 @@ class SurfaceScrollView: NSView { private var observers: [NSObjectProtocol] = [] private var cancellables: Set = [] private var isLiveScrolling = false - private var currentScrollbar: Ghostty.Action.Scrollbar? = nil /// The last row position sent via scroll_to_row action. Used to avoid /// sending redundant actions when the user drags the scrollbar but stays @@ -137,13 +136,6 @@ class SurfaceScrollView: NSView { // insets. This is necessary for the content view to match the // surface view if we have the "hidden" titlebar style. override var safeAreaInsets: NSEdgeInsets { return NSEdgeInsetsZero } - - override func setFrameSize(_ newSize: NSSize) { - super.setFrameSize(newSize) - - // Force layout to be called to fix up our various subviews. - needsLayout = true - } override func layout() { super.layout() @@ -151,43 +143,22 @@ class SurfaceScrollView: NSView { // Fill entire bounds with scroll view scrollView.frame = bounds - // Use contentSize to account for visible scrollers - // - // Only update sizes if we have a valid (non-zero) content size. The content size - // can be zero when this is added early to a view, or to an invisible hierarchy. - // Practically, this happened in the quick terminal. - var contentSize = scrollView.contentSize - guard contentSize.width > 0 && contentSize.height > 0 else { - synchronizeSurfaceView() - return - } - - // If we have a legacy scrollbar and its not visible, then we account for that - // in advance, because legacy scrollbars change our contentSize and force reflow - // of our terminal which is not desirable. - // See: https://github.com/ghostty-org/ghostty/discussions/9254 - let style = scrollView.verticalScroller?.scrollerStyle ?? NSScroller.preferredScrollerStyle - if style == .legacy { - if (scrollView.verticalScroller?.isHidden ?? true) { - let scrollerWidth = NSScroller.scrollerWidth(for: .regular, scrollerStyle: .legacy) - contentSize.width -= scrollerWidth - } - } - - // Keep document width synchronized with content width, and update the - // document height as appropriate for the current surface size. - documentView.setFrameSize(CGSize( - width: contentSize.width, - height: documentHeight(currentScrollbar), - )) + // When our scrollview changes make sure our scroller and surface views are synchronized + synchronizeScrollView() + synchronizeSurfaceView() // Inform the actual pty of our size change. This doesn't change the actual view // frame because we do want to render the whole thing, but it will prevent our // rows/cols from going into the non-content area. - surfaceView.sizeDidChange(contentSize) - - // When our scrollview changes make sure our surface view is synchronized - synchronizeSurfaceView() + // + // Only update the pty if we have a valid (non-zero) content size. The content size + // can be zero when this is added early to a view, or to an invisible hierarchy. + // Practically, this happened in the quick terminal. + let width = surfaceContentWidth() + let height = surfaceView.frame.height + if width > 0 && height > 0 { + surfaceView.sizeDidChange(CGSize(width: width, height: height)) + } } // MARK: Scrolling @@ -206,6 +177,38 @@ class SurfaceScrollView: NSView { let visibleRect = scrollView.contentView.documentVisibleRect surfaceView.frame = visibleRect } + + /// Sizes the document view and scrolls the content view according to the scrollbar state + private func synchronizeScrollView() { + // We adjust the document height first, as the content width may depend on it. + documentView.frame.size.height = documentHeight() + + // Our width should be the content width to account for visible scrollers. + // We don't do horizontal scrolling in terminals. The surfaceView width is + // yoked to the document width (this is distinct from the content width + // passed to surfaceView.sizeDidChange, which is only updated on layout). + documentView.frame.size.width = scrollView.contentSize.width + surfaceView.frame.size.width = scrollView.contentSize.width + + // Only update our actual scroll position if we're not actively scrolling. + if !isLiveScrolling { + // Convert row units to pixels using cell height, ignore zero height. + let cellHeight = surfaceView.cellSize.height + if cellHeight > 0, let scrollbar = surfaceView.scrollbar { + // Invert coordinate system: terminal offset is from top, AppKit position from bottom + let offsetY = + CGFloat(scrollbar.total - scrollbar.offset - scrollbar.len) * cellHeight + scrollView.contentView.scroll(to: CGPoint(x: 0, y: offsetY)) + + // Track the current row position to avoid redundant movements when we + // move the scrollbar. + lastSentRow = Int(scrollbar.offset) + } + } + + // Always update our scrolled view with the latest dimensions + scrollView.reflectScrolledClipView(scrollView.contentView) + } // MARK: Notifications @@ -254,32 +257,8 @@ class SurfaceScrollView: NSView { guard let scrollbar = notification.userInfo?[SwiftUI.Notification.Name.ScrollbarKey] as? Ghostty.Action.Scrollbar else { return } - currentScrollbar = scrollbar - - // Convert row units to pixels using cell height, ignore zero height. - let cellHeight = surfaceView.cellSize.height - guard cellHeight > 0 else { return } - - // Our width should be the content width to account for visible scrollers. - // We don't do horizontal scrolling in terminals. - documentView.setFrameSize(CGSize( - width: scrollView.contentSize.width, - height: documentHeight(scrollbar), - )) - - // Only update our actual scroll position if we're not actively scrolling. - if !isLiveScrolling { - // Invert coordinate system: terminal offset is from top, AppKit position from bottom - let offsetY = CGFloat(scrollbar.total - scrollbar.offset - scrollbar.len) * cellHeight - scrollView.contentView.scroll(to: CGPoint(x: 0, y: offsetY)) - - // Track the current row position to avoid redundant movements when we - // move the scrollbar. - lastSentRow = Int(scrollbar.offset) - } - - // Always update our scrolled view with the latest dimensions - scrollView.reflectScrolledClipView(scrollView.contentView) + surfaceView.scrollbar = scrollbar + synchronizeScrollView() } /// Handles a change in the frame of NSScrollPocket styling overlays @@ -325,11 +304,35 @@ class SurfaceScrollView: NSView { // MARK: Calculations + /// Calculate the content width reported to the core surface + /// + /// If we have a legacy scrollbar and its not visible, then we account for that + /// in advance, because legacy scrollbars change our contentSize and force reflow + /// of our terminal which is not desirable. + /// See: https://github.com/ghostty-org/ghostty/discussions/9254 + private func surfaceContentWidth() -> CGFloat { + let contentWidth = scrollView.contentSize.width + if scrollView.hasVerticalScroller { + let style = + scrollView.verticalScroller?.scrollerStyle + ?? NSScroller.preferredScrollerStyle + // We only subtract the scrollbar width if it's hidden or not present, + // otherwise its width is already accounted for in contentSize. + if style == .legacy && (scrollView.verticalScroller?.isHidden ?? true) { + let scrollerWidth = + scrollView.verticalScroller?.frame.width + ?? NSScroller.scrollerWidth(for: .regular, scrollerStyle: .legacy) + return max(0, contentWidth - scrollerWidth) + } + } + return contentWidth + } + /// Calculate the appropriate document view height given a scrollbar state - private func documentHeight(_ scrollbar: Ghostty.Action.Scrollbar?) -> CGFloat { + private func documentHeight() -> CGFloat { let contentHeight = scrollView.contentSize.height let cellHeight = surfaceView.cellSize.height - if cellHeight > 0, let scrollbar = scrollbar { + if cellHeight > 0, let scrollbar = surfaceView.scrollbar { // The document view must have the same vertical padding around the // scrollback grid as the content view has around the terminal grid // otherwise the content view loses alignment with the surface. diff --git a/macos/Sources/Ghostty/SurfaceView.swift b/macos/Sources/Ghostty/SurfaceView.swift index c650bdf8f..b42e34314 100644 --- a/macos/Sources/Ghostty/SurfaceView.swift +++ b/macos/Sources/Ghostty/SurfaceView.swift @@ -407,8 +407,8 @@ extension Ghostty { } func updateOSView(_ scrollView: SurfaceScrollView, context: Context) { - // Our scrollview always takes up the full size. - scrollView.frame.size = size + // Nothing to do: SwiftUI automatically updates the frame size, and + // SurfaceScrollView handles the rest in response to that } #else func makeOSView(context: Context) -> SurfaceView { diff --git a/macos/Sources/Ghostty/SurfaceView_AppKit.swift b/macos/Sources/Ghostty/SurfaceView_AppKit.swift index a04d00761..3ab2f3e94 100644 --- a/macos/Sources/Ghostty/SurfaceView_AppKit.swift +++ b/macos/Sources/Ghostty/SurfaceView_AppKit.swift @@ -152,6 +152,9 @@ extension Ghostty { var surface: ghostty_surface_t? { surfaceModel?.unsafeCValue } + /// Current scrollbar state, cached here for persistence across rebuilds + /// of the SwiftUI view hierarchy, for example when changing splits + var scrollbar: Ghostty.Action.Scrollbar? // Notification identifiers associated with this surface var notificationIdentifiers: Set = []