-
Notifications
You must be signed in to change notification settings - Fork 59
Add fractional scroll offsets for embedders #61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1229,6 +1229,7 @@ fn selectionScrollTick(self: *Surface) !void { | |||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Scroll the viewport as required | ||||||||||||||||||||||||||||||||||||||||||||||||||
| self.renderer_state.resetSmoothScrollOffset(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| t.scrollViewport(.{ .delta = delta }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Next, trigger our drag behavior | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -3491,6 +3492,53 @@ const ScrollAmount = struct { | |||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Scroll the viewport to a fractional row offset from the top of scrollback. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| /// The terminal model stays row based; the renderer shifts the current frame | ||||||||||||||||||||||||||||||||||||||||||||||||||
| /// by the remaining pixel fraction. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| pub fn scrollToOffset(self: *Surface, offset: f64) !void { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // Crash metadata in case we crash in here | ||||||||||||||||||||||||||||||||||||||||||||||||||
| crash.sentry.thread_state = self.crashThreadState(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| defer crash.sentry.thread_state = null; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| self.renderer_state.mutex.lock(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| defer self.renderer_state.mutex.unlock(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const t: *terminal.Terminal = self.renderer_state.terminal; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const scrollbar = t.screens.active.pages.scrollbar(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const max_offset: usize = if (scrollbar.total > scrollbar.len) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| scrollbar.total - scrollbar.len | ||||||||||||||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||||||||||||||
| 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const max_offset_float: f64 = @floatFromInt(max_offset); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const clamped_offset = @min(@max(offset, 0.0), max_offset_float); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const row_offset_float = @floor(clamped_offset); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const row_offset: usize = @intFromFloat(row_offset_float); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const fractional_offset = clamped_offset - row_offset_float; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| t.screens.active.scroll(.{ .row = row_offset }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| self.mouse.pending_scroll_x = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| self.mouse.pending_scroll_y = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| self.renderer_state.resetSmoothScrollOffset(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const cell_height_float: f64 = @floatFromInt(self.size.cell.height); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (fractional_offset > 0 and cell_height_float > 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const pixel_offset = @trunc(-fractional_offset * cell_height_float); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Do not truncate the computed pixel offset here; truncation removes sub-pixel smooth-scroll movement and causes visible stair-stepping. Prompt for AI agents
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| self.renderer_state.smooth_scroll_offset = @floatCast(pixel_offset); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const extra_rows_float = @ceil(@abs(pixel_offset) / cell_height_float); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const max_u16_float: f64 = @floatFromInt(std.math.maxInt(u16)); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const extra_rows: u16 = if (extra_rows_float >= max_u16_float) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| std.math.maxInt(u16) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||||||||||||||
| @intFromFloat(extra_rows_float); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| self.renderer_state.image_scroll_offset = .{ extra_rows, extra_rows }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+3524
to
+3535
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Keep the pixel offset fractional. Line 3526 truncates the requested smooth-scroll displacement to a whole pixel, so small offset updates are dropped and the new API will visibly stair-step instead of tracking the embedder’s fractional position. Proposed fix const cell_height_float: f64 = `@floatFromInt`(self.size.cell.height);
if (fractional_offset > 0 and cell_height_float > 0) {
- const pixel_offset = `@trunc`(-fractional_offset * cell_height_float);
+ const pixel_offset = -fractional_offset * cell_height_float;
self.renderer_state.smooth_scroll_offset = `@floatCast`(pixel_offset);
const extra_rows_float = `@ceil`(`@abs`(pixel_offset) / cell_height_float);
const max_u16_float: f64 = `@floatFromInt`(std.math.maxInt(u16));
const extra_rows: u16 = if (extra_rows_float >= max_u16_float)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| try self.queueRender(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Mouse scroll event. Negative is down, left. Positive is up, right. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||||||||||||||||||
| /// "Natural scrolling" is a macOS term for inverting the scroll direction. | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -3665,6 +3713,7 @@ pub fn scrollCallback( | |||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| if (y.delta != 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| self.renderer_state.resetSmoothScrollOffset(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // Modify our viewport, this requires a lock since it affects | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // rendering. We have to switch signs here because our delta | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // is negative down but our viewport is positive down. | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -5511,6 +5560,7 @@ pub fn performBindingAction(self: *Surface, action: input.Binding.Action) !bool | |||||||||||||||||||||||||||||||||||||||||||||||||
| self.renderer_state.mutex.lock(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| defer self.renderer_state.mutex.unlock(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const t: *terminal.Terminal = self.renderer_state.terminal; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| self.renderer_state.resetSmoothScrollOffset(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| t.screens.active.scroll(.{ .row = n }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -5523,6 +5573,7 @@ pub fn performBindingAction(self: *Surface, action: input.Binding.Action) !bool | |||||||||||||||||||||||||||||||||||||||||||||||||
| defer self.renderer_state.mutex.unlock(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const sel = self.io.terminal.screens.active.selection orelse return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const tl = sel.topLeft(self.io.terminal.screens.active); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| self.renderer_state.resetSmoothScrollOffset(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| self.io.terminal.screens.active.scroll(.{ .pin = tl }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -737,6 +737,8 @@ pub fn Renderer(comptime GraphicsAPI: type) type { | |
| .cell_size = undefined, | ||
| .grid_size = undefined, | ||
| .grid_padding = undefined, | ||
| .smooth_scroll_offset = 0, | ||
| .image_scroll_offset = .{ 0, 0 }, | ||
| .screen_size = undefined, | ||
| .padding_extend = .{}, | ||
| .min_contrast = options.config.min_contrast, | ||
|
|
@@ -1212,6 +1214,7 @@ pub fn Renderer(comptime GraphicsAPI: type) type { | |
| self.last_bottom_y = br.y; | ||
|
|
||
| // Scroll | ||
| state.resetSmoothScrollOffset(); | ||
| state.terminal.scrollViewport(.bottom); | ||
| } | ||
|
|
||
|
|
@@ -1230,6 +1233,8 @@ pub fn Renderer(comptime GraphicsAPI: type) type { | |
| // can be expensive) and also makes it so we don't need another | ||
| // cross-thread mailbox message within the IO path. | ||
| const scrollbar = state.terminal.screens.active.pages.scrollbar(); | ||
| self.uniforms.smooth_scroll_offset = state.smooth_scroll_offset; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Write Prompt for AI agents |
||
| self.uniforms.image_scroll_offset = state.image_scroll_offset; | ||
|
Comment on lines
+1236
to
+1237
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move uniform writes under
🔧 Suggested fix const Critical = struct {
links: terminal.RenderState.CellSet,
mouse: renderer.State.Mouse,
preedit: ?renderer.State.Preedit,
scrollbar: terminal.Scrollbar,
+ smooth_scroll_offset: f32,
+ image_scroll_offset: [2]u16,
overlay_features: []const Overlay.Feature,
};
...
-const scrollbar = state.terminal.screens.active.pages.scrollbar();
-self.uniforms.smooth_scroll_offset = state.smooth_scroll_offset;
-self.uniforms.image_scroll_offset = state.image_scroll_offset;
+const scrollbar = state.terminal.screens.active.pages.scrollbar();
+const smooth_scroll_offset = state.smooth_scroll_offset;
+const image_scroll_offset = state.image_scroll_offset;
...
break :critical .{
.links = links,
.mouse = state.mouse,
.preedit = preedit,
.scrollbar = scrollbar,
+ .smooth_scroll_offset = smooth_scroll_offset,
+ .image_scroll_offset = image_scroll_offset,
.overlay_features = overlay_features,
};
};
...
self.draw_mutex.lock();
defer self.draw_mutex.unlock();
+self.uniforms.smooth_scroll_offset = critical.smooth_scroll_offset;
+self.uniforms.image_scroll_offset = critical.image_scroll_offset;🤖 Prompt for AI AgentsThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| // Get our preedit state | ||
| const preedit: ?renderer.State.Preedit = preedit: { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -178,6 +178,12 @@ pub const Uniforms = extern struct { | |
| /// top, right, bottom, left. | ||
| grid_padding: [4]f32 align(16), | ||
|
|
||
| /// Fractional vertical scroll offset in pixels. | ||
| smooth_scroll_offset: f32 align(4), | ||
|
|
||
| /// Extra image-row range to draw while smooth scrolling. | ||
| image_scroll_offset: [2]u16 align(4), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Prompt for AI agents |
||
|
|
||
| /// Bit mask defining which directions to | ||
| /// extend cell colors in to the padding. | ||
| /// Order, LSB first: left, right, up, down | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@truncquantises the pixel offset to whole pixels, so smooth-scroll animations will visibly step in 1-pixel increments instead of moving continuously. Becausesmooth_scroll_offsetis af32uniform the shader can already accept fractional pixels; drop the truncation to preserve sub-pixel precision.