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

Fix slow focus transitions to the terminal panel #24172

Merged
merged 3 commits into from
Feb 4, 2025

Conversation

mikayla-maki
Copy link
Member

@mikayla-maki mikayla-maki commented Feb 4, 2025

This long standing bug was caused by Pane's focus_in handler bouncing the focus to another handle.
Because focus resolution happens after a frame has been rendered, the only way to deal with this case is to schedule another frame to be redrawn. However, we where suppressing all window refreshes that occur during a focus transfer, causing this focus change to be completely missed. However, changing this behavior can lead to infinite notify loops, due to drawing a frame causing another to be rendered.

This PR fixes this problem narrowly by adding an on_next_frame() callback in the pane's focus handle, so that the focus changes take effect almost immediately. But only for this case, where we know it doesn't cause infinite notify loops.

TODO:

  • Fix the infinite notify loop bug or determine a third way to fix this lag

Release Notes:

  • Fixed a bug where shifting focus to the terminal panel could be slow

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Feb 4, 2025
@mikayla-maki mikayla-maki changed the title Fix focus transitions to the terminal being slow Fix slow focus transitions to the terminal panel Feb 4, 2025
@mikayla-maki mikayla-maki marked this pull request as ready for review February 4, 2025 06:06
@mikayla-maki mikayla-maki requested review from ConradIrwin and removed request for ConradIrwin February 4, 2025 06:07
@mikayla-maki
Copy link
Member Author

On looking deeper into this PR, I think we should actually fix the GPUI issue.

@mikayla-maki mikayla-maki reopened this Feb 4, 2025
@mikayla-maki
Copy link
Member Author

mikayla-maki commented Feb 4, 2025

Turns out the GPUI changes where easier than I thought!

Edit: clicking the right buttons on github however...

@mikayla-maki mikayla-maki reopened this Feb 4, 2025
@mikayla-maki mikayla-maki force-pushed the fix-delayed-focus-in-terminal-panel branch from 7a17b6f to 44823ef Compare February 4, 2025 06:28
@mikayla-maki mikayla-maki enabled auto-merge (squash) February 4, 2025 19:52
@mikayla-maki mikayla-maki disabled auto-merge February 4, 2025 19:58
@mikayla-maki mikayla-maki enabled auto-merge (squash) February 4, 2025 19:59
@mikayla-maki mikayla-maki merged commit 69bb0a0 into main Feb 4, 2025
14 checks passed
@mikayla-maki mikayla-maki deleted the fix-delayed-focus-in-terminal-panel branch February 4, 2025 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant