simon-git: putty (main): Simon Tatham
Commits to Tartarus hosted VCS
tartarus-commits at lists.tartarus.org
Mon Dec 20 15:51:32 GMT 2021
TL;DR:
adf8fc1a GTK: fix return type of window-state-event handler.
8c63125f GTK: avoid trying to resize a maximised window.
8f365e39 Centralise drag-select check into term_out().
5a54b3bf Factor out term_request_resize().
be0cea71 Stop using a local buffer in term_out.
19b12ee5 Try to ensure term_size() after win_resize_request().
420fe755 Suspend terminal output while a window resize is pending.
cfc90236 Windows: try to send only one term_size on request_resize.
bc91a396 Proper buffer management between terminal and backend.
4721571b GTK: run toplevel callbacks when an fd is active.
f780a45c Proper backlog handling in Unix pty backend.
18a3a999 GTK: fix calculation of fixed window size for SUPDUP.
99aac9c4 GTK: stop using geometry hints when not on X11.
Repository: https://git.tartarus.org/simon/putty.git
On the web: https://git.tartarus.org/?p=simon/putty.git
Branch updated: main
Committer: Simon Tatham <anakin at pobox.com>
Date: 2021-12-20 15:51:32
commit adf8fc1ab092cf0d1c4ff4f037b6fe9eb1a409d1
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=adf8fc1ab092cf0d1c4ff4f037b6fe9eb1a409d1;hp=0e630bc4f16bc6019cdb0135cd82c8832502d8ef
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Dec 18 14:49:11 2021 +0000
GTK: fix return type of window-state-event handler.
The event should return a 'gboolean', indicating whether the event
needs propagating any further. We were returning void, which meant
that the default handling might be accidentally suppressed.
On Wayland, this had the particularly nasty effect that window redraws
would stop completely if you maximised the terminal window.
(By trial and error I found that this stopped happening if I removed
GDK_HINT_RESIZE_INC from the geometry hints, from which I guess that
the default window-state-event handler is doing something important
relating to that hint, or would have been if we hadn't accidentally
suppressed it. But the bug is clearly here.)
unix/window.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
commit 8c63125f7ad61a42d17decc249384307e2edb98a
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=8c63125f7ad61a42d17decc249384307e2edb98a;hp=adf8fc1ab092cf0d1c4ff4f037b6fe9eb1a409d1
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Dec 18 14:14:16 2021 +0000
GTK: avoid trying to resize a maximised window.
This is another thing that seems harmless on X11 but causes window
redraws to semipermanently stop happening on Wayland: if we try to
gtk_window_resize() a window that is maximised at the time, then
something mysterious goes wrong and we stop ever getting "draw" events.
unix/window.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
commit 8f365e39f3c1662884361900e2c017d9f2d4875e
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=8f365e39f3c1662884361900e2c017d9f2d4875e;hp=8c63125f7ad61a42d17decc249384307e2edb98a
Author: Simon Tatham <anakin at pobox.com>
Date: Sun Dec 12 14:39:50 2021 +0000
Centralise drag-select check into term_out().
This tiny refactoring replaces three identical checks at call sites,
not all as well commented as each other, with a check in just one
place with the best of the three comments.
terminal/terminal.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
commit 5a54b3bf1707781cd4a46009bc44a8aad59844a9
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=5a54b3bf1707781cd4a46009bc44a8aad59844a9;hp=8f365e39f3c1662884361900e2c017d9f2d4875e
Author: Simon Tatham <anakin at pobox.com>
Date: Mon Dec 13 18:49:45 2021 +0000
Factor out term_request_resize().
This tiny refactoring makes a convenient function for setting all the
'pending' flags and triggering a callback for the next window update.
This saves a bit of code, but that's not really the main point (or
else I'd have done the same to all the other similar things like
window moves). The point is that in a future commit I'm going to want
to do an extra thing on every server-controlled window resize, and
this refactoring gives me a single place to put that extra action.
terminal/terminal.c | 81 +++++++++++++++++++++++------------------------------
1 file changed, 35 insertions(+), 46 deletions(-)
commit be0cea71304c353f66e4411e449bc85367e3b702
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=be0cea71304c353f66e4411e449bc85367e3b702;hp=5a54b3bf1707781cd4a46009bc44a8aad59844a9
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Dec 18 15:07:41 2021 +0000
Stop using a local buffer in term_out.
There's no actual need to copy the data from term->inbuf into a local
variable inside term_out(). We can simply store a pointer and length,
and use the data _in situ_ - as long as we remember how much of it
we've used, and bufchain_consume() it when the routine exits.
Getting rid of that awkward and size-limited local array should
marginally improve performance. But also, it opens up the possibility
to suddenly suspend handling of terminal data and leave everything not
yet processed in the bufchain, because now we never remove anything
from the bufchain until _after_ it's been processed, so there's no
need to awkwardly push the unused segment of localbuf[] back on to the
front of the bufchain if we need to do that.
NFC, but as usual, I have a plan to use the new capability in a
followup commit.
terminal/terminal.c | 48 +++++++++++++++++++++++++++++++-----------------
1 file changed, 31 insertions(+), 17 deletions(-)
commit 19b12ee56c623db2331ef44859445169bdcad9e2
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=19b12ee56c623db2331ef44859445169bdcad9e2;hp=be0cea71304c353f66e4411e449bc85367e3b702
Author: Simon Tatham <anakin at pobox.com>
Date: Sun Dec 19 10:21:11 2021 +0000
Try to ensure term_size() after win_resize_request().
When the terminal asks its TermWin for a resize, the resize operation
happens asynchronously (or can do), and sooner or later, the terminal
will see a term_size() telling it the resize has actually taken
effect.
If the resize _doesn't_ take effect for any reason - e.g. because the
window is maximised, or because the X11 window manager is a tiling one
which will refuse requests to change the window size anyway - then the
terminal never got any explicit notification of refusal to resize. Now
it should, in as many cases as I can arrange.
One obvious case of this is the early exit I recently added to
gtkwin_request_resize() when the window is known to be in a maximised
or tiled state preventing a resize: in that situation, when our own
code knows we're not even attempting the resize, we also queue a
toplevel callback to tell the terminal so.
The more interesting case is when the request is refused for a reason
GTK _didn't_ know in advance, e.g. because the user is running an X11
tiling window manager such as i3, which generally refuses windows'
resize requests. In X11, if a window manager refuses an attempt to
change the window's size via ConfigureWindow, ICCCM says it should
respond by sending a synthetic ConfigureNotify event restating the
same size. Such no-op configure events do reach the "configure_event"
handler in a GTK program, but they weren't previously getting as far
as term_size(), because the call to term_size() was triggered from the
GTK "size_allocate" event on the GtkDrawingArea inside the window (via
drawing_area_setup()), so GTK would detect that nothing had changed.
Now we queue a last-ditch toplevel callback which ensures that if the
configure event doesn't also cause a size_allocate and a call to
drawing_area_setup(), then we cause one of our own once the dust has
settled. And drawing_area_setup(), in turn, now unconditionally calls
term_size() even if the size is the same as it was last time, instead
of taking an early exit. (It still does take an early exit to avoid
unnecessary faffing with Cairo surfaces etc, but _after_ term_size()).
This won't be 100% reliable, because it's the window manager's
responsibility to send those synthetic ConfigureNotify events, and a
window manager is a fallible process which could get into a stuck
state. But it covers all the cases I know of that _can_ be sensibly
covered - so now, when terminal.c asks the front end to resize the
window, it ought to find out in a timely manner whether or not that
has happened, in _almost_ all cases.
unix/window.c | 52 +++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 39 insertions(+), 13 deletions(-)
commit 420fe75552afa9490c7ca2ff08e92ec96933cfe1
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=420fe75552afa9490c7ca2ff08e92ec96933cfe1;hp=19b12ee56c623db2331ef44859445169bdcad9e2
Author: Simon Tatham <anakin at pobox.com>
Date: Sun Dec 19 10:37:02 2021 +0000
Suspend terminal output while a window resize is pending.
This is the payoff from the last few commits of refactoring. It fixes
the following race-condition bug in terminal application redraw:
* server sends a window-resizing escape sequence
* terminal requests a window resize from the front end
* server sends further escape sequences to perform a redraw of some
full-screen application, which assume that the window resize has
occurred and the window is already its new size
* terminal processes all those sequences in the context of the old
window size, while the front end is still thinking
* window resize completes in the front end and term_size() tells the
terminal it now has its new size, but it's too late, the screen
redraw has made a total mess.
(Perhaps the server might even send its window resize + followup
redraw all in one SSH packet, so that it's all queued in term->inbuf
in one go.)
As far as I can see, handling of this case has been broken more or
less forever in the GTK frontend (where window resizes are inherently
asynchronous due to the way X11 works, and we've never done anything
to compensate for that). On Windows, where window size is changed via
SetWindowPos which is synchronous, it used to work, but broke in
commit d74308e90e3813a (i.e. between 0.74 and 0.75), which made all
the ancillary window updates run on the same delayed-action timer as
ordinary text display.
So, it's time to fix it, and I think now I should be able to fix it in
GTK as well as on Windows.
Now, as soon as we've set the term->win_resize_pending flag (in
response to a resize escape sequence), the next return to the top of
the main loop in term_out will terminate output processing early,
leaving any further terminal data still in the term->inbuf bufchain.
Once we get a term_size() callback from the front end telling us our
new size, we reset term->win_resize_pending, which unblocks output
processing again, and we also queue a toplevel callback to have
another try at term_out() so that it will be unblocked promptly.
To implement this I've changed term->win_resize_pending from a bool
into a three-state enumeration, so that we can tell the difference
between 'pending' in the sense of not yet having sent our resize
request to the frontend, and in the sense of waiting for the frontend
to reply. That way, a window resize from the GUI user at least won't
be mistaken for the response to our resize request if it arrives in
the former state. (It can still be mistaken for one in the latter
case, but if the user is resizing the window at the same time as the
server-side application is doing critically size-dependent redrawing,
I don't think there can be any reasonable expectation of nothing going
wrong.)
As mentioned in the previous commit, some failure modes under X11 (in
particular the window manager process getting wedged in some way) can
result in no response being received to a ConfigureWindow request. In
that situation, it seems to me that we really _shouldn't_ sit there
waiting forever - perhaps it's technically the WM's fault and not
ours, but what kind of X window are you most likely to want to use to
do emergency WM repair? A terminal window, of course, so it would be
exceptionally unhelpful to make any terminal window stop working
completely in this situation! Hence, there's a fallback timeout in
terminal.c, so that if we don't receive a response in _too_ long,
we'll assume one is not forthcoming, and resume processing terminal
data at the old window size. The fallback timeout is set to 5 seconds,
following existing practice in libXt (DEFAULT_WM_TIMEOUT).
terminal/terminal.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++----
terminal/terminal.h | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 101 insertions(+), 6 deletions(-)
commit cfc902361633915646cd1384830d758f16aa701d
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=cfc902361633915646cd1384830d758f16aa701d;hp=420fe75552afa9490c7ca2ff08e92ec96933cfe1
Author: Simon Tatham <anakin at pobox.com>
Date: Mon Dec 13 06:54:49 2021 +0000
Windows: try to send only one term_size on request_resize.
Previously, the Windows implementation of win_request_resize would
call term_size() to tell the terminal about its new size, _before_
calling SetWindowPos to actually change the window size.
If SetWindowPos ends up not getting the exact window size it asks for,
Windows notifies the application by sending back a WM_SIZE message -
synchronously, by calling back to the window procedure from within
SetWindowPos. So after the first over-optimistic term_size(), the
WM_SIZE handler would trigger a second one, resetting the terminal
again to the _actual_ size.
This was more or less harmless, since current handling of terminal
resizes is such that no terminal data gets too confused: any lines
pushed into the scrollback by the first term_size will be pulled back
out by the second one if needed (or vice versa), and since commit
271de3e4ec5682e, individual termlines are not eagerly truncated by
resizing them twice.
However, it's more work than we really wanted to be doing, and it
seems presumptuous to send term_size() before we know it's right! So
now we send term_size() after SetWindowPos returns, unless it already
got sent by a re-entrant call to the WM_SIZE handler _before_
SetWindowPos returned. That way, the terminal should get exactly one
term_size() call in response to win_request_resize(), whether it got
the size it was expecting or not.
windows/window.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
commit bc91a39670a7a74bb231c35a8ea1b020a58f0917
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=bc91a39670a7a74bb231c35a8ea1b020a58f0917;hp=cfc902361633915646cd1384830d758f16aa701d
Author: Simon Tatham <anakin at pobox.com>
Date: Sun Dec 12 10:57:23 2021 +0000
Proper buffer management between terminal and backend.
The return value of term_data() is used as the return value from the
GUI-terminal versions of the Seat output method, which means backends
will take it to be the amount of standard-output data currently
buffered, and exert back-pressure on the remote peer if it gets too
big (e.g. by ceasing to extend the window in that particular SSH-2
channel).
Historically, as a comment in term_data() explained, we always just
returned 0 from that function, on the basis that we were processing
all the terminal data through our terminal emulation code immediately,
and never retained any of it in the buffer at all. If the terminal
emulation code were to start running slowly, then it would slow down
the _whole_ PuTTY system, due to single-threadedness, and
back-pressure of a sort would be exerted on the remote by it simply
failing to get round to reading from the network socket. But by the
time we got back to the top level of term_data(), we'd have finished
reading all the data we had, so it was still appropriate to return 0.
That comment is still correct if you're thinking about the limiting
factor on terminal data processing being the CPU usage in term_out().
But now that's no longer the whole story, because sometimes we leave
data in term->inbuf without having processed it: during drag-selects
in the terminal window, and (just introduced) while waiting for the
response to a pending window resize request. For both those reasons,
we _don't_ always have a buffer size of zero when we return from
term_data().
So now that hole in our buffer size management is filled in:
term_data() returns the true size of the remaining unprocessed
terminal output, so that back-pressure will be exerted if the terminal
is currently not consuming it. And when processing resumes and we
start to clear our backlog, we call backend_unthrottle to let the
backend know it can relax the back-pressure if necessary.
putty.h | 7 +++++++
terminal/terminal.c | 43 +++++++++++++------------------------------
test/fuzzterm.c | 2 ++
unix/window.c | 8 ++++++++
windows/window.c | 8 ++++++++
5 files changed, 38 insertions(+), 30 deletions(-)
commit 4721571b8bcfc2c0266d0f20dcd5ddc7b0e58889
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=4721571b8bcfc2c0266d0f20dcd5ddc7b0e58889;hp=bc91a39670a7a74bb231c35a8ea1b020a58f0917
Author: Simon Tatham <anakin at pobox.com>
Date: Sun Dec 19 13:13:37 2021 +0000
GTK: run toplevel callbacks when an fd is active.
Normally, the GTK code runs toplevel callbacks from a GTK 'idle
function'. But those mean what they say: they are considered
low-priority, to be run _only_ when the system is idle - so they can
fail to run at all in conditions of a steady stream of higher-priority
things, e.g. something is throwing data at the application so fast
that every main-loop iteration finds a readable fd.
And that's not good, because _we_ don't think our callbacks are
low-priority: they do a lot of really important work like redrawing
the window. So if they never get round to happening, PuTTY or pterm
can appear to lock up.
Simple solution to that one: whenever we process a select notification
on any fd, we _also_ call run_toplevel_callbacks(). Then our callbacks
are bound to happen reasonably regularly.
unix/gtk-common.c | 4 ++++
1 file changed, 4 insertions(+)
commit f780a45c571f7623766300731b243e485393bf15
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=f780a45c571f7623766300731b243e485393bf15;hp=4721571b8bcfc2c0266d0f20dcd5ddc7b0e58889
Author: Simon Tatham <anakin at pobox.com>
Date: Sun Dec 19 11:15:50 2021 +0000
Proper backlog handling in Unix pty backend.
If the Seat that the pty backend is talking to starts to back up, then
we ought to temporarily stop reading from the pty device, to pass that
back-pressure on to whatever's running in the terminal.
Previously, this didn't matter because a Seat running a GUI terminal
never backed up anyway. But now it does, so we should support it all
the way through the system.
unix/pty.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
commit 18a3a999f6671763755b270713f7e43d6e0fa9fc
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=18a3a999f6671763755b270713f7e43d6e0fa9fc;hp=f780a45c571f7623766300731b243e485393bf15
Author: Simon Tatham <anakin at pobox.com>
Date: Mon Dec 20 10:18:38 2021 +0000
GTK: fix calculation of fixed window size for SUPDUP.
The window size set in the geometry hints when the backend has the
BACKEND_RESIZE_FORBIDDEN flag was computed in a simplistic way that
forgot to take account of window furniture like scrollbars and menu
bars. Now it's computed based on the rest of the geometry hints, which
are more accurate.
unix/window.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
commit 99aac9c4f4f9b9153284b499668b3ba1300c9e67
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=99aac9c4f4f9b9153284b499668b3ba1300c9e67;hp=18a3a999f6671763755b270713f7e43d6e0fa9fc
Author: Simon Tatham <anakin at pobox.com>
Date: Mon Dec 20 11:06:57 2021 +0000
GTK: stop using geometry hints when not on X11.
While re-testing on Wayland after all this churn of the window
resizing code, I discovered that the window constantly came out a few
pixels too small, losing a character cell in width and height. This
stopped happening once I experimentally stopped setting geometry
hints.
Source-diving in GTK, it turns out that this is because the GDK
Wayland backend is applying the geometry hints to the size of the
window including 'margins', which are a very large extra space around
a window beyond even the visible 'non-client-area' furniture like the
title bar. And I have no idea how you find out the size of those
margins, so I can't allow for them in the geometry hints.
I also noticed that gtk_window_set_geometry_hints is removed in GTK 4,
which suggests that GTK upstream are probably not interested in
fiddling with them until they work more usefully (even if they would
agree with me that this is a bug in the first place, which I have no
idea). A simpler workaround is to avoid setting geometry hints at all
on any GDK backend other than X11.
So, that's what this commit does. On Wayland (or other backends), the
window can now be resized a pixel at a time, and if its size doesn't
work out to a whole number of character cells, then you just get some
dead space at the edges. Not especially nice, but better than the
alternatives I can see.
One other job the geometry hints were doing was to forbid resizing if
the backend sets the BACKEND_RESIZE_FORBIDDEN flag (which SUPDUP
does). That's now done at window creation time, via
gtk_window_set_resizable.
unix/window.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
More information about the tartarus-commits
mailing list