[gdb/tui] Refactor tui_layout_split::apply

Message ID 20231120161911.19715-1-tdevries@suse.de
State New
Headers
Series [gdb/tui] Refactor tui_layout_split::apply |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Tom de Vries Nov. 20, 2023, 4:19 p.m. UTC
  I tried to make some modifications in tui_layout_split::apply, and ran
into difficulty understanding it.

The overall approach is explained well, but I found the implementation hard to
understand because of:
- large and complex loop bodies, and
- handling disjoint issues in the same loop, where one issue is addressed for
  fixed-size layout and another one for variable-sized layout.

I've tried to refactor things such that loop bodies handle:
- not too much things at the same time, allowing a clear comment before the
  loop, and
- either fixed-size or variable-size layout only, if that's appropriate.

Finally, I've pulled apart this needlessly complex statement:
...
      /* If we fall off the bottom, just make allocations overlap.
	 GIGO.  */
      if (size_accum + info[i].size > maximum)
	size_accum = maximum - info[i].size;
      else if (info[i].share_box)
	--size_accum;
...
where the comment in fact just matches the first two lines.

Tested on x86_64-linux.
---
 gdb/tui/tui-layout.c | 155 +++++++++++++++++++++++++++----------------
 1 file changed, 96 insertions(+), 59 deletions(-)


base-commit: fdb4c2e02e6600b51f38a60cd70882887007cbdf
  

Patch

diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 159445dc520..f0cc1bf744c 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -826,58 +826,92 @@  tui_layout_split::apply (int x_, int y_, int width_, int height_,
   tui_debug_printf ("weights are: %s",
 		    tui_debug_weights_to_string ().c_str ());
 
-  /* Step 1: Find the min and max size of each sub-layout.
-     Fixed-sized layouts are given their desired size, and then the
-     remaining space is distributed among the remaining windows
-     according to the weights given.  */
-  int available_size = m_vertical ? height : width;
-  int last_index = -1;
-  int total_weight = 0;
+  /* Overall approach:
+     - find the min and max size of each sub-layout.
+     - give fixed-sized (min size == max size) layouts their desired size.
+     - distribute the remaining space among the variable-sized layouts,
+       according to the weights given.  */
+
+  /* Find the command window.  */
+  int cmd_win = -1;
+  bool cmd_win_already_exists = TUI_CMD_WIN != nullptr;
+  if (cmd_win_already_exists)
+    for (int i = 0; i < m_splits.size (); ++i)
+      if (m_splits[i].layout->get_name () != nullptr
+	  && strcmp (m_splits[i].layout->get_name (), "cmd") == 0)
+	{
+	  cmd_win = i;
+	  break;
+	}
+
+  /* Calculate info[i].min_size and info[i].max_size.  */
   for (int i = 0; i < m_splits.size (); ++i)
     {
-      bool cmd_win_already_exists = TUI_CMD_WIN != nullptr;
-
       /* Always call get_sizes, to ensure that the window is
 	 instantiated.  This is a bit gross but less gross than adding
 	 special cases for this in other places.  */
       m_splits[i].layout->get_sizes (m_vertical, &info[i].min_size,
 				     &info[i].max_size);
+    }
 
-      if (preserve_cmd_win_size_p
-	  && cmd_win_already_exists
-	  && m_splits[i].layout->get_name () != nullptr
-	  && strcmp (m_splits[i].layout->get_name (), "cmd") == 0)
+  /* Update info[cmd_win].min_size and info[cmd_win].max_size, if
+     necessary.  */
+  if (preserve_cmd_win_size_p && cmd_win != -1)
+    {
+      /* Save the old cmd window information, in case we need to
+	 restore it later.  */
+      old_cmd_info.emplace (cmd_win, info[cmd_win].min_size,
+			    info[cmd_win].max_size);
+
+      /* If this layout has never been applied, then it means the
+	 user just changed the layout.  In this situation, it's
+	 desirable to keep the size of the command window the
+	 same.  Setting the min and max sizes this way ensures
+	 that the resizing step, below, does the right thing with
+	 this window.  */
+      info[cmd_win].min_size = (m_vertical
+				? TUI_CMD_WIN->height
+				: TUI_CMD_WIN->width);
+      info[cmd_win].max_size = info[cmd_win].min_size;
+    }
+
+  /* Calculate info[i].share_box.  */
+  for (int i = 1; i < m_splits.size (); ++i)
+    {
+      /* Two adjacent boxed windows will share a border, making a bit
+	 more size available.  */
+      info[i].share_box = (m_splits[i - 1].layout->last_edge_has_border_p ()
+			   && m_splits[i].layout->first_edge_has_border_p ());
+    }
+
+  /* Give fixed-sized layouts their desired size.  Calculate what's left for
+     variable-sized layouts in available_size.  */
+  int available_size = m_vertical ? height : width;
+  for (int i = 0; i < m_splits.size (); ++i)
+    {
+      if (info[i].min_size != info[i].max_size)
 	{
-	  /* Save the old cmd window information, in case we need to
-	     restore it later.  */
-	  old_cmd_info.emplace (i, info[i].min_size, info[i].max_size);
-
-	  /* If this layout has never been applied, then it means the
-	     user just changed the layout.  In this situation, it's
-	     desirable to keep the size of the command window the
-	     same.  Setting the min and max sizes this way ensures
-	     that the resizing step, below, does the right thing with
-	     this window.  */
-	  info[i].min_size = (m_vertical
-			      ? TUI_CMD_WIN->height
-			      : TUI_CMD_WIN->width);
-	  info[i].max_size = info[i].min_size;
+	  /* Not a fixed-size layout.  */
+	  continue;
 	}
 
+      info[i].size = info[i].min_size;
+      available_size -= info[i].size;
+    }
+
+  /* Calculate total_weight of variable-size layouts.  */
+  int last_index = -1;
+  int total_weight = 0;
+  for (int i = 0; i < m_splits.size (); ++i)
+    {
       if (info[i].min_size == info[i].max_size)
-	available_size -= info[i].min_size;
-      else
 	{
-	  last_index = i;
-	  total_weight += m_splits[i].weight;
+	  /* Not a variable-size layout.  */
+	  continue;
 	}
 
-      /* Two adjacent boxed windows will share a border, making a bit
-	 more size available.  */
-      if (i > 0
-	  && m_splits[i - 1].layout->last_edge_has_border_p ()
-	  && m_splits[i].layout->first_edge_has_border_p ())
-	info[i].share_box = true;
+      last_index = i;
+      total_weight += m_splits[i].weight;
     }
 
   /* If last_index is set then we have a window that is not of a fixed
@@ -886,31 +920,32 @@  tui_layout_split::apply (int x_, int y_, int width_, int height_,
      want a floating-point exception).  */
   gdb_assert (last_index == -1 || total_weight > 0);
 
-  /* Step 2: Compute the size of each sub-layout.  Fixed-sized items
-     are given their fixed size, while others are resized according to
-     their weight.  */
+  /* Give variable-sized layouts a size according to their weight.  Accumulate
+     the total into used_size.  */
   int used_size = 0;
   for (int i = 0; i < m_splits.size (); ++i)
     {
-      if (info[i].min_size != info[i].max_size)
+      if (info[i].min_size == info[i].max_size)
 	{
-	  /* Compute the height and clamp to the allowable range.  */
-	  info[i].size = available_size * m_splits[i].weight / total_weight;
-	  if (info[i].size > info[i].max_size)
-	    info[i].size = info[i].max_size;
-	  if (info[i].size < info[i].min_size)
-	    info[i].size = info[i].min_size;
-	  /* Keep a total of all the size we've used so far (we gain some
-	     size back if this window can share a border with a preceding
-	     window).  Any unused space will be distributed between all of
-	     the other windows (while respecting min/max sizes) later in
-	     this function.  */
-	  used_size += info[i].size;
-	  if (info[i].share_box)
-	    --used_size;
+	  /* Not a variable-size layout.  */
+	  continue;
 	}
-      else
+
+      /* Compute the height and clamp to the allowable range.  */
+      info[i].size = available_size * m_splits[i].weight / total_weight;
+      if (info[i].size > info[i].max_size)
+	info[i].size = info[i].max_size;
+      if (info[i].size < info[i].min_size)
 	info[i].size = info[i].min_size;
+
+      /* Keep a total of all the size we've used so far (we gain some
+	 size back if this window can share a border with a preceding
+	 window).  Any unused space will be distributed between all of
+	 the other windows (while respecting min/max sizes) later in
+	 this function.  */
+      used_size += info[i].size;
+      if (info[i].share_box)
+	--used_size;
     }
 
   if (debug_tui)
@@ -1014,17 +1049,19 @@  tui_layout_split::apply (int x_, int y_, int width_, int height_,
 	}
     }
 
-  /* Step 3: Resize.  */
+  /* Apply the calculated layout sizes.  */
   int size_accum = 0;
   const int maximum = m_vertical ? height : width;
   for (int i = 0; i < m_splits.size (); ++i)
     {
+      if (info[i].share_box)
+	--size_accum;
+
       /* If we fall off the bottom, just make allocations overlap.
 	 GIGO.  */
       if (size_accum + info[i].size > maximum)
 	size_accum = maximum - info[i].size;
-      else if (info[i].share_box)
-	--size_accum;
+
       if (m_vertical)
 	m_splits[i].layout->apply (x, y + size_accum, width, info[i].size,
 				   preserve_cmd_win_size_p);