From 052c8e1dcb9a6a132c261388951033a167140f0d Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Tue, 9 Jun 2020 19:13:28 +0200 Subject: [PATCH] transactions: handle preemption take 2 This implementation is far simpler than c0d7e71 as it takes advantage of wlroots's tracking of pending state. Additionally, we now send frame done events if a view that we are configuring commits with the wrong dimensions. This is necessary in order to trigger a redraw for some clients as well as being a more correct implementation of the protocol. --- river/Root.zig | 10 +++++++--- river/View.zig | 14 ++++++++------ river/VoidView.zig | 4 ++++ river/XdgToplevel.zig | 29 ++++++++++++++++++++++++++--- river/XwaylandView.zig | 6 ++++++ 5 files changed, 51 insertions(+), 12 deletions(-) diff --git a/river/Root.zig b/river/Root.zig index 11ec344..f032665 100644 --- a/river/Root.zig +++ b/river/Root.zig @@ -139,9 +139,9 @@ fn startTransaction(self: *Self) void { view.configure(); self.pending_configures += 1; - // We save the current buffer, so we can send an early - // frame done event to give the client a head start on - // redrawing. + // Send a frame done that the client will commit a new frame + // with the dimensions we sent in the configure. Normally this + // event would be sent in the render function. view.sendFrameDone(); } @@ -165,6 +165,10 @@ fn startTransaction(self: *Self) void { self.commitTransaction(); } } else { + // No views need configures, clear the current timer in case we are + // interrupting another transaction and commit. + if (c.wl_event_source_timer_update(self.transaction_timer, 0) < 0) + Log.Error.log("error disarming timer", .{}); self.commitTransaction(); } } diff --git a/river/View.zig b/river/View.zig index af5d9b9..cc07252 100644 --- a/river/View.zig +++ b/river/View.zig @@ -112,12 +112,10 @@ pub fn deinit(self: Self) void { } pub fn needsConfigure(self: Self) bool { - if (self.pending_box) |pending_box| { - return pending_box.width != self.current_box.width or - pending_box.height != self.current_box.height; - } else { - return false; - } + return switch (self.impl) { + .xdg_toplevel => |xdg_toplevel| xdg_toplevel.needsConfigure(), + .xwayland_view => |xwayland_view| xwayland_view.needsConfigure(), + }; } pub fn configure(self: Self) void { @@ -238,6 +236,8 @@ pub fn getTitle(self: Self) [*:0]const u8 { pub fn map(self: *Self) void { const root = self.output.root; + Log.Debug.log("View '{}' mapped", .{self.getTitle()}); + // Add the view to the stack of its output const node = @fieldParentPtr(ViewStack(Self).Node, "view", self); self.output.views.push(node); @@ -260,6 +260,8 @@ pub fn map(self: *Self) void { pub fn unmap(self: *Self) void { const root = self.output.root; + Log.Debug.log("View '{}' unmapped", .{self.getTitle()}); + self.wlr_surface = null; // Inform all seats that the view has been unmapped so they can handle focus diff --git a/river/VoidView.zig b/river/VoidView.zig index fca2321..81392ff 100644 --- a/river/VoidView.zig +++ b/river/VoidView.zig @@ -23,6 +23,10 @@ const c = @import("c.zig"); const Box = @import("Box.zig"); +pub fn needsConfigure(self: Self) bool { + unreachable; +} + pub fn configure(self: Self, pending_box: Box) void { unreachable; } diff --git a/river/XdgToplevel.zig b/river/XdgToplevel.zig index 88e7dce..5d43b92 100644 --- a/river/XdgToplevel.zig +++ b/river/XdgToplevel.zig @@ -63,6 +63,25 @@ pub fn init(self: *Self, view: *View, wlr_xdg_surface: *c.wlr_xdg_surface) void c.wl_signal_add(&self.wlr_xdg_surface.events.unmap, &self.listen_unmap); } +/// Returns true if a configure must be sent to ensure the dimensions of the +/// pending_box are applied. +pub fn needsConfigure(self: Self) bool { + const pending_box = self.view.pending_box orelse return false; + + const wlr_xdg_toplevel: *c.wlr_xdg_toplevel = @field( + self.wlr_xdg_surface, + c.wlr_xdg_surface_union, + ).toplevel; + + // Checking server_pending is sufficient here since it will be either in + // sync with the current dimensions or be the dimensions sent with the + // most recent configure. In both cases server_pending has the values we + // want to check against. + return pending_box.width != wlr_xdg_toplevel.server_pending.width or + pending_box.height != wlr_xdg_toplevel.server_pending.height; +} + +/// Send a configure event, applying the width/height of the pending box. pub fn configure(self: Self, pending_box: Box) void { self.view.pending_serial = c.wlr_xdg_toplevel_set_size( self.wlr_xdg_surface, @@ -106,7 +125,7 @@ pub fn getTitle(self: Self) [*:0]const u8 { self.wlr_xdg_surface, c.wlr_xdg_surface_union, ).toplevel; - return wlr_xdg_toplevel.title; + return wlr_xdg_toplevel.title orelse "NULL"; } /// Called when the xdg surface is destroyed @@ -153,8 +172,6 @@ fn handleMap(listener: ?*c.wl_listener, data: ?*c_void) callconv(.C) void { const state = &wlr_xdg_toplevel.current; const app_id: [*:0]const u8 = if (wlr_xdg_toplevel.app_id) |id| id else "NULL"; - Log.Debug.log("View with app_id '{}' mapped", .{app_id}); - for (root.server.config.float_filter.items) |filter_app_id| { // Make views with app_ids listed in the float filter float if (std.mem.eql(u8, std.mem.span(app_id), std.mem.span(filter_app_id))) { @@ -194,6 +211,12 @@ fn handleCommit(listener: ?*c.wl_listener, data: ?*c_void) callconv(.C) void { if (s == self.wlr_xdg_surface.configure_serial) { view.output.root.notifyConfigured(); view.pending_serial = null; + } else { + // If the view has not yet acked our configure, we need to send a + // frame done event so that they commit another buffer. These + // buffers won't be rendered since we are still rendering our + // stashed buffer from when the transaction started. + view.sendFrameDone(); } } } diff --git a/river/XwaylandView.zig b/river/XwaylandView.zig index 681d9d1..f978e50 100644 --- a/river/XwaylandView.zig +++ b/river/XwaylandView.zig @@ -57,6 +57,12 @@ pub fn init(self: *Self, view: *View, wlr_xwayland_surface: *c.wlr_xwayland_surf c.wl_signal_add(&self.wlr_xwayland_surface.events.unmap, &self.listen_unmap); } +/// Don't really care about efficiency with xwayland, we don't wait for them +/// to ack anyways since they don't use serials. +pub fn needsConfigure(self: Self) bool { + return true; +} + /// Tell the client to take a new size pub fn configure(self: Self, pending_box: Box) void { c.wlr_xwayland_surface_configure(