apprt/gtk: don't use Stacked for surface error status page

Fixes #8533

Replace the usage of `Stacked` for error pages with programmatically
swapping the child of the `adw.Bin`.

I regret to say I don't know the root cause of this. I only know that
the usage of `Stacked` plus `Gtk.Paned` and the way we programmatically 
change the paned position and stack child during initialization causes
major issues.

This change isn't without its warts, too, and you can see them heavily
commented in the diff. 

(1) We have to workaround a GTK template double-free bug that is well known 
to us: if you bind a template child that is also the direct child of the 
template class, GTK does a double free on dispose. We workaround this by
removing our child in dispose. Valgrind verifies the fix.

(2) We have to workaround an issue where setting an `Adw.Bin` child
during a glarea realize causes some kind of critical GTK error that
results in a hard crash. We delay changing our bin child to an idle
tick.
This commit is contained in:
Mitchell Hashimoto
2025-09-05 11:13:04 -07:00
parent 0492cd16fa
commit ef822612d3
2 changed files with 197 additions and 177 deletions

View File

@@ -496,6 +496,9 @@ pub const Surface = extern struct {
/// if this is true, then it means the terminal is non-functional.
@"error": bool = false,
/// The source that handles setting our child property.
idle_rechild: ?c_uint = null,
/// A weak reference to an inspector window.
inspector: ?*InspectorWindow = null,
@@ -504,6 +507,8 @@ pub const Surface = extern struct {
context_menu: *gtk.PopoverMenu,
drop_target: *gtk.DropTarget,
progress_bar_overlay: *gtk.ProgressBar,
error_page: *adw.StatusPage,
terminal_page: *gtk.Overlay,
pub var offset: c_int = 0;
};
@@ -595,17 +600,6 @@ pub const Surface = extern struct {
return @intFromBool(config.@"bell-features".border);
}
fn closureStackChildName(
_: *Self,
error_: c_int,
) callconv(.c) ?[*:0]const u8 {
const err = error_ != 0;
return if (err)
glib.ext.dupeZ(u8, "error")
else
glib.ext.dupeZ(u8, "terminal");
}
pub fn toggleFullscreen(self: *Self) void {
signals.@"toggle-fullscreen".impl.emit(
self,
@@ -1370,6 +1364,19 @@ pub const Surface = extern struct {
priv.progress_bar_timer = null;
}
if (priv.idle_rechild) |v| {
if (glib.Source.remove(v) == 0) {
log.warn("unable to remove idle source", .{});
}
priv.idle_rechild = null;
}
// This works around a GTK double-free bug where if you bind
// to a top-level template child, it frees twice if the widget is
// also the root child of the template. By unsetting the child here,
// we avoid the double-free.
self.as(adw.Bin).setChild(null);
gtk.Widget.disposeTemplate(
self.as(gtk.Widget),
getGObjectType(),
@@ -1651,8 +1658,26 @@ pub const Surface = extern struct {
self.as(gtk.Widget).removeCssClass("background");
}
// Note above: in both cases setting our error view is handled by
// a Gtk.Stack visible-child-name binding.
// We need to set our child property on an idle tick, because the
// error property can be triggered by signals that are in the middle
// of widget mapping and changing our child during that time
// results in a hard gtk crash.
if (priv.idle_rechild == null) priv.idle_rechild = glib.idleAdd(
onIdleRechild,
self,
);
}
fn onIdleRechild(ud: ?*anyopaque) callconv(.c) c_int {
const self: *Self = @ptrCast(@alignCast(ud orelse return 0));
const priv = self.private();
priv.idle_rechild = null;
if (priv.@"error") {
self.as(adw.Bin).setChild(priv.error_page.as(gtk.Widget));
} else {
self.as(adw.Bin).setChild(priv.terminal_page.as(gtk.Widget));
}
return 0;
}
fn propMouseHoverUrl(
@@ -2699,8 +2724,10 @@ pub const Surface = extern struct {
class.bindTemplateChildPrivate("url_right", .{});
class.bindTemplateChildPrivate("child_exited_overlay", .{});
class.bindTemplateChildPrivate("context_menu", .{});
class.bindTemplateChildPrivate("error_page", .{});
class.bindTemplateChildPrivate("progress_bar_overlay", .{});
class.bindTemplateChildPrivate("resize_overlay", .{});
class.bindTemplateChildPrivate("terminal_page", .{});
class.bindTemplateChildPrivate("drop_target", .{});
class.bindTemplateChildPrivate("im_context", .{});
@@ -2736,7 +2763,6 @@ pub const Surface = extern struct {
class.bindTemplateCallback("notify_mouse_shape", &propMouseShape);
class.bindTemplateCallback("notify_bell_ringing", &propBellRinging);
class.bindTemplateCallback("should_border_be_shown", &closureShouldBorderBeShown);
class.bindTemplateCallback("stack_child_name", &closureStackChildName);
// Properties
gobject.ext.registerProperties(class, &.{

View File

@@ -1,6 +1,155 @@
using Gtk 4.0;
using Adw 1;
Adw.StatusPage error_page {
icon-name: "computer-fail-symbolic";
title: _("Oh, no.");
description: _("Unable to acquire an OpenGL context for rendering.");
child: LinkButton {
label: "https://ghostty.org/docs/help/gtk-opengl-context";
uri: "https://ghostty.org/docs/help/gtk-opengl-context";
};
}
Overlay terminal_page {
focusable: false;
focus-on-click: false;
child: Box {
hexpand: true;
vexpand: true;
GLArea gl_area {
realize => $gl_realize();
unrealize => $gl_unrealize();
render => $gl_render();
resize => $gl_resize();
hexpand: true;
vexpand: true;
focusable: true;
focus-on-click: true;
has-stencil-buffer: false;
has-depth-buffer: false;
allowed-apis: gl;
}
PopoverMenu context_menu {
closed => $context_menu_closed();
menu-model: context_menu_model;
flags: nested;
halign: start;
has-arrow: false;
}
};
[overlay]
ProgressBar progress_bar_overlay {
styles [
"osd",
]
visible: false;
halign: fill;
valign: start;
}
[overlay]
// The "border" bell feature is implemented here as an overlay rather than
// just adding a border to the GLArea or other widget for two reasons.
// First, adding a border to an existing widget causes a resize of the
// widget which undesirable side effects. Second, we can make it reactive
// here in the blueprint with relatively little code.
Revealer {
reveal-child: bind $should_border_be_shown(template.config, template.bell-ringing) as <bool>;
transition-type: crossfade;
transition-duration: 500;
Box bell_overlay {
styles [
"bell-overlay",
]
halign: fill;
valign: fill;
}
}
[overlay]
$GhosttySurfaceChildExited child_exited_overlay {
visible: bind template.child-exited;
close-request => $child_exited_close();
}
[overlay]
$GhosttyResizeOverlay resize_overlay {}
[overlay]
Label url_left {
styles [
"background",
"url-overlay",
]
visible: false;
halign: start;
valign: end;
label: bind template.mouse-hover-url;
EventControllerMotion url_ec_motion {
enter => $url_mouse_enter();
leave => $url_mouse_leave();
}
}
[overlay]
Label url_right {
styles [
"background",
"url-overlay",
]
visible: false;
halign: end;
valign: end;
label: bind template.mouse-hover-url;
}
// Event controllers for interactivity
EventControllerFocus {
enter => $focus_enter();
leave => $focus_leave();
}
EventControllerKey {
key-pressed => $key_pressed();
key-released => $key_released();
}
EventControllerMotion {
motion => $mouse_motion();
leave => $mouse_leave();
}
EventControllerScroll {
scroll => $scroll();
scroll-begin => $scroll_begin();
scroll-end => $scroll_end();
flags: both_axes;
}
GestureClick {
pressed => $mouse_down();
released => $mouse_up();
button: 0;
}
DropTarget drop_target {
drop => $drop();
actions: copy;
}
}
template $GhosttySurface: Adw.Bin {
styles [
"surface",
@@ -12,169 +161,14 @@ template $GhosttySurface: Adw.Bin {
notify::mouse-hover-url => $notify_mouse_hover_url();
notify::mouse-hidden => $notify_mouse_hidden();
notify::mouse-shape => $notify_mouse_shape();
Stack {
StackPage {
name: "terminal";
child: Overlay {
focusable: false;
focus-on-click: false;
child: Box {
hexpand: true;
vexpand: true;
GLArea gl_area {
realize => $gl_realize();
unrealize => $gl_unrealize();
render => $gl_render();
resize => $gl_resize();
hexpand: true;
vexpand: true;
focusable: true;
focus-on-click: true;
has-stencil-buffer: false;
has-depth-buffer: false;
allowed-apis: gl;
}
PopoverMenu context_menu {
closed => $context_menu_closed();
menu-model: context_menu_model;
flags: nested;
halign: start;
has-arrow: false;
}
};
[overlay]
ProgressBar progress_bar_overlay {
styles [
"osd",
]
visible: false;
halign: fill;
valign: start;
}
[overlay]
// The "border" bell feature is implemented here as an overlay rather than
// just adding a border to the GLArea or other widget for two reasons.
// First, adding a border to an existing widget causes a resize of the
// widget which undesirable side effects. Second, we can make it reactive
// here in the blueprint with relatively little code.
Revealer {
reveal-child: bind $should_border_be_shown(template.config, template.bell-ringing) as <bool>;
transition-type: crossfade;
transition-duration: 500;
Box bell_overlay {
styles [
"bell-overlay",
]
halign: fill;
valign: fill;
}
}
[overlay]
$GhosttySurfaceChildExited child_exited_overlay {
visible: bind template.child-exited;
close-request => $child_exited_close();
}
[overlay]
$GhosttyResizeOverlay resize_overlay {}
[overlay]
Label url_left {
styles [
"background",
"url-overlay",
]
visible: false;
halign: start;
valign: end;
label: bind template.mouse-hover-url;
EventControllerMotion url_ec_motion {
enter => $url_mouse_enter();
leave => $url_mouse_leave();
}
}
[overlay]
Label url_right {
styles [
"background",
"url-overlay",
]
visible: false;
halign: end;
valign: end;
label: bind template.mouse-hover-url;
}
// Event controllers for interactivity
EventControllerFocus {
enter => $focus_enter();
leave => $focus_leave();
}
EventControllerKey {
key-pressed => $key_pressed();
key-released => $key_released();
}
EventControllerMotion {
motion => $mouse_motion();
leave => $mouse_leave();
}
EventControllerScroll {
scroll => $scroll();
scroll-begin => $scroll_begin();
scroll-end => $scroll_end();
flags: both_axes;
}
GestureClick {
pressed => $mouse_down();
released => $mouse_up();
button: 0;
}
DropTarget drop_target {
drop => $drop();
actions: copy;
}
};
}
StackPage {
name: "error";
child: Adw.StatusPage {
icon-name: "computer-fail-symbolic";
title: _("Oh, no.");
description: _("Unable to acquire an OpenGL context for rendering.");
child: LinkButton {
label: "https://ghostty.org/docs/help/gtk-opengl-context";
uri: "https://ghostty.org/docs/help/gtk-opengl-context";
};
};
}
// The order matters here: we can only set this after the stack
// pages above have been created.
visible-child-name: bind $stack_child_name(template.error) as <string>;
}
// Some history: we used to use a Stack here and swap between the
// terminal and error pages as needed. But a Stack doesn't play nice
// with our SplitTree and Gtk.Paned usage[^1]. Replacing this with
// a manual programmatic child swap fixed this. So if you ever change
// this, be sure to test many splits!
//
// [^1]: https://github.com/ghostty-org/ghostty/issues/8533
child: terminal_page;
}
IMMulticontext im_context {