changeset 34383:a84fe48ae523 v9.1.0118

patch 9.1.0118: Use different restoration strategy in win_splitmove Commit: https://github.com/vim/vim/commit/704966c2545897dfcf426dd9ef946aeb6fa80c38 Author: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Tue Feb 20 22:00:33 2024 +0100 patch 9.1.0118: Use different restoration strategy in win_splitmove Problem: saving and restoring all frames to split-move is overkill now that WinNewPre is not fired when split-moving. Solution: defer the flattening of frames until win_split_ins begins reorganising them, and attempt to restore the layout by undoing our changes. (Sean Dewar) This also means we no longer must allocate. related: #14042 Signed-off-by: Sean Dewar <6256228+seandewar@users.noreply.github.com> Signed-off-by: Christian Brabandt <cb@256bit.org>
author Christian Brabandt <cb@256bit.org>
date Tue, 20 Feb 2024 22:30:07 +0100
parents 17d926feeb82
children 1e826423694b
files src/autocmd.c src/proto/window.pro src/version.c src/window.c
diffstat 4 files changed, 145 insertions(+), 182 deletions(-) [+]
line wrap: on
line diff
--- a/src/autocmd.c
+++ b/src/autocmd.c
@@ -1607,7 +1607,7 @@ aucmd_prepbuf(
 	p_acd = FALSE;
 #endif
 
-	(void)win_split_ins(0, WSP_TOP | WSP_FORCE_ROOM, auc_win, 0);
+	(void)win_split_ins(0, WSP_TOP | WSP_FORCE_ROOM, auc_win, 0, NULL);
 	(void)win_comp_pos();   // recompute window positions
 	p_ea = save_ea;
 #ifdef FEAT_AUTOCHDIR
@@ -1670,7 +1670,7 @@ win_found:
 	    stop_insert_mode = save_stop_insert_mode;
 #endif
 	// Remove the window and frame from the tree of frames.
-	(void)winframe_remove(curwin, &dummy, NULL);
+	(void)winframe_remove(curwin, &dummy, NULL, NULL);
 	win_remove(curwin, NULL);
 
 	// The window is marked as not used, but it is not freed, it can be
--- a/src/proto/window.pro
+++ b/src/proto/window.pro
@@ -7,7 +7,7 @@ void get_wincmd_addr_type(char_u *arg, e
 int check_split_disallowed(win_T *wp);
 int win_split(int size, int flags);
 int win_splitmove(win_T *wp, int size, int flags);
-int win_split_ins(int size, int flags, win_T *new_wp, int dir);
+int win_split_ins(int size, int flags, win_T *new_wp, int dir, frame_T *to_flatten);
 int win_valid_popup(win_T *win);
 int win_valid(win_T *win);
 win_T *win_find_by_id(int id);
@@ -28,7 +28,7 @@ void may_make_initial_scroll_size_snapsh
 void may_trigger_win_scrolled_resized(void);
 void win_close_othertab(win_T *win, int free_buf, tabpage_T *tp);
 void win_free_all(void);
-win_T *winframe_remove(win_T *win, int *dirp, tabpage_T *tp);
+win_T *winframe_remove(win_T *win, int *dirp, tabpage_T *tp, frame_T **to_flatten);
 void close_others(int message, int forceit);
 void unuse_tabpage(tabpage_T *tp);
 void use_tabpage(tabpage_T *tp);
--- a/src/version.c
+++ b/src/version.c
@@ -705,6 +705,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    118,
+/**/
     117,
 /**/
     116,
--- a/src/window.c
+++ b/src/window.c
@@ -30,7 +30,7 @@ static void win_fix_cursor(int normal);
 static void frame_new_height(frame_T *topfrp, int height, int topfirst, int wfh);
 static int frame_fixed_height(frame_T *frp);
 static int frame_fixed_width(frame_T *frp);
-static void frame_add_statusline(frame_T *frp, int adjust_winheight);
+static void frame_add_statusline(frame_T *frp);
 static void frame_new_width(frame_T *topfrp, int width, int leftfirst, int wfw);
 static void frame_add_vsep(frame_T *frp);
 static int frame_minwidth(frame_T *topfrp, win_T *next_curwin);
@@ -53,16 +53,15 @@ static void win_goto_ver(int up, long co
 static void win_goto_hor(int left, long count);
 static void frame_add_height(frame_T *frp, int n);
 static void last_status_rec(frame_T *fr, int statusline);
-
-static int make_snapshot_rec(frame_T *fr, frame_T **frp, int snap_wins);
+static void frame_flatten(frame_T *frp);
+static void winframe_restore(win_T *wp, int dir, frame_T *to_flatten);
+
+static int make_snapshot_rec(frame_T *fr, frame_T **frp);
 static void clear_snapshot(tabpage_T *tp, int idx);
 static void clear_snapshot_rec(frame_T *fr);
 static int check_snapshot_rec(frame_T *sn, frame_T *fr);
 static win_T *restore_snapshot_rec(frame_T *sn, frame_T *fr);
 static win_T *get_snapshot_curwin(int idx);
-static frame_T *make_full_snapshot(void);
-static void restore_full_snapshot(frame_T *sn);
-static void restore_full_snapshot_rec(frame_T *sn);
 
 static int frame_check_height(frame_T *topfrp, int height);
 static int frame_check_width(frame_T *topfrp, int width);
@@ -928,13 +927,16 @@ win_split(int size, int flags)
     else
 	clear_snapshot(curtab, SNAP_HELP_IDX);
 
-    return win_split_ins(size, flags, NULL, 0);
+    return win_split_ins(size, flags, NULL, 0, NULL);
 }
 
 /*
  * When "new_wp" is NULL: split the current window in two.
  * When "new_wp" is not NULL: insert this window at the far
  * top/left/right/bottom.
+ * When "to_flatten" is not NULL: flatten this frame before reorganising frames;
+ * remains unflattened on failure.
+ *
  * On failure, if "new_wp" was not NULL, no changes will have been made to the
  * window layout or sizes.
  * Return FAIL for failure, OK otherwise.
@@ -944,7 +946,8 @@ win_split_ins(
     int		size,
     int		flags,
     win_T	*new_wp,
-    int		dir)
+    int		dir,
+    frame_T	*to_flatten)
 {
     win_T	*wp = new_wp;
     win_T	*oldwin;
@@ -1219,6 +1222,10 @@ win_split_ins(
 	win_init(wp, curwin, flags);
     }
 
+    // Going to reorganize frames now, make sure they're flat.
+    if (to_flatten != NULL)
+	frame_flatten(to_flatten);
+
     /*
      * Reorganise the tree of frames to insert the new window.
      */
@@ -1371,7 +1378,7 @@ win_split_ins(
 	    if (!((flags & WSP_BOT) && p_ls == 0))
 		new_fr_height -= STATUS_HEIGHT;
 	    if (flags & WSP_BOT)
-		frame_add_statusline(curfrp, FALSE);
+		frame_add_statusline(curfrp);
 	    frame_new_height(curfrp, new_fr_height, flags & WSP_TOP, FALSE);
 	}
 	else
@@ -1921,48 +1928,30 @@ win_splitmove(win_T *wp, int size, int f
 {
     int		dir;
     int		height = wp->w_height;
-    frame_T	*frp;
+    frame_T	*unflat_altfr;
 
     if (ONE_WINDOW)
 	return OK;	// nothing to do
     if (check_split_disallowed(wp) == FAIL)
 	return FAIL;
 
-    // Undoing changes to frames if splitting fails is complicated.
-    // Save a full snapshot to restore instead.
-    frp = make_full_snapshot();
-    if (frp == NULL)
-    {
-	emsg(_(e_out_of_memory));
-	return FAIL;
-    }
-
-    // Remove the window and frame from the tree of frames.
-    (void)winframe_remove(wp, &dir, NULL);
+    // Remove the window and frame from the tree of frames.  Don't flatten any
+    // frames yet so we can restore things if win_split_ins fails.
+    winframe_remove(wp, &dir, NULL, &unflat_altfr);
     win_remove(wp, NULL);
     last_status(FALSE);	    // may need to remove last status line
     (void)win_comp_pos();   // recompute window positions
 
     // Split a window on the desired side and put "wp" there.
-    if (win_split_ins(size, flags, wp, dir) == FAIL)
-    {
-	// Restore the previous layout from the snapshot.
-	vim_free(wp->w_frame);
-	restore_full_snapshot(frp);
-
-	// Vertical separators to the left may have been lost.  Restore them.
-	frp = wp->w_frame;
-	if (frp->fr_parent->fr_layout == FR_ROW && frp->fr_prev != NULL)
-	    frame_add_vsep(frp->fr_prev);
-
-	// Statuslines above may have been lost.  Restore them.
-	if (frp->fr_parent->fr_layout == FR_COL && frp->fr_prev != NULL)
-	    frame_add_statusline(frp->fr_prev, TRUE);
-
+    if (win_split_ins(size, flags, wp, dir, unflat_altfr) == FAIL)
+    {
+	// win_split_ins doesn't change sizes or layout if it fails to insert an
+	// existing window, so just undo winframe_remove.
+	winframe_restore(wp, dir, unflat_altfr);
 	win_append(wp->w_prev, wp);
+	(void)win_comp_pos();   // recompute window positions
 	return FAIL;
     }
-    clear_snapshot_rec(frp);
 
     // If splitting horizontally, try to preserve height.
     if (size == 0 && !(flags & WSP_VERT))
@@ -3412,7 +3401,7 @@ win_free_mem(
 
     // Remove the window and its frame from the tree of frames.
     frp = win->w_frame;
-    wp = winframe_remove(win, dirp, tp);
+    wp = winframe_remove(win, dirp, tp, NULL);
     vim_free(frp);
     win_free(win, tp);
 
@@ -3462,7 +3451,9 @@ win_free_all(void)
 winframe_remove(
     win_T	*win,
     int		*dirp UNUSED,	// set to 'v' or 'h' for direction if 'ea'
-    tabpage_T	*tp)		// tab page "win" is in, NULL for current
+    tabpage_T	*tp,		// tab page "win" is in, NULL for current
+    frame_T	**unflat_altfr)	// if not NULL, set to pointer of frame that got
+				// the space, and it is not flattened
 {
     frame_T	*frp, *frp2, *frp3;
     frame_T	*frp_close = win->w_frame;
@@ -3517,7 +3508,7 @@ winframe_remove(
 	    }
 	}
 	frame_new_height(frp2, frp2->fr_height + frp_close->fr_height,
-			    frp2 == frp_close->fr_next ? TRUE : FALSE, FALSE);
+			    frp2 == frp_close->fr_next, FALSE);
 	*dirp = 'v';
     }
     else
@@ -3554,7 +3545,7 @@ winframe_remove(
 	    }
 	}
 	frame_new_width(frp2, frp2->fr_width + frp_close->fr_width,
-			    frp2 == frp_close->fr_next ? TRUE : FALSE, FALSE);
+			    frp2 == frp_close->fr_next, FALSE);
 	*dirp = 'h';
     }
 
@@ -3568,50 +3559,106 @@ winframe_remove(
 	frame_comp_pos(frp2, &row, &col);
     }
 
-    if (frp2->fr_next == NULL && frp2->fr_prev == NULL)
-    {
-	// There is no other frame in this list, move its info to the parent
-	// and remove it.
-	frp2->fr_parent->fr_layout = frp2->fr_layout;
-	frp2->fr_parent->fr_child = frp2->fr_child;
-	FOR_ALL_FRAMES(frp, frp2->fr_child)
-	    frp->fr_parent = frp2->fr_parent;
-	frp2->fr_parent->fr_win = frp2->fr_win;
-	if (frp2->fr_win != NULL)
-	    frp2->fr_win->w_frame = frp2->fr_parent;
-	frp = frp2->fr_parent;
+    if (unflat_altfr == NULL)
+	frame_flatten(frp2);
+    else
+	*unflat_altfr = frp2;
+
+    return wp;
+}
+
+/*
+ * Flatten "frp" into its parent frame if it's the only child, also merging its
+ * list with the grandparent if they share the same layout.
+ * Frees "frp" if flattened; also "frp->fr_parent" if it has the same layout.
+ * "frp" must be valid in the current tabpage.
+ */
+    static void
+frame_flatten(frame_T *frp)
+{
+    frame_T	*frp2, *frp3;
+
+    if (frp->fr_next != NULL || frp->fr_prev != NULL)
+	return;
+
+    // There is no other frame in this list, move its info to the parent
+    // and remove it.
+    frp->fr_parent->fr_layout = frp->fr_layout;
+    frp->fr_parent->fr_child = frp->fr_child;
+    FOR_ALL_FRAMES(frp2, frp->fr_child)
+	frp2->fr_parent = frp->fr_parent;
+    frp->fr_parent->fr_win = frp->fr_win;
+    if (frp->fr_win != NULL)
+	frp->fr_win->w_frame = frp->fr_parent;
+    frp2 = frp->fr_parent;
+    if (topframe->fr_child == frp)
+	topframe->fr_child = frp2;
+    vim_free(frp);
+
+    frp = frp2->fr_parent;
+    if (frp != NULL && frp->fr_layout == frp2->fr_layout)
+    {
+	// The frame above the parent has the same layout, have to merge
+	// the frames into this list.
+	if (frp->fr_child == frp2)
+	    frp->fr_child = frp2->fr_child;
+	frp2->fr_child->fr_prev = frp2->fr_prev;
+	if (frp2->fr_prev != NULL)
+	    frp2->fr_prev->fr_next = frp2->fr_child;
+	for (frp3 = frp2->fr_child; ; frp3 = frp3->fr_next)
+	{
+	    frp3->fr_parent = frp;
+	    if (frp3->fr_next == NULL)
+	    {
+		frp3->fr_next = frp2->fr_next;
+		if (frp2->fr_next != NULL)
+		    frp2->fr_next->fr_prev = frp3;
+		break;
+	    }
+	}
 	if (topframe->fr_child == frp2)
 	    topframe->fr_child = frp;
 	vim_free(frp2);
-
-	frp2 = frp->fr_parent;
-	if (frp2 != NULL && frp2->fr_layout == frp->fr_layout)
-	{
-	    // The frame above the parent has the same layout, have to merge
-	    // the frames into this list.
-	    if (frp2->fr_child == frp)
-		frp2->fr_child = frp->fr_child;
-	    frp->fr_child->fr_prev = frp->fr_prev;
-	    if (frp->fr_prev != NULL)
-		frp->fr_prev->fr_next = frp->fr_child;
-	    for (frp3 = frp->fr_child; ; frp3 = frp3->fr_next)
-	    {
-		frp3->fr_parent = frp2;
-		if (frp3->fr_next == NULL)
-		{
-		    frp3->fr_next = frp->fr_next;
-		    if (frp->fr_next != NULL)
-			frp->fr_next->fr_prev = frp3;
-		    break;
-		}
-	    }
-	    if (topframe->fr_child == frp)
-		topframe->fr_child = frp2;
-	    vim_free(frp);
-	}
-    }
-
-    return wp;
+    }
+}
+
+/*
+ * Undo changes from a prior call to winframe_remove, also restoring lost
+ * vertical separators and statuslines.
+ * Caller must ensure no other changes were made to the layout or window sizes!
+ */
+    static void
+winframe_restore(win_T *wp, int dir, frame_T *unflat_altfr)
+{
+    frame_T	*frp = wp->w_frame;
+
+    // Put "wp"'s frame back where it was.
+    if (frp->fr_prev != NULL)
+	frame_append(frp->fr_prev, frp);
+    else
+	frame_insert(frp->fr_next, frp);
+
+    // Vertical separators to the left may have been lost.  Restore them.
+    if (wp->w_vsep_width == 0
+	    && frp->fr_parent->fr_layout == FR_ROW && frp->fr_prev != NULL)
+	frame_add_vsep(frp->fr_prev);
+
+    // Statuslines above may have been lost.  Restore them.
+    if (wp->w_status_height == 0
+	    && frp->fr_parent->fr_layout == FR_COL && frp->fr_prev != NULL)
+	frame_add_statusline(frp->fr_prev);
+
+    // Restore the lost room that was redistributed to the altframe.
+    if (dir == 'v')
+    {
+	frame_new_height(unflat_altfr, unflat_altfr->fr_height - frp->fr_height,
+		unflat_altfr == frp->fr_next, FALSE);
+    }
+    else if (dir == 'h')
+    {
+	frame_new_width(unflat_altfr, unflat_altfr->fr_width - frp->fr_width,
+		unflat_altfr == frp->fr_next, FALSE);
+    }
 }
 
 /*
@@ -3895,34 +3942,30 @@ frame_fixed_width(frame_T *frp)
 
 /*
  * Add a status line to windows at the bottom of "frp".
- * If "adjust_winheight" is set, reduce the height of windows without a
- * statusline to accommodate one; otherwise, there is no check for room!
+ * Note: Does not check if there is room!
  */
     static void
-frame_add_statusline(frame_T *frp, int adjust_winheight)
+frame_add_statusline(frame_T *frp)
 {
     win_T	*wp;
 
     if (frp->fr_layout == FR_LEAF)
     {
 	wp = frp->fr_win;
-        if (adjust_winheight && wp->w_status_height == 0
-            && wp->w_height >= STATUS_HEIGHT)	// don't make it negative
-            wp->w_height -= STATUS_HEIGHT - wp->w_status_height;
 	wp->w_status_height = STATUS_HEIGHT;
     }
     else if (frp->fr_layout == FR_ROW)
     {
 	// Handle all the frames in the row.
 	FOR_ALL_FRAMES(frp, frp->fr_child)
-	    frame_add_statusline(frp, adjust_winheight);
+	    frame_add_statusline(frp);
     }
     else // frp->fr_layout == FR_COL
     {
 	// Only need to handle the last frame in the column.
 	for (frp = frp->fr_child; frp->fr_next != NULL; frp = frp->fr_next)
 	    ;
-	frame_add_statusline(frp, adjust_winheight);
+	frame_add_statusline(frp);
     }
 }
 
@@ -7554,7 +7597,7 @@ reset_lnums(void)
 make_snapshot(int idx)
 {
     clear_snapshot(curtab, idx);
-    if (make_snapshot_rec(topframe, &curtab->tp_snapshot[idx], FALSE) == FAIL)
+    if (make_snapshot_rec(topframe, &curtab->tp_snapshot[idx]) == FAIL)
     {
 	clear_snapshot(curtab, idx);
 	return FAIL;
@@ -7563,7 +7606,7 @@ make_snapshot(int idx)
 }
 
     static int
-make_snapshot_rec(frame_T *fr, frame_T **frp, int snap_wins)
+make_snapshot_rec(frame_T *fr, frame_T **frp)
 {
     *frp = ALLOC_CLEAR_ONE(frame_T);
     if (*frp == NULL)
@@ -7573,18 +7616,16 @@ make_snapshot_rec(frame_T *fr, frame_T *
     (*frp)->fr_height = fr->fr_height;
     if (fr->fr_next != NULL)
     {
-	if (make_snapshot_rec(fr->fr_next, &((*frp)->fr_next), snap_wins)
-		== FAIL)
+	if (make_snapshot_rec(fr->fr_next, &((*frp)->fr_next)) == FAIL)
 	    return FAIL;
     }
     if (fr->fr_child != NULL)
     {
-	if (make_snapshot_rec(fr->fr_child, &((*frp)->fr_child), snap_wins)
-		== FAIL)
+	if (make_snapshot_rec(fr->fr_child, &((*frp)->fr_child)) == FAIL)
 	    return FAIL;
     }
-    if (fr->fr_layout == FR_LEAF && (snap_wins || fr->fr_win == curwin))
-	(*frp)->fr_win = fr->fr_win;
+    if (fr->fr_layout == FR_LEAF && fr->fr_win == curwin)
+	(*frp)->fr_win = curwin;
     return OK;
 }
 
@@ -7722,86 +7763,6 @@ restore_snapshot_rec(frame_T *sn, frame_
     return wp;
 }
 
-/*
- * Return a snapshot of all frames in the current tabpage and which windows are
- * in them, or NULL if out of memory.
- * Use clear_snapshot_rec to free the snapshot.
- */
-    static frame_T *
-make_full_snapshot(void)
-{
-    frame_T	*frp;
-
-    if (make_snapshot_rec(topframe, &frp, TRUE) == FAIL)
-    {
-	clear_snapshot_rec(frp);
-	return NULL;
-    }
-    return frp;
-}
-
-/*
- * Restore all frames in the full snapshot "sn" for the current tabpage.
- * Caller must ensure that the screen size didn't change, no windows with frames
- * in the snapshot were freed, and windows with frames not in the snapshot are
- * removed from their frames!
- * Doesn't restore changed window vertical separators or statuslines.
- * Frees the old frames.  Don't call clear_snapshot_rec on "sn" afterwards!
- */
-    static void
-restore_full_snapshot(frame_T *sn)
-{
-    if (sn == NULL)
-	return;
-
-    clear_snapshot_rec(topframe);
-    restore_full_snapshot_rec(sn);
-    curtab->tp_topframe = topframe = sn;
-    last_status(FALSE);
-
-    // If the amount of space available changed, first try setting the sizes of
-    // windows with 'winfix{width,height}'. If that doesn't result in the right
-    // size, forget about that option.
-    if (topframe->fr_width != Columns)
-    {
-	frame_new_width(topframe, Columns, FALSE, TRUE);
-	if (!frame_check_width(topframe, Columns))
-	    frame_new_width(topframe, Columns, FALSE, FALSE);
-    }
-    if (topframe->fr_height != ROWS_AVAIL)
-    {
-	frame_new_height(topframe, ROWS_AVAIL, FALSE, TRUE);
-	if (!frame_check_height(topframe, ROWS_AVAIL))
-	    frame_new_height(topframe, ROWS_AVAIL, FALSE, FALSE);
-    }
-
-    win_comp_pos();
-}
-
-    static void
-restore_full_snapshot_rec(frame_T *sn)
-{
-    if (sn == NULL)
-	return;
-
-    if (sn->fr_child != NULL)
-	sn->fr_child->fr_parent = sn;
-    if (sn->fr_next != NULL)
-    {
-	sn->fr_next->fr_parent = sn->fr_parent;
-	sn->fr_next->fr_prev = sn;
-    }
-    if (sn->fr_win != NULL)
-    {
-	sn->fr_win->w_frame = sn;
-	// Resize window to fit the frame.
-	frame_new_height(sn, sn->fr_height, FALSE, FALSE);
-	frame_new_width(sn, sn->fr_width, FALSE, FALSE);
-    }
-    restore_full_snapshot_rec(sn->fr_child);
-    restore_full_snapshot_rec(sn->fr_next);
-}
-
 #if defined(FEAT_GUI) || defined(PROTO)
 /*
  * Return TRUE if there is any vertically split window.