Avoid a flashing window blink during startup

[davvid]

The very first paint events cause a bright flash of unstyled shell
content to appear before the configured colors are applied.

Defer displaying the shell widget until nvim sends us colors.

Instead of seeing the shell with its constructor-provided colors
we will instead see Qt's default window background color, which
follows the user's desktop theme and dark mode settings.

The shell becomes visible once nvim sends a color message
and the colors are applied.

[jgehrig]

We'll need to test this further, are we confident this fix is valid?

1. This will only remove the flashing for dark background users. It may re-introduce flashing for light background users.
2. We'll need to check the nvim-qt -- -u NONE case.
   Neovim may send UI render events with no color to indicate black or white. We need to be careful we don't cause regressions here.

Here is the part of the UI doc that jumps out at me:

The RGB values will always be valid colors, by default. If no colors have been set, they will default to black and white, depending on 'background'. By setting the ext_termcolors option, instead -1 will be used for unset colors. This is mostly useful for a TUI implementation, where using the terminal builtin ("ANSI") defaults are expected.

https://neovim.io/doc/user/ui.html

We will need to explicitly test this with and without ext_linegrid.

---

We may be able to solve this another way...

What if we leverage QSettings to render the MainWindow with the last-known background color on the initial load.

The trickiest part will be knowing what the correct "background color" is. We may be able to parse this out of m_highlightGroupNameMap and m_highlightMap, for cases were the ext_linegrid protocol is used.

[davvid]

Full disclosure ~ I didn't really notice this at first but then my wife walked out of the room complaining that my screen kept flashing and that it was annoying. After she pointed it out I'm unable to not notice the flash on each startup (especially when hacking on neovim-qt itself, which requires a lot of restarts during the edit/compile/run cycle). That's what prompted me to tweak the defaults so I am a bit motivated to make this better 😹 .

This will only remove the flashing for dark background users. It may re-introduce flashing for light background users.

Users with a light background won't see a "flash" because flashes are bright and blinding whereas a brief dark window is perceived differently and not really perceived as a flash. That's subjective, though; the UI docs and the nvim-qt -- -u NONE behavior suggests that this simple patch isn't quite enough.

Thanks for the careful review. I'll have to study the code a bit more ~ I don't know what ext_linegrid is so I have some reading to do. I'll be looking into using m_highlightGroupNameMap and m_highlightMap alongside QSettings as you suggested above since that sounds like a workable approach.

Does that suggestion only apply to the ext_linegrid protocol or is it generic to non-ext_linegrid usage as well? If it's specific to ext_linegrid then I can try to have it remember the last known background and foreground colors when ext_linegrid is not in effect.

I'll plan to loop back around on this topic after the other PRs are sorted since those seem a bit simpler. Cheers!

[jgehrig]

That's what prompted me to tweak the defaults so I am a bit motivated to make this better 😹 .

Haha too funny! I've also noticed this as well and I'm all for a fix :)

Users with a light background won't see a "flash" because flashes are bright and blinding whereas a brief dark window is perceived differently and not really perceived as a flash. That's subjective, though; the UI docs and the nvim-qt -- -u NONE behavior suggests that this simple patch isn't quite enough.

Sounds reasonable.

If other options don't work and this doesn't trigger any regressions, this sounds like this is the way to go.

I don't know what ext_linegrid is so I have some reading to do.

There are two rendering protocols used by Neovim to communicate with the frontend.

Here is documentation for the old cell-based legacy protocol: https://neovim.io/doc/user/ui.html#ui-grid-old
Here is documentation for the new "ext_linegrid" protocol: https://neovim.io/doc/user/ui.html#ui-linegrid

Neovim has supported the new protocol for some time, and the old one shouldn't be used. I think it is reasonable to fix this for the new protocol only.

You can set the protocol in ~/.config/nvim-qt/nvim-qt.conf and the code lives here..

My guess here is my "regression" I references above will only occur in the old protocol. I think Neovim has written some code in to avoid implicit colors like this when using the new ext_linegrid protocol.

Does that suggestion only apply to the ext_linegrid protocol or is it generic to non-ext_linegrid usage as well?

It would only work for the new rendering protocol.

However, if it provides a clean and no-blink solution, I think that's the best path forward.

[davvid]

I pushed a new patch which is very minimal and it avoids this issue.

Since the shell exists at startup (before the messages have been transmitted) then Qt will display it and we can't avoid a flash of unstyled content.

But, what we can do is simply defer showing the shell until nvim has sent over an initial message with the default colors, and only show the shell after those colors have been applied.

The observed behavior is that now we instead see Qt's default window background color for a split second before the shell widget becomes visible. When it does become visible it has the correct colors applied.

Since this default color matches the user's desktop theme settings it'll be a dark background for dark mode users and a light background for light mode users, which is pretty nice.

It is slightly "hacky" in that it's just toggling visibility state, but it's also super simple and just using simple Qt features.

I'm curious to hear your thoughts. There's still some minor flickering during startup, but it's much less pronounced now since it's just switching from the default window background to the shell widget's background.

[jgehrig]

I like the simplicity of this approach, but I think it may need some refinement...

I think this would break the old rendering protocol, since default_colors_set is only called for the new linegrid protocol.

This line from the UI doc also concerns me:

If no colors have been set, they will default to black and white, depending on 'background'.

I'm not sure if there is a guarantee that a call to default_colors_set will be made on startup?

Without the call, the shell will remain invisible and we could get some interesting bugs.

[davvid]

I'm not sure if there is a guarantee that a call to default_colors_set will be made on startup?

Without the call, the shell will remain invisible and we could get some interesting bugs.

I tried both the old and new protocols and interestingly enough the behavior seems to be the same. Is that because I'm using a fairly current version of neovim, and maybe nvim itself defaults to ext_linegrid now?

I wasn't sure if the QSetting was making a difference so I just kinda butchered it locally with these tweaks to forcibly disable ext_linegrid:

diff --git a/src/gui/shell.cpp b/src/gui/shell.cpp
index efc784d..df6d23a 100644
--- a/src/gui/shell.cpp
+++ b/src/gui/shell.cpp
@@ -48,6 +48,9 @@ static ShellOptions GetShellOptionsFromQSettings() noexcept
 		opts.SetIsTablineEnabled(ext_tabline.toBool());
 	}
 
+	opts.SetIsLineGridEnabled(false);
+	opts.SetIsTablineEnabled(false);
+	opts.SetIsPopupmenuEnabled(false);
 	return opts;
 }
 
@@ -340,13 +343,13 @@ void Shell::init()
 	const int64_t shellHeight = screenRect.height() / cellSize().height();
 	QVariantMap options;
 	if (m_options.IsTablineEnabled()) {
-		options.insert("ext_tabline", true);
+		// options.insert("ext_tabline", true);
 	}
 	if (m_options.IsPopupmenuEnabled()) {
-		options.insert("ext_popupmenu", true);
+		// options.insert("ext_popupmenu", true);
 	}
 	if (m_options.IsLineGridEnabled() && m_nvim->hasUIOption("ext_linegrid")) {
-		options.insert("ext_linegrid", true);
+		// options.insert("ext_linegrid", true);
 	}
 	options.insert("rgb", true);
 

My testing with both nvim-qt and nvim-qt -- -u NONE couldn't run into a situation where we don't end up showing the widget.

In addition to nvim latest, I tried nvim 0.7.2 (debian testing) and that version was also well-behaved. Let me know if there's anything else we can try.

Earlier it sounded like maybe we didn't have to worry about the old protocol, but I'm down to try making things work either way. Should I try an older version or is nvim 0.7 old enough?

[davvid]

@jgehrig Here's an idea I just had that can eliminate the chance of the shell widget staying hidden forever.

We can use a single-shot QTimer to show the widget. We can set the timer to fire 750ms after startup. This should be plenty of time for nvim to startup and send the default colors so most of the time this timer will be a no-op.

If the widget is already visible (because we already got the initial color message) then the timer will do nothing.

For the rare case where we don't get the default colors message then the timer will kick in and show the widget. That'll guarantee that we don't ever get into a situation where the shell widget stays hidden.

If you think this is a simple and workable solution I'll take a stab at adding that in shortly.

[jgehrig]

Good idea on the single shot timer! I think this is a reasonably clean and simple approach 👍

I'd like to quickly test this myself with ext_linegrid=false, just to see if there is an obvious event we can use or if the function is actually being used in the old protocol too.

[jgehrig]

Let's move the hard-coded 888 constant to a named variable.

Something along the lines of:
const int timeoutDefaultColorEventNotReceivedMSec { 750 };

[davvid]

I reworked things a bit and this ended up getting called constexpr int timeout_visibility{ 850 }; mostly since it avoided too long of a line and because it looked a little more consistent alongside the other variables in scope.

The timeout was kept at 850 just to keep the chances higher of it firing after the first event, e.g. on slower hardware or when launching nvim + nvim-qt binaries over slow storage such as NFS. I hope that's cool.

I figured out that this was the snippet I had to add to ~/.config/nvim-qt/nvim-qt.conf to enable the old protocol.

[General]
ext_linegrid=false

Once I did that I tested it and as far as I can tell the default_colors_set message is still received and the widget is still made visible (even without the timer). I still like the peace of mind of having the timer, though.

The blink is still seen with my nvim config w/ ext_linegrid=false, but that's fine IMO since it's no worse than before and only an issue when using the old protocol.

When using nvim-qt -- -u NONE the old protocol does seem to mostly avoid the blink, possibly due to a different sequence of commands getting received. Anyways, that's just my testing w/ the two nvim versions I tried. I'm curious to hear if you see similar results in your test.

Thanks for the review ~ I'm more than happy to tweak things further if you have other preferences.

[jgehrig]

This looks great!

Agreed, that's a better variable name. I just wanted some context for the constant.

Makes sense, 850 sounds like a good value.

Excellent, that is the way to use the old protocol. If you see default_colors_set with the old protocol, that alleviates any concerns I have.

I'll get this merged 👍