InputPopup: fix minor issues, simplify code

deps/zig-wlroots

@@ -1 +1 @@
-Subproject commit a579f9f7dae72c960e8804737109815c78e471f4
+Subproject commit 5bc01a9f597e051c0dfab6dd9991f08697794e3d

river/InputPopup.zig

@@ -17,40 +17,27 @@
 
 const InputPopup = @This();
 
-const build_options = @import("build_options");
 const std = @import("std");
 const assert = std.debug.assert;
-const mem = std.mem;
 const wlr = @import("wlroots");
 const wl = @import("wayland").server.wl;
-const server = &@import("main.zig").server;
 
+const server = &@import("main.zig").server;
 const util = @import("util.zig");
+
 const InputRelay = @import("InputRelay.zig");
-const TextInput = @import("TextInput.zig");
+const SceneNodeData = @import("SceneNodeData.zig");
-const Root = @import("Root.zig");
-const View = @import("View.zig");
-const LayerSurface = @import("LayerSurface.zig");
-const XdgToplevel = @import("XdgToplevel.zig");
-const XwaylandView = @import("XwaylandView.zig");
 
 link: wl.list.Link,
-scene_tree: ?*wlr.SceneTree = null,
-parent_scene_tree: ?*wlr.SceneTree = null,
-scene_surface: ?*wlr.SceneTree = null,
-view: ?*View = null,
-
 input_relay: *InputRelay,
-wlr_popup: *wlr.InputPopupSurfaceV2,
 
-destroy: wl.Listener(void) =
+wlr_popup: *wlr.InputPopupSurfaceV2,
-    wl.Listener(void).init(handleDestroy),
+surface_tree: *wlr.SceneTree,
-map: wl.Listener(void) =
+
-    wl.Listener(void).init(handleMap),
+destroy: wl.Listener(void) = wl.Listener(void).init(handleDestroy),
-unmap: wl.Listener(void) =
+map: wl.Listener(void) = wl.Listener(void).init(handleMap),
-    wl.Listener(void).init(handleUnmap),
+unmap: wl.Listener(void) = wl.Listener(void).init(handleUnmap),
-commit: wl.Listener(*wlr.Surface) =
+commit: wl.Listener(*wlr.Surface) = wl.Listener(*wlr.Surface).init(handleCommit),
-    wl.Listener(*wlr.Surface).init(handleCommit),
 
 pub fn create(wlr_popup: *wlr.InputPopupSurfaceV2, input_relay: *InputRelay) !void {
     const input_popup = try util.gpa.create(InputPopup);
@@ -60,35 +47,32 @@ pub fn create(wlr_popup: *wlr.InputPopupSurfaceV2, input_relay: *InputRelay) !vo
         .link = undefined,
         .input_relay = input_relay,
         .wlr_popup = wlr_popup,
+        .surface_tree = try server.root.hidden.tree.createSceneSubsurfaceTree(wlr_popup.surface),
     };
 
+    input_relay.input_popups.append(input_popup);
+
     input_popup.wlr_popup.events.destroy.add(&input_popup.destroy);
     input_popup.wlr_popup.surface.events.map.add(&input_popup.map);
     input_popup.wlr_popup.surface.events.unmap.add(&input_popup.unmap);
     input_popup.wlr_popup.surface.events.commit.add(&input_popup.commit);
 
-    input_relay.input_popups.append(input_popup);
     input_popup.update();
 }
 
 fn handleDestroy(listener: *wl.Listener(void)) void {
     const input_popup = @fieldParentPtr(InputPopup, "destroy", listener);
 
+    input_popup.destroy.link.remove();
     input_popup.map.link.remove();
     input_popup.unmap.link.remove();
     input_popup.commit.link.remove();
-    input_popup.destroy.link.remove();
+
     input_popup.link.remove();
 
     util.gpa.destroy(input_popup);
 }
 
-fn handleCommit(listener: *wl.Listener(*wlr.Surface), _: *wlr.Surface) void {
-    const input_popup = @fieldParentPtr(InputPopup, "commit", listener);
-
-    input_popup.update();
-}
-
 fn handleMap(listener: *wl.Listener(void)) void {
     const input_popup = @fieldParentPtr(InputPopup, "map", listener);
 
@@ -98,132 +82,107 @@ fn handleMap(listener: *wl.Listener(void)) void {
 fn handleUnmap(listener: *wl.Listener(void)) void {
     const input_popup = @fieldParentPtr(InputPopup, "unmap", listener);
 
-    input_popup.scene_tree.?.node.destroy();
+    input_popup.surface_tree.node.reparent(server.root.hidden.tree);
-    input_popup.scene_tree = null;
+}
+
+fn handleCommit(listener: *wl.Listener(*wlr.Surface), _: *wlr.Surface) void {
+    const input_popup = @fieldParentPtr(InputPopup, "commit", listener);
+
+    input_popup.update();
 }
 
 pub fn update(input_popup: *InputPopup) void {
-    var text_input = input_popup.getTextInputFocused() orelse return;
+    const text_input = input_popup.input_relay.text_input orelse {
+        input_popup.surface_tree.node.reparent(server.root.hidden.tree);
+        return;
+    };
+
+    if (!input_popup.wlr_popup.surface.mapped) return;
+
+    // This seems like it could be null if the focused surface is destroyed
     const focused_surface = text_input.wlr_text_input.focused_surface orelse return;
 
-    if (!input_popup.wlr_popup.surface.mapped) {
+    // Focus should never be sent to subsurfaces
+    assert(focused_surface.getRootSurface() == focused_surface);
+
+    const focused = SceneNodeData.fromSurface(focused_surface) orelse return;
+
+    const output = switch (focused.data) {
+        .view => |view| view.current.output orelse return,
+        .layer_surface => |layer_surface| layer_surface.output,
+        .lock_surface => |lock_surface| lock_surface.getOutput(),
+        // Xwayland doesn't use the text-input protocol
+        .override_redirect => unreachable,
+    };
+
+    const popup_tree = switch (focused.data) {
+        .view => |view| view.popup_tree,
+        .layer_surface => |layer_surface| layer_surface.popup_tree,
+        .lock_surface => |lock_surface| lock_surface.getOutput().layers.popups,
+        // Xwayland doesn't use the text-input protocol
+        .override_redirect => unreachable,
+    };
+
+    input_popup.surface_tree.node.reparent(popup_tree);
+
+    if (!text_input.wlr_text_input.current.features.cursor_rectangle) {
+        // If the text-input client does not inform us where in the surface
+        // the active text input is there's not much we can do. Placing the
+        // popup at the top left corner of the window is nice and simple
+        // while not looking terrible.
+        input_popup.surface_tree.node.setPosition(0, 0);
         return;
     }
 
+    var focused_x: c_int = undefined;
+    var focused_y: c_int = undefined;
+    _ = focused.node.coords(&focused_x, &focused_y);
+
     var output_box: wlr.Box = undefined;
-    var parent: wlr.Box = undefined;
+    server.root.output_layout.getBox(output.wlr_output, &output_box);
 
-    input_popup.getParentAndOutputBox(focused_surface, &parent, &output_box);
+    // Relative to the surface with the active text input
+    var cursor_box = text_input.wlr_text_input.current.cursor_rectangle;
 
-    var cursor_rect = if (text_input.wlr_text_input.current.features.cursor_rectangle)
+    // Adjust to be relative to the output
-        text_input.wlr_text_input.current.cursor_rectangle
+    cursor_box.x += focused_x - output_box.x;
-    else
+    cursor_box.y += focused_y - output_box.y;
-        wlr.Box{
-            .x = 0,
-            .y = 0,
-            .width = parent.width,
-            .height = parent.height,
-        };
 
-    const popup_width = input_popup.wlr_popup.surface.current.width;
+    // Choose popup x/y relative to the output:
-    const popup_height = input_popup.wlr_popup.surface.current.height;
 
-    const cursor_rect_left = parent.x + cursor_rect.x;
+    // Align the left edge of the popup with the left edge of the cursor.
-    const popup_anchor_left = blk: {
+    // If the popup wouldn't fit on the output instead align the right edge
-        const cursor_rect_right = cursor_rect_left + cursor_rect.width;
+    // of the popup with the right edge of the cursor.
-        const available_right_of_cursor = output_box.x + output_box.width - cursor_rect_left;
+    const popup_x = blk: {
-        const available_left_of_cursor = cursor_rect_right - output_box.x;
+        const popup_width = input_popup.wlr_popup.surface.current.width;
-        if (available_right_of_cursor < popup_width and available_left_of_cursor > popup_width) {
+        if (output_box.width - cursor_box.x >= popup_width) {
-            break :blk cursor_rect_right - popup_width;
+            break :blk cursor_box.x;
         } else {
-            break :blk cursor_rect_left;
+            break :blk cursor_box.x + cursor_box.width - popup_width;
         }
     };
 
-    const cursor_rect_up = parent.y + cursor_rect.y;
+    // Align the top edge of the popup with the bottom edge of the cursor.
-    const popup_anchor_up = blk: {
+    // If the popup wouldn't fit on the output instead align the bottom edge
-        const cursor_rect_down = cursor_rect_up + cursor_rect.height;
+    // of the popup with the top edge of the cursor.
-        const available_down_of_cursor = output_box.y + output_box.height - cursor_rect_down;
+    const popup_y = blk: {
-        const available_up_of_cursor = cursor_rect_up - output_box.y;
+        const popup_height = input_popup.wlr_popup.surface.current.height;
-        if (available_down_of_cursor < popup_height and available_up_of_cursor > popup_height) {
+        if (output_box.height - (cursor_box.y + cursor_box.height) >= popup_height) {
-            break :blk cursor_rect_up - popup_height;
+            break :blk cursor_box.y + cursor_box.height;
         } else {
-            break :blk cursor_rect_down;
+            break :blk cursor_box.y - popup_height;
         }
     };
 
-    if (text_input.wlr_text_input.current.features.cursor_rectangle) {
+    // Scene node position is relative to the parent so adjust popup x/y to
-        var box = wlr.Box{
+    // be relative to the focused surface.
-            .x = cursor_rect_left - popup_anchor_left,
+    input_popup.surface_tree.node.setPosition(
-            .y = cursor_rect_up - popup_anchor_up,
+        popup_x - focused_x + output_box.x,
-            .width = cursor_rect.width,
+        popup_y - focused_y + output_box.y,
-            .height = cursor_rect.height,
+    );
-        };
-        input_popup.wlr_popup.sendTextInputRectangle(&box);
-    }
 
-    if (input_popup.scene_tree == null) {
+    // The text input rectangle sent to the input method is relative to the popup.
-        input_popup.scene_tree = input_popup.parent_scene_tree.?.createSceneTree() catch {
+    cursor_box.x -= popup_x;
-            std.log.err("out of memory", .{});
+    cursor_box.y -= popup_y;
-            return;
+    input_popup.wlr_popup.sendTextInputRectangle(&cursor_box);
-        };
-
-        input_popup.scene_surface = input_popup.scene_tree.?
-            .createSceneSubsurfaceTree(
-            input_popup.wlr_popup.surface,
-        ) catch {
-            std.log.err("out of memory", .{});
-            input_popup.wlr_popup.surface.resource.getClient().postNoMemory();
-            return;
-        };
-    }
-    input_popup.scene_tree.?.node.setPosition(popup_anchor_left - parent.x, popup_anchor_up - parent.y);
-}
-
-pub fn getTextInputFocused(input_popup: *InputPopup) ?*TextInput {
-    var it = input_popup.input_relay.text_inputs.iterator(.forward);
-    while (it.next()) |text_input| {
-        if (text_input.wlr_text_input.focused_surface != null) return text_input;
-    }
-    return null;
-}
-
-pub fn getParentAndOutputBox(
-    input_popup: *InputPopup,
-    focused_surface: *wlr.Surface,
-    parent: *wlr.Box,
-    output_box: *wlr.Box,
-) void {
-    if (wlr.LayerSurfaceV1.tryFromWlrSurface(focused_surface)) |wlr_layer_surface| {
-        const layer_surface: *LayerSurface = @ptrFromInt(wlr_layer_surface.data);
-        input_popup.parent_scene_tree = layer_surface.popup_tree;
-        const output = layer_surface.output.wlr_output;
-        server.root.output_layout.getBox(output, output_box);
-        _ = layer_surface.popup_tree.node.coords(&parent.x, &parent.y);
-    } else {
-        const view = getViewFromWlrSurface(focused_surface) orelse return;
-        input_popup.parent_scene_tree = view.tree;
-        _ = view.tree.node.coords(&parent.x, &parent.y);
-        const output = view.current.output orelse return;
-        server.root.output_layout.getBox(output.wlr_output, output_box);
-        parent.width = view.current.box.width;
-        parent.height = view.current.box.height;
-    }
-}
-
-fn getViewFromWlrSurface(wlr_surface: *wlr.Surface) ?*View {
-    if (wlr.XdgSurface.tryFromWlrSurface(wlr_surface)) |xdg_surface| {
-        const xdg_toplevel: *XdgToplevel = @ptrFromInt(xdg_surface.data);
-        return xdg_toplevel.view;
-    }
-    if (build_options.xwayland) {
-        if (wlr.XwaylandSurface.tryFromWlrSurface(wlr_surface)) |xwayland_surface| {
-            const xwayland_view: *XwaylandView = @ptrFromInt(xwayland_surface.data);
-            return xwayland_view.view;
-        }
-    }
-    if (wlr.Subsurface.tryFromWlrSurface(wlr_surface)) |wlr_subsurface| {
-        if (wlr_subsurface.parent) |parent| return getViewFromWlrSurface(parent);
-    }
-    return null;
 }

river/InputRelay.zig

@@ -52,8 +52,8 @@ grab_keyboard: wl.Listener(*wlr.InputMethodV2.KeyboardGrab) =
     wl.Listener(*wlr.InputMethodV2.KeyboardGrab).init(handleInputMethodGrabKeyboard),
 input_method_destroy: wl.Listener(*wlr.InputMethodV2) =
     wl.Listener(*wlr.InputMethodV2).init(handleInputMethodDestroy),
-input_method_new_popup_surface: wl.Listener(*wlr.InputPopupSurfaceV2) =
+input_method_new_popup: wl.Listener(*wlr.InputPopupSurfaceV2) =
-    wl.Listener(*wlr.InputPopupSurfaceV2).init(handleInputMethodNewPopupSurface),
+    wl.Listener(*wlr.InputPopupSurfaceV2).init(handleInputMethodNewPopup),
 
 grab_keyboard_destroy: wl.Listener(*wlr.InputMethodV2.KeyboardGrab) =
     wl.Listener(*wlr.InputMethodV2.KeyboardGrab).init(handleInputMethodGrabKeyboardDestroy),
@@ -82,7 +82,7 @@ pub fn newInputMethod(relay: *InputRelay, input_method: *wlr.InputMethodV2) void
     input_method.events.commit.add(&relay.input_method_commit);
     input_method.events.grab_keyboard.add(&relay.grab_keyboard);
     input_method.events.destroy.add(&relay.input_method_destroy);
-    input_method.events.new_popup_surface.add(&relay.input_method_new_popup_surface);
+    input_method.events.new_popup_surface.add(&relay.input_method_new_popup);
 
     if (seat.focused.surface()) |surface| {
         relay.focus(surface);
@@ -133,7 +133,7 @@ fn handleInputMethodDestroy(
     relay.input_method_commit.link.remove();
     relay.grab_keyboard.link.remove();
     relay.input_method_destroy.link.remove();
-    relay.input_method_new_popup_surface.link.remove();
+    relay.input_method_new_popup.link.remove();
     relay.input_method = null;
 
     relay.focus(null);
@@ -154,13 +154,13 @@ fn handleInputMethodGrabKeyboard(
     keyboard_grab.events.destroy.add(&relay.grab_keyboard_destroy);
 }
 
-fn handleInputMethodNewPopupSurface(
+fn handleInputMethodNewPopup(
     listener: *wl.Listener(*wlr.InputPopupSurfaceV2),
-    input_method_new_popup_surface: *wlr.InputPopupSurfaceV2,
+    wlr_popup: *wlr.InputPopupSurfaceV2,
 ) void {
-    log.debug("new input_method_popup_surface", .{});
+    const relay = @fieldParentPtr(InputRelay, "input_method_new_popup", listener);
-    const relay = @fieldParentPtr(InputRelay, "input_method_new_popup_surface", listener);
+
-    InputPopup.create(input_method_new_popup_surface, relay) catch {
+    InputPopup.create(wlr_popup, relay) catch {
         log.err("out of memory", .{});
         return;
     };
@@ -180,13 +180,16 @@ fn handleInputMethodGrabKeyboardDestroy(
 
 pub fn disableTextInput(relay: *InputRelay) void {
     assert(relay.text_input != null);
+    relay.text_input = null;
 
     if (relay.input_method) |input_method| {
+        {
+            var it = relay.input_popups.iterator(.forward);
+            while (it.next()) |popup| popup.update();
+        }
         input_method.sendDeactivate();
         input_method.sendDone();
     }
-
-    relay.text_input = null;
 }
 
 pub fn sendInputMethodState(relay: *InputRelay) void {
@@ -215,11 +218,11 @@ pub fn sendInputMethodState(relay: *InputRelay) void {
         );
     }
 
-    // Update input popups
+    {
-    var it = relay.input_popups.iterator(.forward);
+        var it = relay.input_popups.iterator(.forward);
-    while (it.next()) |popup| {
+        while (it.next()) |popup| popup.update();
-        popup.update();
     }
+
     input_method.sendDone();
 }
 

river/Output.zig

@@ -99,7 +99,7 @@ layers: struct {
     fullscreen: *wlr.SceneTree,
     /// Overlay layer shell layer
     overlay: *wlr.SceneTree,
-    /// xdg-popups of views and layer-shell surfaces
+    /// Popups from xdg-shell and input-method-v2 clients.
     popups: *wlr.SceneTree,
 },
 

river/SceneNodeData.zig

@@ -24,6 +24,7 @@ const util = @import("util.zig");
 
 const LayerSurface = @import("LayerSurface.zig");
 const LockSurface = @import("LockSurface.zig");
+const InputPopup = @import("InputPopup.zig");
 const View = @import("View.zig");
 const XwaylandOverrideRedirect = @import("XwaylandOverrideRedirect.zig");
 
@@ -65,10 +66,8 @@ pub fn fromNode(node: *wlr.SceneNode) ?*SceneNodeData {
 }
 
 pub fn fromSurface(surface: *wlr.Surface) ?*SceneNodeData {
-    if (surface.getRootSurface()) |root_surface| {
+    if (@as(?*wlr.SceneNode, @ptrFromInt(surface.getRootSurface().data))) |node| {
-        if (@as(?*wlr.SceneNode, @ptrFromInt(root_surface.data))) |node| {
+        return fromNode(node);
-            return fromNode(node);
-        }
     }
     return null;
 }

river/XwaylandView.zig

@@ -163,7 +163,6 @@ pub fn handleMap(listener: *wl.Listener(void)) void {
     const view = xwayland_view.view;
 
     const xwayland_surface = xwayland_view.xwayland_surface;
-    xwayland_surface.data = @intFromPtr(xwayland_view);
     const surface = xwayland_surface.surface.?;
     surface.data = @intFromPtr(&view.tree.node);