layer-shell: handle commits before map

A client is free to change its mind and request a different
size/anchor/etc after recieving the initial configure but before
attaching and committing the first buffer. This means that we should
respond to such a situation with a new configure.

mako has been observed doing this in the wild for example.

river/InputManager.zig

@@ -220,7 +220,7 @@ fn handleInhibitDeactivate(
     // keyboard-interactive surfaces will re-grab focus.
     var output_it = server.root.outputs.first;
     while (output_it) |output_node| : (output_it = output_node.next) {
-        output_node.data.arrangeLayers();
+        output_node.data.arrangeLayers(.mapped);
     }
 
     // After ensuring that any possible layer surface focus grab has occured,

river/LayerSurface.zig

@@ -18,6 +18,7 @@
 const Self = @This();
 
 const std = @import("std");
+const assert = std.debug.assert;
 const wlr = @import("wlroots");
 const wl = @import("wayland").server.wl;
 
@@ -37,14 +38,11 @@ wlr_layer_surface: *wlr.LayerSurfaceV1,
 box: Box = undefined,
 state: wlr.LayerSurfaceV1.State,
 
-// Listeners active the entire lifetime of the layser surface
 destroy: wl.Listener(*wlr.LayerSurfaceV1) = wl.Listener(*wlr.LayerSurfaceV1).init(handleDestroy),
 map: wl.Listener(*wlr.LayerSurfaceV1) = wl.Listener(*wlr.LayerSurfaceV1).init(handleMap),
 unmap: wl.Listener(*wlr.LayerSurfaceV1) = wl.Listener(*wlr.LayerSurfaceV1).init(handleUnmap),
 new_popup: wl.Listener(*wlr.XdgPopup) = wl.Listener(*wlr.XdgPopup).init(handleNewPopup),
 new_subsurface: wl.Listener(*wlr.Subsurface) = wl.Listener(*wlr.Subsurface).init(handleNewSubsurface),
-
-// Listeners only active while the layer surface is mapped
 commit: wl.Listener(*wlr.Surface) = wl.Listener(*wlr.Surface).init(handleCommit),
 
 pub fn init(self: *Self, output: *Output, wlr_layer_surface: *wlr.LayerSurfaceV1) void {
@@ -55,21 +53,19 @@ pub fn init(self: *Self, output: *Output, wlr_layer_surface: *wlr.LayerSurfaceV1
     };
     wlr_layer_surface.data = @ptrToInt(self);
 
-    // Temporarily add to the output's list to allow for inital arrangement
-    // which sends the first configure.
-    const node = @fieldParentPtr(std.TailQueue(Self).Node, "data", self);
-    const list = &output.layers[@intCast(usize, @enumToInt(self.state.layer))];
-    list.append(node);
-    output.arrangeLayers();
-    list.remove(node);
-
     // Set up listeners that are active for the entire lifetime of the layer surface
     wlr_layer_surface.events.destroy.add(&self.destroy);
     wlr_layer_surface.events.map.add(&self.map);
     wlr_layer_surface.events.unmap.add(&self.unmap);
     wlr_layer_surface.events.new_popup.add(&self.new_popup);
+    wlr_layer_surface.surface.events.commit.add(&self.commit);
     wlr_layer_surface.surface.events.new_subsurface.add(&self.new_subsurface);
 
+    // wlroots only informs us of the new surface after the first commit,
+    // so our listener does not get called for this first commit. However,
+    // we do want our listener called in order to send the initial configure.
+    handleCommit(&self.commit, wlr_layer_surface.surface);
+
     // There may already be subsurfaces present on this surface that we
     // aren't aware of and won't receive a new_subsurface event for.
     var it = wlr_layer_surface.surface.subsurfaces.iterator(.forward);
@@ -86,6 +82,7 @@ fn handleDestroy(listener: *wl.Listener(*wlr.LayerSurfaceV1), wlr_layer_surface:
     self.map.link.remove();
     self.unmap.link.remove();
     self.new_popup.link.remove();
+    self.commit.link.remove();
     self.new_subsurface.link.remove();
 
     const node = @fieldParentPtr(std.TailQueue(Self).Node, "data", self);
@@ -97,13 +94,11 @@ fn handleMap(listener: *wl.Listener(*wlr.LayerSurfaceV1), wlr_layer_surface: *wl
 
     log.debug("layer surface '{s}' mapped", .{wlr_layer_surface.namespace});
 
-    // Add listeners that are only active while mapped
-    wlr_layer_surface.surface.events.commit.add(&self.commit);
-
     wlr_layer_surface.surface.sendEnter(wlr_layer_surface.output.?);
 
     const node = @fieldParentPtr(std.TailQueue(Self).Node, "data", self);
-    self.output.layers[@intCast(usize, @enumToInt(self.state.layer))].append(node);
+    self.output.getLayer(self.state.layer).append(node);
+    self.output.arrangeLayers(.mapped);
 }
 
 fn handleUnmap(listener: *wl.Listener(*wlr.LayerSurfaceV1), wlr_layer_surface: *wlr.LayerSurfaceV1) void {
@@ -111,9 +106,6 @@ fn handleUnmap(listener: *wl.Listener(*wlr.LayerSurfaceV1), wlr_layer_surface: *
 
     log.debug("layer surface '{s}' unmapped", .{self.wlr_layer_surface.namespace});
 
-    // remove listeners only active while the layer surface is mapped
-    self.commit.link.remove();
-
     // Remove from the output's list of layer surfaces
     const self_node = @fieldParentPtr(std.TailQueue(Self).Node, "data", self);
     self.output.layers[@intCast(usize, @enumToInt(self.state.layer))].remove(self_node);
@@ -128,7 +120,7 @@ fn handleUnmap(listener: *wl.Listener(*wlr.LayerSurfaceV1), wlr_layer_surface: *
 
     // This gives exclusive focus to a keyboard interactive top or overlay layer
     // surface if there is one.
-    self.output.arrangeLayers();
+    self.output.arrangeLayers(.mapped);
 
     // Ensure that focus is given to the appropriate view if there is no
     // other top/overlay layer surface to grab focus.
@@ -144,8 +136,14 @@ fn handleUnmap(listener: *wl.Listener(*wlr.LayerSurfaceV1), wlr_layer_surface: *
 fn handleCommit(listener: *wl.Listener(*wlr.Surface), wlr_surface: *wlr.Surface) void {
     const self = @fieldParentPtr(Self, "commit", listener);
 
-    if (self.wlr_layer_surface.output == null) {
+    assert(self.wlr_layer_surface.output != null);
-        log.err("layer surface committed with null output", .{});
+
+    // If a surface is committed while it is not mapped, we may need to send a configure.
+    if (!self.wlr_layer_surface.mapped) {
+        const node = @fieldParentPtr(std.TailQueue(Self).Node, "data", self);
+        self.output.getLayer(self.state.layer).append(node);
+        self.output.arrangeLayers(.unmapped);
+        self.output.getLayer(self.state.layer).remove(node);
         return;
     }
 
@@ -154,13 +152,13 @@ fn handleCommit(listener: *wl.Listener(*wlr.Surface), wlr_surface: *wlr.Surface)
         // If the layer changed, move the LayerSurface to the proper list
         if (self.state.layer != new_state.layer) {
             const node = @fieldParentPtr(std.TailQueue(Self).Node, "data", self);
-            self.output.layers[@intCast(usize, @enumToInt(self.state.layer))].remove(node);
+            self.output.getLayer(self.state.layer).remove(node);
-            self.output.layers[@intCast(usize, @enumToInt(new_state.layer))].append(node);
+            self.output.getLayer(new_state.layer).append(node);
         }
 
         self.state = new_state.*;
 
-        self.output.arrangeLayers();
+        self.output.arrangeLayers(.mapped);
         server.root.startTransaction();
     }
 

river/Output.zig

@@ -198,8 +198,12 @@ pub fn arrangeViews(self: *Self) void {
     }
 }
 
+const ArrangeLayersTarget = enum { mapped, unmapped };
+
 /// Arrange all layer surfaces of this output and adjust the usable area
-pub fn arrangeLayers(self: *Self) void {
+/// If target is unmapped, this function is pure aside from the
+/// wlr.LayerSurfaceV1.configure() calls made on umapped layer surfaces.
+pub fn arrangeLayers(self: *Self, target: ArrangeLayersTarget) void {
     const effective_resolution = self.getEffectiveResolution();
     const full_box: Box = .{
         .x = 0,
@@ -215,16 +219,18 @@ pub fn arrangeLayers(self: *Self) void {
 
     // Arrange all layer surfaces with exclusive zones, applying them to the
     // usable box along the way.
-    for (layers) |layer| self.arrangeLayer(self.getLayer(layer).*, full_box, &usable_box, true);
+    for (layers) |layer| self.arrangeLayer(self.getLayer(layer).*, full_box, &usable_box, true, target);
 
     // If the the usable_box has changed, we need to rearrange the output
-    if (!std.meta.eql(self.usable_box, usable_box)) {
+    if (target == .mapped and !std.meta.eql(self.usable_box, usable_box)) {
         self.usable_box = usable_box;
         self.arrangeViews();
     }
 
     // Arrange the layers without exclusive zones
-    for (layers) |layer| self.arrangeLayer(self.getLayer(layer).*, full_box, &usable_box, false);
+    for (layers) |layer| self.arrangeLayer(self.getLayer(layer).*, full_box, &usable_box, false, target);
+
+    if (target == .unmapped) return;
 
     // Find the topmost layer surface in the top or overlay layers which
     // requests keyboard interactivity if any.
@@ -268,6 +274,7 @@ fn arrangeLayer(
     full_box: Box,
     usable_box: *Box,
     exclusive: bool,
+    target: ArrangeLayersTarget,
 ) void {
     var it = layer.first;
     while (it) |node| : (it = node.next) {
@@ -348,8 +355,6 @@ fn arrangeLayer(
             new_box.height = current_state.desired_height;
         }
 
-        layer_surface.box = new_box;
-
         // Apply the exclusive zone to the current bounds
         const edges = [4]struct {
             single: zwlr.LayerSurfaceV1.Anchor,
@@ -399,9 +404,16 @@ fn arrangeLayer(
             }
         }
 
-        // Tell the client to assume the new size
+        switch (target) {
-        std.log.scoped(.layer_shell).debug("send configure, {} x {}", .{ layer_surface.box.width, layer_surface.box.height });
+            .mapped => {
-        layer_surface.wlr_layer_surface.configure(layer_surface.box.width, layer_surface.box.height);
+                assert(layer_surface.wlr_layer_surface.mapped);
+                layer_surface.box = new_box;
+                layer_surface.wlr_layer_surface.configure(new_box.width, new_box.height);
+            },
+            .unmapped => if (!layer_surface.wlr_layer_surface.mapped) {
+                layer_surface.wlr_layer_surface.configure(new_box.width, new_box.height);
+            },
+        }
     }
 }
 
@@ -466,7 +478,7 @@ fn handleFrame(listener: *wl.Listener(*wlr.OutputDamage), wlr_output: *wlr.Outpu
 
 fn handleMode(listener: *wl.Listener(*wlr.Output), wlr_output: *wlr.Output) void {
     const self = @fieldParentPtr(Self, "mode", listener);
-    self.arrangeLayers();
+    self.arrangeLayers(.mapped);
     self.arrangeViews();
     server.root.startTransaction();
 }

river/Root.zig

@@ -490,7 +490,7 @@ fn applyOutputConfig(self: *Self, config: *wlr.OutputConfigurationV1) bool {
         // Arrange layers to adjust the usable_box
         // We dont need to call arrangeViews() since arrangeLayers() will call
         // it for us because the usable_box changed
-        output.arrangeLayers();
+        output.arrangeLayers(.mapped);
         self.startTransaction();
     }