From 65853812306414b525ea6beb25dc915249dc922e Mon Sep 17 00:00:00 2001 From: Aleksei Bavshin Date: Mon, 8 Feb 2021 22:30:01 -0800 Subject: [PATCH 1/2] fix(client): remove unnecessary wl_output_roundtrip At this point we're not awaiting any protocol events and flushing wayland queue makes little sense. As #1019 shows, it may be even harmful as an extra roundtrip could process wl_output disappearance and delete output object right from under our code. --- src/client.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/client.cpp b/src/client.cpp index 0ad4e6bb..9983bb56 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -124,7 +124,6 @@ void waybar::Client::handleOutputDone(void *data, struct zxdg_output_v1 * /*xdg_ auto configs = client->getOutputConfigs(output); if (!configs.empty()) { - wl_display_roundtrip(client->wl_display); for (const auto &config : configs) { client->bars.emplace_back(std::make_unique(&output, config)); } From 89b5e819a34388e39829bd3cbe52d5fb28a08297 Mon Sep 17 00:00:00 2001 From: Aleksei Bavshin Date: Mon, 8 Feb 2021 23:05:31 -0800 Subject: [PATCH 2/2] fix(client): improve guard from repeated xdg_output.done events Multiple .done events may arrive in batch. In this case libwayland would queue xdg_output.destroy and dispatch all pending events, triggering this callback several times for the same output. Delete xdg_output pointer immediately on the first event and use the value as a guard for reentering. --- src/client.cpp | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/client.cpp b/src/client.cpp index 9983bb56..fcfcd98c 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -120,16 +120,26 @@ void waybar::Client::handleOutputDone(void *data, struct zxdg_output_v1 * /*xdg_ auto client = waybar::Client::inst(); try { auto &output = client->getOutput(data); - spdlog::debug("Output detection done: {} ({})", output.name, output.identifier); + /** + * Multiple .done events may arrive in batch. In this case libwayland would queue + * xdg_output.destroy and dispatch all pending events, triggering this callback several times + * for the same output. .done events can also arrive after that for a scale or position changes. + * We wouldn't want to draw a duplicate bar for each such event either. + * + * All the properties we care about are immutable so it's safe to delete the xdg_output object + * on the first event and use the ptr value to check that the callback was already invoked. + */ + if (output.xdg_output) { + output.xdg_output.reset(); + spdlog::debug("Output detection done: {} ({})", output.name, output.identifier); - auto configs = client->getOutputConfigs(output); - if (!configs.empty()) { - for (const auto &config : configs) { - client->bars.emplace_back(std::make_unique(&output, config)); + auto configs = client->getOutputConfigs(output); + if (!configs.empty()) { + for (const auto &config : configs) { + client->bars.emplace_back(std::make_unique(&output, config)); + } } } - // unsubscribe - output.xdg_output.reset(); } catch (const std::exception &e) { std::cerr << e.what() << std::endl; }