From 2fb84343a4b7055032ba2b3e1ad57b61e49a0aba Mon Sep 17 00:00:00 2001 From: Jon Parise Date: Thu, 25 Jul 2024 09:50:15 -0400 Subject: [PATCH] config: switch to a Surface.Options-based approach This introduces an ArenaAllocator that models the lifetime of the Surface.Options structure from initial surface creation command to the actual surface creation. This implementation is a little rough and relies on an `anyopaque` pointer (because `extern struct`), but it demonstrates a more apprt-centric approach. Still only implemented for embedded apprt and macOS. The other apprts will need their own plumbing to support this approach. We can also chose to clean up newConfig() and call Config.shallowClone() directly after this change. --- include/ghostty.h | 8 +--- macos/Sources/Ghostty/SurfaceView.swift | 4 -- src/Surface.zig | 2 +- src/apprt/embedded.zig | 57 +++++++++++++++++++++---- src/apprt/glfw.zig | 3 +- src/apprt/gtk/Surface.zig | 3 +- src/apprt/surface.zig | 25 +---------- 7 files changed, 54 insertions(+), 48 deletions(-) diff --git a/include/ghostty.h b/include/ghostty.h index f33428fb6..9d69f5d05 100644 --- a/include/ghostty.h +++ b/include/ghostty.h @@ -31,12 +31,6 @@ typedef void* ghostty_surface_t; typedef void* ghostty_inspector_t; // Enums are up top so we can reference them later. -typedef enum { - GHOSTTY_SURFACE_KIND_SPLIT, - GHOSTTY_SURFACE_KIND_TAB, - GHOSTTY_SURFACE_KIND_WINDOW, -} ghostty_surface_kind_e; - typedef enum { GHOSTTY_PLATFORM_INVALID, GHOSTTY_PLATFORM_MACOS, @@ -401,7 +395,6 @@ typedef union { } ghostty_platform_u; typedef struct { - ghostty_surface_kind_e kind; ghostty_platform_e platform_tag; ghostty_platform_u platform; void* userdata; @@ -409,6 +402,7 @@ typedef struct { float font_size; const char* working_directory; const char* command; + void* _arena; } ghostty_surface_config_s; typedef void (*ghostty_runtime_wakeup_cb)(void*); diff --git a/macos/Sources/Ghostty/SurfaceView.swift b/macos/Sources/Ghostty/SurfaceView.swift index 8172b01b6..92d899300 100644 --- a/macos/Sources/Ghostty/SurfaceView.swift +++ b/macos/Sources/Ghostty/SurfaceView.swift @@ -291,8 +291,6 @@ extension Ghostty { /// The configuration for a surface. For any configuration not set, defaults will be chosen from /// libghostty, usually from the Ghostty configuration. struct SurfaceConfiguration { - var kind: ghostty_surface_kind_e? = nil; - /// Explicit font size to use in points var fontSize: Float32? = nil @@ -305,7 +303,6 @@ extension Ghostty { init() {} init(from config: ghostty_surface_config_s) { - self.kind = config.kind; self.fontSize = config.font_size self.workingDirectory = String.init(cString: config.working_directory, encoding: .utf8) self.command = String.init(cString: config.command, encoding: .utf8) @@ -337,7 +334,6 @@ extension Ghostty { #error("unsupported target") #endif - if let kind = kind { config.kind = kind } if let fontSize = fontSize { config.font_size = fontSize } if let workingDirectory = workingDirectory { config.working_directory = (workingDirectory as NSString).utf8String diff --git a/src/Surface.zig b/src/Surface.zig index d8eb0865a..b1093de5f 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -126,7 +126,7 @@ config: DerivedConfig, child_exited: bool = false, /// The kind of surface. -pub const Kind = enum(c_int) { +pub const Kind = enum { split, tab, window, diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig index 406fa0a11..148456fba 100644 --- a/src/apprt/embedded.zig +++ b/src/apprt/embedded.zig @@ -8,6 +8,7 @@ const std = @import("std"); const builtin = @import("builtin"); const assert = std.debug.assert; const Allocator = std.mem.Allocator; +const ArenaAllocator = std.heap.ArenaAllocator; const objc = @import("objc"); const apprt = @import("../apprt.zig"); const font = @import("../font/main.zig"); @@ -308,9 +309,6 @@ pub const Surface = struct { /// Surface initialization options. pub const Options = extern struct { - /// The initial kind of surface. - kind: CoreSurface.Kind = .window, - /// The platform that this surface is being initialized for and /// the associated platform-specific configuration. platform_tag: c_int = 0, @@ -332,6 +330,17 @@ pub const Surface = struct { /// the "wait-after-command" option is also automatically set to true, /// since this is used for scripting. command: [*:0]const u8 = "", + + _arena: ?*anyopaque = null, + + pub fn deinit(self: *Options) void { + if (self._arena) |ptr| { + const arena: *ArenaAllocator = @ptrCast(@alignCast(ptr)); + const alloc = arena.child_allocator; + arena.deinit(); + alloc.destroy(arena); + } + } }; /// This is the key event sent for ghostty_surface_key. @@ -368,7 +377,7 @@ pub const Surface = struct { errdefer app.core_app.deleteSurface(self); // Shallow copy the config so that we can modify it. - var config = try apprt.surface.newConfig(app.core_app, app.config, opts.kind); + var config = try apprt.surface.newConfig(app.core_app, app.config); defer config.deinit(); // If we have a working directory from the options then we set it. @@ -476,7 +485,7 @@ pub const Surface = struct { return; }; - const options = self.newSurfaceOptions(.split); + const options = try self.newSurfaceOptions(.split); func(self.userdata, direction, options); } @@ -1027,7 +1036,7 @@ pub const Surface = struct { return; }; - const options = self.newSurfaceOptions(.tab); + const options = try self.newSurfaceOptions(.tab); func(self.userdata, options); } @@ -1037,7 +1046,7 @@ pub const Surface = struct { return; }; - const options = self.newSurfaceOptions(.window); + const options = try self.newSurfaceOptions(.window); func(self.userdata, options); } @@ -1068,15 +1077,44 @@ pub const Surface = struct { func(self.userdata, width, height); } - fn newSurfaceOptions(self: *const Surface, kind: CoreSurface.Kind) apprt.Surface.Options { + fn newSurfaceOptions(self: *const Surface, kind: CoreSurface.Kind) !apprt.Surface.Options { const font_size: f32 = font_size: { if (!self.app.config.@"window-inherit-font-size") break :font_size 0; break :font_size self.core_surface.font_size.points; }; + const working_directory: [*:0]const u8, const arena: ?*ArenaAllocator = dir: { + const prev = self.app.core_app.focusedSurface(); + if (prev) |p| { + const inherit_pwd: bool = switch (kind) { + .split => self.app.config.@"window-inherit-working-directory".split, + .tab => self.app.config.@"window-inherit-working-directory".tab, + .window => self.app.config.@"window-inherit-working-directory".window, + }; + if (inherit_pwd) { + const app_alloc = self.app.core_app.alloc; + + var arena = try app_alloc.create(ArenaAllocator); + errdefer app_alloc.destroy(arena); + + arena.* = ArenaAllocator.init(app_alloc); + errdefer arena.deinit(); + + const arena_alloc = arena.allocator(); + + if (try p.pwd(app_alloc)) |pwd| { + defer app_alloc.free(pwd); + break :dir .{ try arena_alloc.dupeZ(u8, pwd), arena }; + } + } + } + break :dir .{ "", null }; + }; + return .{ - .kind = kind, .font_size = font_size, + .working_directory = working_directory, + ._arena = arena, }; } @@ -1503,6 +1541,7 @@ pub const CAPI = struct { app: *App, opts: *const apprt.Surface.Options, ) !*Surface { + defer @constCast(opts).deinit(); return try app.newSurface(opts.*); } diff --git a/src/apprt/glfw.zig b/src/apprt/glfw.zig index a50ce4f2a..81063bc69 100644 --- a/src/apprt/glfw.zig +++ b/src/apprt/glfw.zig @@ -448,8 +448,7 @@ pub const Surface = struct { errdefer app.app.deleteSurface(self); // Get our new surface config - const kind: CoreSurface.Kind = .window; // TODO - var config = try apprt.surface.newConfig(app.app, &app.config, kind); + var config = try apprt.surface.newConfig(app.app, &app.config); defer config.deinit(); // Initialize our surface now that we have the stable pointer. diff --git a/src/apprt/gtk/Surface.zig b/src/apprt/gtk/Surface.zig index 29b6c9ab5..ee6c6869e 100644 --- a/src/apprt/gtk/Surface.zig +++ b/src/apprt/gtk/Surface.zig @@ -543,8 +543,7 @@ fn realize(self: *Surface) !void { errdefer self.app.core_app.deleteSurface(self); // Get our new surface config - const kind: CoreSurface.Kind = .window; // TODO - var config = try apprt.surface.newConfig(self.app.core_app, &self.app.config, kind); + var config = try apprt.surface.newConfig(self.app.core_app, &self.app.config); defer config.deinit(); if (!self.parent_surface) { // A hack, see the "parent_surface" field for more information. diff --git a/src/apprt/surface.zig b/src/apprt/surface.zig index 92ed85b68..384cce0a2 100644 --- a/src/apprt/surface.zig +++ b/src/apprt/surface.zig @@ -1,4 +1,3 @@ -const std = @import("std"); const apprt = @import("../apprt.zig"); const App = @import("../App.zig"); const Surface = @import("../Surface.zig"); @@ -86,27 +85,7 @@ pub const Mailbox = struct { /// Returns a new config for a surface for the given app that should be /// used for any new surfaces. The resulting config should be deinitialized /// after the surface is initialized. -pub fn newConfig(app: *const App, config: *const Config, kind: Surface.Kind) !Config { +pub fn newConfig(app: *const App, config: *const Config) !Config { // Create a shallow clone - var copy = config.shallowClone(app.alloc); - - // Our allocator is our config's arena - const alloc = copy._arena.?.allocator(); - - // Get our previously focused surface for some inherited values. - const prev = app.focusedSurface(); - if (prev) |p| { - const inherit_pwd: bool = switch (kind) { - .split => config.@"window-inherit-working-directory".split, - .tab => config.@"window-inherit-working-directory".tab, - .window => config.@"window-inherit-working-directory".window, - }; - if (inherit_pwd) { - if (try p.pwd(alloc)) |pwd| { - copy.@"working-directory" = pwd; - } - } - } - - return copy; + return config.shallowClone(app.alloc); }