vhakulinen / gnvim

GUI for neovim, without any web bloat
MIT License
1.85k stars 69 forks source link

Initial grid size is too small #82

Closed YaLTeR closed 5 years ago

YaLTeR commented 5 years ago

I use the sway Wayland compositor. When I initially open gnvim, the grid size is smaller than the actual window size, and I need to resize it for it to fix itself.

Right after opening: image

After resizing to fix the issue: image

This also happens with smaller window sizes: image

smolck commented 5 years ago

I was previously going to open an issue on this topic, but after working on #78 I think I know what the problem is. GNvim isn't handling each redraw event when given multiple at once (see #77)~, and so when given multiple (on opening the window) events for drawing the window, it doesn't execute them all, and (I think) this causes the problem.~

EDIT: I seem to have been wrong, at least somewhat. After trying #78 out with a local fix of ui.rs, it didn't fix the problem....

badosu commented 5 years ago

I confirm seeing this issue though not easily reproducible.

smolck commented 5 years ago

For me this doesn't happen when using cargo run. @badosu Can you confirm this?

I remember when using xorg/i3-gaps this happened inconsistently, but when using wayland/sway it happened every time I opened GNvim. Now, on AwesomeWM, it happens every time as well (keybindings or term command).

badosu commented 5 years ago

This is with i3 (can't use sway due to a certain GPU brand laziness), just tested.

Opening as a new split:

Screenshot from 2019-07-22 14-50-15

Moving the splits and forcing a redraw:

Screenshot from 2019-07-22 14-50-37

Confirm that with cargo run I can't reproduce even though I can reproduce with build from latest commit (https://github.com/vhakulinen/gnvim/commit/1d8a0425b851cd4cb821c38ac2b6a50c6b52fd07) but it happens when the debug binary is ran directly, I would be surprised if there's a factor related.

smolck commented 5 years ago

This may actually be a bug with Neovim (I'm using neovim-nightly from the AUR). I use termite, which supports the -e option to run something on opening the terminal. Running Neovim on startup of termite with $ termite -e nvim causes the same problem to occur (this is the TUI):

image

Upon forcing a redraw by opening another window, it fixes itself (as in GNvim).

Since using Neovim with <terminal> -e nvim is something that is (probably) almost never used, it makes sense why this wouldn’t be noticed if it is a Neovim bug. @badosu Can you confirm this? It doesn't seem to happen with urxvt -e nvim.

@bfredl Could this be a Neovim problem? This only happens when using Termite AFAIK (although I haven't tested beyond urxvt and termite: tried xfce4-terminal but the -e option didn't work due to my environment setup).

YaLTeR commented 5 years ago

For me alacritty -e nvim works correctly.

smolck commented 5 years ago

I am able to make this issue happen with cargo run when I comment out this line in ui.rs: https://github.com/vhakulinen/gnvim/blob/1d8a0425b851cd4cb821c38ac2b6a50c6b52fd07/src/ui/ui.rs#L107

vhakulinen commented 5 years ago

I've encountered the same problem, and its rather annoying. @smolck if you can produce this with TUI, you should open a bug report on neovim's repo.

bfredl commented 5 years ago

Not sure if it is causing this particular issue, but there is a slight inefficiency in gnvim's initialization. First neovim is attached with fixed size nvim.ui_attach(80, 30, &ui_opts), and immediately after that, when the drawing area is realized, the true initial size is set by nvim.ui_try_resize_async in the first drawing area resize event. It would be better to delay attachment until the initial size is known.

smolck commented 5 years ago

@smolck if you can produce this with TUI, you should open a bug report on neovim's repo.

It may be a Termite bug (can only reproduce in Termite): https://github.com/thestinger/termite/issues/686#issue-461756403

In the case of Termite, Neovim isn't getting the first resize event. Maybe the same is true of GNvim (still wouldn't explain the cargo run inconsistency though).

smolck commented 5 years ago

After throwing a dbg!(&args) here in nvim_bridge/mod.rs and looking at the output, when this bug happens GNvim is only getting a single (when I tested it) grid_resize event from Neovim, with the wrong width and height. Not sure why that is happening?

It would be better to delay attachment until the initial size is known.

I may be able to do some work on this and submit a PR. It's a good idea, and I think that may be why neovim-gtk doesn't get this problem; if it starts nvim with the correct ui size (which it appears to from reading the source), then there is no need to depend on grid_resize events to make the grid's initial size correct on startup.

smolck commented 5 years ago

@vhakulinen @badosu After some testing, GNvim doesn’t get the right values in the grid_resize events from Neovim on startup (i.e. it gets different width and height vals from those it gets when forced to redraw/resize the grid), so the grid doesn’t fill the window on initial draw (I think it redraws correctly for the width and height that it is given though). Any ideas of what could cause that? Bfredl mentioned above that GNvim is inefficient in attaching the UI, but could that really be the cause? It seems slight enough that it wouldn’t cause this issue, but IDK.

As a side note, neovim-gtk delays the UI attachment as necessary, removing that inefficiency. However, it may require a decently-sized refactor to add this delayed attachment to GNvim, and I’m not sure how high of a priority adding the necessary code is (unless it’s causing this problem, which I don’t think I can test very easily ATM). For now, I can try to get a working branch where the UI attachment is delayed until the drawing area is realized to test if that slight inefficiency is the issue (unless you guys have a better idea, which I’m fully open to!).

bfredl commented 5 years ago

I'm not sure if it needs to be a big refractor. You can still create/connect the nvim instance in the same place. What is needed is a flag to indicate if ui_attach is called yet, and then the gtk resize handler should call it the first time instead of ui_try_resize and change the flag.

It would at least fix issues with error messages in init.vim being drawn properly.

smolck commented 5 years ago

@YaLTeR could you try out #86's branch (https://github.com/smolck/gnvim/tree/fix-ui-attach-inefficiency) and see if that fixes this issue for you?

YaLTeR commented 5 years ago

It does start with the correct size, but with a small graphical flicker before and also scrolled a couple of lines down.

smolck commented 5 years ago

. . . but with a small graphical flicker before . . .

I may be wrong but I think that was already there. I think #86 makes it more noticeable though, so we may want to look into ways of removing that (if possible).

. . . and also scrolled a couple of lines down.

@YaLTeR Not sure I can reproduce this. Do you mean your cursor is on a different line from the first one, or something else?

YaLTeR commented 5 years ago

Well, I use the Startify plugin which provides a start screen, and this is how it looks on start: image as opposed to: image

smolck commented 5 years ago

I can reproduce it using Startify, and I was able to position it correctly by scrolling up with the mouse wheel; I wonder why it starts scrolled downwards? It seems to only happen with Startify, not when opening a file from gnvim via $ gnvim <filename>.

smolck commented 5 years ago

@vhakulinen @badosu Is there a way we could optimize/change how GNvim is drawn initially? Ideally we'd only call window.show_all() after everything's loaded "behind the scenes", but is that possible? (Alternatively, maybe we could change when and in what order everything's drawn?) It seems drawing GNvim initially is a bit finicky (e.g. using ui.win.borrow().show_all() here instead of window.show_all() here doesn't draw things correctly).

badosu commented 5 years ago

@smolck I've not had too much time lately, but will try to take a look at this issue this week.

By the way, I tested the latest master and the issue persists with the gap on the bottom part when opening gnvim with a split on i3. The UI loads much snappier now though, so definitely an improvement, good job!

Screenshot from 2019-08-05 10-11-09

smolck commented 5 years ago

@badosu Okay, and thanks!

By the way, I tested the latest master and the issue persists . . .

Just so I am certain, when you say latest master, you mean you tried my branch, right? Strange; it fixes my problem on AwesomeWM. I’ll have to start up i3 & Sway and see if I get the same result.

Currently (I think, may be wrong here) when the grid size changes (the window resizes) GNvim tells Neovim to resize itself, which in turn causes GNvim to handle the grid_resize event that Neovim sends after we call ui_try_resize_async. The grid size problem may have something to do with this indirect resizing, but I’m not certain. See here for what I’m talking about.

badosu commented 5 years ago

Just so I am certain, when you say latest master, you mean you tried my branch, right?

Ah, thought it was already merged. Tested and it's fixed :clap:

I think the snappy improvement is due to the internals refactor then, great job!