output: fix assert when re-adding output to layout #315
Conversation
wlr_scene_output_layout_add_output() aborts when called with an already-added output. To reproduce, run wlr-randr to reconfigure one of the enabled outputs.
|
I ran into the same issue but my patch was not ready yet. Instead of checking existence each time, I assume wlr_scene_output_layout_add_output() has been called in handle_new_output(). Then, these output_layout_add*() inline functions can be replaced by the only call done to the wlr API. |
That won't work if a user disables an output via the wlr-output-management-v1 protocol, then enables it. |
| bool exists = wlr_output_layout_get(output->server->output_layout, output->wlr_output); | ||
| struct wlr_output_layout_output *layout_output = | ||
| wlr_output_layout_add(output->server->output_layout, output->wlr_output, x, y); | ||
| if (exists) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Alternatively one could just wlr_output_layout_remove before calls to wlr_output_layout_add(_auto), avoiding the need to check anything.
There was a problem hiding this comment.
Yeah, but that would re-create the wl_output global which I'd prefer not to do.
There was a problem hiding this comment.
Hmm, good point, that would be a bit annoying on the wire.
Maybe we should replace that assert in scene_output_layout with a an early return to better match how wlr_output_layout works...
There was a problem hiding this comment.
This was added in https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/4380 FWIW.
There was a problem hiding this comment.
https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/4559
(Note that unless we're backporting that patch to wlroots 0.17, this PR is still necessary.)
An assert was added in [1] to avoid tracking the same output multiple times. However, this is cumbersome for compositors [2]: they need to add a special check for this. Additionally, this is inconsistent with wlr_output_layout_add(). [1]: https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/4380 [2]: cage-kiosk/cage#315
|
Let's merge for now, and revert later if the wlroots patch ends up backported + released. |
wlr_scene_output_layout_add_output() aborts when called with an already-added output.
To reproduce, run wlr-randr to reconfigure one of the enabled outputs.
cc @kennylevinsen