gdb/tui: fairer distribution of excess space during apply
authorAndrew Burgess <aburgess@redhat.com>
Tue, 1 Feb 2022 10:28:18 +0000 (10:28 +0000)
committerAndrew Burgess <aburgess@redhat.com>
Sun, 3 Apr 2022 14:31:47 +0000 (15:31 +0100)
When applying layouts gdb computes the size of each window (or rather,
each sub-layout within a layout) using integer arithmetic.  As this
rounds down the results, then, when all sub-layouts are sized, there
is the possibility that we have some space left over.

Currently, this space is just assigned to an arbitrary sub-layout.

This can result in some unbalanced results.  Consider this set of
steps with current master:

  (gdb) tui enable
  (gdb) layout regs
  (gdb) info win
  Name       Lines Columns Focus
  regs           7      80
  src            9      80 (has focus)
  status         1      80
  cmd            8      80

Notice the weird split between the src and regs windows, the original
layout specification has these windows given equal weight.  The
problem is that, with rounding, both the regs and src windows are
initially sized to 7, the extra 2 lines are then arbitrarily added to
the src window.

In this commit, rather than add all the extra space to one single
window, I instead hand out the extra space 1 line at a time, looping
over all the sub-layouts.  We take care to respect the min/max sizes,
and so, we now get this result:

  (gdb) tui enable
  (gdb) layout regs
  (gdb) info win
  Name       Lines Columns Focus
  regs           8      80
  src            8      80 (has focus)
  status         1      80
  cmd            8      80

This looks more natural to me.

This is obviously a change in behaviour, and so, lots of the existing
tests need to be updated to take this into account.  None of the
changes are huge, it's just a line or two (or column for width) moved
between windows.

gdb/testsuite/gdb.tui/basic.exp
gdb/testsuite/gdb.tui/empty.exp
gdb/testsuite/gdb.tui/regs.exp
gdb/testsuite/gdb.tui/winwidth.exp
gdb/tui/tui-layout.c

index 0e9f2e3eab28d9cff6b03f9e5f4fb28d81fd98fb..7afb28b08581a38c2ad89c35a0d179ba2e25fe09 100644 (file)
@@ -104,5 +104,5 @@ Term::command "layout split"
 Term::check_contents "split layout contents" \
     "$main_line *$main_re.*$hex <main>"
 
-Term::check_box "source box in split layout" 0 0 80 7
-Term::check_box "asm box in split layout" 0 6 80 9
+Term::check_box "source box in split layout" 0 0 80 8
+Term::check_box "asm box in split layout" 0 7 80 8
index ef14fe4dac0fbe9342c328e763d782a30cb6e769..3efa55d812ed5bcc64429962c0f9cd61b6b17ba1 100644 (file)
@@ -35,26 +35,26 @@ if {![Term::enter_tui]} {
 set layouts {
     {src src {{0 0 80 15}} {{0 0 90 26}}
        {{"no source" "No Source Available"}}}
-    {regs src-regs {{0 0 80 7} {0 6 80 9}} {{0 0 90 13} {0 12 90 13}}
+    {regs src-regs {{0 0 80 8} {0 7 80 8}} {{0 0 90 13} {0 12 90 13}}
        {
            {"no source" "No Source Available"}
            {"no regs" "Register Values Unavailable"}
        }}
-    {asm asm {{0 0 80 13}} {{0 0 90 26}}
+    {asm asm {{0 0 80 15}} {{0 0 90 26}}
        {
            {"no asm" "No Assembly Available"}
        }}
-    {regs asm-regs {{0 0 80 7} {0 6 80 9}} {{0 0 90 13} {0 12 90 13}}
+    {regs asm-regs {{0 0 80 8} {0 7 80 8}} {{0 0 90 13} {0 12 90 13}}
        {
            {"no asm" "No Assembly Available"}
            {"no regs" "Register Values Unavailable"}
        }}
-    {split split {{0 0 80 6} {0 5 80 8}} {{0 0 90 13} {0 12 90 13}}
+    {split split {{0 0 80 8} {0 7 80 8}} {{0 0 90 13} {0 12 90 13}}
        {
            {"no source" "No Source Available"}
            {"no asm" "No Assembly Available"}
        }}
-    {regs split-regs {{0 0 80 6} {0 5 80 8}} {{0 0 90 13} {0 12 90 13}}
+    {regs split-regs {{0 0 80 8} {0 7 80 8}} {{0 0 90 13} {0 12 90 13}}
        {
            {"no asm" "No Assembly Available"}
            {"no regs" "Register Values Unavailable"}
@@ -97,5 +97,9 @@ foreach layout $layouts {
            check_text $text_list
        }
        Term::resize 24 80
+       with_test_prefix "80x24 again" {
+           check_boxes $small_boxes
+           check_text $text_list
+       }
     }
 }
index 178eba03f19cfb4b17c8b1bedbbc3e80c7ebe3c3..2f3482f5d38f255a57e3d42fecbdb4dc25e9397f 100644 (file)
@@ -38,8 +38,8 @@ if {![Term::enter_tui]} {
 Term::check_contents "source at startup" "\\|.*21 *return 0"
 
 Term::command "layout regs"
-Term::check_box "register box" 0 0 80 7
-Term::check_box "source box in regs layout" 0 6 80 9
+Term::check_box "register box" 0 0 80 8
+Term::check_box "source box in regs layout" 0 7 80 8
 
 set text [Term::get_line 1]
 # Just check for any register window content at all.
index b0a838b578fc129f8cabe3fe4ae2ed0f7b4de6f5..1767b25c8d37f3ba2b12ae1dd4457b74bbf99201 100644 (file)
@@ -39,24 +39,25 @@ with_test_prefix "original window sizes" {
 }
 
 with_test_prefix "after src +5" {
-    Term::check_box "source box" 0 0 44 15
-    Term::check_box "asm box" 43 0 37 15
+    Term::check_box "source box" 0 0 45 15
+    Term::check_box "asm box" 44 0 36 15
     Term::command "winwidth asm -5"
 }
 
 with_test_prefix "after asm -5" {
-    Term::check_box "source box" 0 0 48 15
-    Term::check_box "asm box" 47 0 33 15
+    Term::dump_screen
+    Term::check_box "source box" 0 0 50 15
+    Term::check_box "asm box" 49 0 31 15
     Term::command "winwidth asm +8"
 }
 
 with_test_prefix "after asm +8" {
-    Term::check_box "source box" 0 0 39 15
-    Term::check_box "asm box" 38 0 42 15
+    Term::check_box "source box" 0 0 42 15
+    Term::check_box "asm box" 41 0 39 15
     Term::command "winwidth src -2"
 }
 
 with_test_prefix "after src -2" {
-    Term::check_box "source box" 0 0 36 15
-    Term::check_box "asm box" 35 0 45 15
+    Term::check_box "source box" 0 0 40 15
+    Term::check_box "asm box" 39 0 41 15
 }
index 52105cca6121360cfbef0160f75eeab4e64ed15f..30beefddfdb9f9e6aadc95cf7f852f66bda08817 100644 (file)
@@ -810,11 +810,11 @@ tui_layout_split::apply (int x_, int y_, int width_, int height_)
            info[i].size = info[i].max_size;
          if (info[i].size < info[i].min_size)
            info[i].size = info[i].min_size;
-         /* If there is any leftover size, just redistribute it to the
-            last resizeable window, by dropping it from the allocated
-            size.  We could try to be fancier here perhaps, by
-            redistributing this size among all windows, not just the
-            last window.  */
+         /* 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;
@@ -834,9 +834,33 @@ tui_layout_split::apply (int x_, int y_, int width_, int height_)
     }
 
   /* Allocate any leftover size.  */
-  if (available_size >= used_size && last_index != -1)
+  if (available_size > used_size && last_index != -1)
     {
-      info[last_index].size += available_size - used_size;
+      /* Loop over all windows until all available space is used up.  */
+      bool found_window_that_can_grow_p = true;
+      for (int idx = last_index;
+          available_size > used_size;
+          idx = (idx + 1) % m_splits.size ())
+       {
+         /* Once we have visited all of the windows, check that we did
+            manage to allocate some more space.  This prevents us getting
+            stuck in the loop forever if we can't allocate anything
+            more.  */
+         if (idx == last_index)
+           {
+             if (!found_window_that_can_grow_p)
+               break;
+             found_window_that_can_grow_p = false;
+           }
+
+         if (available_size > used_size
+             && info[idx].size < info[idx].max_size)
+           {
+             found_window_that_can_grow_p = true;
+             info[idx].size += 1;
+             used_size += 1;
+           }
+       }
 
       if (debug_tui)
        {