fix: Scrolling of windows with winbar or native borders #2165

fredizzimo · 2023-12-03T23:10:58Z

What kind of change does this PR introduce?

This fixes the scrolling of windows with winbar or native borders by only scrolling the inner region.

The solution is quite hacky, see the new neovide_unlink_border_highlights option for example. But I think this is the best we can do with the current version of Neovim.

I also changed Arc<Mutex<Line>> <RC<RefCell<Line>> because it's never accessed concurrently.

NOTE: This probably needs a bit more testing before merging.

Did this PR introduce a breaking change?

No

fredizzimo · 2023-12-04T09:27:54Z

I fixed a couple of issues with terminal buffers. The Neovim documentation does not seem to reflect what's actually sent.

Kethku · 2023-12-07T20:35:59Z

src/bridge/events.rs

+                    match kind_str.as_str() {
+                        "ui" => kind = Some(HighlightKind::Ui),
+                        "syntax" => kind = Some(HighlightKind::Syntax),
+                        // The documentation says terminal but Neovim 0.9.4 sends term...


Is there an issue to link here?

Kethku · 2023-12-07T20:36:20Z

src/bridge/events.rs

+    } else {
+        String::default()
+    };
+    // hi_name can actually be absent for terminal, even though the documentation indicates otherwise


Kethku · 2023-12-07T20:39:50Z

Looks good to me. Ship it ✅

cshuaimin · 2023-12-08T03:12:44Z

I tested this and it works for simple case like set winbar=%F, but not works with dropbar.nvim. When I change the highlight group, winbar scrolls with the content too, try set winbar=%#Visual#%F.

fredizzimo · 2023-12-08T08:35:13Z

@cshuaimin, yes I should have put that in the description, it's a known problem and we discussed it on Discord.

But the highlight groups are the only thing we have to detect the borders and winbars. Neovim simply does not provide us with any other info. Through the UI protocol.

For the winbar we could maybe make an even more hacky solution and listen to the option being set, both the global and window window local ones, and infer if there's a winbar or not in a given window based on that. But the floating borders can also use arbitrary highlight groups and that same hack does not work for them, since there's no way to listen for a window configuration change.

I will open a discussion on the Neovim repo, so that we can get a proper solution for Neovim 0.10.

src/renderer/rendered_window.rs

cshuaimin · 2023-12-08T10:51:34Z

That makes sense, thanks for the explanation. Float window border works well, great!

ttytm · 2023-12-13T14:24:56Z

I greatly appreciate the recent work of @fredizzimo. I want to say thanks at this point.

For this PR, I don't think it is ready for a merge. With the change, when using a winbar content that is more complex than something like set winbar=%F, the winbar content changes when e.g. opening a telescope picker.

A partial fix for the issue should not come with a downside for cases like @cshuaimin is mentioning or for users that use things like dropbar. The fix is not working there anyway atm. Unfortunately - as the PR description already mentions - this again requires that upstream neovim allows for a proper fix.

E.g. current master opening a telescope file picker

With the changes:

fredizzimo · 2023-12-13T14:48:49Z

Hm.. this should be strictly better than main in all cases, winbars with custom highlight groups the bug should be the same as on main, and the content scrolls with the content like @cshuaimin explains,

I can't quite explain the different highlights in the screenshot above. But maybe the highlight groups are dynamically updated, and in that case the neovide_unlink_border_highlights hack does not work. @ttytm, can you test if setting that neovide option to false fixes it?

If it does, there's no more hacks I can think of to get this work, and we will have to get a fix Neovim itself. Maybe we could merge this with the option set to false, to allow people to still have proper border and winbar scrolling with simple winbars and borders.

ttytm · 2023-12-13T17:37:25Z

[...] If it does, there's no more hacks I can think of to get this work, and we will have to get a fix Neovim itself. Maybe we could merge this with the option set to false, to allow people to still have proper border and winbar scrolling with simple winbars and borders.

Good suggestion, I unfortunately cannot test it today anymore. I'll leave an update tomorrow.

ttytm · 2023-12-14T12:59:44Z

@fredizzimo the option has no effect on the mentioned behavior.

fredizzimo · 2023-12-14T13:28:28Z

@ttytm, I think I need a Neovim configuration, so that I can re-produce the problem myself.

Ideally following this format https://github.com/neovim/neovim/blob/master/contrib/minimal.lua

But if that is not possible, a complete .config/nvim directory with lazy as the plugin manager is also OK, since I can use NVIM_APPNAME to run the config without messing with my own.

ttytm · 2023-12-15T00:22:08Z

Hey @fredizzimo sorry bout the delay. I don't have the capacity atm to create a minimal repro. It's very unfortunate as I don't like to be a blocking factor.

I can share a full nvim config where this happens with: https://github.com/tenxsoydev/nxvim.
Also, please feel free to ignore the mentioned objections and merge the PR if you think that's appropriate.

The documentation seem to be wrong on a couple of points...

fredizzimo · 2023-12-18T22:54:41Z

@ttytm, I tested your configuration, and I can repeat the problems with the winbar highlights getting brighter when telescope is opened. But I can also repeat that on the current master, so there must be something else going on here, mabye some earlier PR that broke it.

In any case, I will try to make this more robust by comparing the content instead of using highlight groups. We know how many lines are scrolled, and from that we see which lines stay in place. If it works, it should not take long to implement, so I'm putting this as draft until then.

I also tested without this ce0174a, but the same thing, so it's something earlier.

ttytm · 2023-12-18T23:15:27Z

I'm discovering that it might not be a problem with Neovide but a problem with the config.
As I can reproduce the winbar problem when using nvim in Kitty on a mac. Somehow no the issue only appears in neovide with the PR but not in kitty at current master on Linux.

Edit:
Just updated to the latest nvim nightly version on my linux machine, now it also happens in kitty. So it just appearead that it was an issue with the PR. I was using the november 28 nvim nightly release. I'm sorry :/

Edit2:
It's fixed for the config when handling the WinbarNC highlight group. The PR should be good I think. Sorry again.

fredizzimo · 2023-12-19T22:32:03Z

I'm moving this out of draft again. The content based approach has too many special cases, and I'm not confident that it can be reliable enough.

My idea was basically to check how many lines have stayed in place at the top and bottom, every time the screen is updated, and consider those borders or winbars, which are treated the same way in this PR. The biggest issue with that approach is duplicate lines, but also lines that changes due to highlight updates and stuff like that, or winbars with dynamic content.

So instead I think we can go with what we have, perhaps even with neovide_unlink_border_highlights set to false by default, to make sure that it has no negative effects at all. And people that want the support can set that to true. Note that it needs to be set before the colorscheme is loaded.

But I'm leaving it enabled by default for now, until I get more feedback.

cshuaimin · 2023-12-21T04:20:19Z

src/renderer/rendered_window.rs

+                            })
+                        })
+                    })
+                    .all(|v| v);


Suggested change

.all(|v| v);

.any(|v| v);

lines with purly border highlight groups are considered borders.

I believe if we change this to "lines contain border highlight groups are considered borders", then dropbar.nvim will work! Don't know if this has false positives. @fredizzimo what do you think?

However with this change, it seems that we need another way to ignore left and right borders.

That's a great idea!

We could use all to detect the borders and any to detect winbars. I will try that later today.

I implemented this, and it does seem to work.

But I have only done a very quick test yet and it's a bit late for me to do more testing now.

I will also make a bit more efficient implementation later.

MultisampledNight · 2023-12-23T21:57:42Z

Thank you! Choooooooooooo choooooooooooo 🚂

This was referenced Dec 4, 2023

winbar moves with the text in multigrid mode #1550

Closed

Floating windows with borders doesn't work well with smooth scrolling #1446

Closed

Breaking changes and new issues on main #2095

Closed

fredizzimo force-pushed the fsundvik/fix-border-scroll branch from f776736 to 43177f6 Compare December 4, 2023 10:21

Kethku reviewed Dec 7, 2023

View reviewed changes

MultisampledNight approved these changes Dec 8, 2023

View reviewed changes

src/renderer/rendered_window.rs Outdated Show resolved Hide resolved

fredizzimo and others added 9 commits December 19, 2023 00:03

Parse highlight infos

4601c74

Draw borders

b6219e2

Unlink border highlight groups so that they can be detected by Neovim.

6144417

Fix the cursor position in windows with borders

2cde2cf

Change Arc/Mutex to RC/RefCell

955c40a

Clarify the border classification

4a6f991

Fix empty grid detection, taking borders into account

5300292

Fix terminal buffers

4f7a150

The documentation seem to be wrong on a couple of points...

style: let rustc infer generic

f2b7ec7

fredizzimo force-pushed the fsundvik/fix-border-scroll branch from d206834 to f2b7ec7 Compare December 18, 2023 22:03

fredizzimo marked this pull request as draft December 18, 2023 22:54

fredizzimo marked this pull request as ready for review December 19, 2023 22:32

cshuaimin reviewed Dec 21, 2023

View reviewed changes

Consider lines with any winbar highlight as borders

09075b7

fredizzimo mentioned this pull request Dec 22, 2023

UI protocol: forward information about winbar and other window grid widgets neovim/neovim#23610

Open

Optimize the border and winbar checks

dc6ccca

fredizzimo force-pushed the fsundvik/fix-border-scroll branch from 8abd998 to dc6ccca Compare December 23, 2023 21:16

MultisampledNight merged commit b496ab2 into neovide:main Dec 23, 2023
3 checks passed

razvanazamfirei mentioned this pull request Dec 24, 2023

neovide 0.12.0 Homebrew/homebrew-core#158189

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Scrolling of windows with winbar or native borders #2165

fix: Scrolling of windows with winbar or native borders #2165

fredizzimo commented Dec 3, 2023 •

edited

fredizzimo commented Dec 4, 2023

Kethku Dec 7, 2023

Kethku Dec 7, 2023

Kethku commented Dec 7, 2023

cshuaimin commented Dec 8, 2023

fredizzimo commented Dec 8, 2023

cshuaimin commented Dec 8, 2023

ttytm commented Dec 13, 2023 •

edited

fredizzimo commented Dec 13, 2023

ttytm commented Dec 13, 2023

ttytm commented Dec 14, 2023

fredizzimo commented Dec 14, 2023

ttytm commented Dec 15, 2023 •

edited

fredizzimo commented Dec 18, 2023 •

edited

ttytm commented Dec 18, 2023 •

edited

fredizzimo commented Dec 19, 2023

cshuaimin Dec 21, 2023 •

edited

cshuaimin Dec 21, 2023

fredizzimo Dec 21, 2023

fredizzimo Dec 21, 2023 •

edited

MultisampledNight commented Dec 23, 2023

fix: Scrolling of windows with winbar or native borders #2165

fix: Scrolling of windows with winbar or native borders #2165

Conversation

fredizzimo commented Dec 3, 2023 • edited

What kind of change does this PR introduce?

Did this PR introduce a breaking change?

fredizzimo commented Dec 4, 2023

Kethku Dec 7, 2023

Choose a reason for hiding this comment

Kethku Dec 7, 2023

Choose a reason for hiding this comment

Kethku commented Dec 7, 2023

cshuaimin commented Dec 8, 2023

fredizzimo commented Dec 8, 2023

cshuaimin commented Dec 8, 2023

ttytm commented Dec 13, 2023 • edited

fredizzimo commented Dec 13, 2023

ttytm commented Dec 13, 2023

ttytm commented Dec 14, 2023

fredizzimo commented Dec 14, 2023

ttytm commented Dec 15, 2023 • edited

fredizzimo commented Dec 18, 2023 • edited

ttytm commented Dec 18, 2023 • edited

fredizzimo commented Dec 19, 2023

cshuaimin Dec 21, 2023 • edited

Choose a reason for hiding this comment

cshuaimin Dec 21, 2023

Choose a reason for hiding this comment

fredizzimo Dec 21, 2023

Choose a reason for hiding this comment

fredizzimo Dec 21, 2023 • edited

Choose a reason for hiding this comment

MultisampledNight commented Dec 23, 2023

fredizzimo commented Dec 3, 2023 •

edited

ttytm commented Dec 13, 2023 •

edited

ttytm commented Dec 15, 2023 •

edited

fredizzimo commented Dec 18, 2023 •

edited

ttytm commented Dec 18, 2023 •

edited

cshuaimin Dec 21, 2023 •

edited

fredizzimo Dec 21, 2023 •

edited