Fix initial and tabnew/tabclose window size errors#19853
Fix initial and tabnew/tabclose window size errors#19853ghgary wants to merge 5 commits intovim:masterfrom
Conversation
For some time, gvim has had a couple of problems with window sizing.
1. The initial window size did not match the -geometry spec: it was
one line too short and one column too narrow.
2. The command sequence ":tabnew" ":tabclose" resulted in a window
shorter and narrow than the original.
I have been working on those problems on-and-off for several weeks and
have made little progress. I don't know much about windows or graphics
toolkits.
A few days ago, I installed GitHub Copilot CLI and was looking for
something to try it out on, so I started it in my Vim repo, described
the :tabnew/:tabclose problem to it, and asked for a solution. It took
several hours, and a few fixes that didn't work, but eventually it came
up with a working solution. I then asked it for a fix to the
initial-window-size problem, and it fixed that as well. Finally, I told
it where to find the Vim test directory and what I wanted the test to
verify, and it wrote and tested the test script, too.
The code looks reasonable, but again, I'm not very familiar with that
part of Vim nor with GTK. Except for some touchups to the formatting,
it was written entirely by Copilot.
The two problems and the affected code seemed related enough to release
as a single PR.
Below are the commit messages written by Copilot.
Change Set 1
------------
gui: fix Rows/Cols shrinking on tabnew/tabclose in GTK3/Wayland (CSD)
On GTK3 with client-side decorations (Wayland), gtk_window_resize() and
gtk_window_get_size() disagree by the CSD margin. Each tabnew/tabclose
cycle triggered gui_mch_show_tabline() -> async GTK layout -> configure
events with stale sizes -> Rows and Cols decreased by one or more per
cycle.
Fixes applied in src/gui_gtk_x11.c:
1. get_item_dimensions(): fall back to preferred (natural) height when
the widget allocation is stale (height <= 1), so chrome height is
always accurate regardless of whether the tabline has been realised.
2. gui_mch_show_tabline(): pre-record the current mainwin size as a
stale TRACK_RESIZE_HISTORY entry before calling
gtk_notebook_set_show_tabs(). The configure event that fires in
response to the tab-bar appearing/disappearing is then discarded
rather than being mis-interpreted as a user resize.
3. mch_csd_height / mch_csd_width: measure the CSD frame size from the
first confirmed resize response (requested size minus reported size)
and add it back in all subsequent gtk_window_resize() calls so the
form widget receives the pixel dimensions that were intended.
4. mch_pending_form_w: record the expected form widget width at the
time gui_mch_set_shellsize() issues gtk_window_resize(). In
form_configure_event(), clamp a configure event's reported width up
to mch_pending_form_w when the deficit is less than one char_width.
This corrects for the steady-state CSD underestimate (startup
measurement is 2 px; actual is 4 px) that caused Columns to drift
downward with every cycle.
Fixes applied in src/gui.c:
5. gui_update_tabline(): save and restore Columns/Rows around the
gui_mch_show_tabline() call so that any configure event processed
inside that call (before the window reaches its final geometry)
cannot corrupt the column/row count.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
src/gui.c
src/gui_gtk_x11.c
Change Set 2
------------
gui: fix initial window size too small with -geometry on GTK3/Wayland
When vim starts with -geometry=NxM on Wayland/GTK3 (CSD), the first
gui_mch_set_shellsize() call runs before the CSD offsets are known, so
gtk_window_resize() undershoots: the window is csd_width pixels too
narrow and csd_height pixels too short. The result was that the
command-line row was visually clipped at the bottom.
Fix in form_configure_event(): when the CSD offsets are first measured
from the startup resize response:
- Set mch_pending_form_w retroactively so the existing width-clamp
gives the correct Columns immediately (avoids Cols = N-1).
- Add mch_csd_height to usable_height so gui_resize_shell() computes
the correct Rows immediately (avoids Rows = M-1).
- Register a one-shot g_idle_add(startup_resize_correction_cb) that
calls gui_set_shellsize() after the signal handler returns. Now
that the CSD offsets are known, this issues a corrective
gtk_window_resize() that makes the physical window actually tall
and wide enough to display all M rows and N columns without clipping.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
src/gui_gtk_x11.c
Change Set 3
------------
test: add test_gui_geometry for GTK3/Wayland CSD window size fix
Test_geometry_exact_size: verifies that a gvim window opened with
-geometry WxH has exactly W columns and H lines, catching the CSD
(client-side decoration) pixel-subtraction bug that made the window
one character cell too small.
Test_tabnew_tabclose_size_stable: verifies that the initial window
size is correct and does not drift after three :tabnew/:tabclose
cycles, catching the bug where stale configure events caused &columns
and &lines to shrink with each tabline show/hide.
Both tests pass with the window_size_bugfix changes and fail on master.
Note: the gvim after script avoids multi-line call expressions because
line continuation with \ does not work in scripts sourced via -S in
GUI mode.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
src/testdir/Make_all.mak
src/testdir/test_gui_geometry.vim
Inside #if defined(FEAT_GUI_TABLINE), directives need one space: #ifdef → # ifdef, #endif → # endif In form_configure_event (not nested in outer #ifdef at that level), directives need no space: # ifdef → #ifdef, # endif → #endif Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
I looked at the test coverage issue with the aid of copilot. It says:
I think that means that this coverage error should be ignored, or the CI procedure changed to account for this sort of thing. |
You can ignore the coverage warnings, its always like that I suppose |
CI build is failing because of an error in ci/lychee.toml, which I didn't change and apparently hasn't changed upstream, either, so I'm trying an update to master.
|
This PR has 3 failing checks, but I don't know what I can do to resolve them. See the comments above about the code coverage failures. The windows Build MinGW failure is due to an inability to include Python.h from if_python.c. This PR doesn't touch either of those files. The last failure would probably go away if I could rerun the check, but I can't find the button to do that, either. Would someone CI savvy please take a look at this and either fix the issue(s) or tell me how I can fix this? |
|
Can you move the test into |
chrisbra
left a comment
There was a problem hiding this comment.
I am not an expert on the GTK code, but I think this change makes sense. But if anybody has some comments, please do
| // Allocation hasn't been updated yet (widget just became visible, | ||
| // e.g. tab bar shown asynchronously on Wayland). Query the preferred | ||
| // height so the caller gets a valid value before the layout pass | ||
| // runs. Use minimum height: GTK may allocate min_h even when |
There was a problem hiding this comment.
that comment is wrong, it uses maximum height
|
thanks |
For some time, gvim has had a couple of problems with window sizing.
The initial window size did not match the -geometry spec: it was one line too short and one column too narrow.
The command sequence ":tabnew" ":tabclose" resulted in a window shorter and narrow than the original.
I have been working on those problems on-and-off for several weeks and have made little progress. I don't know much about windows or graphics toolkits.
A few days ago, I installed GitHub Copilot CLI and was looking for something to try it out on, so I started it in my Vim repo, described the :tabnew/:tabclose problem to it, and asked for a solution. It took several hours, and a few fixes that didn't work, but eventually it came up with a working solution. I then asked it for a fix to the initial-window-size problem, and it fixed that as well. Finally, I told it where to find the Vim test directory and what I wanted the test to verify, and it wrote and tested the test script, too.
The code looks reasonable, but again, I'm not very familiar with that part of Vim nor with GTK. Except for some touchups to the formatting, it was written entirely by Copilot.
The two problems and the affected code seemed related enough to release as a single PR.
Below are the commit messages written by Copilot.
Change Set 1
src/gui.c
src/gui_gtk_x11.c
Change Set 2
src/gui_gtk_x11.c
Change Set 3
src/testdir/Make_all.mak
src/testdir/test_gui_geometry.vim