superrob1500
/
mirror-ghostty
mirror of https://github.com/ghostty-org/ghostty
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

macos: Fix documentView padding calculations (#9423) 

Fixes #9420 

The problem was ultimately that the padding calculations assumed that
the total vertical padding is always less than one cell height (just
doing `fmod(contentHeight, cellHeight)` instead of the more careful
`contentHeight - scrollbar.len * cellHeight`). This is not at all true,
and as a result, the calculated document height was often a cell height
short of what it should be.

For similar reasons, we shouldn't rely on `fmod`-based padding update
calculations during relayouting. This PR takes the proper approach of
saving and reusing the current scrollbar state to calculate the correct
document height on every layout. As a bonus, this removes the flickering
scrollbar during resize that I complained about in #9296.
pull/9427/head
Lukas 2025-10-31 11:32:37 +01:00 committed by GitHub
parent bc0f5e4d57 951374cd1c
commit b043623bb2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 24 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
macos/Sources/Ghostty/SurfaceScrollView.swift
View File

@ -19,6 +19,7 @@ class SurfaceScrollView: NSView {
private var observers: [NSObjectProtocol] = [] private var observers: [NSObjectProtocol] = []
private var cancellables: Set<AnyCancellable> = [] private var cancellables: Set<AnyCancellable> = []
private var isLiveScrolling = false private var isLiveScrolling = false
private var currentScrollbar: Ghostty.Action.Scrollbar? = nil
/// The last row position sent via scroll_to_row action. Used to avoid /// The last row position sent via scroll_to_row action. Used to avoid
/// sending redundant actions when the user drags the scrollbar but stays /// sending redundant actions when the user drags the scrollbar but stays
@ -174,19 +175,11 @@ class SurfaceScrollView: NSView {
} }
} }
// Keep document width synchronized with content width, and // Keep document width synchronized with content width, and update the
// recalculate the height of the document view to account for the // document height as appropriate for the current surface size.
// change in padding around the cell grid due to the resize.
var documentHeight = documentView.frame.height
let cellHeight = surfaceView.cellSize.height
if cellHeight > 0 {
let oldPadding = fmod(documentHeight, cellHeight)
let newPadding = fmod(contentSize.height, cellHeight)
documentHeight += newPadding - oldPadding
}
documentView.setFrameSize(CGSize( documentView.setFrameSize(CGSize(
width: contentSize.width, width: contentSize.width,
height: documentHeight height: documentHeight(currentScrollbar),
)) ))
// Inform the actual pty of our size change. This doesn't change the actual view // Inform the actual pty of our size change. This doesn't change the actual view
@ -262,21 +255,18 @@ class SurfaceScrollView: NSView {
guard let scrollbar = notification.userInfo?[SwiftUI.Notification.Name.ScrollbarKey] as? Ghostty.Action.Scrollbar else { guard let scrollbar = notification.userInfo?[SwiftUI.Notification.Name.ScrollbarKey] as? Ghostty.Action.Scrollbar else {
return return
} }
currentScrollbar = scrollbar
// Convert row units to pixels using cell height, ignore zero height. // Convert row units to pixels using cell height, ignore zero height.
let cellHeight = surfaceView.cellSize.height let cellHeight = surfaceView.cellSize.height
guard cellHeight > 0 else { return } guard cellHeight > 0 else { return }
// The full document height must include the vertical padding around the cell
// grid, otherwise the content view ends up misaligned with the surface.
let documentGridHeight = CGFloat(scrollbar.total) * cellHeight
let padding = fmod(scrollView.contentSize.height, cellHeight)
let documentHeight = documentGridHeight + padding
// Our width should be the content width to account for visible scrollers. // Our width should be the content width to account for visible scrollers.
// We don't do horizontal scrolling in terminals. // We don't do horizontal scrolling in terminals.
let newSize = CGSize(width: scrollView.contentSize.width, height: documentHeight) documentView.setFrameSize(CGSize(
documentView.setFrameSize(newSize) width: scrollView.contentSize.width,
height: documentHeight(scrollbar),
))
// Only update our actual scroll position if we're not actively scrolling. // Only update our actual scroll position if we're not actively scrolling.
if !isLiveScrolling { if !isLiveScrolling {
@ -292,4 +282,19 @@ class SurfaceScrollView: NSView {
// Always update our scrolled view with the latest dimensions // Always update our scrolled view with the latest dimensions
scrollView.reflectScrolledClipView(scrollView.contentView) scrollView.reflectScrolledClipView(scrollView.contentView)
} }
/// Calculate the appropriate document view height given a scrollbar state
private func documentHeight(_ scrollbar: Ghostty.Action.Scrollbar?) -> CGFloat {
let contentHeight = scrollView.contentSize.height
let cellHeight = surfaceView.cellSize.height
if cellHeight > 0, let scrollbar = 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.
let documentGridHeight = CGFloat(scrollbar.total) * cellHeight
let padding = contentHeight - (CGFloat(scrollbar.len) * cellHeight)
return documentGridHeight + padding
}
return contentHeight
}
} }