Fix Ghostty viewport sync scroll jitter#1512
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughAdds a new viewport synchronization module and integrates it into the Ghostty terminal view: introduces pure sync logic, anchor-based top-visible-row tracking, explicit/passive viewport change handling, wiring to existing scroll view/terminal surface code, and unit + harness tests to validate reconciliation and dispatch behavior. Changes
Sequence DiagramsequenceDiagram
participant User
participant ScrollView as GhosttyScrollView
participant TerminalView as GhosttyTerminalView
participant ViewportSync as ViewportSync<br/>(functions)
participant Surface as TerminalSurface
participant Scrollbar as ScrollbarUI
User->>ScrollView: user scroll / scroll wheel
ScrollView->>TerminalView: mark explicit viewport change
TerminalView->>ViewportSync: ghosttyShouldBeginExplicitViewportChange()
ViewportSync-->>TerminalView: explicit intent
TerminalView->>ViewportSync: ghosttyScrollViewportSyncPlan(scrollbar, storedAnchor, isExplicit)
ViewportSync-->>TerminalView: sync plan (targetTopVisibleRow / targetRowFromBottom)
TerminalView->>Surface: updateScrollViewportAnchorTopVisibleRow()
Surface-->>TerminalView: stored anchor updated
TerminalView->>Scrollbar: apply sync target (UI update)
Note over TerminalView,ViewportSync: Passive/incoming scrollbar update (new output)
TerminalView->>ViewportSync: ghosttyReconciledViewportScrollbar(incoming, storedAnchor, isExplicit)
ViewportSync-->>TerminalView: reconciled scrollbar or plan
TerminalView->>Surface: storedScrollViewportAnchorTopVisibleRow()
Surface-->>TerminalView: returns anchor
TerminalView->>Scrollbar: apply reconciled state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53b1750547
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| guard incomingScrollbar.totalRows < previousScrollbar.totalRows else { | ||
| return false | ||
| } | ||
| return resolvedStoredTopVisibleRow != nil || resultingStoredTopVisibleRow == nil |
There was a problem hiding this comment.
This stale-update predicate also returns true when both resolvedStoredTopVisibleRow and resultingStoredTopVisibleRow are nil (the normal "at bottom" case), so any passive scrollbar update where totalRows decreases is dropped even when it is legitimate (for example, switching to an alternate screen or clearing scrollback). In handleScrollbarUpdate, dropped updates never refresh lastAcceptedScrollbar, so subsequent smaller snapshots keep getting rejected and the scroll view can stay pinned to stale geometry. The ignore condition should be limited to cases where an existing anchor would be lost, not unconditional bottom-state regressions.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Sources/GhosttyViewportSync.swift (1)
215-227: Consider consolidating identical implementations.
ghosttyBaselineScrollbarForIncomingUpdateandghosttyEffectiveViewportScrollbarhave identical bodies. If the semantic distinction is important for call-site clarity and they may diverge later, this is fine. Otherwise, one could call the other to reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyViewportSync.swift` around lines 215 - 227, Both functions ghosttyBaselineScrollbarForIncomingUpdate and ghosttyEffectiveViewportScrollbar have identical implementations; to remove duplication, have one delegate to the other (e.g., implement ghosttyBaselineScrollbarForIncomingUpdate by returning ghosttyEffectiveViewportScrollbar(lastAcceptedScrollbar:currentSurfaceScrollbar:) or vice versa) or extract the shared expression into a small helper function used by both. Update callers if needed to preserve the original semantic names while consolidating the actual logic in a single place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 5874-5876: The scroll-wheel path in
GhosttyNSView.scrollWheel(with:) bypasses the explicit-viewport marker so
pendingExplicitViewportChange stays false; modify
GhosttyNSView.scrollWheel(with:) to mirror GhosttyScrollView.scrollWheel(with:)
by checking ghosttyShouldBeginExplicitViewportChange(for: .scrollWheel) and,
when true, calling surfaceContainer?.markExplicitViewportChange() before
invoking ghostty_surface_mouse_scroll(), ensuring pendingExplicitViewportChange
is set for the main wheel path so handleScrollbarUpdate treats replies as
explicit.
---
Nitpick comments:
In `@Sources/GhosttyViewportSync.swift`:
- Around line 215-227: Both functions ghosttyBaselineScrollbarForIncomingUpdate
and ghosttyEffectiveViewportScrollbar have identical implementations; to remove
duplication, have one delegate to the other (e.g., implement
ghosttyBaselineScrollbarForIncomingUpdate by returning
ghosttyEffectiveViewportScrollbar(lastAcceptedScrollbar:currentSurfaceScrollbar:)
or vice versa) or extract the shared expression into a small helper function
used by both. Update callers if needed to preserve the original semantic names
while consolidating the actual logic in a single place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 153d13a3-8ed8-4eef-9b2e-756f3659f77e
📒 Files selected for processing (6)
GhosttyTabs.xcodeproj/project.pbxprojSources/GhosttyTerminalView.swiftSources/GhosttyViewportSync.swiftcmuxTests/GhosttyScrollbarSyncPlanTests.swifttests/test_ghostty_viewport_sync_logic.shtests/test_ghostty_viewport_sync_logic.swift
| if ghosttyShouldBeginExplicitViewportChange(for: .scrollWheel) { | ||
| surfaceContainer?.markExplicitViewportChange() | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's look at the specific code sections mentioned in the review
# Check lines around 5619-5666 for GhosttyNSView.scrollWheel
echo "=== GhosttyNSView.scrollWheel (lines 5619-5666) ==="
sed -n '5619,5666p' Sources/GhosttyTerminalView.swift
echo ""
echo "=== GhosttyScrollView.scrollWheel (lines 5874-5876 and context) ==="
sed -n '5870,5880p' Sources/GhosttyTerminalView.swift
echo ""
echo "=== Search for markExplicitViewportChange definition ==="
rg "func markExplicitViewportChange" -A 5Repository: manaflow-ai/cmux
Length of output: 3145
🏁 Script executed:
#!/bin/bash
# Look at the full scrollWheel implementations with more context
echo "=== GhosttyNSView.scrollWheel full context ==="
sed -n '5615,5675p' Sources/GhosttyTerminalView.swift
echo ""
echo "=== GhosttyScrollView.scrollWheel full context ==="
sed -n '5865,5895p' Sources/GhosttyTerminalView.swiftRepository: manaflow-ai/cmux
Length of output: 3680
🏁 Script executed:
#!/bin/bash
# Search for where pendingExplicitViewportChange is used
echo "=== pendingExplicitViewportChange usage ==="
rg "pendingExplicitViewportChange" -B 2 -A 2
echo ""
echo "=== handleScrollbarUpdate implementation ==="
rg "func handleScrollbarUpdate" -A 20Repository: manaflow-ai/cmux
Length of output: 7029
🏁 Script executed:
#!/bin/bash
# Check TerminalSurface definition and hostedView property
echo "=== TerminalSurface hostedView property ==="
rg "struct TerminalSurface" -A 30
echo ""
echo "=== Check how terminalSurface is accessed in GhosttyNSView ==="
sed -n '5619,5640p' Sources/GhosttyTerminalView.swift | head -20
echo ""
echo "=== Look for hostedView or surfaceContainer in TerminalSurface ==="
rg "var hostedView|var surfaceContainer" -B 2 -A 2Repository: manaflow-ai/cmux
Length of output: 2311
🏁 Script executed:
#!/bin/bash
# Check where terminalSurface comes from in GhosttyNSView
echo "=== GhosttyNSView.terminalSurface definition ==="
rg "class GhosttyNSView" -A 20 | head -40
echo ""
echo "=== Search for terminalSurface property ==="
rg "terminalSurface.*TerminalSurface" -B 1 -A 1 | grep -A 1 "GhosttyNSView"Repository: manaflow-ai/cmux
Length of output: 2126
🏁 Script executed:
#!/bin/bash
# Verify the relationship between surfaceView and GhosttySurfaceScrollView
echo "=== GhosttySurfaceScrollView and markExplicitViewportChange ==="
rg "class GhosttySurfaceScrollView" -A 10
echo ""
echo "=== Check surfaceView property that holds GhosttyNSView ==="
rg "var surfaceView.*GhosttyNSView" -B 2 -A 2
echo ""
echo "=== Verify terminalSurface relationship ==="
rg "terminalSurface.*?TerminalSurface" Sources/GhosttyTerminalView.swift | head -5Repository: manaflow-ai/cmux
Length of output: 1862
🏁 Script executed:
#!/bin/bash
# Check TerminalSurface struct definition more carefully
echo "=== TerminalSurface definition in Sources ==="
rg "struct TerminalSurface\s*\{" -A 50 | head -100
echo ""
echo "=== Check if TerminalSurface has hostedView property ==="
rg "class TerminalSurface|struct TerminalSurface" -A 100 | grep -A 100 "^Sources" | grep "hostedView" -B 5 -A 2Repository: manaflow-ai/cmux
Length of output: 2194
🏁 Script executed:
#!/bin/bash
# Look for TerminalSurface file
echo "=== Find TerminalSurface definition file ==="
fd ".*TerminalSurface.*" --type f --extension swift
echo ""
echo "=== Search for hostedView assignment on TerminalSurface ==="
rg "\.hostedView.*=" -B 3 -A 1 | grep -A 3 -B 3 "TerminalSurface\|GhosttySurfaceScrollView"Repository: manaflow-ai/cmux
Length of output: 726
🏁 Script executed:
#!/bin/bash
# Check if GhosttyNSView can access parent container
echo "=== Check GhosttyNSView properties and relationships ==="
sed -n '5580,5610p' Sources/GhosttyTerminalView.swift
echo ""
echo "=== Look for parent traversal or superview patterns ==="
rg "superview.*GhosttyScrollView|as\?.*GhosttyScrollView" -B 2 -A 2Repository: manaflow-ai/cmux
Length of output: 1580
🏁 Script executed:
#!/bin/bash
# Check GhosttyNSView's access to terminalSurface property
echo "=== GhosttyNSView class definition and terminalSurface ==="
sed -n '5440,5480p' Sources/GhosttyTerminalView.swift
echo ""
echo "=== Search for terminalSurface assignment in GhosttyNSView ==="
rg "var terminalSurface.*\?" Sources/GhosttyTerminalView.swift | head -5
echo ""
echo "=== Check GhosttySurfaceCallbackContext where terminalSurface is defined ==="
rg "private final class GhosttySurfaceCallbackContext" -A 20Repository: manaflow-ai/cmux
Length of output: 3355
🏁 Script executed:
#!/bin/bash
# Verify the event routing: does GhosttyNSView.scrollWheel bypass GhosttyScrollView.scrollWheel?
echo "=== Check if GhosttyNSView is a subview of GhosttyScrollView ==="
rg "documentView|setDocumentView" -B 2 -A 2
echo ""
echo "=== Verify AppKit scrollWheel routing - check for super.scrollWheel ==="
sed -n '5619,5670p' Sources/GhosttyTerminalView.swift | grep -i "super\|scrollwheel"
echo ""
echo "=== Check if GhosttyScrollView delegates to inner view ==="
sed -n '5865,5885p' Sources/GhosttyTerminalView.swiftRepository: manaflow-ai/cmux
Length of output: 6251
🏁 Script executed:
#!/bin/bash
# Verify the exact view hierarchy and scroll wheel routing
echo "=== Complete GhosttySurfaceScrollView init and setup ==="
rg "private let scrollView: GhosttyScrollView" -A 50 | head -80
echo ""
echo "=== Check if there's any direct scrollWheel override or handling in GhosttySurfaceScrollView ==="
rg "class GhosttySurfaceScrollView" -A 200 | grep -i "scrollwheel\|scroll"Repository: manaflow-ai/cmux
Length of output: 5182
🏁 Script executed:
#!/bin/bash
# Verify if GhosttyNSView.scrollWheel could be called independently
echo "=== Check all scrollWheel overrides ==="
rg "override func scrollWheel" -B 3 -A 8
echo ""
echo "=== Verify the precise location where markExplicitViewportChange is called ==="
sed -n '5870,5885p' Sources/GhosttyTerminalView.swiftRepository: manaflow-ai/cmux
Length of output: 4901
🏁 Script executed:
#!/bin/bash
# Check if GhosttyScrollView.scrollWheel is actually invoked
echo "=== Verify GhosttyScrollView extends NSScrollView ==="
grep -A 3 "private final class GhosttyScrollView"
echo ""
echo "=== Check AppKit event routing - does NSScrollView.scrollWheel intercept? ==="
sed -n '5860,5890p' Sources/GhosttyTerminalView.swiftRepository: manaflow-ai/cmux
Length of output: 1337
🏁 Script executed:
#!/bin/bash
# Final verification: check view hierarchy and hitTest behavior
echo "=== View hierarchy setup in GhosttySurfaceScrollView.init ==="
rg "GhosttySurfaceScrollView.*init" -A 40 | head -60
echo ""
echo "=== Verify GhosttyNSView doesn't call super.scrollWheel ==="
sed -n '5619,5670p' Sources/GhosttyTerminalView.swift | grep -i "super"
echo ""
echo "=== Check if GhosttyScrollView or GhosttySurfaceScrollView override hitTest ==="
rg "override func hitTest" -B 2 -A 8 | grep -A 8 "GhosttyScrollView\|GhosttySurfaceScrollView"Repository: manaflow-ai/cmux
Length of output: 263
🏁 Script executed:
#!/bin/bash
# Get the full GhosttySurfaceScrollView init to understand view hierarchy
echo "=== Full GhosttySurfaceScrollView init code ==="
rg "final class GhosttySurfaceScrollView" -A 100 | grep -A 60 "init("
echo ""
echo "=== Look for where terminalSurface is set on GhosttyNSView ==="
rg "\.terminalSurface\s*=" Sources/GhosttyTerminalView.swift | head -10Repository: manaflow-ai/cmux
Length of output: 218
🏁 Script executed:
#!/bin/bash
# Verify the exact accessor path for the fix
echo "=== Check TerminalSurface.hostedView type ==="
rg "let hostedView:" Sources/GhosttyTerminalView.swift -B 2 -A 2
echo ""
echo "=== Verify markExplicitViewportChange is accessible from hostedView ==="
rg "fileprivate func markExplicitViewportChange" -B 5 -A 5
echo ""
echo "=== Verify GhosttyNSView.terminalSurface access ==="
sed -n '5380,5400p' Sources/GhosttyTerminalView.swiftRepository: manaflow-ai/cmux
Length of output: 1883
Mark the actual wheel-event path as explicit too.
Line 5619's GhosttyNSView.scrollWheel(with:) handles normal trackpad/mouse-wheel input and directly calls ghostty_surface_mouse_scroll() without triggering the marker at line 5874. Since hit-testing targets the inner view, GhosttyScrollView.scrollWheel(with:) is typically bypassed. This leaves pendingExplicitViewportChange false for the main scrolling path, causing handleScrollbarUpdate to classify Ghostty's reply as passive again.
Add the marker in GhosttyNSView.scrollWheel(with:) to catch all wheel events:
override func scrollWheel(with event: NSEvent) {
guard let surface = surface else { return }
+ terminalSurface?.hostedView.markExplicitViewportChange()
lastScrollEventTime = CACurrentMediaTime()
Self.focusLog("scrollWheel: surface=\(terminalSurface?.id.uuidString ?? "nil") firstResponder=\(String(describing: window?.firstResponder))")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/GhosttyTerminalView.swift` around lines 5874 - 5876, The scroll-wheel
path in GhosttyNSView.scrollWheel(with:) bypasses the explicit-viewport marker
so pendingExplicitViewportChange stays false; modify
GhosttyNSView.scrollWheel(with:) to mirror GhosttyScrollView.scrollWheel(with:)
by checking ghosttyShouldBeginExplicitViewportChange(for: .scrollWheel) and,
when true, calling surfaceContainer?.markExplicitViewportChange() before
invoking ghostty_surface_mouse_scroll(), ensuring pendingExplicitViewportChange
is set for the main wheel path so handleScrollbarUpdate treats replies as
explicit.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Sources/GhosttyTerminalView.swift (1)
5874-5876:⚠️ Potential issue | 🟠 MajorExplicit viewport marking still misses the primary wheel-event path.
This marker only covers the
GhosttyScrollView.scrollWheel(with:)path. The core wheel handling still happens inGhosttyNSView.scrollWheel(with:)(Line 5619), sopendingExplicitViewportChangecan remain unset for common wheel input and regress to passive classification.⚙️ Suggested patch
override func scrollWheel(with event: NSEvent) { guard let surface = surface else { return } + if ghosttyShouldBeginExplicitViewportChange(for: .scrollWheel) { + terminalSurface?.hostedView.markExplicitViewportChange() + } lastScrollEventTime = CACurrentMediaTime() Self.focusLog("scrollWheel: surface=\(terminalSurface?.id.uuidString ?? "nil") firstResponder=\(String(describing: window?.firstResponder))")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 5874 - 5876, The explicit-viewport marker is only set in GhosttyScrollView.scrollWheel(with:) and misses the primary wheel-event path handled by GhosttyNSView.scrollWheel(with:), so update GhosttyNSView.scrollWheel(with:) to call ghosttyShouldBeginExplicitViewportChange(for: .scrollWheel) and, if true, call surfaceContainer?.markExplicitViewportChange() (same logic as in GhosttyScrollView); ensure you reference and set pendingExplicitViewportChange via the existing ghosttyShouldBeginExplicitViewportChange(for:) check and reuse the same markExplicitViewportChange() invocation to keep behavior consistent.
🧹 Nitpick comments (1)
Sources/GhosttyViewportSync.swift (1)
221-233: Consider consolidating duplicated scrollbar-selection helpers.
ghosttyBaselineScrollbarForIncomingUpdateandghosttyEffectiveViewportScrollbarcurrently do the same thing (lastAcceptedScrollbar ?? currentSurfaceScrollbar). Keeping one implementation reduces drift risk.♻️ Optional refactor
-func ghosttyBaselineScrollbarForIncomingUpdate( - lastAcceptedScrollbar: GhosttyScrollbar?, - currentSurfaceScrollbar: GhosttyScrollbar? -) -> GhosttyScrollbar? { - lastAcceptedScrollbar ?? currentSurfaceScrollbar -} - -func ghosttyEffectiveViewportScrollbar( +func ghosttyPreferredScrollbar( lastAcceptedScrollbar: GhosttyScrollbar?, currentSurfaceScrollbar: GhosttyScrollbar? ) -> GhosttyScrollbar? { lastAcceptedScrollbar ?? currentSurfaceScrollbar }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyViewportSync.swift` around lines 221 - 233, Both functions ghosttyBaselineScrollbarForIncomingUpdate and ghosttyEffectiveViewportScrollbar are duplicate (both return lastAcceptedScrollbar ?? currentSurfaceScrollbar); consolidate by keeping a single implementation (e.g., ghosttyEffectiveViewportScrollbar) and have the other function delegate to it or replace both call sites to use the single helper; update references to the removed/redirected function(s) so callers use the chosen unique symbol and remove the duplicate implementation to avoid drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 5874-5876: The explicit-viewport marker is only set in
GhosttyScrollView.scrollWheel(with:) and misses the primary wheel-event path
handled by GhosttyNSView.scrollWheel(with:), so update
GhosttyNSView.scrollWheel(with:) to call
ghosttyShouldBeginExplicitViewportChange(for: .scrollWheel) and, if true, call
surfaceContainer?.markExplicitViewportChange() (same logic as in
GhosttyScrollView); ensure you reference and set pendingExplicitViewportChange
via the existing ghosttyShouldBeginExplicitViewportChange(for:) check and reuse
the same markExplicitViewportChange() invocation to keep behavior consistent.
---
Nitpick comments:
In `@Sources/GhosttyViewportSync.swift`:
- Around line 221-233: Both functions ghosttyBaselineScrollbarForIncomingUpdate
and ghosttyEffectiveViewportScrollbar are duplicate (both return
lastAcceptedScrollbar ?? currentSurfaceScrollbar); consolidate by keeping a
single implementation (e.g., ghosttyEffectiveViewportScrollbar) and have the
other function delegate to it or replace both call sites to use the single
helper; update references to the removed/redirected function(s) so callers use
the chosen unique symbol and remove the duplicate implementation to avoid drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f862a4c3-9821-4494-b888-d789476422cd
📒 Files selected for processing (4)
Sources/GhosttyTerminalView.swiftSources/GhosttyViewportSync.swiftcmuxTests/GhosttyScrollbarSyncPlanTests.swifttests/test_ghostty_viewport_sync_logic.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- cmuxTests/GhosttyScrollbarSyncPlanTests.swift
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e90c7d9c47
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let offsetY = | ||
| CGFloat(scrollbar.total - scrollbar.offset - scrollbar.len) * cellHeight | ||
| let targetOrigin = CGPoint(x: 0, y: offsetY) | ||
| if cellHeight > 0, let syncPlan = makeViewportSyncPlan(isExplicitViewportChange: false) { |
There was a problem hiding this comment.
synchronizeScrollView() always requests makeViewportSyncPlan(isExplicitViewportChange: false), so even when handleScrollbarUpdate has just classified an incoming snapshot as explicit (e.g. a user scroll_to_bottom from scrollback) and cleared the stored anchor, the follow-up sync immediately runs passive recovery from the current clip view and can restore the old top-visible row. In practice this can undo explicit viewport changes and make keyboard/menu jumps to bottom/top snap back. The sync path needs to carry explicit/passive mode through this call instead of forcing passive mode.
Useful? React with 👍 / 👎.
1 participant
Summary
Restart from current
mainand rebuild the Ghostty viewport sync fix with a fresh red/green pair.GhosttyTerminalViewso plain scrolling and passive updates stop fighting each otherVerification
./tests/test_ghostty_viewport_sync_logic.shxcodebuild -project GhosttyTabs.xcodeproj -scheme cmux-unit -destination "platform=macOS" -derivedDataPath /tmp/cmux-task-scroll-jank-reset-green4 -only-testing:cmuxTests/GhosttyScrollbarSyncPlanTests test./scripts/reload.sh --tag task-scroll-jank-resetSummary by cubic
Fixes scroll jitter in Ghostty by separating user-driven scrolls from passive output updates and anchoring the top visible row. Keeps the initial viewport at the bottom and uses a stateless sync planner to stabilize scrollbar-to-viewport mapping and stop rubber-banding.
Bug Fixes
Refactors
GhosttyViewportSync.swiftwith small, pure helpers used byGhosttySurfaceScrollView/GhosttyTerminalView.GhosttyScrollbarSyncPlanTests.swiftand a lightweight CLI test (tests/test_ghostty_viewport_sync_logic.*); wired tests into the Xcode project.Written for commit e90c7d9. Summary will update on new commits.
Summary by CodeRabbit
New Features
Refactor
Tests