From patchwork Mon Nov 20 16:19:11 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 80373 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 282E73858424 for ; Mon, 20 Nov 2023 16:17:33 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by sourceware.org (Postfix) with ESMTPS id CD4783858D39 for ; Mon, 20 Nov 2023 16:17:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CD4783858D39 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org CD4783858D39 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2001:67c:2178:6::1c ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700497040; cv=none; b=DpiGC4WqPWSwkwJnKEN1by+Ir+rcyKYYlSHlPj0N/VL6Nl9gR7KMUk0uLIR9nJhGD4MwiNTRf76vBr0BIoULwa6i7amobimYHK4bC45opGECW3Uqh79atvJfJgGy7OqujonqhXP9w4cFWKqCfjc0BIk/n75ky7yNm4sQ0kT7aQo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700497040; c=relaxed/simple; bh=nYXTft9qnINX7m5h9/lyLdu9uyqkpOeirssHis2rCNI=; h=DKIM-Signature:DKIM-Signature:From:To:Subject:Date:Message-Id: MIME-Version; b=pFtI5S40gRnoijUSe0pl/ogcMINf0j+a4n9THzQJU/xBjcIYRZeyIB/pZmsjJNG9XBuRjYALDX2N5f+mm7dsdLxDco13SP40odn5/+YOZ8+9cHgH4JGjgHUj4FwVWgbkxWGyPT4Iajd0ZNwU/uO6CLl+jbS8KpT+/bgqtZLb5dw= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id F1C5E21910 for ; Mon, 20 Nov 2023 16:17:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1700497037; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=Ugv8CId7ujACuKOMN1bxYdZBkBx1II6NHmk5vKsro3o=; b=b4WvxzbKZkNJhFYt9QFQ5nP1ou9tJCaa+8BdrAspxQw3j7+H9EqOoYk0vs/PQ6kHWWlUtq 3Dz2zM82kKB11qKnwYn4jVy1I3/CXzRgmvyyXLB7KUzS39D3ZGZUZ8aq1aEIKo3BX2L+Xp 16thTgCHw1VyVMus6Ptqu3MAyxQm79k= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1700497037; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=Ugv8CId7ujACuKOMN1bxYdZBkBx1II6NHmk5vKsro3o=; b=2XvyzRSg3pM7jsbsD8B+nIwBFXHqJwK/9p8VMrOxt/9lUR6E1H3zSjIG69nHfCq6PvMa9K RCmvXfEV+CPWKnCA== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id D4CD113499 for ; Mon, 20 Nov 2023 16:17:17 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id LE/OMo2GW2VLZQAAMHmgww (envelope-from ) for ; Mon, 20 Nov 2023 16:17:17 +0000 From: Tom de Vries To: gdb-patches@sourceware.org Subject: [PATCH] [gdb/tui] Refactor tui_layout_split::apply Date: Mon, 20 Nov 2023 17:19:11 +0100 Message-Id: <20231120161911.19715-1-tdevries@suse.de> X-Mailer: git-send-email 2.35.3 MIME-Version: 1.0 Authentication-Results: smtp-out1.suse.de; none X-Spam-Level: X-Spam-Score: 0.70 X-Spamd-Result: default: False [0.70 / 50.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; FROM_HAS_DN(0.00)[]; R_MISSING_CHARSET(2.50)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_GOOD(-0.10)[text/plain]; PREVIOUSLY_DELIVERED(0.00)[gdb-patches@sourceware.org]; BROKEN_CONTENT_TYPE(1.50)[]; RCPT_COUNT_ONE(0.00)[1]; NEURAL_HAM_LONG(-1.00)[-1.000]; TO_DN_NONE(0.00)[]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; NEURAL_HAM_SHORT(-0.20)[-0.997]; MID_CONTAINS_FROM(1.00)[]; FUZZY_BLOCKED(0.00)[rspamd.com]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; RCVD_COUNT_TWO(0.00)[2]; RCVD_TLS_ALL(0.00)[]; BAYES_HAM(-3.00)[100.00%] X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org 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 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);