Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wezterm does not always notice window resize #1992

Closed
yuttie opened this issue May 15, 2022 · 39 comments
Closed

Wezterm does not always notice window resize #1992

yuttie opened this issue May 15, 2022 · 39 comments
Labels
bug Something isn't working nvidia nvidia specific

Comments

@yuttie
Copy link
Sponsor

yuttie commented May 15, 2022

What Operating System(s) are you seeing this problem on?

Linux X11

WezTerm version

20220514-175933-64b6e63f

Did you try the latest nightly build to see if the issue is better (or worse!) than your current version?

Yes, and I updated the version box above to show the version of the nightly that I tried

Describe the bug

Sometimes wezterm does not recognize its window resize.
Especially toggling fullscreen mostly does not work while resizing horizontally or vertically in a incremental fashion works a bit better.
Please watch my screen recording.

It seems that continuously (frequently) changing window size makes wezterm notice resize events, e.g., by using mouse or by holding keys for shrinking/enlarging a window or toggling fullscreen.

I tried both my window manager's key binding and wezterm's Alt+Enter to toggle fullscreen state.

I'm using i3, specifically i3-gaps v4.20.1, as the window manager on Gentoo Linux.

To Reproduce

Launch a wezterm and resize its window.

Configuration

no config

Expected Behavior

Resizing a terminal window should affect the terminal's content area size.

Logs

wezterm-20220514-175933-64b6e63f.log

wezterm-20220514-175933-64b6e63f.mp4

Anything else?

I bisected at release level and found that WezTerm-20210502-154244-3f7122cb-Ubuntu16.04.AppImage worked much better than WezTerm-20210814-124438-54e29167-Ubuntu16.04.AppImage.
Although the former do fail to response to fullscreen toggle but much less frequently.

I've checked the release notes of the latter, it seems that the most relevant issue is only #695.

@yuttie yuttie added the bug Something isn't working label May 15, 2022
@yuttie
Copy link
Sponsor Author

yuttie commented May 19, 2022

Today, I built another set of binaries of version 20220517-184546-9b5c8878 on an Arch Linux machine, and rsync-ed them to a different machine, which is the Gentoo Linux machine I mentioned in the previous comment.

I found the reported issue only reproduced on the Gentoo machine and any resize-related problems were not found on the Arch Linux machine.

I'm using i3-gaps of version 4.20.1 on both machines. So, some other difference(s) must be causing the issue. I will try different settings on Gentoo side.

@yuttie
Copy link
Sponsor Author

yuttie commented May 20, 2022

As the first step, I tried different Nvidia driver versions available on Gentoo.
But none of them worked.

@yuttie
Copy link
Sponsor Author

yuttie commented May 20, 2022

@wez BTW, I tried to modify source code and found only a couple of lines of change solved the problem although repaint is somewhat delayed when toggling fullscreen state.

The change was made on top of bc7c838 as follows:

diff --git a/window/src/os/x11/window.rs b/window/src/os/x11/window.rs
index f2931fd5..c84bcd79 100644
--- a/window/src/os/x11/window.rs
+++ b/window/src/os/x11/window.rs
@@ -205,7 +205,7 @@ impl XWindowInner {
         }
 
         if let Some(resize) = resize.take() {
-            self.dispatched_any_resize = true;
+            self.dispatched_any_resize = false;
             self.events.dispatch(resize);
         }
 
@@ -231,7 +231,7 @@ impl XWindowInner {
                 }
 
                 if !self.dispatched_any_resize {
-                    self.dispatched_any_resize = true;
+                    self.dispatched_any_resize = false;
 
                     log::trace!(
                         "About to paint, but we've never dispatched a Resized \

I tested 4 patterns of combinations of boolean values, only setting both to false helped.

Of course, we have not understood the actual cause yet, however, I believe we have confirmed that the window manager is giving the window a resize notification.

@wez
Copy link
Owner

wez commented May 20, 2022

I think the root cause is that your WM isn't sending resize events consistently.

The change that you've sketched out pessimizes painting by requiring that wezterm ask the X server for the dimensions prior to every paint. I think that makes sense for this situation, but I'd prefer to only use it when we can't trust the WM.

Have you considered filing a bug report with the i3-gaps folks to see if they can fix this at the source?

@yuttie
Copy link
Sponsor Author

yuttie commented May 20, 2022

The change that you've sketched out pessimizes painting

OK, I understand what those changes actually mean.
Because I love Wezterm's performance, I don't want to sacrifice the performance.

I think the root cause is that your WM isn't sending resize events consistently.

Have you considered filing a bug report with the i3-gaps folks to see if they can fix this at the source?

Yes, but I'm not sure if it is really caused by i3-gaps because wezterm works well with i3-gaps on my Arch machine.
Please do not misunderstand me. It does not mean that I think the root cause is on your side.

I will continue my investigation on the difference between my machines and report here when I find something important.
Thank you for taking the time to give comments.

@yuttie
Copy link
Sponsor Author

yuttie commented May 20, 2022

I found using Nouveau driver instead of Nvidia's proprietary one fixes the issue.
I confirmed with the initially reported version 20220514-175933-64b6e63f.

Other things I have tried but did not work:

  • Using the original i3 window manager 4.20.1 (not i3-gaps)
  • Using awesome window manager 4.3-r101
  • EDIT: Using different version of Nvidia drivers: 470.103.01, 510.68.02, 510.73.05

Now I'm a bit more confident with the irrelevance of window manager to this issue.
However, it could still be a problem specific to my environment.

Since Nouveau's performance is not good on my GeForce GTX 1070, I don't want to solve the issue by switching the driver.
I hope any other solution could be found...

EDIT: It seems that __EGL_VENDOR_LIBRARY_FILENAMES=/usr/share/glvnd/egl_vendor.d/50_mesa.json wezterm also fixes the issue. However, it leads to another problem that wezterm crashes when I make the terminal fullscreen. I found the workaround from #484.

@erf
Copy link
Contributor

erf commented May 20, 2022

I recently noticed that the window_padding on window-resized events are no longer called on my macOS, so maybe this is related.

@Lenbok
Copy link

Lenbok commented May 20, 2022

I am noticing resize related problems on WindowMaker window manager on Ubuntu 20.04. I tested this scenario with git bisect:

  • create new wezterm window (my default is approximately full screen)
  • open launcher, verify it looks good, escape out of it
  • resize window to quarter of the screen
  • open launcher, notice whether it has the top lines truncated (bad) or if it looks good.

Git bisect had a bunch of commits that needed to be skipped due to e.g.:

$ cargo build --release
    Updating git repository `https://github.com/wez/xkbcommon-rs.git`
error: failed to get `xkbcommon` as a dependency of package `window v0.1.0 (/home2/len/dev/wezterm/window)`

Caused by:
  failed to load source for dependency `xkbcommon`

Caused by:
  Unable to update https://github.com/wez/xkbcommon-rs.git?rev=5cc8f233ac2b8bfa0d7a26fd981b77e68c3f2219#5cc8f233

Caused by:
  object not found - no match for id (5cc8f233ac2b8bfa0d7a26fd981b77e68c3f2219); class=Odb (9); code=NotFound (-3)

At the end:

$ git bisect skip
There are only 'skip'ped commits left to test.
The first bad commit could be any of:
4b018f564ab7469cba929a525950207721cd45ce
e9e590c64e1628f524e460f1ea67909e4f9d7e3a
0a0a1004eb3d6f619c43ad9f5669d0ac1809a839
6484b3adc0ecb9ae14d44292fc8f43c8cc9800f7
7858f652fb6a49e1dee44a47864492d0212e57d3
b365cdbb94b4a17d0c81cd169ce49c779aad4356
74e1cdcb40b79df4bf64da972c9407c67bb19213
abc42f7bcbf3f712727e4bff4db2115d23063b29
8523a1b0e72e2539ad9d9a5455047a854908155f
b25d4d22eeeaf835ae3c242d6ddde8dd18cee667
e5a483ff8b351079e502d91852c17060d27d86ba
We cannot bisect more!

@Lenbok
Copy link

Lenbok commented May 26, 2022

I think this is also related (but I've not explicitly tested if it's the same commit that introduced the issue). If I minimize a wezterm window and then unminimize it, the cursor is hollow (indicating it doesn't have focus, although keypresses etc actually do go there).

@Mange
Copy link

Mange commented May 27, 2022

I have the same issue, on Awesome WM + nvidia drivers.
(Just to add another piece on the pile.)

Problem for me is that I'm on a new machine, so everything installed the latest versions at the same time and I'm not sure which package "caused" this. Could be the latest nightly, or the latest version of nvidia drivers.

wez added a commit that referenced this issue May 27, 2022
This commit:

* Logs atom names in property change events (makes it easier to
  understand user's logs)
* Sets flags in cases where property changes might imply that a
  configure or focus event should have or should be sent
* Adjusts the "unsure about state" logic so that it doesn't just
  trigger on the initial paint, but also on those flags being set

refs: #1992
@wez
Copy link
Owner

wez commented May 27, 2022

I just pushed a commit that may help with this; could you give it a try when you have a chance?

It should show up in a nightly build within about 20 minutes from now.

If you prefer to use packages provided by your distribution or package manager of choice and don't want to replace that with a nightly download, keep in mind that you can download portable packages (eg: a .dmg file on macOS, a .zip file on Windows and an .AppImage file on Linux) that can be run without permanently installing or replacing an existing package, and can then simply be deleted once you no longer need them.

If you are eager and can build from source then you may be able to try this out more quickly.

@Lenbok
Copy link

Lenbok commented May 27, 2022

I built from source and tested. Unfortunately, no change for my test described in #1992 (comment)

Edit: I also tested using the __EGL_VENDOR_LIBRARY_FILENAMES define mentioned in #1992 (comment) and can confirm setting that variable as described does fix the issue (although there is a weird flicker upon resize). I am using nvidia driver version 470.129.06

@wez
Copy link
Owner

wez commented Jun 1, 2022

@Lenbok: I think you're experiencing something different, as this issue is about the X11 WM not consistently reporting resize events, and that doesn't vary based on the EGL driver.

@yuttie: please try a more recent nightly and report back!

@wez wez added the waiting-on-op Waiting for more information from the original poster label Jun 1, 2022
@Lenbok
Copy link

Lenbok commented Jun 1, 2022

@wez Are you sure? My issue is not just painting related. When I run the last of the steps described above to reproduce the bug, (opening the launcher when the window has been sized to a quarter of the screen), the launcher itself thinks the size of the window is full screen rather than quarter screen (you can still move the selected item up and down, although most of the time it's not visible until you move down enough items), so wezterm has the wrong idea of the size. Setting the EGL variable somehow makes this test ensure that wezterm has the correct size.

@yuttie
Copy link
Sponsor Author

yuttie commented Jun 2, 2022

@wez Sorry for my late reply because I was quite busy recently.
Thank you for your concern about this issue.
I will give it a try later and report back on the results.

@github-actions github-actions bot removed the waiting-on-op Waiting for more information from the original poster label Jun 2, 2022
@wez wez added the waiting-on-op Waiting for more information from the original poster label Jun 2, 2022
@yuttie
Copy link
Sponsor Author

yuttie commented Jun 2, 2022

@wez I compared binaries that I built from the latest commit 9270e3b and from the first reported commit 64b6e63.
I cannot see big difference between them, and also same as before.
But, in the case of the latest binary, it seems that the window content gets redrawn after the window lose focus while the old binary does not.

@github-actions github-actions bot removed the waiting-on-op Waiting for more information from the original poster label Jun 2, 2022
wez added a commit that referenced this issue Jun 2, 2022
From the logs shared in #2063 (comment)
it looks like focus change events can correlate with the WM tiling
windows, even though the WM doesn't send a CONFIGURE_NOTIFY event.

So let's set a flag to query the geometry when the focus changes

refs: #2063
refs: #1992
wez added a commit that referenced this issue Jun 2, 2022
Main idea here is to add expose events to the trace so that we
can get a sense of whether those are emitted when a WM isn't delivering
CONFIGURE_NOTIFY consistently.

If the exposed area is bigger than what we think the window is,
then we mark geometry as unsure.

I'm considering always marking as unsure for an expose event,
or at least, adding a config option to enable that, as a way
to workaround this situation.

refs: #2063
refs: #1992
@yuttie
Copy link
Sponsor Author

yuttie commented Jun 3, 2022

@wez I built the binary from the latest commit 402e5b7 just now.
I found that the binary solved the issue completely so far!
I'll perform bisect.

@yuttie
Copy link
Sponsor Author

yuttie commented Jun 3, 2022

@wez I identified the commit that solved my issue, it is c3b1aa7.

EDIT: I'll use wezterm all day tomorrow and see if there are any problems with long term use.

wez added a commit that referenced this issue Jun 4, 2022
The hope here is that the nvidia-specific resize issue might have
a workaround if it is emitting some other events that we were
previously not listening for.

This commit optionally enables the Present extension and listens
for its version of CONFIGURE_NOTIFY, routing it through the same
logic as the base CONFIGURE_NOTIFY event.

On my AMD hardware under Gnome, I see something like:

```
18:04:26.476  TRACE  window::os::x11::window > Present::ConfigureNotify: width 1168 -> 1180, height 858 -> 873, dpi 124.7998046875 -> 124.7998046875
18:04:26.478  TRACE  window::os::x11::window > Ignoring X::ConfigureNotify (1180x873 dpi=124.7998046875) because width,height,dpi are unchanged
```

with the Present event firing before the X event.

Let's see how this goes.

refs: #2063
refs: #1992
@Lenbok
Copy link

Lenbok commented Jun 4, 2022

@wez Are you able to restore the missing commits that prevent earlier builds from being built from source?

Builds between 2022-05-08 and 2022-05-10 complain about not being able to find:
https://github.com/wez/xkbcommon-rs.git?rev=69a29877e99369b4ec74703d3193b6dd694145e9#69a29877
https://github.com/wez/xcb-imdkit-rs.git?rev=7498cce1085ec23535f1b8d6729407485bf66c00#7498cce1

@Lenbok
Copy link

Lenbok commented Jun 4, 2022

BTW, I built from source just now (after seeing your commit da59a22) and this seems to fix this issue for me - my test case has wezterm using the correct window size for the launcher when the window size is reduced.

The only remaining niggle I can see is the hollow / solid cursor weirdness I mentioned above, where after I maximise the window I sometimes get a hollow cursor (even though the window has focus and accepts keystrokes). Moving focus out and back gives the expected solid cursor.

@wez wez added the nvidia nvidia specific label Jun 4, 2022
@wez
Copy link
Owner

wez commented Jun 4, 2022

Are you able to restore the missing commits that prevent earlier builds from being built from source?

done!

@wez
Copy link
Owner

wez commented Jun 4, 2022

Summarizing here the findings from over in #2063:

  • This does appear to be nvidia driver related
  • The WM/XServer doesn't generate correct (or in some cases, any) FocusIn, FocusOut or ConfigureNotify events various situations with the nvidia driver enabled when EGL is in use
  • The current state of main has a couple of hacks to try to work around this, but they are imperfect; the real fix needs to be made in the nvidia driver

@Lenbok
Copy link

Lenbok commented Jun 5, 2022

Also, with the restored commits I was able to complete the git bisect to confirm that abc42f7 was the commit that introduced the regression.

@wez
Copy link
Owner

wez commented Jun 5, 2022

@Lenbok could you summarize the difference in behavior you see between that rev and the one prior? I don't have a clear picture

@yuttie
Copy link
Sponsor Author

yuttie commented Jun 5, 2022

Just an additional test.
I confirmed in my environment that abc42f7 and abc42f7~ (i.e., b25d4d2) behaved differently; the former has the resize problem, but the latter does not.

@wez
Copy link
Owner

wez commented Jun 5, 2022

Well, that's weird! Thanks for confirming!

@yuttie
Copy link
Sponsor Author

yuttie commented Jun 5, 2022

Interestingly, it seems that abc42f7~ (i.e., b25d4d2) also does not have the problem of #1628 while abc42f7 causes input hang.
These issues are common in that they involve resizing.

I'm not completely sure that I have the same problem as reported in #1628, but I have found some common points.
I'm not using Pop!_OS but using ibus as input method (I have $XMODIFIERS be set to @im=ibus), and I confirmed that disabling IME by use_ime = false solves the problem.

patrickjonesuk added a commit to patrickjonesuk/wezterm that referenced this issue Jun 12, 2022
Adds a configuration option, `focus_change_repaint_delay` to allow customisation of the delay added in 9b6329b.

The default delay remains 100ms, and can be disabled by setting it to `0`, if the workaround is not required on the user's system.

refs: wez#2063
refs: wez#1992
patrickjonesuk added a commit to patrickjonesuk/wezterm that referenced this issue Jun 12, 2022
The focus events used to trigger a query of geometry are innacurate, so the window's focus state should also be queried.

refs: wez#2063
refs: wez#1992
patrickjonesuk added a commit to patrickjonesuk/wezterm that referenced this issue Jun 12, 2022
The focus events used to trigger a query of geometry are inaccurate, so the window's focus state should also be queried.

refs: wez#2063
refs: wez#1992
wez pushed a commit that referenced this issue Jun 16, 2022
Adds a configuration option, `focus_change_repaint_delay` to allow customisation of the delay added in 9b6329b.

The default delay remains 100ms, and can be disabled by setting it to `0`, if the workaround is not required on the user's system.

refs: #2063
refs: #1992
wez pushed a commit that referenced this issue Jun 16, 2022
The focus events used to trigger a query of geometry are inaccurate, so the window's focus state should also be queried.

refs: #2063
refs: #1992
@wez
Copy link
Owner

wez commented Jun 16, 2022

@yuttie are you saying that setting use_ime=false resolves this weird resize issue?
@pjones123: do you think you could try that?

@yuttie
Copy link
Sponsor Author

yuttie commented Jun 17, 2022

@wez No. I mean that both the problems #1628 and #1992 do not exist at b25d4d2 but do exist at abc42f7 (b25d4d2's child commit).

I suspect abc42f7 introduced the problems. These issues might have the same root.
In fact, I have been using wezterm 20220506-201914-b25d4d22 and encountered no problems so far.

wez added a commit that referenced this issue Jun 18, 2022
This does two things:

* Sets the event queue owner explicitly to xcb
* Adopts a dri2 resize related workaround from the rust-xcb opengl
  example

I think the latter is probably a NOP, but the former sounds like
something important.

refs: #1992
@wez
Copy link
Owner

wez commented Jun 18, 2022

I took a look at the rust-xcb opengl example and adapted a couple of bits from it to wezterm's code base; could you give 1485cf3 a try when you have a chance?

@yuttie
Copy link
Sponsor Author

yuttie commented Jun 18, 2022

@wez I tried the commit with the default config and found no problems.
I could not reproduce either #1628 or #1992.

@wez
Copy link
Owner

wez commented Jun 18, 2022

Great!

@yuttie
Copy link
Sponsor Author

yuttie commented Jun 18, 2022

@wez I also tried its parent commit 00e5b9a.

Regarding #1628, I could reproduce the problem with binary built from 00e5b9a, and therefore it seems that those changes introduced by 1485cf3 solved the issue.

On the other hand, regarding the present issue (#1992), I could not reproduce the problem with binary built from 00e5b9a.
I think this is because you added workarounds for the problem.

Is it possible for you to create a branch that reverts those workarounds related to #1992?
I would like to give it a try to confirm that 1485cf3 alone solved #1992.
(I mean that the branch would be based on 1485cf3 but not include workarounds.)

@chaoyi
Copy link

chaoyi commented Jun 18, 2022

I had the same problem. After window resize, wezterm does not accept keyboard input (mouse input still works). An error pops up when that happens: X11 conneciton is broken: xcb conneciton error: Connection error, possible I/O error. Latest commit fixed the problem, as well as using use_ime = false.

If related, I use Nvidia driver and fcitx IME. I didn't notice the issue was associated with window resize (because of titling window manager). Thanks for making wezterm awesome.

@wez
Copy link
Owner

wez commented Jun 18, 2022

@yuttie

This was the bit that I think made the difference: conn.set_event_queue_owner(xcb::EventQueueOwner::Xcb);

I'd suggest checking out abc42f7 and then adding that line; here's the relevant section of code in main for context:

pub(crate) fn create_new() -> anyhow::Result<Rc<XConnection>> {
let (conn, screen_num) = xcb::Connection::connect_with_xlib_display_and_extensions(
&[xcb::Extension::Xkb],
&[
xcb::Extension::Present,
xcb::Extension::RandR,
xcb::Extension::Render,
],
)?;
conn.set_event_queue_owner(xcb::EventQueueOwner::Xcb);

Thanks for being thorough!

@yuttie
Copy link
Sponsor Author

yuttie commented Jun 18, 2022

@wez Thank you for your instruction!

Following your suggestion, I only examined the existence of the present issue (#1992) this time:

  • abc42f7: resize problem exists
  • abc42f7 + conn.set_event_queue_owner(...): resize problem do not exist.

Bingo! That line is the key to solve the problem! 👍

wez added a commit that referenced this issue Jun 18, 2022
We found the real root of the problem in so we shouldn't need this any
longer

refs: #1992
refs: #1628
refs: #2063
@wez wez closed this as completed Jun 22, 2022
@yuttie
Copy link
Sponsor Author

yuttie commented Jun 25, 2022

@wez I'd like to say thank you for your time to develop and improve wezterm.
I'm really happy to have been able to help improve it in any way I could.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 3, 2023

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working nvidia nvidia specific
Projects
None yet
Development

No branches or pull requests

6 participants