From 5f0c9e2ccfb225827ab041d2d43a383aee8899fd Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Fri, 29 Dec 2023 17:15:51 -0600 Subject: output-management: fix output config application Currently wlr-output-management config application is broken since the pre 0.17 code relied on the (now removed) output enable/disable event to be emitted as part of the state application. The old code was pretty smelly and hard to understand, I'm glad the upstream improvements pushed river's code in this directions. --- river/Output.zig | 28 ++++++++++++++++++++++------ river/Root.zig | 46 ++++++++++++++++------------------------------ 2 files changed, 38 insertions(+), 36 deletions(-) diff --git a/river/Output.zig b/river/Output.zig index 3b083c6..04d92ff 100644 --- a/river/Output.zig +++ b/river/Output.zig @@ -419,22 +419,36 @@ fn handleDestroy(listener: *wl.Listener(*wlr.Output), _: *wlr.Output) void { fn handleRequestState(listener: *wl.Listener(*wlr.Output.event.RequestState), event: *wlr.Output.event.RequestState) void { const output = @fieldParentPtr(Self, "request_state", listener); - // TODO double buffer output state changes for frame perfection and cleaner code. - // Schedule a frame and commit in the frame handler. - if (!output.wlr_output.commitState(event.state)) { + output.applyState(event.state) catch { log.err("failed to commit requested state", .{}); return; + }; + + server.root.applyPending(); +} + +// TODO double buffer output state changes for frame perfection and cleaner code. +// Schedule a frame and commit in the frame handler. +// Get rid of this function. +pub fn applyState(output: *Self, state: *wlr.Output.State) error{CommitFailed}!void { + + // We need to be precise about this state change to make assertions + // in handleEnableDisable() possible. + const enable_state_change = state.committed.enabled and + (state.enabled != output.wlr_output.enabled); + + if (!output.wlr_output.commitState(state)) { + return error.CommitFailed; } - if (event.state.committed.enabled) { + if (enable_state_change) { output.handleEnableDisable(); } - if (event.state.committed.mode) { + if (state.committed.mode) { output.updateBackgroundRect(); output.arrangeLayers(); server.lock_manager.updateLockSurfaceSize(output); - server.root.applyPending(); } } @@ -462,6 +476,8 @@ fn handleEnableDisable(output: *Self) void { output.lock_render_state = .blanked; output.normal_content.node.setEnabled(false); output.locked_content.node.setEnabled(true); + + server.root.deactivateOutput(output); } } diff --git a/river/Root.zig b/river/Root.zig index 71cc1ef..b0c1d8d 100644 --- a/river/Root.zig +++ b/river/Root.zig @@ -264,8 +264,8 @@ fn handleNewOutput(_: *wl.Listener(*wlr.Output), wlr_output: *wlr.Output) void { }; } -/// Remove the output from root.active_outputs and evacuate views if it is a -/// member of the list. +/// Remove the output from root.active_outputs and the output layout. +/// Evacuate views if necessary. pub fn deactivateOutput(root: *Self, output: *Output) void { { // If the output has already been removed, do nothing @@ -273,11 +273,14 @@ pub fn deactivateOutput(root: *Self, output: *Output) void { while (it.next()) |o| { if (o == output) break; } else return; - - output.active_link.remove(); - output.active_link.init(); } + root.output_layout.remove(output.wlr_output); + output.tree.node.setEnabled(false); + + output.active_link.remove(); + output.active_link.init(); + { var it = output.inflight.focus_stack.iterator(.forward); while (it.next()) |view| { @@ -391,8 +394,6 @@ pub fn activateOutput(root: *Self, output: *Output) void { } assert(root.fallback.pending.focus_stack.empty()); assert(root.fallback.pending.wm_stack.empty()); - - root.applyPending(); } /// Trigger asynchronous application of pending state for all outputs and views. @@ -774,33 +775,18 @@ fn processOutputConfig( if (!wlr_output.testState(&proposed_state)) success = false; }, .apply => { - if (wlr_output.commitState(&proposed_state)) { - if (head.state.enabled) { - // Just updates the output's position if it is already in the layout - _ = self.output_layout.add(output.wlr_output, head.state.x, head.state.y) catch { - std.log.err("out of memory", .{}); - success = false; - continue; - }; - output.tree.node.setEnabled(true); - output.tree.node.setPosition(head.state.x, head.state.y); - output.scene_output.setPosition(head.state.x, head.state.y); - // Even though we call this in the output's handler for the mode event - // it is necessary to call it here as well since changing e.g. only - // the transform will require the dimensions of the background to be - // updated but will not trigger a mode event. - output.updateBackgroundRect(); - output.arrangeLayers(); - } else { - self.deactivateOutput(output); - self.output_layout.remove(output.wlr_output); - output.tree.node.setEnabled(false); - } - } else { + output.applyState(&proposed_state) catch { std.log.scoped(.output_manager).err("failed to apply config to output {s}", .{ output.wlr_output.name, }); success = false; + }; + if (output.wlr_output.enabled) { + // applyState() will always add the output to the layout on success, which means + // that this function cannot fail as it does not need to allocate a new layout output. + _ = self.output_layout.add(output.wlr_output, head.state.x, head.state.y) catch unreachable; + output.tree.node.setPosition(head.state.x, head.state.y); + output.scene_output.setPosition(head.state.x, head.state.y); } }, } -- cgit v1.2.3