fix(tui): initialize clear attrs with current terminal background #28676

luukvbaal · 2024-05-09T00:18:53Z

Problem: Invalidated regions that are flushed during startup are
cleared with unitialized "clear_attrs", which is perceived as
flickering.
Solution: Initialize "clear_attrs" with current terminal background color.

Fix #28667

luukvbaal · 2024-05-09T00:20:44Z

Was unsuccessful in adding a test for this so far, can try again tomorrow if this fix seems right to you @bfredl.

gpanders · 2024-05-09T00:33:01Z

I am not sure this fixes #28667. Using the init script mentioned in that file:

colorscheme blue
au VimEnter * sleep 1

with VIMRUNTIME=runtime build/bin/nvim --clean -u init.vim, I still see the black screen.

It might fix #28668, but I'm not at my laptop at the moment to verify. I can check tomorrow.

luukvbaal · 2024-05-09T00:56:49Z

Oh? I don't see the black screen. Are you actually seeing a black screen or is your terminal background black and is that what you're seeing? That's what I see and what I would expect after this change. I.e. no flickering but colorscheme being applied after the delay since that's the first moment the screen is drawn.

EDIT: Hmm seems terminal dependent, I do still get a black screen in wezterm, not in st or kitty. Not sure about the fix then. Moving tui->starting = false to after_startup_cb() also fixes the black screen on wezterm, but that seems to break other tests... @bfredl do you think the premise of this PR makes sense and do you see a way to do it without breaking behavior? Can look at it again tomorrow...

It does also fix #28668 (on my end) but didn't link it here because we may want to keep that open until it has been decided when to call ui_flush() in nvim__redraw().

gpanders · 2024-05-09T03:07:40Z

EDIT: Hmm seems terminal dependent, I do still get a black screen in wezterm, not in st or kitty. Not sure about the fix then

Weird, I still see the black screen on every terminal I've tried (Ghostty, Alacritty, WezTerm, and Kitty).

Are you actually seeing a black screen or is your terminal background black and is that what you're seeing?

My terminal background color is #2e3440, which is not black. The "black" is, I believe, the default colorscheme background, which is #14161b.

luukvbaal · 2024-05-09T06:48:23Z

Weird, I still see the black screen on every terminal I've tried (Ghostty, Alacritty, WezTerm, and Kitty).

Probably timing related instead then(for me the results are consistent depending on the terminal on a relatively old laptop)...

My terminal background color is #2e3440, which is not black. The "black" is, I believe, the default colorscheme background, which is #14161b.

No I don't think it is the default colorscheme background in these cases(the ones from #28667 and #28668). What we're seeing is actually black(#000000) because the clear_attr becomes 0 when the grid is completely undrawn during startup. I.e. even with this patch you will still see a black screen momentarily even though the default colorscheme background is set to white:

diff --git a/src/nvim/highlight_group.c b/src/nvim/highlight_group.c
index c4fb57c95a..43725a8214 100644
--- a/src/nvim/highlight_group.c
+++ b/src/nvim/highlight_group.c
@@ -2912,12 +2912,12 @@ color_name_table_T color_name_table[] = {
   { "NvimDarkBlue", RGB_(0x00, 0x4c, 0x73) },
   { "NvimDarkCyan", RGB_(0x00, 0x73, 0x73) },
   { "NvimDarkGray1", RGB_(0x07, 0x08, 0x0d) },
-  { "NvimDarkGray2", RGB_(0x14, 0x16, 0x1b) },
+  { "NvimDarkGrey2", RGB_(0xff, 0xff, 0xff) },
   { "NvimDarkGray3", RGB_(0x2c, 0x2e, 0x33) },
   { "NvimDarkGray4", RGB_(0x4f, 0x52, 0x58) },
   { "NvimDarkGreen", RGB_(0x00, 0x55, 0x23) },
   { "NvimDarkGrey1", RGB_(0x07, 0x08, 0x0d) },
-  { "NvimDarkGrey2", RGB_(0x14, 0x16, 0x1b) },
+  { "NvimDarkGrey2", RGB_(0xff, 0xff, 0xff) },
   { "NvimDarkGrey3", RGB_(0x2c, 0x2e, 0x33) },
   { "NvimDarkGrey4", RGB_(0x4f, 0x52, 0x58) },
   { "NvimDarkMagenta", RGB_(0x47, 0x00, 0x45) },

luukvbaal · 2024-05-09T16:24:10Z

Updated to avoid flushing in a way that shouldn't be inter-process timing dependent, while still avoiding any flickering and without breaking any existing expected results. Still not sure how to add a test for this...

Updated again with a simpler solution that achieves the same result in more cases(I noticed the previous solution didn't work when resizing during startup). Also added a comment which better explains the current situation and it's limitations(as I understand it).

Problem: Invalidated regions that are flushed during startup are cleared with unitialized "clear_attrs", which is perceived as flickering. Solution: Initialize "clear_attrs" with current terminal background color.

luukvbaal · 2024-05-10T12:16:32Z

test/functional/terminal/tui_spec.lua

@@ -2003,6 +2003,39 @@ describe('TUI', function()
    ]])
  end)

+  it('invalidated regions are cleared with terminal background attr', function()
+    local screen = Screen.new(50, 10)
+    screen:set_default_attr_ids({ [1] = { foreground = Screen.colors.Black } })


Added a test that at least fails on master, although I'm not sure if it tests for the behavior correctly. Presumably Screen.colors.Black is normal_bg, in which case it does. Could also mean that the test still produces an unwanted black screen here? I can no longer reproduce that locally.

gpanders · 2024-05-10T17:36:46Z

Confirmed this fixes both #28667 and #28668 for me.

Problem: Fix added in neovim#28676 worked accidentally for RGB color, and did not work for cterm. Solution: Intentionally initialize clear_attrs to -1 and update comment.

Problem: Fix added in neovim#28676 worked accidentally(used variables were themselves uninitialized at this point during startup) and does not always work. Solution: Reset attributes when clearing regions during startup.

Problem: Fix added in #28676 worked accidentally(used variables were themselves uninitialized at this point during startup) and does not always work. Solution: Reset attributes when clearing regions during startup.

Problem: Fix added in neovim#28676 worked accidentally(used variables were themselves uninitialized at this point during startup) and does not always work. Solution: Reset attributes when clearing regions during startup.

github-actions bot added the tui label May 9, 2024

github-actions bot requested a review from gpanders May 9, 2024 00:21

zeertzjq added this to the 0.10 milestone May 9, 2024

luukvbaal force-pushed the tuiflush branch from ab73c3b to f2cfc55 Compare May 9, 2024 02:01

luukvbaal force-pushed the tuiflush branch from f2cfc55 to b75dbe8 Compare May 9, 2024 16:18

luukvbaal changed the title ~~fix(tui): don't flush invalid regions during startup~~ fix(tui): avoid invalidating regions with uninitialized grid May 9, 2024

luukvbaal force-pushed the tuiflush branch from b75dbe8 to 272869e Compare May 9, 2024 22:43

luukvbaal changed the title ~~fix(tui): avoid invalidating regions with uninitialized grid~~ fix(tui): initialize clear attrs with current terminal background May 9, 2024

luukvbaal force-pushed the tuiflush branch from 272869e to 9b56e36 Compare May 9, 2024 22:45

luukvbaal mentioned this pull request May 9, 2024

fix(redraw): only flush screen state when "flush" is true #28669

Closed

fix(tui): initialize clear attrs with current terminal background

fd50185

Problem: Invalidated regions that are flushed during startup are cleared with unitialized "clear_attrs", which is perceived as flickering. Solution: Initialize "clear_attrs" with current terminal background color.

luukvbaal force-pushed the tuiflush branch from 9b56e36 to fd50185 Compare May 10, 2024 12:13

luukvbaal commented May 10, 2024

View reviewed changes

gpanders approved these changes May 10, 2024

View reviewed changes

bfredl merged commit a2c158a into neovim:master May 10, 2024
27 of 28 checks passed

gpanders mentioned this pull request May 10, 2024

nvim__redraw causes some visible screen tearing #28668

Closed

luukvbaal mentioned this pull request May 12, 2024

fix(tui): reset clear_region attributes during startup #28713

Merged

Jendker mentioned this pull request May 14, 2024

Don't set vim.o.termguicolors = true navarasu/onedark.nvim#219

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(tui): initialize clear attrs with current terminal background #28676

fix(tui): initialize clear attrs with current terminal background #28676

luukvbaal commented May 9, 2024 •

edited

luukvbaal commented May 9, 2024

gpanders commented May 9, 2024 •

edited

luukvbaal commented May 9, 2024 •

edited

gpanders commented May 9, 2024

luukvbaal commented May 9, 2024 •

edited

luukvbaal commented May 9, 2024 •

edited

luukvbaal May 10, 2024

gpanders commented May 10, 2024

fix(tui): initialize clear attrs with current terminal background #28676

fix(tui): initialize clear attrs with current terminal background #28676

Conversation

luukvbaal commented May 9, 2024 • edited

luukvbaal commented May 9, 2024

gpanders commented May 9, 2024 • edited

luukvbaal commented May 9, 2024 • edited

gpanders commented May 9, 2024

luukvbaal commented May 9, 2024 • edited

luukvbaal commented May 9, 2024 • edited

luukvbaal May 10, 2024

Choose a reason for hiding this comment

gpanders commented May 10, 2024

luukvbaal commented May 9, 2024 •

edited

gpanders commented May 9, 2024 •

edited

luukvbaal commented May 9, 2024 •

edited

luukvbaal commented May 9, 2024 •

edited

luukvbaal commented May 9, 2024 •

edited