[PATCH 3/3] Remove flickering from the TUI

[PATCH 3/3] Remove flickering from the TUI

[Tom Tromey]

In some cases, the TUI flickers when redrawing.  This can be seen
mostly easily when switching layouts.

This patch fixes the problem by exploiting the double buffering that
curses already does.  In some spots, the TUI will now disable flushing
the curses buffers to the screen; and then flush them all at once when
the rendering is complete.

gdb/ChangeLog
2019-12-27  Tom Tromey  <tom@tromey.com>

	* tui/tui.c (tui_show_assembly): Use tui_suppress_output.
	* tui/tui-wingeneral.h (class tui_suppress_output): New.
	(tui_wrefresh): Declare.
	* tui/tui-wingeneral.c (suppress_output): New global.
	(tui_suppress_output, ~tui_suppress_output): New constructor and
	destructor.
	(tui_wrefresh): New function.
	(tui_gen_win_info::refresh_window): Use tui_wrefresh.
	(tui_gen_win_info::make_window): Call wnoutrefresh when needed.
	* tui/tui-regs.h (struct tui_data_window) <no_refresh>: Declare
	method.
	* tui/tui-regs.c (tui_data_window::erase_data_content): Call
	tui_wrefresh.
	(tui_data_window::no_refresh): New method.
	(tui_data_item_window::refresh_window): Call tui_wrefresh.
	(tui_reg_command): Use tui_suppress_output
	* tui/tui-layout.c (tui_set_layout): Use tui_suppress_output.
	* tui/tui-data.h (struct tui_gen_win_info) <no_refresh>: New
	method.
	* tui/tui-command.c (tui_refresh_cmd_win): Call tui_wrefresh.

Change-Id: Icb832ae100b861de3af3307488e636fa928d5c9f
---
 gdb/ChangeLog            | 23 ++++++++++++++++++++
 gdb/tui/tui-command.c    |  2 +-
 gdb/tui/tui-data.h       |  7 ++++++
 gdb/tui/tui-layout.c     |  2 ++
 gdb/tui/tui-regs.c       | 14 ++++++++++--
 gdb/tui/tui-regs.h       |  2 ++
 gdb/tui/tui-wingeneral.c | 46 ++++++++++++++++++++++++++++++++++++++--
 gdb/tui/tui-wingeneral.h | 23 ++++++++++++++++++++
 gdb/tui/tui.c            |  2 ++
 9 files changed, 116 insertions(+), 5 deletions(-)

diff --git a/gdb/tui/tui-command.c b/gdb/tui/tui-command.c
index 5f676b2e659..ca0e8f612bd 100644
--- a/gdb/tui/tui-command.c
+++ b/gdb/tui/tui-command.c
@@ -70,7 +70,7 @@ tui_refresh_cmd_win (void)
 {
   WINDOW *w = TUI_CMD_WIN->handle.get ();
 
-  wrefresh (w);
+  tui_wrefresh (w);
 
   /* FIXME: It's not clear why this is here.
      It was present in the original tui_puts code and is kept in order to
diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h
index 2a822487fce..25a23860ef8 100644
--- a/gdb/tui/tui-data.h
+++ b/gdb/tui/tui-data.h
@@ -99,6 +99,13 @@ public:
     return handle != nullptr;
   }
 
+  /* Disable output until the next call to doupdate.  */
+  virtual void no_refresh ()
+  {
+    if (handle != nullptr)
+      wnoutrefresh (handle.get ());
+  }
+
   /* Window handle.  */
   std::unique_ptr<WINDOW, curses_deleter> handle;
   /* Type of window.  */
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index bd2e6aee651..740178ca083 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -113,6 +113,8 @@ tui_set_layout (enum tui_layout_type layout_type)
 
   if (new_layout != cur_layout)
     {
+      tui_suppress_output suppress;
+
       show_layout (new_layout);
 
       /* Now determine where focus should be.  */
diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index 85c7a31b0b6..cfcd80b7745 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -381,7 +381,7 @@ tui_data_window::erase_data_content (const char *prompt)
 	x_pos = half_width - strlen (prompt);
       mvwaddstr (handle.get (), (height / 2), x_pos, (char *) prompt);
     }
-  wrefresh (handle.get ());
+  tui_wrefresh (handle.get ());
 }
 
 /* See tui-regs.h.  */
@@ -434,6 +434,14 @@ tui_data_window::refresh_window ()
     win.refresh_window ();
 }
 
+void
+tui_data_window::no_refresh ()
+{
+  tui_gen_win_info::no_refresh ();
+  for (auto &&win : m_regs_content)
+    win.no_refresh ();
+}
+
 /* This function check all displayed registers for changes in values,
    given a particular frame.  If the values have changed, they are
    updated with the new value and highlighted.  */
@@ -502,7 +510,7 @@ tui_data_item_window::refresh_window ()
 	 windows, which according to the ncurses man pages aren't well
 	 supported.  */
       touchwin (handle.get ());
-      wrefresh (handle.get ());
+      tui_wrefresh (handle.get ());
     }
 }
 
@@ -574,6 +582,8 @@ tui_reg_command (const char *args, int from_tty)
       /* Make sure the curses mode is enabled.  */
       tui_enable ();
 
+      tui_suppress_output suppress;
+
       /* Make sure the register window is visible.  If not, select an
 	 appropriate layout.  We need to do this before trying to run the
 	 'next' or 'prev' commands.  */
diff --git a/gdb/tui/tui-regs.h b/gdb/tui/tui-regs.h
index 92df6183677..3ba4b28ddf6 100644
--- a/gdb/tui/tui-regs.h
+++ b/gdb/tui/tui-regs.h
@@ -70,6 +70,8 @@ struct tui_data_window : public tui_win_info
 
   void refresh_window () override;
 
+  void no_refresh () override;
+
   const char *name () const override
   {
     return DATA_NAME;
diff --git a/gdb/tui/tui-wingeneral.c b/gdb/tui/tui-wingeneral.c
index 17be3e23e9a..091cd73986e 100644
--- a/gdb/tui/tui-wingeneral.c
+++ b/gdb/tui/tui-wingeneral.c
@@ -30,13 +30,51 @@
 
 #include "gdb_curses.h"
 
+/* This is true if we're currently suppressing output, via
+   wnoutrefresh.  This is needed in case we create a new window while
+   in this mode.  */
+
+static bool suppress_output;
+
+/* See tui-data.h.  */
+
+tui_suppress_output::tui_suppress_output ()
+  : m_saved_suppress (suppress_output)
+{
+  suppress_output = true;
+
+  for (const auto &win : all_tui_windows ())
+    win->no_refresh ();
+}
+
+/* See tui-data.h.  */
+
+tui_suppress_output::~tui_suppress_output ()
+{
+  suppress_output = m_saved_suppress;
+  if (!suppress_output)
+    doupdate ();
+
+  for (const auto &win : all_tui_windows ())
+    win->refresh_window ();
+}
+
+/* See tui-data.h.  */
+
+void
+tui_wrefresh (WINDOW *win)
+{
+  if (!suppress_output)
+    wrefresh (win);
+}
+
 /* See tui-data.h.  */
 
 void
 tui_gen_win_info::refresh_window ()
 {
   if (handle != NULL)
-    wrefresh (handle.get ());
+    tui_wrefresh (handle.get ());
 }
 
 /* Draw a border arround the window.  */
@@ -133,7 +171,11 @@ tui_gen_win_info::make_window ()
 {
   handle.reset (newwin (height, width, y, x));
   if (handle != NULL)
-    scrollok (handle.get (), TRUE);
+    {
+      if (suppress_output)
+	wnoutrefresh (handle.get ());
+      scrollok (handle.get (), TRUE);
+    }
 }
 
 void
diff --git a/gdb/tui/tui-wingeneral.h b/gdb/tui/tui-wingeneral.h
index 9995250ce9a..982a9364516 100644
--- a/gdb/tui/tui-wingeneral.h
+++ b/gdb/tui/tui-wingeneral.h
@@ -33,4 +33,27 @@ extern void tui_unhighlight_win (struct tui_win_info *);
 extern void tui_highlight_win (struct tui_win_info *);
 extern void tui_refresh_all ();
 
+/* An RAII class that suppresses output on construction (calling
+   wnoutrefresh on the existing windows), and then flushes the output
+   (via doupdate) when destroyed.  */
+
+class tui_suppress_output
+{
+public:
+
+  tui_suppress_output ();
+  ~tui_suppress_output ();
+
+  DISABLE_COPY_AND_ASSIGN (tui_suppress_output);
+
+private:
+
+  /* Save the state of the suppression global.  */
+  bool m_saved_suppress;
+};
+
+/* Call wrefresh on the given window.  However, if output is being
+   suppressed via tui_suppress_output, do not call wrefresh.  */
+extern void tui_wrefresh (WINDOW *win);
+
 #endif /* TUI_TUI_WINGENERAL_H */
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index 6fdfa4579f1..b0d31c3bc0c 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -30,6 +30,7 @@
 #include "tui/tui-regs.h"
 #include "tui/tui-stack.h"
 #include "tui/tui-win.h"
+#include "tui/tui-wingeneral.h"
 #include "tui/tui-winsource.h"
 #include "tui/tui-source.h"
 #include "target.h"
@@ -575,6 +576,7 @@ tui_disable_command (const char *args, int from_tty)
 void
 tui_show_assembly (struct gdbarch *gdbarch, CORE_ADDR addr)
 {
+  tui_suppress_output suppress;
   tui_add_win_to_layout (DISASSEM_WIN);
   tui_update_source_windows_with_addr (gdbarch, addr);
 }
-- 
2.17.2

[PATCH 2/3] Make "disassemble" always use TUI disassembly window

[Tom Tromey]

Currently the "disassemble" command will use the TUI disassembly
window if it is already showing.  I think it makes more sense to
unconditionally switch to it.  This patch implements this.

gdb/ChangeLog
2019-12-27  Tom Tromey  <tom@tromey.com>

	* cli/cli-cmds.c (print_disassembly): Just check tui_active.

gdb/testsuite/ChangeLog
2019-12-27  Tom Tromey  <tom@tromey.com>

	* gdb.tui/disasm.exp: New file.

Change-Id: I3bf3a93f618b2138c49c28156cf6c6f5ca0fa762
---
 gdb/ChangeLog                    |  4 ++++
 gdb/cli/cli-cmds.c               |  2 +-
 gdb/testsuite/ChangeLog          |  4 ++++
 gdb/testsuite/gdb.tui/disasm.exp | 36 ++++++++++++++++++++++++++++++++
 4 files changed, 45 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.tui/disasm.exp

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index e1410690aaf..b2ef904ff60 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -1338,7 +1338,7 @@ print_disassembly (struct gdbarch *gdbarch, const char *name,
 		   gdb_disassembly_flags flags)
 {
 #if defined(TUI)
-  if (tui_is_window_visible (DISASSEM_WIN))
+  if (tui_active)
     tui_show_assembly (gdbarch, low);
   else
 #endif
diff --git a/gdb/testsuite/gdb.tui/disasm.exp b/gdb/testsuite/gdb.tui/disasm.exp
new file mode 100644
index 00000000000..362aa7087ec
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/disasm.exp
@@ -0,0 +1,36 @@
+# Copyright 2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test that "disassemble" shows the disassembly window.
+
+load_lib "tuiterm.exp"
+
+standard_testfile tui-layout.c
+
+if {[build_executable "failed to prepare" ${testfile} ${srcfile}] == -1} {
+    return -1
+}
+
+Term::clean_restart 24 80 $testfile
+if {![Term::enter_tui]} {
+    unsupported "TUI not supported"
+}
+
+set text [Term::get_all_lines]
+gdb_assert {![string match "No Source Available" $text]} \
+    "initial source listing"
+
+Term::command "disassemble main"
+Term::check_contents "asm window is showing" "$hex <main>"
-- 
2.17.2

[PATCH 0/3] More TUI improvements

[Tom Tromey]

This series holds a few TUI improvements.

Patch 1 is just a bug fix.  Patch 2 changes the behavior in a visible
way, so please give it some thought.  Patch 3 is a visual
optimization.

Tested on x86-64 Fedora 28.

Tom

[PATCH 1/3] Make "file" clear TUI source window

[Tom Tromey]

I noticed that a plain "file" will leave the current source file in
the TUI source window.  Instead, I think, it should clear the source
window.  This patch implements this.

2019-12-21  Tom Tromey  <tom@tromey.com>

	* tui/tui-winsource.c (tui_update_source_windows_with_line):
	Handle case where symtab is null.

gdb/testsuite/ChangeLog
2019-12-22  Tom Tromey  <tom@tromey.com>

	* gdb.tui/main.exp: Add check for plain "file".

Change-Id: I8424acf837f1a47f75bc6a833d1e917d4c10b51e
---
 gdb/ChangeLog                  |  5 +++++
 gdb/testsuite/ChangeLog        |  4 ++++
 gdb/testsuite/gdb.tui/main.exp |  7 +++++++
 gdb/tui/tui-winsource.c        | 11 ++++++-----
 4 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/gdb/testsuite/gdb.tui/main.exp b/gdb/testsuite/gdb.tui/main.exp
index 57ddb52bf36..b62e0f420d9 100644
--- a/gdb/testsuite/gdb.tui/main.exp
+++ b/gdb/testsuite/gdb.tui/main.exp
@@ -26,9 +26,16 @@ if {[build_executable "failed to prepare" ${testfile} ${srcfile}] == -1} {
 # Note: don't pass the executable here
 Term::clean_restart 24 80
 
+# Later on we'd like to avoid having to answer a question.
+gdb_test_no_output "set interactive-mode off"
+
 if {![Term::enter_tui]} {
     unsupported "TUI not supported"
 }
 
 Term::command "file [standard_output_file $testfile]"
 Term::check_contents "show main after file" "\\|.*21 *return 0"
+
+# Ensure that "file" clears the source window.
+Term::command "file"
+Term::check_contents "file clears window" "No Source Available"
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index 1ac650b6987..d720e99c6ab 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -212,11 +212,12 @@ tui_update_source_windows_with_addr (struct gdbarch *gdbarch, CORE_ADDR addr)
 void
 tui_update_source_windows_with_line (struct symtab_and_line sal)
 {
-  if (!sal.symtab)
-    return;
-
-  find_line_pc (sal.symtab, sal.line, &sal.pc);
-  struct gdbarch *gdbarch = get_objfile_arch (SYMTAB_OBJFILE (sal.symtab));
+  struct gdbarch *gdbarch = nullptr;
+  if (sal.symtab != nullptr)
+    {
+      find_line_pc (sal.symtab, sal.line, &sal.pc);
+      gdbarch = get_objfile_arch (SYMTAB_OBJFILE (sal.symtab));
+    }
 
   for (struct tui_source_window_base *win_info : tui_source_windows ())
     win_info->update_source_window (gdbarch, sal);
-- 
2.17.2

Re: [PATCH 2/3] Make "disassemble" always use TUI disassembly window

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> Currently the "disassemble" command will use the TUI disassembly
Tom> window if it is already showing.  I think it makes more sense to
Tom> unconditionally switch to it.  This patch implements this.

I wonder if this should be limited to the case where the options to
disassemble are compatible with what the TUI does.

Tom

Re: [PATCH 0/3] More TUI improvements

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> This series holds a few TUI improvements.
Tom> Patch 1 is just a bug fix.  Patch 2 changes the behavior in a visible
Tom> way, so please give it some thought.  Patch 3 is a visual
Tom> optimization.

Tom> Tested on x86-64 Fedora 28.

I'm going to check in patch 1 and patch 3, but hold patch 2 for
discussion.

Tom

Re: [PATCH 2/3] Make "disassemble" always use TUI disassembly window

[Andrew Burgess]

* Tom Tromey <tom@tromey.com> [2020-01-19 13:07:30 -0700]:

> >>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
> 
> Tom> Currently the "disassemble" command will use the TUI disassembly
> Tom> window if it is already showing.  I think it makes more sense to
> Tom> unconditionally switch to it.  This patch implements this.
> 
> I wonder if this should be limited to the case where the options to
> disassemble are compatible with what the TUI does.

I've wondered for a while if we should have something like:

  set tui disassemble-flags ...

where a user can set the flags used by the disassembler.  My main
interest initially was /r, but it might be nice if /m and /s could be
supported too, though I'm not quite sure how that might look or work.

Anyway, if we had something like this then using a disassemble command
could effectively be an alternative path to set the flags. So
something like:

  (gdb) show tui disassemble-flags
  TUI will disassemble using no flags.
  (gdb) disassemble /r 0x.....
  # TUI ASM window updates, and includes opcodes.
  (gdb) show tui disassemble-flags
  TUI will disassemble using flags /r.
  # User doesn't want the opcodes any more, no problem.
  (gdb) set tui disassemble-flags    # <-- passing nothing here to
                                     #  clear the flags
  # TUI updates, but not longer shows opcodes.

Just a thought.

Thanks,
Andrew

Re: [PATCH 2/3] Make "disassemble" always use TUI disassembly window

[Tom Tromey]

Tom> I wonder if this should be limited to the case where the options to
Tom> disassemble are compatible with what the TUI does.

Andrew> I've wondered for a while if we should have something like:

Andrew>   set tui disassemble-flags ...

Andrew> where a user can set the flags used by the disassembler.  My main
Andrew> interest initially was /r, but it might be nice if /m and /s could be
Andrew> supported too, though I'm not quite sure how that might look or work.

I've considered this too (and also adding support for the new binutils
mode that graphically shows jump targets).

However, I wasn't sure how these flags would interact with the current
TUI model of disassembly, where it shows a window on the entire memory
space, and not just a single function.

Tom