From 678bd0de0c4938e2150e1627fb75038b915f9db5 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 13 Sep 2023 08:34:09 -0700 Subject: [PATCH] core: surface should not use app mailbox The surface runs on the same thread as the app so if we use the app mailbox then we risk filling the queue before it can drain. The surface should use the app directly. This commit just changes all the calls to use the app directly. We may also want to coalesce certain changes to avoid too much CPU but I defer that to a future change. --- src/App.zig | 22 ++++++--------------- src/Surface.zig | 43 +++++++++++++----------------------------- src/apprt/embedded.zig | 2 +- src/apprt/glfw.zig | 2 +- src/apprt/gtk.zig | 2 +- 5 files changed, 22 insertions(+), 49 deletions(-) diff --git a/src/App.zig b/src/App.zig index 4b88c30e7..bdf5cb77b 100644 --- a/src/App.zig +++ b/src/App.zig @@ -199,8 +199,7 @@ fn drainMailbox(self: *App, rt_app: *apprt.App) !void { switch (message) { .reload_config => try self.reloadConfig(rt_app), .new_window => |msg| try self.newWindow(rt_app, msg), - .close => |surface| try self.closeSurface(rt_app, surface), - .focus => |surface| try self.focusSurface(rt_app, surface), + .close => |surface| try self.closeSurface(surface), .quit => try self.setQuit(), .surface_message => |msg| try self.surfaceMessage(msg.surface, msg.message), .redraw_surface => |surface| try self.redrawSurface(rt_app, surface), @@ -208,7 +207,7 @@ fn drainMailbox(self: *App, rt_app: *apprt.App) !void { } } -fn reloadConfig(self: *App, rt_app: *apprt.App) !void { +pub fn reloadConfig(self: *App, rt_app: *apprt.App) !void { log.debug("reloading configuration", .{}); if (try rt_app.reloadConfig()) |new| { log.debug("new configuration received, applying", .{}); @@ -216,16 +215,12 @@ fn reloadConfig(self: *App, rt_app: *apprt.App) !void { } } -fn closeSurface(self: *App, rt_app: *apprt.App, surface: *Surface) !void { - _ = rt_app; - +pub fn closeSurface(self: *App, surface: *Surface) !void { if (!self.hasSurface(surface)) return; surface.close(); } -fn focusSurface(self: *App, rt_app: *apprt.App, surface: *Surface) !void { - _ = rt_app; - +pub fn focusSurface(self: *App, surface: *Surface) void { if (!self.hasSurface(surface)) return; self.focused_surface = surface; } @@ -236,7 +231,7 @@ fn redrawSurface(self: *App, rt_app: *apprt.App, surface: *apprt.Surface) !void } /// Create a new window -fn newWindow(self: *App, rt_app: *apprt.App, msg: Message.NewWindow) !void { +pub fn newWindow(self: *App, rt_app: *apprt.App, msg: Message.NewWindow) !void { if (!@hasDecl(apprt.App, "newWindow")) { log.warn("newWindow is not supported by this runtime", .{}); return; @@ -253,7 +248,7 @@ fn newWindow(self: *App, rt_app: *apprt.App, msg: Message.NewWindow) !void { } /// Start quitting -fn setQuit(self: *App) !void { +pub fn setQuit(self: *App) !void { if (self.quit) return; self.quit = true; } @@ -292,11 +287,6 @@ pub const Message = union(enum) { /// should close. close: *Surface, - /// The last focused surface. The app keeps track of this to - /// enable "inheriting" various configurations from the last - /// surface. - focus: *Surface, - /// Quit quit: void, diff --git a/src/Surface.zig b/src/Surface.zig index 922175e2d..6392e855f 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -42,10 +42,11 @@ const Renderer = renderer.Renderer; /// Allocator alloc: Allocator, -/// The mailbox for sending messages to the main app thread. -app_mailbox: App.Mailbox, +/// The app that this surface is attached to. +app: *App, -/// The windowing system surface +/// The windowing system surface and app. +rt_app: *apprt.runtime.App, rt_surface: *apprt.runtime.Surface, /// The font structures @@ -180,7 +181,7 @@ pub fn init( alloc: Allocator, config: *const configpkg.Config, app: *App, - app_mailbox: App.Mailbox, + rt_app: *apprt.runtime.App, rt_surface: *apprt.runtime.Surface, ) !void { // Initialize our renderer with our initialized surface. @@ -351,6 +352,7 @@ pub fn init( }; // Create our terminal grid with the initial size + const app_mailbox: App.Mailbox = .{ .rt_app = rt_app, .mailbox = &app.mailbox }; var renderer_impl = try Renderer.init(alloc, .{ .config = try Renderer.DerivedConfig.init(alloc, config), .font_group = font_group, @@ -409,7 +411,8 @@ pub fn init( self.* = .{ .alloc = alloc, - .app_mailbox = app_mailbox, + .app = app, + .rt_app = rt_app, .rt_surface = rt_surface, .font_lib = font_lib, .font_group = font_group, @@ -1019,11 +1022,7 @@ pub fn focusCallback(self: *Surface, focused: bool) !void { }, .{ .forever = {} }); // Notify our app if we gained focus. - if (focused) { - _ = self.app_mailbox.push(.{ - .focus = self, - }, .{ .forever = {} }); - } + if (focused) self.app.focusSurface(self); // Schedule render which also drains our mailbox try self.queueRender(); @@ -1871,11 +1870,7 @@ pub fn performBindingAction(self: *Surface, action: input.Binding.Action) !void .unbind => unreachable, .ignore => {}, - .reload_config => { - _ = self.app_mailbox.push(.{ - .reload_config = {}, - }, .{ .instant = {} }); - }, + .reload_config => try self.app.reloadConfig(self.rt_app), .csi => |data| { // We need to send the CSI sequence as a single write request. @@ -2072,13 +2067,7 @@ pub fn performBindingAction(self: *Surface, action: input.Binding.Action) !void try self.io_thread.wakeup.notify(); }, - .new_window => { - _ = self.app_mailbox.push(.{ - .new_window = .{ - .parent = self, - }, - }, .{ .instant = {} }); - }, + .new_window => try self.app.newWindow(self.rt_app, .{ .parent = self }), .new_tab => { if (@hasDecl(apprt.Surface, "newTab")) { @@ -2130,15 +2119,9 @@ pub fn performBindingAction(self: *Surface, action: input.Binding.Action) !void .close_surface => self.close(), - .close_window => { - _ = self.app_mailbox.push(.{ .close = self }, .{ .instant = {} }); - }, + .close_window => try self.app.closeSurface(self), - .quit => { - _ = self.app_mailbox.push(.{ - .quit = {}, - }, .{ .instant = {} }); - }, + .quit => try self.app.setQuit(), } } diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig index e3b8df6e0..f35765275 100644 --- a/src/apprt/embedded.zig +++ b/src/apprt/embedded.zig @@ -231,7 +231,7 @@ pub const Surface = struct { app.core_app.alloc, &config, app.core_app, - .{ .rt_app = app, .mailbox = &app.core_app.mailbox }, + app, self, ); errdefer self.core_surface.deinit(); diff --git a/src/apprt/glfw.zig b/src/apprt/glfw.zig index c3c07af07..47b37ec9a 100644 --- a/src/apprt/glfw.zig +++ b/src/apprt/glfw.zig @@ -378,7 +378,7 @@ pub const Surface = struct { app.app.alloc, &config, app.app, - .{ .rt_app = app, .mailbox = &app.app.mailbox }, + app, self, ); errdefer self.core_surface.deinit(); diff --git a/src/apprt/gtk.zig b/src/apprt/gtk.zig index 54e805dd1..580970711 100644 --- a/src/apprt/gtk.zig +++ b/src/apprt/gtk.zig @@ -852,7 +852,7 @@ pub const Surface = struct { self.app.core_app.alloc, &config, self.app.core_app, - .{ .rt_app = self.app, .mailbox = &self.app.core_app.mailbox }, + self.app, self, ); errdefer self.core_surface.deinit();