Skip to content

fix(gtk-wayland): track decorations#19603

Draft
dezza wants to merge 1 commit intovim:masterfrom
dezza:fix-gui-tiny-cmdline
Draft

fix(gtk-wayland): track decorations#19603
dezza wants to merge 1 commit intovim:masterfrom
dezza:fix-gui-tiny-cmdline

Conversation

@dezza
Copy link
Copy Markdown
Contributor

@dezza dezza commented Mar 7, 2026

Closes: #19483

Decorations are not tracked in wayland GTK3 GUI.

Previously FEAT_NORMAL and FEAT_HUGE worked accidentally, because its height rounded up to correct Rows value. This happened because these feature-targets ship with additional decorations like FEAT_MENU and FEAT_GUI_TABLINE, which shifted to next row on gui.char_height.

A very large font (row) could also highlight the issue if it exceeds the base_height of a row.

Note that if there were previous dirty math due to shifting these should be reverted as tracking decorations is now in effect. However, this is limited to wayland for now.

@dezza dezza marked this pull request as draft March 7, 2026 11:14
Closes: vim#19483

Decorations are not tracked in wayland GTK3 GUI.

Previously `FEAT_NORMAL` and `FEAT_HUGE` worked accidentally, because
its height rounded up to correct `Rows` value. This happened because
these feature-targets ship with additional decorations like `FEAT_MENU`
and `FEAT_GUI_TABLINE`, which shifted to next row on `gui.char_height`.

A very large font (row) could also highlight the issue if it exceeds the
`base_height` of a row.

Note that if there were previous dirty math due to shifting these
should be reverted as tracking decorations is now in effect. However,
this is limited to wayland for now.
@dezza dezza force-pushed the fix-gui-tiny-cmdline branch from 36132b8 to f872182 Compare March 7, 2026 11:22
@dezza dezza marked this pull request as ready for review March 7, 2026 11:41
@chrisbra
Copy link
Copy Markdown
Member

chrisbra commented Mar 8, 2026

thanks

gtk_widget_get_allocation(gui.formwin, &formwin_alloc);
csd_height = mainwin_alloc.height - formwin_alloc.height;
csd_width = mainwin_alloc.width - formwin_alloc.width;
}
Copy link
Copy Markdown
Member

@chrisbra chrisbra Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only done initially (when old_csd_height is zero). But if we change the theme, don't we need to re-calculate the decoration height and width? So perhaps this should rather be:
if (gui.is_wayland && gtk_widget_get_realized(gui.mainwin))

Copy link
Copy Markdown
Contributor Author

@dezza dezza Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean gtk3 theme? A different and specific condition must exist for theme-change. I assume what you suggested would run on every call, I don't think a theme change triggers a new unrealize/realize event, but I'm not sure.

I appreciate the questions and commitment to audit it!

I have osc52 builtin (C impl) poc working btw (#19441). I'm considering an experimental non-default --enable-clipboard-osc52 configure flag.

current draft can be forced enabled with VIM_OSC52=1 on startup so it works out-the-box as + and * register, maybe a vim option is better not sure what the convention is for env variables or if new options generally should be avoided for FEAT_TINY why that idea. But it does give very accessible clipboard function on servers where vimrc is not configured usually. I don't think security (read clipboard) is a concern for vim - if it is exposed in terminal emulator, it does not require vim, why its a term emu concern.

requires blob_free, blob_alloc, blob_from_string and base64_decode, base64_encode moved to FEAT_CLIPBOARD_OSC52 as standalone feature enabled at level with FEAT_TINY optionally..

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you create a separate PR for this? Please also mention by how much this will grow vim-tiny. But let's not get side-tracked by the OSC52 change. Do you think it makes sense to change that if statement?

Copy link
Copy Markdown
Contributor Author

@dezza dezza Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will when its ready. I will not grow tiny per default, for now it will be optional and as experimental flag until its been approved and tested thoroughly. I will keep it as slim as possible.

I would assume myself changing a gtk3 theme would require most apps to fully restart to apply, after all its legacy at this point. But I can look into it, however I'm unaware of this being handled elsewhere.

As the condition is now it runs only 1 time at startup by the old_csd_height condition, the realized guard is only there to prevent several nil checks while not realized.

The formwin mainwin alloc subtraction is the exact diff I measured between x11 and wayland by debug logs.

@dezza
Copy link
Copy Markdown
Contributor Author

dezza commented Mar 13, 2026

Converted to draft. I have deeper knowledge now with the other issues.

@dezza
Copy link
Copy Markdown
Contributor Author

dezza commented Mar 13, 2026

@chrisbra almost done fixing #19640, #19483 one bug is very old maybe pre-9.0.

Creating unittest to solidify it, so no one easily changes by mistake.

I created has('gui_display_wayland') || has ('gui_display_x11'), could just rely on GDK_BACKEND, but currently no way to tell from within vim besides using xlsclients 3rdparty shell tool.

Unittest Validate that &lines does not change when :tabnew | close runs and triggers resize, I want to test both display servers.

util/check.vim

" Command to Check for Wayland GUI
command -nargs=0 CheckWaylandGui call CheckWaylandGui()
func CheckWaylandGui()
  if !has('gui_display_wayland')
    throw 'Skipped: no wayland gui display'
  endif
endfunc

" Command to Check for X11 GUI
command -nargs=0 CheckX11Gui call CheckX11Gui()
func CheckX11Gui()
  if !has('gui_display_x11')
    throw 'Skipped: no x11 gui display'
  endif
endfunc

test_gui.vim

" Test if ':tabnew | close' causes &lines change on x11
func Test_tabline_lines_x11()
  CheckX11Gui

  let old_lines = &lines
  tabnew
  close
  let new_lines = &lines

  call assert_equal(old_lines, new_lines)
endfunc

" (should move to test_gui_wayland.vim, see below)
" Test if ':tabnew | close' causes &lines change on wayland
source util/window_manager.vim
func Test_tabline_lines_wayland()
  CheckWaylandGui

  let old_lines = &lines
  tabnew
  close
  let new_lines = &lines

  call assert_equal(old_lines, new_lines)
endfunc

^ For above test to work, I need to set GDK_BACKEND=x11

1. guess only way to supply execution env variable GDK_BACKEND is creating a separate test_gui_wayland.vim ?

Like:

# x11 default
test_gui.res: test_gui.vim
	@echo "$(VIMPROG)" > vimcmd
	@echo "$(RUN_GVIMTEST)" >> vimcmd
	if test -n "$${ASAN_OPTIONS}"; then \
		GDK_BACKEND=x11 ASAN_OPTIONS="$${ASAN_OPTIONS}_$*" UBSAN_OPTIONS="$${UBSAN_OPTIONS}_$*" $(RUN_VIMTEST) -u NONE $(NO_INITS) -S runtest.vim $< ; \
	else \
		GDK_BACKEND=x11 $(RUN_VIMTEST) -u NONE $(NO_INITS) -S runtest.vim $< ; \
	fi

	@rm vimcmd

# wayland specific
test_gui_wayland.res: test_gui_wayland.vim
	@echo "$(VIMPROG)" > vimcmd
	@echo "$(RUN_GVIMTEST)" >> vimcmd
	if test -n "$${ASAN_OPTIONS}"; then \
		GDK_BACKEND=wayland ASAN_OPTIONS="$${ASAN_OPTIONS}_$*" UBSAN_OPTIONS="$${UBSAN_OPTIONS}_$*" $(RUN_VIMTEST) -u NONE $(NO_INITS) -S runtest.vim $< ; \
	else \
		GDK_BACKEND=wayland $(RUN_VIMTEST) -u NONE $(NO_INITS) -S runtest.vim $< ; \
	fi

	@rm vimcmd

2. Can CI currently run gui gtk3 Wayland gui?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gvim-tiny commandline half-visible on startup

2 participants