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

Support for terminal resizing #2

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

NthStarFarer
Copy link

@NthStarFarer NthStarFarer commented Aug 29, 2023

Still work in progress. resolves #1

@NthStarFarer
Copy link
Author

This could be overkill, but I don't really like the idea of polling every frame if it isn't necessary. Unfortunately, I haven't found a good alternative on windows. The closest seems to be WINDOWS_BUFFER_SIZE_EVENT, but that doesn't necessarily match sigwinch's behvaiour afaik.

@NthStarFarer
Copy link
Author

Ok I think that this is ready. Using the alternative screen prevents the artifacting that got left behind after shrinking the screen, and #4 fixes the rest. Ideally we could also prevent the odd patterns that appear when resizing by disabling line-wrapping but I didn't have any success there. Since there only appear for a frame or so they aren't really high priority IMO.

@NthStarFarer NthStarFarer marked this pull request as ready for review September 3, 2023 21:04
@lilyyy411
Copy link
Owner

I've been playing around with this for a bit and I've noticed that there's the occasional assertion failures when resizing too quickly.

@NthStarFarer
Copy link
Author

Took a bit but I was able to replicate it, it seems to depend a bit on performance which is why I didn't catch it.

The actual issue is a race condition between the main thread and the producer, since there isn't anything to keep each frame ready message synced with the frame data. Normally the desync just means that a frame gets dropped, but if a resize happens in between sending the first FrameReady message and updating the data then the sizes won't match.

Reverting the size of the notify channel to one and only updating the frame data after resizing should fix it, but this does mean that one frame is almost always dropped after resizing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for resizing the terminal
2 participants