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

WINDOW_BUFFER_SIZE_EVENT generated during window scrolling #281

Open
alabuzhev opened this issue Oct 11, 2018 · 26 comments
Open

WINDOW_BUFFER_SIZE_EVENT generated during window scrolling #281

alabuzhev opened this issue Oct 11, 2018 · 26 comments
Labels
Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Conhost For issues in the Console codebase Work-Item It's being tracked by an actual work item internally. (to be removed soon)
Milestone

Comments

@alabuzhev
Copy link
Contributor

Windows Version 10.0.17763.1

SetConsoleWindowInfo can be used to scroll the contents of the console screen buffer by shifting the position of the window rectangle without changing its size.

Starting from Windows 10 1709 (FCU) such scrolling generates a WINDOW_BUFFER_SIZE_EVENT even though the console buffer size remains unchanged.

This breaks our application behaviour and does not make sense for the following reasons:

  • The documentation explicitly says that WINDOW_BUFFER_SIZE_RECORD "describes a change in the size of the console screen buffer", but there's no change in this case.
  • The event is generated only if the contents of the console screen buffer is scrolled via SetConsoleWindowInfo API, but it's useless - the application already knows that the console is being scrolled because the scrolling is initiated by the application itself.
  • event is not generated when the user moves the scrollbar manually, so the application does not know that the console it being scrolled in that case.
  • It does not happen in Legacy mode and never happened before for 20+ years.

A minimal project to reproduce the issue attached.
BufferSizeEventBug.zip

@miniksa miniksa self-assigned this Oct 11, 2018
@miniksa miniksa added Work-Item It's being tracked by an actual work item internally. (to be removed soon) Product-Conhost For issues in the Console codebase labels Oct 11, 2018
@miniksa
Copy link
Member

miniksa commented Oct 11, 2018

Thank you for the report. I've copied this into my internal bug queue as MSFT: 19275577.

@alabuzhev
Copy link
Contributor Author

Another (possibly related) observation: v2 console sends WINDOW_BUFFER_SIZE_EVENT even if ENABLE_WINDOW_INPUT flag is not set:

ENABLE_WINDOW_INPUT 0x0008 | User interactions that change the size of the console screen buffer are reported in the console's input buffer.

alabuzhev added a commit to FarGroup/FarManager that referenced this issue Oct 20, 2018
1. Hello again, rainbow: continuation of build 2070 after seven years.
   - Added a new option "Use Virtual Terminal for rendering" to Interface settings.
   - Added "Advanced" button to Color dialog.
   - Fixed bugs related to colors combination.
   In other words - happy Windows 10 owners now can enjoy TrueColor without intermediaries and right on the spot.

2. Workaround for microsoft/terminal#281.
@miniksa
Copy link
Member

miniksa commented Nov 20, 2018

It looks like this was an intentional change to fix Maximus5/ConEmu#1123.

We don't really have any other notification mechanism for the viewport or buffer changing, so it looks like we're triggering that event whenever the view changes even when the buffer size does not to try to keep things in sync.

It looks like at the time we thought that applications receiving an additional message with no actual change wouldn't really be harmed by that information as whatever they were doing would probably be able to detect nothing really happened with GetConsoleScreenBufferInfoEx before performing a further action after the event.

In theory, we could revert the change and come up with a new message type or notification type for the viewport change behavior and then change other downstream components that are now relying on it for synchrony, but I'm a bit hesitant to do that because this is the first report I've seen of it causing issues after being in the wild for over a year. I'm also afraid changing it to fix your issue will then cause me to have to completely revisit the ConEmu one and potentially cause other bugs as a result.

Is it an extremely onerous change in your software to realize that nothing has actually changed when this message comes through with the same buffer size as before? If so, can you further describe your scenario and estimated usage market? It would be useful to weigh your report versus the installed market of ConEmu and WSL users to see whether it's worth the risk of changing this further at this time or if I need to defer a more complete solution to a future milestone.

@alabuzhev
Copy link
Contributor Author

Thanks for looking into this.

Indeed, there was no way for applications to receive notifications about window changing. And now there is one (sort of).

Sadly this looks like a quick hack rather than a long-term solution - the only information provided is basically "something's happened, go figure it out yourself":

  • Has the buffer size changed? You can't say for sure any more without storing the previous size somewhere
  • Has the window size/position changed? Maybe, but you need to call another API to find it out and yes, also compare with the previous value, stored somewhere.

Also, what about this (see above):

  • The event is generated only if the contents of the console screen buffer is scrolled via SetConsoleWindowInfo API ...
  • event is not generated when the user moves the scrollbar manually ...

- Shouldn't the event be generated in both cases if you're triggering that event "whenever the view changes"?

Our scenario:

  1. On WINDOW_BUFFER_SIZE_EVENT the app recalculates it's interface, redraws everything and resets the viewport position
  2. The app provides several hotkeys to scroll the viewport using SetConsoleWindowInfo (very similar to the example in the first post)
  3. The user presses the hotkey to, say, scroll up, the app receives WINDOW_BUFFER_SIZE_EVENT and... see point 1. The viewport flickers and remains in the same position, the user is unhappy and files a bug report.

Detecting that nothing has actually changed is not extremely onerous, and that's what I already did a month ago (a commit reference is just above your comment), so it's not a blocker for us at least.
I can't say anything about other apps in the wild which might be also affected (or not) of course.

I'd say reverting this and introducing a new notification type for the window changes is a Good Thing To Do in general:

  • it won't break any code that existed before this change
  • this change is not documented, so, technically, any code that relies on it since last year relies on undocumented behaviour and should stop doing that anyway
  • any new code that needs to monitor window changes can be more straightforward and more efficient with the new notification type.

Obviously, the final decision is yours. But, in any case:

  • the inconsistency with the manual scrolling (see above) should probably be fixed?
  • the absence of ENABLE_WINDOW_INPUT console mode (see above) should probably be respected?
  • any changes should probably be reflected in the documentation?

@miniksa
Copy link
Member

miniksa commented Nov 20, 2018

You're right, the event should probably be generated in both of those cases if it should be generated in either one.

You're also right that the documentation should probably be reflecting what happens.

And you're right that we would have to look into the ENABLE_WINDOW_INPUT console mode.

You're also right that the solution of sending this event in hindsight is a dirtier quick hack than we thought it was at the time.

So I think what I'm going to do is promote the work item type here from a simple bug into a deliverable. It's obvious to me by how many nuances there are to this particular scenario that it needs a more concerted effort to resolve this than a simple bug fix. I do think The Right Thing To Do is for it to have its own event notification and try to resolve the ConEmu/WSL thing through that mechanism instead.

It looks like you are unblocked for now through your own workaround (sorry, I didn't see the link attached at first). That takes a bit of the pressure off and means that taking longer to craft the right solution (Deliverable style) is easier to justify than making another hack on top of this hack (Bug style).

@miniksa
Copy link
Member

miniksa commented Nov 20, 2018

Deliverable ID: 19275577 - Craft comprehensive solution to buffer/window overload on Input event
Task ID: 19686586 - Ensure that docs.microsoft.com reflects actual behavior of buffer/window overloads
Task ID: 19686595 - Create new/different eventing mechanism for window sizing concerns that isn't overloaded with buffer sizing
Task ID: 19686633 - Ensure that events for window buffer sizing and viewport sizing respect ENABLE_WINDOW_INPUT flag
Task ID: 19686624 - Fix WSL listener to receive new event type
Task ID: 19686633 - Verify ConEmu hosting WSL is proper after adjustments (see Maximus5/ConEmu#1123)
Task ID: 19686646 - Check consistency on event generation between SetConsoleWindowInfo API and manual scrollbar adjustment

@oising
Copy link
Collaborator

oising commented Nov 21, 2018

@miniksa I noticed (unless I'm doing something wrong) that ENABLE_VIRTUAL_TERMINAL_INPUT and ENABLE_WINDOWS_INPUT seem to be mutually exclusive. This is a real pain in the ass since there's no other way to get console host resize events. Is this a bug you're also tracking? If not, I'll open a new issue. I'm on latest insider build, 18282.

@DHowett-MSFT
Copy link
Contributor

@oising The answer to that depends on how you're reading the console. I think that ENABLE_VIRTUAL_TERMINAL_INPUT | ENABLE_WINDOW_INPUT requires you to ReadConsoleInput. You'll get different types of INPUT_RECORD: key events containing the VT data, and window events containing the window data. You'll also be able to learn about focus events, mouse events and menu events.

@oising
Copy link
Collaborator

oising commented Nov 21, 2018

@DHowett-MSFT -- that's exactly what I'm doing, except I'm not seeing any events in the buffer. I'm testing by resizing the console window (dragging the corner) and also by adjusting from the control box (top left). Nothing in the input buffer.

@zadjii-msft
Copy link
Member

Do you have a minimal repro sample code I could debug? What @DHowett-MSFT describes is how WSL does it, so I'm 99% sure that should be working for you.

@oising
Copy link
Collaborator

oising commented Nov 26, 2018

@zadjii-msft I think the problem is that I had never really taken a good look at WINDOW_BUFFER_SIZE_EVENT before. I was expecting it to be emitted any time the console window was resized. It's only emitted when the console window is sized beyond the current buffer. It's never emitted when the window size is reduced (as the buffer won't shrink to match.) What I really wanted is a way to reliably monitor the viewport not the buffer.

@zadjii-msft
Copy link
Member

@oising Yea, that's what I would have expected as well, but apparently whoever wrote that API originally hadn't considered that.

@miniksa miniksa added Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels Jan 18, 2019
@zadjii-msft
Copy link
Member

So for the record, sometime between RS2 and RS5, we changed the behavior of the console with respect to WINDOW_BUFFER_SIZE_EVENTs. We made the console send the events anytime the viewport size changed, not only the buffer size. However, we didn't change the actual values of the message, it still had the buffer size as the payload, not the viewport size. A client app can now use that message as a notification that the window might have changed size, and they could query the actual window size using GetConsoleScreenBufferInfo.

We haven't added more messages yet, nor have we changed the content of any messages, only increased the frequency of the messages.

I believe there were earlier builds where this change was scoped only to apps using VT input, but IIRC we changed it later to just send the events for all viewport changes regardless. This was probably a year ago, so I'm definitely hazy on the specifics.

@oising
Copy link
Collaborator

oising commented Mar 7, 2019

Thanks @zadjii-msft for the update. I'd still rather see a new event WINDOWS_VIEWPORT_SIZE_EVENT introduced rather than overload the buffer one, but I understand to ship is to choose. This will unblock a lot of people, but it would be nice to formalize it before it becomes a backcompat issue.

@joaobzrr
Copy link

So how do you sync UI update due to console resize and a subsequent resize? As far as I know there's no way to ask Windows to wait or postpone resizing the console.

@zadjii-msft
Copy link
Member

zadjii-msft commented Mar 22, 2023

/me necros thread

I believe there were earlier builds where this change was scoped only to apps using VT input, but IIRC we changed it later to just send the events for all viewport changes regardless. This was probably a year ago, so I'm definitely hazy on the specifics.

Well, if we meant to do this, we sure didn't. This works as expected for VT input, but not for apps that didn't ask for VT input.

I can still repro the behavior from OP even on Windows 11 (circa 1.15 conhost builds), so we haven't ever accidentally fixed the bug at the root of the OP, and we surely didn't get around to adding the WINDOW_VIEWPORT_SIZE_RECORD, which we considered the Good Thing To Do.


So I don't totally lose it: sample code for reproing this
https://gist.github.com/zadjii-msft/13d2f14e2125b674fdcb92525e2bde97

@zadjii-msft zadjii-msft added the Needs-Discussion Something that requires a team discussion before we can proceed label Mar 22, 2023
@zadjii-msft zadjii-msft removed the Needs-Discussion Something that requires a team discussion before we can proceed label Apr 3, 2023
@awsdert
Copy link

awsdert commented Jul 17, 2023

I just mentioned it on issue #305 but I realised the best way to trigger the SIGWINCH manually is to catch an event like WM_ERASEBKGND from the console window and poll the sizes there, here's a snippet from my code (which I also posted on issue #305):

pawttyxy _pawsig_ttysize = {0};
pawld _pawsig_catch_PAWSIG_TTYSIZE( pawsig *sig, UINT msg, WPARAM wp, LPARAM lp )
{
	pawttyxy xy = {0};
	CONSOLE_SCREEN_BUFFER_INFO csbi = {{0}};
	if ( msg == WM_ERASEBKGND )
		return 0;
	GetConsoleScreenBufferInfo(GetStdHandle(STD_OUTPUT_HANDLE), &csbi);
	xy.x = csbi.dwSize.X;
	xy.y = csbi.dwSize.Y;
	/* Only trigger the event if at least one of limits have changed */
	if ( xy.x && _pawsig_ttysize.x && xy.y == _pawsig_ttysize.y )
		return 0;
	sig->signal = PAWSIG_TTYSIZE;
	sig->extinf.size = xy;
	return 1;
}

I'm looping through the few events that I catch of the window so ignore the return statements and the library specific stuff like pawsig (making an all round solution that should run on all systems with the same binaries - excluding libpaw at mininum).

@determin1st
Copy link

determin1st commented Apr 22, 2024

window buffer size.. to me, is a mixture of names by previous naming artist, SCROLL_EVENT sounds better oriented.

scroll event happens only with the terminal that has a scrollback buffer, which is defined by 4 parameters - columns, rows, left-offset, top-offset. terminal that is switched in alternate-screen-thing doesnt have a scroll so the scroll always equals to the screen size (aka equals to zero) and is not generated

a change of any of the above parameters made by the user or by sending certain ESC sequences cause the scroll event. the scroll is not related to SIGWINCH that nix terminals have

resize event or window size event or screen size event is a separate event, it may occur with the scroll or alone. it is similar to SIGWINCH

*fix: resize event absorbs (includes) scroll event, resize > scroll, so only one shall be generated

@determin1st
Copy link

determin1st commented Apr 22, 2024

@awsdert those msg == WM_ERASEBKGND would NOT execute down below, seem like a CPU=100% loop.. im not in a window manipulation for now, better shall have described the subscription mechanism. a polling could be implemented without this window magic

@awsdert
Copy link

awsdert commented Apr 23, 2024

@awsdert those msg == WM_ERASEBKGND would NOT execute down below, seem like a CPU=100% loop.. im not in a window manipulation for now, better shall have described the subscription mechanism. a polling could be implemented without this window magic

Oh darn, being a linux user I would never have found out until some time after releasing the library & launcher pair I'm working on. I'm making it because I'm tired of all the ABI and executable/library type issues caused by data models and os specific APIs. Far too many variants so I'm working on the pair to just link static libs against native objects that deal with the main vs WinMain vs wMainMain vs DllMain vs etc to produce what the OS expects (*.exe vs *.AppImage vs *.so vs etc). via makefiles and a few variables handed to them for the downstream software to just pair with their static libraries to produce whatever the intended result is. For example:

build:
	$(CC) -o $(LIBPFX)foo$(LIBEXT) foo.a $(OSLIBS:%=-l %) ...

My project is on gitlab under the same username, paw repo, if anyone is interested in occasionally taking a look (I'm a lazy dev so will take a while).

@determin1st

This comment was marked as off-topic.

@awsdert

This comment was marked as off-topic.

@G1style

This comment was marked as off-topic.

@determin1st
Copy link

determin1st commented Apr 24, 2024

Did I say anything about "evil corps"? I don't think I did, I chose to move to gitlab for convenience, not because I think m$ is inherently evil. Though pretty much everyone on the planet (including myself) is (at the very least) vile when compared to God.

Github was certainly convenient but ever since the mandatory ssh key usage when uploading it became inconvenient for my small projects. I'm the only one besides God who knows my passwords, they're pretty damn secure with high entropy, they're kept in a KeePassXC database with at maximum encryption likewise high entropy password.

If I deem it safe to use my passwords instead of ssh keys then that should be my choice, not github's. For multi-dev projects, sure ssh keys should be mandatory but not single dev projects. Instead the ssh keys made it less secure just because I had to keep a copy of the unencrypted keys on my drive where any hacker that happens to break through my security can just copy it from.

too much to worry about, not lazy. git has some masterpassword too, though i dont use git except for git clone something, to check others code. lazily pushing GithubDesktop button in the FTP manner.. those comments and stuff are for corporate workers, so managers can track their salaries around.

As far as the binary compatibility stuff goes, you know about data models like LP32 vs ILP32 vs LP64 vs ILP64 vs LLP64 vs SILP64 right? Or how wchar_t on linux is 32bit while on windows it's 16bit right?

Those sort of inconsistencies are what my project aims to hide in the library, exposing only the fixed size integers, floats, etc that the compiler will map to the CPU's word/dword/qword for me via the _BitInt addition in C2X. I also mandate in the library that it must be a compiler like GCC or Clang that produces GNU ABI only.

This allows me to abuse the objects and static libraries that are produce by them along with makefiles to produce truly cross-platform software that relies on the launcher to hook it up to a native build of my library to handle the OS specific stuff in a consistent way.

I'm even making a custom protocol for handling common directories so that something like paw://~/ will always mean the home directory (so that $HOME vs %USERPROFILE% doesn't need to be detected).

The final goal is that users will not need to re-install many apps and libraries every time they find they need to re-install their os/distro or change to a different one. They just keep their home directory as is or redefine the OTG_DIR variable to whatever drive/directory is acting as a portable home directory.

uniformity at the API level is what matters, sure cant use some platform dependent types or structs, call it ABI or BABABI concept's the same

relating to window magic you mentioned previously, i found out that it happens in FAR and also with another user, they probably send that WM_ erase background message to the window. can check it by slowly resizing the window frame, content redraws even before mouse btn been depressed. one of the polling properties should be the focus, when it's on, then poll, no sense otherwise.

@awsdert
Copy link

awsdert commented Apr 24, 2024

uniformity at the API level is what matters

An ABI is that plus binary compatibility. And yes ABI matters, just take a look at all the workarounds the standard libraries (stdlib.h, stdint.h, etc) pull to keep their libraries working for native builds. What I'm doing just takes that a step further.

relating to window magic you mentioned previously, i found out that it happens in FAR and also with another user, they probably send that WM_ erase background message to the window. can check it by slowly resizing the window frame, content redraws even before mouse btn been depressed. one of the polling properties should be the focus, when it's on, then poll, no sense otherwise.

Good to know, will still test with wine primarily and via a copy of windows in qemu or similar once I'm satisfied.

I don't think there's anything left to extract from either of these topics for now so let's just leave end the convo here for now so the admin doesn't need to decide if any more of our comments are off topic or not.

@determin1st
Copy link

determin1st commented May 5, 2024

i've re-read the topic and want to DISAGREE with the author's statement.

buffer (scroll) is defined by size and offset, so the change in the offset shall generate the scroll event.

the ENABLE_WINDOW_INPUT flag is pretty useless though, because the scroll event alone is not what terminal user wants, it is desired AFTER (less important than) the resize event, which is unreliable with this flag, because window resize by Y coordinate (columns) does not trigger WINDOW_BUFFER_SIZE_EVENT (ive answered here #14975) SIGWINCH is also unreliable for size change detection, so implementations in both worlds should use polling routine to detect size change (scroll is only a windows feature)

regarding to above window magic - it doesnt work (in the sense that window refreshes content and GetConsoleScreenBufferInfo reports new values), though on windows 6.1+ (ive tested the resize on windows 10 conhost) the window updates in the process of resizing, so no special magic is needed, not an issue (here's demo of the problem Maximus5/ConEmu#2583)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Conhost For issues in the Console codebase Work-Item It's being tracked by an actual work item internally. (to be removed soon)
Projects
None yet
Development

No branches or pull requests

9 participants