From 7929e0bc09d4524d982c6ac369013eba40762fd0 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Wed, 14 Aug 2024 19:35:52 -0400 Subject: [PATCH 1/3] fix: prevent flicker while shrinking screen by eliminating thread race Before this fix, if vsync was on the GPU cells buffer could be cleared for a frame while resizing the terminal down. This was due to the fact that the surface sent messages for the resize to both the renderer and the IO thread. If the renderer thread was processed first then the GPU cells buffer(s) would be cleared and not rebuilt, because the terminal state would be larger than the GPU cell buffers causing updateFrame to bail out early, leaving empty cell buffers. This fixes the problem by changing the origin of the renderer's resize message to be the IO thread, only after properly updating the terminal state, to avoid clearing the GPU cells buffers at a time they can't be successfully rebuilt. --- src/Surface.zig | 9 --------- src/renderer/Metal.zig | 10 ++++++---- src/renderer/Thread.zig | 6 +++--- src/termio/Termio.zig | 8 +++++++- 4 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index 2e283f985..fc3144ade 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -1197,15 +1197,6 @@ fn resize(self: *Surface, size: renderer.ScreenSize) !void { // Save our screen size self.screen_size = size; - // Mail the renderer so that it can update the GPU and re-render - _ = self.renderer_thread.mailbox.push(.{ - .resize = .{ - .screen_size = self.screen_size, - .padding = self.padding, - }, - }, .{ .forever = {} }); - try self.queueRender(); - // Recalculate our grid size. Because Ghostty supports fluid resizing, // its possible the grid doesn't change at all even if the screen size changes. // We have to update the IO thread no matter what because we send diff --git a/src/renderer/Metal.zig b/src/renderer/Metal.zig index ebd5d9b0d..4f669d474 100644 --- a/src/renderer/Metal.zig +++ b/src/renderer/Metal.zig @@ -2012,11 +2012,13 @@ pub fn setScreenSize( .cursor_color = old.cursor_color, }; - // Reset our cell contents. - try self.cells.resize(self.alloc, grid_size); + // Reset our cell contents if our grid size has changed. + if (!self.cells.size.equals(grid_size)) { + try self.cells.resize(self.alloc, grid_size); - // Reset our viewport to force a rebuild - self.cells_viewport = null; + // Reset our viewport to force a rebuild + self.cells_viewport = null; + } // If we have custom shaders then we update the state if (self.custom_shader_state) |*state| { diff --git a/src/renderer/Thread.zig b/src/renderer/Thread.zig index 459fc105a..2725a2175 100644 --- a/src/renderer/Thread.zig +++ b/src/renderer/Thread.zig @@ -480,7 +480,7 @@ fn drawCallback( r: xev.Timer.RunError!void, ) xev.CallbackAction { _ = r catch unreachable; - const t = self_ orelse { + const t: *Thread = self_ orelse { // This shouldn't happen so we log it. log.warn("render callback fired without data set", .{}); return .disarm; @@ -504,7 +504,7 @@ fn renderCallback( r: xev.Timer.RunError!void, ) xev.CallbackAction { _ = r catch unreachable; - const t = self_ orelse { + const t: *Thread = self_ orelse { // This shouldn't happen so we log it. log.warn("render callback fired without data set", .{}); return .disarm; @@ -543,7 +543,7 @@ fn cursorTimerCallback( }, }; - const t = self_ orelse { + const t: *Thread = self_ orelse { // This shouldn't happen so we log it. log.warn("render callback fired without data set", .{}); return .disarm; diff --git a/src/termio/Termio.zig b/src/termio/Termio.zig index 246b9b2f8..7573b000b 100644 --- a/src/termio/Termio.zig +++ b/src/termio/Termio.zig @@ -378,7 +378,13 @@ pub fn resize( // immediately for a resize. This is allowed by the spec. self.terminal.modes.set(.synchronized_output, false); - // Wake up our renderer so any changes will be shown asap + // Mail the renderer so that it can update the GPU and re-render + _ = self.renderer_mailbox.push(.{ + .resize = .{ + .screen_size = screen_size, + .padding = padding, + }, + }, .{ .forever = {} }); self.renderer_wakeup.notify() catch {}; // If we have size reporting enabled we need to send a report. From 900aab10f2c744f943d65942654c7e90fae28a34 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Wed, 14 Aug 2024 22:43:58 -0400 Subject: [PATCH 2/3] renderer: don't update frame if renderer grid size != terminal size --- src/renderer/Metal.zig | 14 ++++++++------ src/renderer/OpenGL.zig | 13 +++++++++++++ src/renderer/Thread.zig | 2 ++ 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/src/renderer/Metal.zig b/src/renderer/Metal.zig index 4f669d474..80ba787b2 100644 --- a/src/renderer/Metal.zig +++ b/src/renderer/Metal.zig @@ -908,12 +908,14 @@ pub fn updateFrame( } // If our terminal screen size doesn't match our expected renderer - // size then we skip a frame. This can happen if we get resized - // before the terminal gets resized. The terminal resize event also - // wakes up the renderer so we'll get another chance to update frame - // data. - if (self.cells.size.rows < state.terminal.rows or - self.cells.size.columns < state.terminal.cols) + // size then we skip a frame. This can happen if the terminal state + // is resized between when the renderer mailbox is drained and when + // the state mutex is acquired inside this function. + // + // For some reason this doesn't seem to cause any significant issues + // with flickering while resizing. '\_('-')_/' + if (self.cells.size.rows != state.terminal.rows or + self.cells.size.columns != state.terminal.cols) { return; } diff --git a/src/renderer/OpenGL.zig b/src/renderer/OpenGL.zig index a8d0198e2..498a1e967 100644 --- a/src/renderer/OpenGL.zig +++ b/src/renderer/OpenGL.zig @@ -739,6 +739,19 @@ pub fn updateFrame( self.foreground_color = bg; } + // If our terminal screen size doesn't match our expected renderer + // size then we skip a frame. This can happen if the terminal state + // is resized between when the renderer mailbox is drained and when + // the state mutex is acquired inside this function. + // + // For some reason this doesn't seem to cause any significant issues + // with flickering while resizing. '\_('-')_/' + if (self.grid_size.rows != state.terminal.rows or + self.grid_size.columns != state.terminal.cols) + { + return; + } + // Get the viewport pin so that we can compare it to the current. const viewport_pin = state.terminal.screen.pages.pin(.{ .viewport = .{} }).?; diff --git a/src/renderer/Thread.zig b/src/renderer/Thread.zig index 2725a2175..2521d18a4 100644 --- a/src/renderer/Thread.zig +++ b/src/renderer/Thread.zig @@ -522,6 +522,8 @@ fn renderCallback( t.flags.cursor_blink_visible, ) catch |err| log.warn("error rendering err={}", .{err}); + + // Draw t.drawFrame(false); return .disarm; From ff6a0bf9a21715e71434f72f7dca07aed4d435bb Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Wed, 14 Aug 2024 23:46:52 -0400 Subject: [PATCH 3/3] termio: wake renderer outside of critical area in resize --- src/termio/Termio.zig | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/termio/Termio.zig b/src/termio/Termio.zig index 7573b000b..ae38eb043 100644 --- a/src/termio/Termio.zig +++ b/src/termio/Termio.zig @@ -378,20 +378,20 @@ pub fn resize( // immediately for a resize. This is allowed by the spec. self.terminal.modes.set(.synchronized_output, false); - // Mail the renderer so that it can update the GPU and re-render - _ = self.renderer_mailbox.push(.{ - .resize = .{ - .screen_size = screen_size, - .padding = padding, - }, - }, .{ .forever = {} }); - self.renderer_wakeup.notify() catch {}; - // If we have size reporting enabled we need to send a report. if (self.terminal.modes.get(.in_band_size_reports)) { try self.sizeReportLocked(td, .mode_2048); } } + + // Mail the renderer so that it can update the GPU and re-render + _ = self.renderer_mailbox.push(.{ + .resize = .{ + .screen_size = screen_size, + .padding = padding, + }, + }, .{ .forever = {} }); + self.renderer_wakeup.notify() catch {}; } /// Make a size report.