From 9df5236c468839ead6627131886f3d1153035bf3 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 27 Aug 2024 13:43:50 +0200 Subject: [PATCH 1/4] progress-bar: Only write when truly updated --- src/libmain/progress-bar.cc | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index bb4c52ef7..d864d9473 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -75,6 +75,9 @@ private: bool active = true; bool paused = false; bool haveUpdate = true; + + /** Helps avoid unnecessary redraws, see `draw()` */ + std::string lastOutput; }; Sync state_; @@ -360,6 +363,31 @@ public: } std::chrono::milliseconds draw(State & state) + { + // Call draw() and render if the output has changed. + + // Excessive redrawing is noticable on slow terminals, and it interferes + // with text selection in some terminals, including libvte-based terminal + // emulators. + + std::optional newOutput; + auto nextWakeup = draw(state, newOutput); + { + auto state(state_.lock()); + if (newOutput && *newOutput != state->lastOutput) { + writeToStderr(*newOutput); + state->lastOutput = std::move(*newOutput); + } + } + return nextWakeup; + } + + /** + * @param output[out] `nullopt` if nothing is to be drawn. Otherwise, a + * string of ANSI terminal output that can be used to + * render the progress bar. + */ + std::chrono::milliseconds draw(State & state, std::optional & output) { auto nextWakeup = std::chrono::milliseconds::max(); @@ -412,7 +440,7 @@ public: auto width = getWindowSize().second; if (width <= 0) width = std::numeric_limits::max(); - writeToStderr("\r" + filterANSIEscapes(line, false, width) + ANSI_NORMAL + "\e[K"); + output = "\r" + filterANSIEscapes(line, false, width) + ANSI_NORMAL + "\e[K"; return nextWakeup; } From 047d9643b54aa857dfff81ba8aed7881a97938a4 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 8 Sep 2024 01:09:51 +0200 Subject: [PATCH 2/4] refact: Extract ProgressBar::redraw(newOutput) --- src/libmain/progress-bar.cc | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index d864d9473..2ea743034 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -362,23 +362,28 @@ public: updateCV.notify_one(); } + /** + * Redraw, if the output has changed. + * + * Excessive redrawing is noticable on slow terminals, and it interferes + * with text selection in some terminals, including libvte-based terminal + * emulators. + */ + void redraw(std::string newOutput) + { + auto state(state_.lock()); + if (newOutput != state->lastOutput) { + writeToStderr(newOutput); + state->lastOutput = std::move(newOutput); + } + } + std::chrono::milliseconds draw(State & state) { - // Call draw() and render if the output has changed. - - // Excessive redrawing is noticable on slow terminals, and it interferes - // with text selection in some terminals, including libvte-based terminal - // emulators. - std::optional newOutput; auto nextWakeup = draw(state, newOutput); - { - auto state(state_.lock()); - if (newOutput && *newOutput != state->lastOutput) { - writeToStderr(*newOutput); - state->lastOutput = std::move(*newOutput); - } - } + if (newOutput) + redraw(*newOutput); return nextWakeup; } From e10ea78f9313d8df381301ac2fc3023de3993689 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 8 Sep 2024 01:21:40 +0200 Subject: [PATCH 3/4] refact: Inline ProgressBar::draw(state, newOutput), inline local output --- src/libmain/progress-bar.cc | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index 2ea743034..ce513d204 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -379,20 +379,6 @@ public: } std::chrono::milliseconds draw(State & state) - { - std::optional newOutput; - auto nextWakeup = draw(state, newOutput); - if (newOutput) - redraw(*newOutput); - return nextWakeup; - } - - /** - * @param output[out] `nullopt` if nothing is to be drawn. Otherwise, a - * string of ANSI terminal output that can be used to - * render the progress bar. - */ - std::chrono::milliseconds draw(State & state, std::optional & output) { auto nextWakeup = std::chrono::milliseconds::max(); @@ -445,7 +431,7 @@ public: auto width = getWindowSize().second; if (width <= 0) width = std::numeric_limits::max(); - output = "\r" + filterANSIEscapes(line, false, width) + ANSI_NORMAL + "\e[K"; + redraw("\r" + filterANSIEscapes(line, false, width) + ANSI_NORMAL + "\e[K"); return nextWakeup; } From c955563b6440ffe7abb22f15744ceea8b4ce2d9c Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 8 Sep 2024 11:44:24 +0200 Subject: [PATCH 4/4] fix: Avoid deadlock in ProgressBar::redraw() --- src/libmain/progress-bar.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index ce513d204..e63d4f13f 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -75,11 +75,11 @@ private: bool active = true; bool paused = false; bool haveUpdate = true; - - /** Helps avoid unnecessary redraws, see `draw()` */ - std::string lastOutput; }; + /** Helps avoid unnecessary redraws, see `redraw()` */ + Sync lastOutput_; + Sync state_; std::thread updateThread; @@ -371,10 +371,10 @@ public: */ void redraw(std::string newOutput) { - auto state(state_.lock()); - if (newOutput != state->lastOutput) { + auto lastOutput(lastOutput_.lock()); + if (newOutput != *lastOutput) { writeToStderr(newOutput); - state->lastOutput = std::move(newOutput); + *lastOutput = std::move(newOutput); } }