Conversation
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.
36132b8 to
f872182
Compare
|
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; | ||
| } |
There was a problem hiding this comment.
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))
There was a problem hiding this comment.
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..
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
|
Converted to draft. I have deeper knowledge now with the other issues. |
|
@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 Unittest Validate that
" 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 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 1. guess only way to supply execution env variable Like: 2. Can CI currently run gui gtk3 Wayland gui? |
Closes: #19483
Decorations are not tracked in wayland GTK3 GUI.
Previously
FEAT_NORMALandFEAT_HUGEworked accidentally, because its height rounded up to correctRowsvalue. This happened because these feature-targets ship with additional decorations likeFEAT_MENUandFEAT_GUI_TABLINE, which shifted to next row ongui.char_height.A very large font (row) could also highlight the issue if it exceeds the
base_heightof 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.