transactions: handle preemption properly

when a transaction interrupts an ongoing transaction, we must be careful
to handle the configures properly. This commit adds a new member to view
so that we can store the dimensions sent with the last configure in
order to determine if the preempting transaction should override the
ongoing configure or not.

Additionally, some views do not ack a configure if they already have the
requested dimensions. This can happen if a pending configure setting
alternative dimensions is overridden, so in this case we do not wait for
an ack before committing the transaction.
This commit is contained in:
Isaac Freund 2020-06-08 00:10:30 +02:00
parent 7b1e07d3d5
commit c0d7e71ec4
No known key found for this signature in database
GPG key ID: 86DED400DDFD7A11
6 changed files with 94 additions and 37 deletions

View file

@ -334,8 +334,8 @@ pub fn layoutMasterStack(self: *Self, visible_count: u32, output_tags: u32, posi
new_box.width -= delta_size;
new_box.height -= delta_size;
// Set the view's pending box to the new dimensions
view.pending_box = new_box;
// Set the view's next box to the new dimensions
view.next_box = new_box;
i += 1;
}
@ -391,7 +391,7 @@ pub fn layoutFull(self: *Self, visible_count: u32, output_tags: u32) void {
.height = layout_height,
};
view.pending_box = new_box;
view.next_box = new_box;
i += 1;
}

View file

@ -132,27 +132,57 @@ fn startTransaction(self: *Self) void {
var view_it = ViewStack(View).iterator(output.views.first, std.math.maxInt(u32));
while (view_it.next()) |view_node| {
const view = &view_node.view;
// Clear the serial in case this transaction is interrupting a prior one.
view.pending_serial = null;
if (view.needsConfigure()) {
view.configure();
self.pending_configures += 1;
switch (view.configureAction()) {
.override => {
view.configure();
// Some clients do not ack a configure if the requested
// size is the same as their current size. Configures of
// this nature may be sent if a pending configure is
// interrupted by a configure returning to the original
// size.
if (view.pending_box.?.width == view.current_box.width and
view.pending_box.?.height == view.current_box.height)
{
view.pending_serial = null;
} else {
std.debug.assert(view.pending_serial != null);
self.pending_configures += 1;
}
},
.new_configure => {
view.configure();
self.pending_configures += 1;
std.debug.assert(view.pending_serial != null);
},
.old_configure => {
self.pending_configures += 1;
view.next_box = null;
std.debug.assert(view.pending_serial != null);
},
.noop => {
view.next_box = null;
std.debug.assert(view.pending_serial == null);
},
}
// If there is a saved buffer present, then this transaction is
// interrupting a previous transaction and we should keep the old
// buffer.
if (view.stashed_buffer == null) {
view.stashBuffer();
// We save the current buffer, so we can send an early
// frame done event to give the client a head start on
// redrawing.
view.sendFrameDone();
}
// If there is a saved buffer present, then this transaction is interrupting
// a previous transaction and we should keep the old buffer.
if (view.stashed_buffer == null) {
view.stashBuffer();
}
}
}
// If there are views that need configures, start a timer and wait for
// configure events before committing.
if (self.pending_configures > 0) {
Log.Debug.log(
"Started transaction with {} pending configures.",
@ -160,11 +190,15 @@ fn startTransaction(self: *Self) void {
);
// Set timeout to 200ms
if (c.wl_event_source_timer_update(self.transaction_timer, 200) < 0) {
if (c.wl_event_source_timer_update(self.transaction_timer, 1000) < 0) {
Log.Error.log("failed to update timer.", .{});
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();
}
}
@ -183,7 +217,7 @@ pub fn notifyConfigured(self: *Self) void {
self.pending_configures -= 1;
if (self.pending_configures == 0) {
// Disarm the timer, as we didn't timeout
if (c.wl_event_source_timer_update(self.transaction_timer, 0) == -1)
if (c.wl_event_source_timer_update(self.transaction_timer, 0) < 0)
Log.Error.log("Error disarming timer", .{});
self.commitTransaction();
}
@ -223,6 +257,7 @@ fn commitTransaction(self: *Self) void {
const view = &view_node.view;
// Ensure that all pending state is cleared
view.pending_serial = null;
std.debug.assert(view.next_box == null);
if (view.pending_box) |state| {
view.current_box = state;
view.pending_box = null;

View file

@ -55,8 +55,13 @@ focused: bool,
/// The current output-relative coordinates and dimensions of the view
current_box: Box,
/// The dimensions sent with the most recent configure
pending_box: ?Box,
/// The dimensions to be used for the next configure
next_box: ?Box,
/// The dimensions the view would have taken if we didn't force it to tile
natural_width: u32,
natural_height: u32,
@ -111,23 +116,40 @@ 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;
}
/// Returns true if a configure needs to be sent to ensure the next_box is
/// applied correctly.
pub fn configureAction(self: Self) enum { override, new_configure, old_configure, noop } {
// If we have a pending box, check if the next box is different from the
// pending box. If we do not have a pending box, check if the next box is
// different from the current box.
if (self.pending_box) |pending_box|
if (self.next_box) |next_box| {
if (next_box.width != pending_box.width or next_box.height != pending_box.height) {
return .override;
} else {
return .old_configure;
}
};
if (self.next_box) |next_box|
if (next_box.width != self.current_box.width or next_box.height != self.current_box.height)
return .new_configure;
return .noop;
}
pub fn configure(self: Self) void {
if (self.pending_box) |pending_box| {
/// Tell the client to assume the size of next_box. Set pending_box to
/// next_box and next_box to null.
pub fn configure(self: *Self) void {
if (self.next_box) |next_box| {
switch (self.impl) {
.xdg_toplevel => |xdg_toplevel| xdg_toplevel.configure(pending_box),
.xwayland_view => |xwayland_view| xwayland_view.configure(pending_box),
.xdg_toplevel => |xdg_toplevel| xdg_toplevel.configure(next_box),
.xwayland_view => |xwayland_view| xwayland_view.configure(next_box),
}
self.pending_box = next_box;
self.next_box = null;
} else {
Log.Error.log("Configure called on a View with no pending box", .{});
Log.Error.log("configure called on View with null next_box", .{});
}
}

View file

@ -23,7 +23,7 @@ const c = @import("c.zig");
const Box = @import("Box.zig");
pub fn configure(self: Self, pending_box: Box) void {
pub fn configure(self: Self, box: Box) void {
unreachable;
}

View file

@ -63,11 +63,11 @@ 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);
}
pub fn configure(self: Self, pending_box: Box) void {
pub fn configure(self: Self, box: Box) void {
self.view.pending_serial = c.wlr_xdg_toplevel_set_size(
self.wlr_xdg_surface,
pending_box.width,
pending_box.height,
box.width,
box.height,
);
}

View file

@ -58,13 +58,13 @@ pub fn init(self: *Self, view: *View, wlr_xwayland_surface: *c.wlr_xwayland_surf
}
/// Tell the client to take a new size
pub fn configure(self: Self, pending_box: Box) void {
pub fn configure(self: Self, box: Box) void {
c.wlr_xwayland_surface_configure(
self.wlr_xwayland_surface,
@intCast(i16, pending_box.x),
@intCast(i16, pending_box.y),
@intCast(u16, pending_box.width),
@intCast(u16, pending_box.height),
@intCast(i16, box.x),
@intCast(i16, box.y),
@intCast(u16, box.width),
@intCast(u16, box.height),
);
// Xwayland surfaces don't use serials, so we will just assume they have
// configured the next time they commit. Set pending serial to a dummy