[lldb] Optimize statusline redrawing on terminal size change

[JDevlieghere]

Avoid some of the work to disable/enable the statusline when the terminal dimensions change.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

Avoid some of the work to disable/enable the statusline when the terminal dimensions change.

---

Full diff: https://github.com/llvm/llvm-project/pull/146435.diff

2 Files Affected:

• (modified) lldb/include/lldb/Core/Statusline.h (-3)
• (modified) lldb/source/Core/Statusline.cpp (+19-10)

diff --git a/lldb/include/lldb/Core/Statusline.h b/lldb/include/lldb/Core/Statusline.h
index 521b9f2526f6b..98c4f0681d61e 100644
--- a/lldb/include/lldb/Core/Statusline.h
+++ b/lldb/include/lldb/Core/Statusline.h
@@ -36,9 +36,6 @@ class Statusline {
   /// Draw the statusline with the given text.
   void Draw(std::string msg);
 
-  /// Update terminal dimensions.
-  void UpdateTerminalProperties();
-
   enum ScrollWindowMode {
     EnableStatusline,
     DisableStatusline,
diff --git a/lldb/source/Core/Statusline.cpp b/lldb/source/Core/Statusline.cpp
index 8ec57c9fa5bac..327ba0ea62465 100644
--- a/lldb/source/Core/Statusline.cpp
+++ b/lldb/source/Core/Statusline.cpp
@@ -41,10 +41,26 @@ Statusline::Statusline(Debugger &debugger)
 Statusline::~Statusline() { Disable(); }
 
 void Statusline::TerminalSizeChanged() {
-  UpdateTerminalProperties();
+  const uint64_t terminal_width = m_debugger.GetTerminalWidth();
+  const uint64_t terminal_height = m_debugger.GetTerminalHeight();
+
+  // Remember whether the terminal height changed.
+  const bool terminal_height_changed = terminal_height != m_terminal_height;
+
+  // Avoid clearing the old statusline if it's not visible (i.e. when the
+  // terminal height decreases), unless the width changed and the old statusline
+  // wrapped.
+  if (terminal_height > m_terminal_height || terminal_width < m_terminal_width)
+    UpdateScrollWindow(DisableStatusline);
+
+  // Update the terminal dimensions.
+  m_terminal_width = terminal_width;
+  m_terminal_height = terminal_height;
+
+  // Update the scroll window if the terminal height changed.
+  if (terminal_height_changed)
+    UpdateScrollWindow(EnableStatusline);
 
-  // This definitely isn't signal safe, but the best we can do, until we
-  // have proper signal-catching thread.
   Redraw(/*update=*/false);
 }
 
@@ -85,13 +101,6 @@ void Statusline::Draw(std::string str) {
   locked_stream << ANSI_RESTORE_CURSOR;
 }
 
-void Statusline::UpdateTerminalProperties() {
-  UpdateScrollWindow(DisableStatusline);
-  m_terminal_width = m_debugger.GetTerminalWidth();
-  m_terminal_height = m_debugger.GetTerminalHeight();
-  UpdateScrollWindow(EnableStatusline);
-}
-
 void Statusline::UpdateScrollWindow(ScrollWindowMode mode) {
   assert(m_terminal_width != 0 && m_terminal_height != 0);
 

[labath]

This messes up my terminal when reducing the height. It goes from

to

The cursor ends up in the right place, but it ends up inside the former (inactive) copy of the status line. I guess that's related to the fact that the status line is automatically disabled on resize. Previous (unoptimized) code worked fine. (Note my window manager is configured to not to do "dynamic resizes" so lldb only gets one SIGWINCH with the new final size -- not all the ones in between. I haven't tried what would happen with dynamic resizes -- it might be better, or it might be much worse).

Since I was trying this out, I also noticed that changing the size upwards also has a bug (unrelated to this patch). This is how it looks like:

->

I don't know exactly why, but I suspect it has something to do with the clearing code. It's definitely triggered by lldb, as this is what I get if I resize without any lldb involvement (I run lldb to set up the scroll window, but then I kill it so it doesn't do any extra work after resizing):

->

I.e., the terminal just happily show the extra lines from the history and the (inactive) status line will stay at the bottom. (inactive means that if I press "return" the prompt will go into the status line remnant).

[labath]

FWIW, my terminal is not wrapping the status line when the width decreases. In fact, it seems to be internally resetting (disabling) the scroll windows whenever the size changes.

[labath]

So I think we'd need to do this unconditionally.

[JDevlieghere]

Thanks for the report. I've reached the point where I'm skeptical that I'm able to get this right for every combination of OS/WM/Terminal so I think #146578 is the safest path forward.

[JDevlieghere]

Closing in favor of #146578