public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCHv3 10/15] gdb/tui: fairer distribution of excess space during apply
Date: Mon,  7 Mar 2022 22:13:42 +0000	[thread overview]
Message-ID: <12dd572021d8f0bc711dc711b64cfbcfe8364a5f.1646691034.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1646691034.git.aburgess@redhat.com>

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    |  4 ++--
 gdb/testsuite/gdb.tui/empty.exp    | 14 +++++++----
 gdb/testsuite/gdb.tui/regs.exp     |  4 ++--
 gdb/testsuite/gdb.tui/winwidth.exp | 17 ++++++-------
 gdb/tui/tui-layout.c               | 38 ++++++++++++++++++++++++------
 5 files changed, 53 insertions(+), 24 deletions(-)

diff --git a/gdb/testsuite/gdb.tui/basic.exp b/gdb/testsuite/gdb.tui/basic.exp
index 0e9f2e3eab2..7afb28b0858 100644
--- a/gdb/testsuite/gdb.tui/basic.exp
+++ b/gdb/testsuite/gdb.tui/basic.exp
@@ -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
diff --git a/gdb/testsuite/gdb.tui/empty.exp b/gdb/testsuite/gdb.tui/empty.exp
index ef14fe4dac0..3efa55d812e 100644
--- a/gdb/testsuite/gdb.tui/empty.exp
+++ b/gdb/testsuite/gdb.tui/empty.exp
@@ -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
+	}
     }
 }
diff --git a/gdb/testsuite/gdb.tui/regs.exp b/gdb/testsuite/gdb.tui/regs.exp
index 178eba03f19..2f3482f5d38 100644
--- a/gdb/testsuite/gdb.tui/regs.exp
+++ b/gdb/testsuite/gdb.tui/regs.exp
@@ -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.
diff --git a/gdb/testsuite/gdb.tui/winwidth.exp b/gdb/testsuite/gdb.tui/winwidth.exp
index b0a838b578f..1767b25c8d3 100644
--- a/gdb/testsuite/gdb.tui/winwidth.exp
+++ b/gdb/testsuite/gdb.tui/winwidth.exp
@@ -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
 }
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 8758bfe70e7..4988d48a8d5 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -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)
 	{
-- 
2.25.4


  parent reply	other threads:[~2022-03-07 22:14 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-28 15:55 [PATCH 0/7] TUI command changes, including new winwidth command Andrew Burgess
2022-01-28 15:55 ` [PATCH 1/7] gdb/tui: add window width information to 'info win' output Andrew Burgess
2022-01-28 17:00   ` Eli Zaretskii
2022-02-06 13:43   ` Andrew Burgess
2022-01-28 15:55 ` [PATCH 2/7] gdb/doc: update docs for 'info win' and 'winheight' commands Andrew Burgess
2022-01-28 17:03   ` Eli Zaretskii
2022-02-06 13:43     ` Andrew Burgess
2022-01-28 15:55 ` [PATCH 3/7] gdb: move some commands into the tui namespace Andrew Burgess
2022-01-28 17:04   ` Eli Zaretskii
2022-01-28 15:55 ` [PATCH 4/7] gdb/tui: rename tui_layout_base::adjust_size to ::set_height Andrew Burgess
2022-01-28 15:55 ` [PATCH 5/7] gdb/tui: rename tui_layout_split:set_weights_from_heights Andrew Burgess
2022-01-28 15:55 ` [PATCH 6/7] gdb/testing/tui: add new functionality to tuiterm.exp Andrew Burgess
2022-01-28 15:55 ` [PATCH 7/7] gdb/tui: add new 'tui window width' command and 'winwidth' alias Andrew Burgess
2022-01-28 17:05   ` Eli Zaretskii
2022-02-06 14:12 ` [PATCHv2 00/15] TUI changes, new winwidth command and resizing changes Andrew Burgess
2022-02-06 14:12   ` [PATCHv2 01/15] gdb: move some commands into the tui namespace Andrew Burgess
2022-02-06 15:50     ` Eli Zaretskii
2022-02-06 14:12   ` [PATCHv2 02/15] gdb/tui: rename tui_layout_base::adjust_size to ::set_height Andrew Burgess
2022-02-06 14:12   ` [PATCHv2 03/15] gdb/tui: rename tui_layout_split:set_weights_from_heights Andrew Burgess
2022-02-06 14:12   ` [PATCHv2 04/15] gdb/testing/tui: add new functionality to tuiterm.exp Andrew Burgess
2022-03-04 16:29     ` Tom Tromey
2022-02-06 14:12   ` [PATCHv2 05/15] gdb/tui: add new 'tui window width' command and 'winwidth' alias Andrew Burgess
2022-02-06 15:52     ` Eli Zaretskii
2022-02-09 15:33       ` Andrew Burgess
2022-02-09 17:03         ` Eli Zaretskii
2022-03-03 18:52     ` Pedro Alves
2022-02-06 14:12   ` [PATCHv2 06/15] gdb/tui: add a tui debugging flag Andrew Burgess
2022-02-06 15:53     ` Eli Zaretskii
2022-03-04 16:35     ` Tom Tromey
2022-02-06 14:12   ` [PATCHv2 07/15] gdb/tui: add left_boxed_p and right_boxed_p member functions Andrew Burgess
2022-03-04 16:37     ` Tom Tromey
2022-02-06 14:12   ` [PATCHv2 08/15] gdb/tui/testsuite: refactor new-layout.exp test Andrew Burgess
2022-02-06 14:12   ` [PATCHv2 09/15] gdb/tui: avoid fp exception when applying layouts Andrew Burgess
2022-02-06 14:12   ` [PATCHv2 10/15] gdb/tui: fairer distribution of excess space during apply Andrew Burgess
2022-03-04 16:42     ` Tom Tromey
2022-02-06 14:12   ` [PATCHv2 11/15] gdb/tui: allow cmd window to change size in tui_layout_split::apply Andrew Burgess
2022-02-06 14:12   ` [PATCHv2 12/15] gdb/tui: support placing the cmd window into a horizontal layout Andrew Burgess
2022-03-04 17:17     ` Tom Tromey
2022-03-07 20:05       ` Andrew Burgess
2022-03-07 21:24         ` Tom Tromey
2022-02-06 14:12   ` [PATCHv2 13/15] gdb/testsuite: some additional tests in gdb.tui/scroll.exp Andrew Burgess
2022-02-06 14:12   ` [PATCHv2 14/15] gdb/tui: relax restrictions on window max height and width Andrew Burgess
2022-03-04 17:20     ` Tom Tromey
2022-03-07 20:08       ` Andrew Burgess
2022-02-06 14:12   ` [PATCHv2 15/15] gdb/tui: fair split of delta after a resize Andrew Burgess
2022-03-04 17:22     ` Tom Tromey
2022-03-07 22:07       ` Andrew Burgess
2022-03-07 23:42         ` Tom Tromey
2022-02-21 17:29   ` [PATCHv2 00/15] TUI changes, new winwidth command and resizing changes Andrew Burgess
2022-03-02 17:59     ` Andrew Burgess
2022-03-07 22:13   ` [PATCHv3 " Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 01/15] gdb: move some commands into the tui namespace Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 02/15] gdb/tui: rename tui_layout_base::adjust_size to ::set_height Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 03/15] gdb/tui: rename tui_layout_split:set_weights_from_heights Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 04/15] gdb/testing/tui: add new functionality to tuiterm.exp Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 05/15] gdb/tui: add new 'tui window width' command and 'winwidth' alias Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 06/15] gdb/tui: add a tui debugging flag Andrew Burgess
2022-03-08 12:16       ` Eli Zaretskii
2022-03-09 11:48         ` Andrew Burgess
2022-03-09 12:58           ` Eli Zaretskii
2022-03-09 17:53           ` Tom Tromey
2022-03-07 22:13     ` [PATCHv3 07/15] gdb/tui: add left_boxed_p and right_boxed_p member functions Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 08/15] gdb/tui/testsuite: refactor new-layout.exp test Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 09/15] gdb/tui: avoid fp exception when applying layouts Andrew Burgess
2022-03-07 22:13     ` Andrew Burgess [this message]
2022-03-07 22:13     ` [PATCHv3 11/15] gdb/tui: allow cmd window to change size in tui_layout_split::apply Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 12/15] gdb/tui: support placing the cmd window into a horizontal layout Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 13/15] gdb/testsuite: some additional tests in gdb.tui/scroll.exp Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 14/15] gdb/tui: relax restrictions on window max height and width Andrew Burgess
2022-03-07 22:13     ` [PATCHv3 15/15] gdb/tui: fair split of delta after a resize Andrew Burgess
2022-03-21 17:52     ` [PATCHv3 00/15] TUI changes, new winwidth command and resizing changes Andrew Burgess
2022-03-30  9:13       ` Andrew Burgess
2022-04-03 14:43         ` Andrew Burgess
2022-03-04 17:23 ` [PATCH 0/7] TUI command changes, including new winwidth command Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=12dd572021d8f0bc711dc711b64cfbcfe8364a5f.1646691034.git.aburgess@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).