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

Snap to character grid when resizing window #2834

Closed
mcpiroman opened this issue Sep 21, 2019 · 11 comments · Fixed by #3181
Closed

Snap to character grid when resizing window #2834

mcpiroman opened this issue Sep 21, 2019 · 11 comments · Fixed by #3181
Assignees
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@mcpiroman
Copy link
Contributor

Description of the new feature/enhancement

Snap to character grid when resizing window, like many terminals do (gnome-based, git terminal etc.). Questions:

  • Make it a setting?
  • If so. enable by default? (I'd say yes)
@mcpiroman mcpiroman added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Sep 21, 2019
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Sep 21, 2019
@mcpiroman
Copy link
Contributor Author

Assigning myself if anyone cares

@oising oising added the Area-User Interface Issues pertaining to the user interface of the Console or Terminal label Sep 23, 2019
@oising oising removed the Needs-Tag-Fix Doesn't match tag requirements label Sep 23, 2019
@zadjii-msft
Copy link
Member

My biggest curiosity is how does this work with multiple panes open? Especially if the panes have different fonts or font sizes?

Otherwise yea, this seems like a good feature request

@zadjii-msft zadjii-msft added the Product-Terminal The new Windows Terminal. label Sep 23, 2019
@mdtauk
Copy link

mdtauk commented Sep 23, 2019

Could I argue for a small point. Not sure the feasibility of it. But part of the Window UI pattern is to display part of an item if there is scrolling. So could half of the line at the top be visible when there is scrolling supported?

@mcpiroman
Copy link
Contributor Author

@zadjii-msft Exactly - that's the biggest and hopefully the only problem here. I suppose we should try to:

  • keep all panes aligned
  • snap to the closes cell when above is not possible
  • maintain size ratio of splitted pane's children close to original one

That should be hard but possible.

@DHowett-MSFT DHowett-MSFT added this to the Terminal Backlog milestone Sep 23, 2019
@DHowett-MSFT DHowett-MSFT added Issue-Task It's a feature request, but it doesn't really need a major design. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels Sep 23, 2019
@DHowett-MSFT
Copy link
Contributor

This requires no small amount of care: it is trivially easy to break the win32 "move size loop" -- the one that handles snapping and window friction and not letting you size things under the taskbar and all that stuff. Be careful!

@egmontkob
Copy link

Other tricky cases are:

  • Switching between tabs of different font sizes
  • Changing font size of the current tab
  • Letting a maximized or fullscreen window really be maximized/fullscreen, without forcing to the character grid.

GNOME Terminal aims to maintain a fixed number of character rows and columns, resulting in the overall window changing its size. It's a constant source of user complaints (many people wish the window to stay at a fixed size, as other apps do, and don't think about how it's incompatible with grid size), as well as a constant source of technical problems.

As a user, I love the terminal size being snapped to the grid, mainly for two reasons:

  • Especially with apps that fill the entire terminal area with the nondefault background (such as Midnight Commander), seeing a random amount of gap at the bottom and right gives me an unsettling, frustrating feeling as it's really anything but pleasing to my eyes. I also have a constant urge with such terminals to manually resize to the grid, which – if I do so – wastes a lot of time.

  • When editing a long command (that wraps into multiple lines), not knowing whether it's a space character on the right separating two words, or just an almost-a-cell-wide gap, does not let me know if there's actually a space character there or not. It's a potential source of every now and then not noticing typos, faulty commands.

The second problem could be fixed by having a gap that's of a different color than the terminal area, but it might look even uglier.

Yet another approach could be what the Kitty emulator does: cheat a little bit and distribute that extra space among some of the cells. Maybe even do subpixel letter positioning.

As a developer, though, maintaining a resize constraint is often a pain. Part of the story is the "how should it work", which you already started discussing and probably doesn't have clear answers; another part of it is technical problems (I could link to quite a few past or present bugs with GNOME Terminal, but it's sure quite different on Windows, so it wouldn't be useful for you).

Also there's some overlap between these two areas, e.g. (randomly made up example) you might have two panes next to each other, one with font width 9, the other with font width 10, and at a current window width of 800 pixels you might decide that if the user increases the width then you give a cell to the second pane, then one to the first, then another one to the first, i.e. you allow width of 800, 810, 819, 828 etc., whatever seemingly random sequence of numbers computed by the application. On Unix, the X Window system only allows a constant amount of increment (you have to preconfigure that the width is fixed_base + N * fixed_increment), and as far as I know, that's what GTK offers on Wayland too, so we're limited by our surrounding infrastructure. Maybe (hopefully) these limitations aren't there on Windows, I don't know.

@egmontkob
Copy link

But part of the Window UI pattern is to display part of an item if there is scrolling. So could half of the line at the top be visible when there is scrolling supported?

@mdtauk This is a great observation, and a really interesting question.

I've thought about it when implementing smooth scrolling in VTE. If you scroll back to the history using a touchpad, this scrolling happens per-pixel in VTE, rather than per-character-row as in most terminals.

I think there are two problems with your idea of showing a partial row at the top. One is that then the initial shell prompt would already have to be displayed with a "random" amount of gap above it, and it's probably confusing and ugly.

The other problem is that there are application that occupy the entire area of the terminal, and don't switch to the alternate screen. top is such an example, but quite a few text editors (e.g. vim) can also be configured so. These apps need to have a concept of the terminal size (rows × columns), and the only reasonable thing to do is to round the available area downwards to an integer. Which, in case of these applications, results in a partial row from a previous command's output (or the shell command line) getting stuck at the top. This would again definitely be misleading and confusing.

However, we can simply turn it "upside down", have that extra area at the bottom instead of the top. This solves both aforementioned problems, there's guaranteed not to be any data below the currently writable area. As soon as the user starts to scroll back, this area on the bottom can also be used to display contents, as done in a non-grid-aligned VTE (e.g. Tilix, or maximized/fullscreen GNOME Terminal).

@mcpiroman
Copy link
Contributor Author

@egmontkob So, keeping the size aligned on various user actions is quite different thing than proposed 'simple' snapping during window resize; however I think we should (eventually) try to also do the former. The problem is that it cannot be done 'right' in many cases, we'd often have to either:

  • (Slightly) resize the window (not possible with fullscreen)
  • Increase (or maybe even decrease) configured padding
  • Adjust cell spacing
  • Otherwise gather/get rid of some space

And there are even more such tricky operations, like resizing panes. I guess that deserves its own issue-tracker entry. Here we could discuss how should it work just for the resizing part.

On Unix, the X Window system only allows a constant amount of increment (you have to preconfigure that the width is fixed_base + N * fixed_increment) [...] Maybe (hopefully) these limitations aren't there on Windows, I don't know.

Fortunately Window's system is much more flexible - for any user's input window rectangle a program can choose arbitrary output rectangle.

@egmontkob
Copy link

So, keeping the size aligned on various user actions is quite different thing than proposed 'simple' snapping during window resize; however I think we should (eventually) try to also do the former.

What I'm thinking of is that due to all these problematic cases, probably the best is to do some "best effort" snapping on certain user actions only. Starting with window resize, as you originally proposed, maybe only for when there are no panes?? Trickier cases might come later on.

With panes, I'm wondering if it makes sense to snap to sizes where at least one terminal that actually gets resized (along the relevant axes) resizes to an aligned size? With a truly complex pane layout it might behave silly in the end.

And there are even more such tricky operations, like resizing panes. I guess that deserves its own issue-tracker entry. Here we could discuss how should it work just for the resizing part.

Indeed, this one didn't occur to me. It's technically quite different than resizing the window, isn't it? Can you also specify for each input position the desired output position of the separator?

Fortunately Window's system is much more flexible - for any user's input window rectangle a program can choose arbitrary output rectangle.

Great to hear it!

It's an area where there's no single good solution, there's lots of possibilities to experiment with. I'll be curious to see what comes out of this story :)

@mcpiroman
Copy link
Contributor Author

To these interested, you may follow the work at https://github.com/mcpiroman/terminal/tree/2834-snap-to-char-grid - it seems to work almost fine now but I'd like to know if there are cases I've missed or what can be done better. Like I said, the biggest and really only problem (yet) are the panes.

With panes, I'm wondering if it makes sense to snap to sizes where at least one terminal that actually gets resized (along the relevant axes) resizes to an aligned size?

Just want to let you know that you saved me quite a lot of time by saying this.

@ghost ghost added the In-PR This issue has a related PR label Oct 13, 2019
DHowett-MSFT pushed a commit that referenced this issue Jan 8, 2020
When user resizes window, snap the size to align with the character grid
(like e.g. putty, mintty and most unix terminals). Properly resolves
arbitrary pane configuration (even with different font sizes and
padding) trying to align each pane as close as possible.

It also fixes terminal minimum size enforcement which was not quite well
handled, especially with multiple panes.

This PR does not however try to keep the terminals aligned at other user
actions (e.g. font change or pane split). That is to be tracked by some
other activity.

Snapping is resolved in the pane tree, recursively, so it (hopefully)
works for any possible layout.

Along the way I had to clean up some things as so to make the resulting
code not so cumbersome:
1. Pane.cpp: Replaced _firstPercent and _secondPercent with single
   _desiredSplitPosition to reduce invariants - these had to be kept in
   sync so their sum always gives 1 (and were not really a percent). The
   desired part refers to fact that since panes are aligned, there is
   usually some deviation from that ratio.
2. Pane.cpp: Fixed _GetMinSize() - it was improperly accounting for
   split direction
3. TerminalControl: Made dedicated member for padding instead of
   reading it from a control itself. This is because the winrt property
   functions turned out to be slow and this algorithm needs to access it
   many times. I also cached scrollbar width for the same reason.
4. AppHost: Moved window to client size resolution to virtual method,
   where IslandWindow and NonClientIslandWindow have their own
   implementations (as opposite to pointer casting).

One problem with current implementation is I had to make a long call
chain from the window that requests snapping to the (root) pane that
implements it: IslandWindow -> AppHost's callback -> App ->
TerminalPage -> Tab -> Pane. I don't know if this can be done better.

## Validation Steps Performed
Spam split pane buttons, randomly change font sizes with ctrl+mouse
wheel and drag the window back and forth.

Closes #2834
Closes #2277
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Jan 8, 2020
@ghost
Copy link

ghost commented Jan 14, 2020

🎉This issue was addressed in #3181, which has now been successfully released as Windows Terminal Preview v0.8.10091.0.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants