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

"Layout / Wrap text output on resize" option breaks pseudographic UI #383

Closed
alabuzhev opened this issue Mar 7, 2019 · 17 comments
Closed
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Tag-Fix Doesn't match tag requirements Product-Conhost For issues in the Console codebase

Comments

@alabuzhev
Copy link
Contributor

When "Layout / Wrap text output on resize" is enabled, the console host assumes that rows were split due to wrapping and on resize tries to merge them back and re-split again to fit in a new window size.

This works fine for cmd.exe and probably other command processors due to their stream-like nature, but for apps with a ixed pseudographic UI it leads to a complete mess (example).

Is there a way to programmatically suppress this feature, similarly to SetConsoleMode?

Alternatively, could the merging algorithm be smarter and don't try to merge box-drawing characters (│┤╡╢╖╕╣║╗╝╜╛┐etc.)?

@zadjii-msft
Copy link
Member

If you ask me, it should be up to the client application to listen for resize events, and redraw it's UI if it notices that it's been resized. I'm pretty sure that's how most linux-like applications handle resizing.

@zadjii-msft zadjii-msft added the Issue-Question For questions or discussion label Mar 7, 2019
@oising
Copy link
Collaborator

oising commented Mar 7, 2019

If only we had resize events, @zadjii-msft -- are you guys planning to add some kind of SIGWINCH functionality?

@DHowett-MSFT
Copy link
Contributor

ReadConsoleInput produces WINDOW_BUFFER_SIZE_EVENT input events when the window or buffer size is changed. That's within a couple standard deviations of a resize event, right?

@oising
Copy link
Collaborator

oising commented Mar 7, 2019

@DHowett-MSFT -- as I posted elsewhere, that only fires when the buffer size is increased due to the window size being increased. If you reduce the size of the window, the buffer remains unchanged and no event fires. In effect, WINDOW_BUFFER_SIZE_EVENT is useless ;)

@zadjii-msft
Copy link
Member

I'm pretty certain in the last few releases we've changed that, so that all resizes send the WINDOW_BUFFER_SIZE_EVENT. It's possible however that I only made that change for apps that enable VT input...

@oising
Copy link
Collaborator

oising commented Mar 7, 2019

@zadjii-msft -- way to miss out on announcing something major. But I will reserve judgment until I check. If you did change that ( I don't believe you did ) then I think there will be other issues. Let's not conflate the BUFFER with the VIEWPORT. Applications rarely concern themselves with the former.

update - remember we talked about it here? see: #305

@miniksa
Copy link
Member

miniksa commented Mar 7, 2019

You're all looking for #281. We need to figure out the right story for this and I made a giant deliverable to figure it out because it's an unmitigated disaster.

@zadjii-msft
Copy link
Member

Indeed, which is the exact reason we made the change. It was originally for WSL, which couldn't tell that the viewport size changed. However, we couldn't change the actual API to add another event at the time, so we just decided that any time the viewport changes in VT input mode to fire off a WINDOW_BUFFER_SIZE_EVENT, then update wsl.exe to call GetConsoleScreenBufferInfo to find the actual window size.

@oising
Copy link
Collaborator

oising commented Mar 7, 2019

Okay, then if you get a moment on your twenty fifth coffee break of the day (update: meaning you guys are overworked already), then could you drop a note on #281 to say what you've changed? Changing the behaviour of WINDOW_BUFFER_SIZE_EVENT seems bothersome to me. I'd rather see a WINDOW_VIEWPORT_SIZE_EVENT be introduced, if anything. I'm guessing that the buffer size isn't changed when reducing the viewport, but we get the event anyway, right?

@DHowett-MSFT
Copy link
Contributor

There's no need to be hostile, Oisin.

@oising
Copy link
Collaborator

oising commented Mar 7, 2019

There's no need to be hostile, Oisin.

That wasn't meant to be hostile - I didn't mean to imply that you guys are lazy, I meant "need more coffee" in the sense of being overworked ;)

@alabuzhev
Copy link
Contributor Author

If you ask me, it should be up to the client application to listen for resize events, and redraw it's UI if it notices that it's been resized.

@zadjii-msft, that's exactly what we do. But when this option is enabled, the host is actively trying to 'help'. It looks like sometimes it does its magic before sending the event and we simply overwrite these artefacts with the correct UI. Sometimes, however (for example, when leaving the full screen mode) it gets weirder - it does something, sends the event and then does something else, so the artefacts stay.

@DHowett-MSFT
Copy link
Contributor

There's no need to be hostile, Oisin.

That wasn't meant to be hostile - I didn't mean to imply that you guys are lazy, I meant "need more coffee" in the sense of being overworked ;)

Cool 👍 sorry for misunderstanding!

@oising
Copy link
Collaborator

oising commented Mar 7, 2019

I updated the comment with some context (overworked), Dustin. Sorry for the terseness.

I just tested on 18351_19h1 and I can indeed see that WINDOWS_BUFFER_RESIZE_EVENT gets raised on window resize. So I guess that's a workaround for the moment until something more formal comes out of this.

Is this a temporary workaround or a more permanent one? It doesn't smell quite right to me. I know there's a lot of things going on and so much to figure out as @miniksa says above. That said, in effect, we're writing a terminal emulator here. It's not the first time it's been done. But yes, the innards are at an insane level of indirection and misdirection (conpty, VT state machine, buffers, reverse API calls, etc) and while they are a super clever approach to solving the problem of legacy code, it's hard to give feedback on a black box. We're learning all the quirks and decisions being made in Redmond by trial and by accidental discovery.

I hope you guys can offer a ConPTY passthrough mode for those of us working with our own VT state machines and buffers soon, even as a feature switch. I get that the bidirectional translation of VT/API work is important, but a passthrough mode would unblock a lot of us and cut out a lot of complaints while you guys took your time figuring out the harder things.

Thanks for all your hard work.

@zadjii-msft
Copy link
Member

@alabuzhev I'm definitely curious about the ordering here. I'm under the impression that we should only ever be firing off the WINDOW_BUFFER_SIZE_EVENT after the resize (and reflow) is done, but if there's a specific repro scenario that causes the events to come in the wrong order, I'm happy to file a bug to get it investigated and fixed.


@oising The workaroud is definitely temporary for now. Once we get to the list of tasks Niksa mentioned, we'll probably have a cleaner solution here.

Conpty passthrough mode would be awesome. It's definitely one of my personal preferences to move up on the backlog, though it would have to be something that client apps would opt themselves into, should they know they won't be using other APIs to write to the console.

@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label May 17, 2019
@miniksa miniksa added Area-Interaction Interacting with the vintage console window (as opposed to driving via API or hooks) Area-Settings Issues related to settings and customizability, for console or terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Conhost For issues in the Console codebase and removed Mass-Chaos labels May 17, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label May 18, 2019
@miniksa miniksa added Needs-Tag-Fix Doesn't match tag requirements and removed Issue-Question For questions or discussion labels May 18, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label May 18, 2019
@zadjii-msft zadjii-msft added Area-User Interface Issues pertaining to the user interface of the Console or Terminal and removed Area-Interaction Interacting with the vintage console window (as opposed to driving via API or hooks) labels Jul 7, 2020
@zadjii-msft
Copy link
Member

Trying to do some backlog cleanup. Just to confirm, yes, this was fixed (some time ago). Not sure the precise build number.

gh-383

We'll use #281 & #1765 for sorting out the remaining weirdness (and any others that are floating around)

@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Dec 13, 2021
@alabuzhev
Copy link
Contributor Author

@zadjii-msft it still looks like a mess in our case, at least in the scrollback area, which we do not touch:
image
I still think if would've been beneficial if individual apps could control this feature through the console mode flags - after all, the app knows better whether it's "wrappable" or not.

Also, a related question: I can't find a corresponding option in WT at all. How can I turn off the wrapping there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Tag-Fix Doesn't match tag requirements Product-Conhost For issues in the Console codebase
Projects
None yet
Development

No branches or pull requests

5 participants