zed
zed copied to clipboard
zed-industries
terminal: Fix copy to clipboard lag
Fixes #7322
Note: I don't know much Rust (yet) and I also believe that this isn't a general fix of the underlying issue. There might be more of the same issue here, e.g. clearing the terminal (via CMD+K) also has a significant delay sometimes (whenever you get unlucky and the time to the next paint is long I assume).
I'm happy to try to fix this properly with some guidance.
We require contributors to sign our Contributor License Agreement, and we don't have @nathanael-ruf on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.
![cla-bot[bot] avatar](https://avatars.githubusercontent.com/in/2404?v=4 "Published by a pub.dev verified publisher"){kind=small}
So is the idea here that you skip the event loop entirely? Is there no alternative solution possible where we wake up the event loop listener on certain events?
So is the idea here that you skip the event loop entirely? Is there no alternative solution possible where we wake up the event loop listener on certain events?
If by event loop you mean paint loop (at least that is my current understanding) then yes. As stated in the issue, I feel like waiting for (or triggering) paint to copy something into the clipboard doesn't make sense. Ifaics copying to the clipboard has no effect on what is rendered, so why do we need to (wait for) paint?
Maybe it makes more sense if we also take other events into account though, such as clearing the terminal which does have an effect on what is rendered.
If by event loop you mean paint loop (at least that is my current understanding) then yes. As stated in the issue, I feel like waiting for (or triggering) paint to copy something into the clipboard doesn't make sense. Ifaics copying to the clipboard has no effect on what is rendered, so why do we need to (wait for) paint?
I don't think the event loop in the terminal is strictly about rendering/painting. It's about the state of the terminal itself.
And this bug (and others that have popped up) make me think that maybe something with the event loop and how often it runs is broken, not just with the copy behaviour itself.
And this bug (and others that have popped up) make me think that maybe something with the event loop and how often it runs is broken, not just with the copy behaviour itself.
Yes it's not just copying.
I also believe that this isn't a general fix of the underlying issue
I don't think the event loop in the terminal is strictly about rendering/painting. It's about the state of the terminal itself.
terminal.sync(cx); is called in TerminalElement:compute_layout which is called in TerminalElement:paint. I don't know yet how GPUI works, i.e. how it decides when to paint, but from the name it sounds like this is just about painting and e.g. copying to the clipboard shouldn't trigger a repaint (maybe?).
The deeper solution to this problem IMO is to take ownership of the Alacritty event loop code and let it own the alacritty Terminal struct. Then we can send everything in the existing, poorly named, EventLoop struct to the Alacritty terminal as messages that can be replied to instantly.
Lacking that refactoring though, I think we should stick to the current solution which maintains the relative ordering of events in time.
Thanks for the suggestion, seems to work, wasn't able to reproduce the linked bug (copy paste).
I'm wondering why it doesn't fix the sporadic delay when clearing the terminal (cmd + K) though (also has a cx.notify()).
I believe those errors are caused upstream by the action resolution mechanism. We’ve merged a fix for those though :)
And the test_multiple_excerpts_large_multibuffer test failure is my fault, I'm about to merge the last PR on the topic that makes it stable: https://github.com/zed-industries/zed/pull/7399
Restarted the tests meanwhile.
