Now that we use gdk_window_create_similar_surface() to make our backing
surface, most of the code to handle scale factors is unnecessary and
indeed slightly harmful. gdk_window_create_similar_surface() creates a
surface suitable for use as backing for a window, so it already has the
scale factor applied. This means that it should be sized in nominal
pixels, we can paint to it in nominal pixels, and the fact that it has
the same extra resolution as the actual window is entirely transparent
to us.
Now the only reason we pay attention to the scale factor at all is to
detect changes and use them as a prompt to re-create the backing
surface.
If gdk_window_create_similar_surface() isn't available, we now fall back
to cairo_surface_create_similar(). This is relevant only on GTK between
2.00 and 2.22 with deprecated calls disabled.
By default, when we're asked to draw a GTK widget, GTK creates a
temporary surface (a Pixmap under X) and redirects our rendering into
that. Then it blits that into the actual window. This is silly for
PuTTY because all that PuTTY does to render its drawing area is to
blit into it from _another_ surface. So now PuTTY asks GTK not to do
that. According to the GTK documentation, GTK as of 3.10 has
completely restructured its drawing routines so that turning off
double-buffering actually makes things worse and slower, so we turn it
off only in GTK 2
Still, this now means that painting text on the screen in GTK 2 causes
precisely one CopyArea operation, which is what we want.
This removes the case where we draw into a Cairo surface and then copy
the results into a GdkPixmap. Now, if we've got a GdkPixmap, we just
draw into it directly using Cairo. This vastly reduces the number of
CopyArea operations needed to draw on the screen.
I just found cairo_paint() while wandering the Cairo documentation. It
just fills the entire clip region with data from the current source,
which is precisely what draw_area() wants to do. This is simpler for us
than requesting the bounding rectangle of the clipping region and then
filling it, and as far as I can tell the clipping rectangle generally
covers the whole window anyway.
Rather than constructing a transformation matrix piece by piece (with
very branchy code), draw_stretch_before now just calls cairo_translate()
and cairo_scale() with values that are almost-obviously correct.
Also, rather than stashing and restoring the transformation matrix
ourselves, it seems simpler to use cairo_save() and cairo_restore().
That requires that draw_stretch_before() and draw_stretch_after() be
called strictly in pairs, but they are so that's OK.
The Cairo documentation is clear that cairo_set_antialias() only
affects shape drawing and not text rendering. To change
anti-aliasing settings for font rendering you need
cairo_font_options_set_antialias() instead. Therefore the comment
can be a bit more certain than just describing what Cairo "appears"
to do.
The terminal code doesn't yet do anything with the text other than feed
it to a debugging printf. The call uses UTF-8 and expects the terminal
to copy the string because that's compatible with
gtk_im_context_get_preedit_string().
GtkIMContext has focus_in and focus_out methods for telling it when the
corresponding widget gains or loses keyboard focus. It's not obvious to
me why these are necessary, but PuTTY now calls them when it sees
focus-in and focus-out events for the terminal window. Somehow, this
has caused Hangul input to start working in PuTTY. I can't yet
see what I'm typing for lack of proper preedit support, though.
I'd forgotten that I'd already chosen a default client-side font, for
NOT_X_WINDOWS builds. I should have made the two defaults match! Now
both default font names are defined in the header file.
If the user's choice of fonts can't be instantiated during initial
terminal-window setup, then we now automatically try two fallback
options, "client:Monospace 10" and "server:fixed", and only give a
fatal error if _no_ option allows us to open a terminal window.
Previously, on Wayland, PuTTY and pterm with default configuration
would completely fail to open a terminal window at all, because our
default font configuration is still the trad X11 "server:fixed", and
on Wayland, X11-style server-side bitmap fonts don't exist at all.
Conversely, in the GTK1 build of PuTTY which we're still supporting,
_client-side_ fonts aren't supported, so if a user had configured one
in their normal PuTTY configuration, then the GTK1 version would
similarly fail to launch.
Now both of those cases should work, because the fallbacks include a
client-side font _and_ a server-side one, and I hope that any usable
Pango system will make "Monospace" map to _some_ locally available
font, and that any remotely sensible X server has 'fixed'
I think it would be even better if there was a mechanism for the Conf
to specify a fallback list of fonts. For example, this might be
specified via a new multifont prefix along the lines of
choices:client:Monospace 10:server:fixed
with the semantics that the "choices:" prefix means that the rest of
the string is split up at every other colon to find a list of fonts to
try to make. Then we could not only set PuTTY's default to a list of
possibilities likely to find a usable font everywhere, but also, users
could configure their own list of preferred fallbacks.
But I haven't thought of a good answer to the design question of what
should happen if a Conf font setting looks like that and the user
triggers the GUI font selector! (Also, you'd need to figure out what
happened if a 'choices:' string had another 'choices' in it...)
When running on Wayland, gdk_display_get_name() can return things like
"wayland-0" rather than valid X display names. PuTTY nonetheless
treated them as X display names, meaning that when running under
Wayland, pterm would set DISPLAY to "wayland-0" in subprocesses, and
PuTTY's X forwarding wouldn't work properly.
To fix this, places that call gdk_display_get_name() now only do so on
displays for which GDK_IS_X_DISPLAY() is true. As with
GDK_IS_X_WINDOW(), this requires some backward-compatibility for GDK
versions where everything is implicitly running on X.
To make this work usefully, pterm now also won't unset DISPLAY if it
can't get an X display name and instead will pass through whatever value
of DISPLAY it received. I think that's better behaviour anyway.
There are two separate parts of PuTTY that call gdk_display_get_name().
platform_get_x_display() in unix/putty.c is used for X forwarding, while
gtk_seat_get_x_display() in unix/window.c is used used for setting DISPLAY
and recording in utmp. I've updated both of them.
The previous mb_to_wc and wc_to_mb had horrible and also buggy APIs.
This commit introduces a fresh pair of functions to replace them,
which generate output by writing to a BinarySink. So it's now up to
the caller to decide whether it wants the output written to a
fixed-size buffer with overflow checking (via buffer_sink), or
dynamically allocated, or even written directly to some other output
channel.
Nothing uses the new functions yet. I plan to migrate things over in
upcoming commits.
What was wrong with the old APIs: they had that awkward undocumented
Windows-specific 'flags' parameter that I described in the previous
commit and took out of the dup_X_to_Y wrappers. But much worse, the
semantics for buffer overflow were not just undocumented but actually
inconsistent. dup_wc_to_mb() in utils assumed that the underlying
wc_to_mb would fill the buffer nearly full and return the size of data
it wrote. In fact, this was untrue in the case where wc_to_mb called
WideCharToMultiByte: that returns straight-up failure, setting the
Windows error code to ERROR_INSUFFICIENT_BUFFER. It _does_ partially
fill the output buffer, but doesn't tell you how much it wrote!
What's wrong with the new API: it's a bit awkward to write a sequence
of wchar_t in native byte order to a byte-oriented BinarySink, so
people using put_mb_to_wc directly have to do some annoying pointer
casting. But I think that's less horrible than the previous APIs.
Another change: in the new API for wc_to_mb, defchr can be "", but not
NULL.
This parameter was undocumented, and Windows-specific: its semantics
date from before PuTTY was cross-platform, and are "Pass this flags
parameter straight through to the Win32 API's conversion functions".
So in Windows platform code you can pass flags like MB_USEGLYPHCHARS,
but in cross-platform code, you dare not pass anything nonzero at all
because the Unix frontend won't recognise it (or, likely, even
compile).
I've kept the flag for now in the underlying mb_to_wc / wc_to_mb
functions. Partly that's because there's one place in the Windows code
where the parameter _is_ used; mostly, it's because I'm about to
replace those functions anyway, so there's no point in editing all the
call sites twice.
CONF_bold_style is a pair of bit flags rather than an enum, so its
values aren't just BOLD_STYLE_FONT and BOLD_STYLE_COLOUR but also the
bitwise OR of them. (Hopefully not neither.)
Constructing a FontSpec in platform-independent code is awkward,
because you can't call fontspec_new() outside the platform subdirs
(since its prototype varies per platform). But sometimes you just need
_some_ valid FontSpec, e.g. to put in a Conf that will be used in some
place where you don't actually care about font settings, such as a
purely CLI program.
Both Unix and Windows _have_ an idiom for this, but they're different,
because their FontSpec constructors have different prototypes. The
existing CLI tools have always had per-platform main source files, so
they just use the locally appropriate method of constructing a boring
don't-care FontSpec.
But if you want a _platform-independent_ main source file, such as you
might find in a test program, then that's rather awkward. Better to
have a platform-independent API for making a default FontSpec.
When I maximised a terminal window today and then used Ctrl-< to
reduce its font size (expecting that the window size would stay the
same but more characters would be squeezed in), pterm failed the
assertion in term_request_resize_completed() that checks
term->win_resize_pending == WIN_RESIZE_AWAIT_REPLY.
This happened because in this situation request_resize_internal() was
called from within window.c rather than from within the terminal code
itself. So the terminal didn't know a resize is pending at all, and
was surprised to be told that one had finished.
request_resize_internal() already has a flag parameter to tell it
whether a given resize came from the terminal or not. On the main code
path, that flag is used to decide whether to notify the terminal. But
on the early exit path when the window is maximised, we weren't
checking the flag. An easy fix.
Horizontal scroll events aren't generated by the traditional mouse
wheel, but they can be generated by trackpad gestures, though this
isn't always configured on.
The cross-platform and Windows parts of this patch is due to
Christopher Plewright; I added the GTK support.
A KDE user observed that if you 'dock' a GTK PuTTY window to the side
of the screen (by dragging it to the RH edge, causing it to
half-maximise over the right-hand half of the display, similarly to
Windows), and then send a terminal resize sequence, then PuTTY fails
the assertion in term_resize_request_completed() which expects that an
unacknowledged resize request was currently in flight.
When drawing_area_setup() calls term_resize_request_completed() in
response to the inst->term_resize_notification_required flag, it
resets the inst->win_resize_pending flag, but doesn't reset
inst->term_resize_notification_required. As a result, the _next_ call
to drawing_area_setup will find that flag still set, and make a
duplicate call to term_resize_request_completed, after the terminal no
longer believes it's waiting for a response to a resize request. And
in this 'docked to the right-hand side of the display' state, KDE
apparently triggers two calls to drawing_area_setup() in quick
succession, making this bug manifest.
I could fix this by clearing inst->term_resize_notification_required.
But inspecting all the other call sites, it seems clear to me that my
original intention was for inst->term_resize_notification_required to
be a flag that's only meaningful if inst->win_resize_pending is set.
So I think a better fix is to conditionalise the check in
drawing_area_setup so that we don't even check
inst->term_resize_notification_required if !inst->win_resize_pending.
From https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h3-Any-event-tracking:
Any-event mode is the same as button-event mode, except that all motion
events are reported, even if no mouse button is down. It is enabled by
specifying 1003 to DECSET.
Normally the front ends only report mouse events when buttons are
pressed, so we introduce a MA_MOVE event with MBT_NOTHING set to
indicate such a mouse movement.
Those versions of GTK (or rather, GDK) don't support the
GDK_WINDOW_STATE_TOP_TILED constants; they only support the
non-directional GDK_WINDOW_STATE_TILED. And GTK < 3.10.0 doesn't even
support that.
All those constants were under #ifdef already; I've just made the
ifdefs a bit more precise.
This is like the seat-independent nonfatal(), but specifies a Seat,
which allows the GUI dialog box to have the right terminal window as
its parent (if there are multiple ones).
Changed over all the nonfatal() calls in the code base that could be
localised to a Seat, which means all the ones that come up if
something goes horribly wrong in host key storage. To make that
possible, I've added a 'seat' parameter to store_host_key(); it turns
out that all its call sites had one available already.
This patch fixes a few other whitespace and formatting issues which
were pointed out by the bulk-reindent or which I spotted in passing,
some involving manual editing to break lines more nicely.
I think the weirdest hunk in here is the one in windows/window.c
TranslateKey() where _half_ of an assignment statement inside an 'if'
was on the same line as the trailing paren of the if condition. No
idea at all how that one managed to happen!
If the function name (or expression) in a function call or declaration
is itself so long that even the first argument doesn't fit after it on
the same line, or if that would leave so little space that it would be
silly to try to wrap all the run-on lines into a tall thin column,
then I used to do this
ludicrously_long_function_name
(arg1, arg2, arg3);
and now prefer this
ludicrously_long_function_name(
arg1, arg2, arg3);
I picked up the habit from Python, where the latter idiom is required
by Python's syntactic significance of newlines (you can write the
former if you use a backslash-continuation, but pretty much everyone
seems to agree that that's much uglier). But I've found it works well
in C as well: it makes it more obvious that the previous line is
incomplete, it gives you a tiny bit more space to wrap the following
lines into (the old idiom indents the _third_ line one space beyond
the second), and I generally turn out to agree with the knock-on
indentation decisions made by at least Emacs if you do it in the
middle of a complex expression. Plus, of course, using the _same_
idiom between C and Python means less state-switching.
So, while I'm making annoying indentation changes in general, this
seems like a good time to dig out all the cases of the old idiom in
this code, and switch them over to the new.
My aim has always been to have those back-dented by 2 spaces (half an
indent level) compared to the statements around them, so that in
particular switch statements have distinct alignment for the
statement, the cases and the interior code without consuming two whole
indent levels.
This patch sweeps up all the violations of that principle found by my
bulk-reindentation exercise.
My bulk indentation check also turned up a lot of cases where a run-on
function call or if statement didn't have its later lines aligned
correctly relative to the open paren.
I think this is quite easy to do by getting things out of
sync (editing the first line of the function call and forgetting to
update the rest, perhaps even because you never _saw_ the rest during
a search-replace). But a few didn't quite fit into that pattern, in
particular an outright misleading case in unix/askpass.c where the
second line of a call was aligned neatly below the _wrong_ one of the
open parens on the opening line.
Restored as many alignments as I could easily find.
In the 'xterm 216+' function key mode, a function key pressed with a
combination of Shift, Ctrl and Alt has its usual sequence like
ESC[n~ (for some integer n) turned into ESC[n;m~ where m-1 is a 3-bit
bitmap of currently pressed modifier keys.
This mode now also applies to the keys on the small keypad above the
arrow keys (Ins, Home, PgUp etc). If xterm 216+ mode is selected,
those keys are modified in the same way as the function keys.
As with the function keys, this doesn't guarantee that PuTTY will
_receive_ any particular shifted key of this kind, and not repurpose
it. Just as Alt+F4 still closes the window (at least on Windows)
rather than sending a modified F4 sequence, Shift+Ins will still
perform a paste action rather than sending a modified Ins sequence,
Shift-PgUp will still scroll the scrollback, etc. But the keys not
already used by PuTTY for other purposes should now have their
modern-xterm behaviour in modern-xterm mode.
Thanks to H.Merijn Brand for developing and testing a version of this
patch.
The text of the host key warnings was replicated in three places: the
Windows rc file, the GTK dialog setup function, and the console.c
shared between both platforms' CLI tools. Now it lives in just one
place, namely ssh/common.c where the rest of the centralised host-key
checking is done, so it'll be easier to adjust the wording in future.
This comes with some extra automation. Paragraph wrapping is no longer
done by hand in any version of these prompts. (Previously we let GTK
do the wrapping on GTK, but on Windows the resource file contained a
bunch of pre-wrapped LTEXT lines, and console.c had pre-wrapped
terminal messages.) And the dialog heights in Windows are determined
automatically based on the amount of stuff in the window.
The main idea of all this is that it'll be easier to set up more
elaborate kinds of host key prompt that deal with certificates (if,
e.g., a server sends us a certified host key which we don't trust the
CA for). But there are side benefits of this refactoring too: each
tool now reliably inserts its own appname in the prompts, and also, on
Windows the entire prompt text is copy-pastable.
Details of implementation: there's a new type SeatDialogText which
holds a set of (type, string) pairs describing the contents of a
prompt. Type codes distinguish ordinary text paragraphs, paragraphs to
be displayed prominently (like key fingerprints), the extra-bold scary
title at the top of the 'host key changed' version of the dialog, and
the various information that lives in the subsidiary 'more info' box.
ssh/common.c constructs this, and passes it to the Seat to present the
actual prompt.
In order to deal with the different UI for answering the prompt, I've
added an extra Seat method 'prompt_descriptions' which returns some
snippets of text to interpolate into the messages. ssh/common.c calls
that while it's still constructing the text, and incorporates the
resulting snippets into the SeatDialogText.
For the moment, this refactoring only affects the host key prompts.
The warnings about outmoded crypto are still done the old-fashioned
way; they probably ought to be similarly refactored to use this new
SeatDialogText system, but it's not immediately critical for the
purpose I have right now.
Instead of maintaining a single sparse table mapping Unicode to the
currently selected code page, we now maintain a collection of such
tables mapping Unicode to any code page we've so far found a need to
work with, and we add code pages to that list as necessary, and never
throw them away (since there are a limited number of them).
This means that the wc_to_mb family of functions are effectively
stateless: they no longer depend on a 'struct unicode_data'
corresponding to the current terminal settings. So I've removed that
parameter from all of them.
This fills in the missing piece of yesterday's commit a216d86106d40c3:
now wc_to_mb too should be able to handle internally-implemented
character sets, by hastily making their reverse mapping table if it
doesn't already have it.
(That was only a _latent_ bug, because the only use of wc_to_mb in the
cross-platform or Windows code _did_ want to convert to the currently
selected code page, so the old strategy worked in that case. But there
was no protection against an unworkable use of it being added later.)
Fixes a cosmetic issue where the new ConPTY error added in 4ae8b742ab
had an ugly "Unable to open connection to".
(Arguably this ought to test a backend property rather than
cmdline_tooltype.)
When the user provides a password on the PuTTY command line, via -pw
or -pwfile, the flag 'tried_once' inside cmdline_get_passwd_input() is
intended to arrange that we only try sending that password once, and
after we've sent it, we don't try again.
But this plays badly with the 'Restart Session' operation. If the
connection is lost and then restarted at user request, we _do_ want to
send that password again!
So this commit moves that static variable out into a small state
structure held by the client of cmdline_get_passwd_input. Each client
can decide how to manage that state itself.
Clients that support 'Restart Session' - i.e. just GUI PuTTY itself -
will initialise the state at the same time as instantiating the
backend, so that every time the session is restarted, we return
to (correctly) believing that we _haven't_ yet tried the password
provided on the command line.
But clients that don't support 'Restart Session' - i.e. Plink and file
transfer tools - can do the same thing that cmdline.c was doing
before: just keep the state in a static variable.
This also means that the GUI login tools will now retain the
command-line password in memory, whereas previously they'd have wiped
it out once it was used. But the other tools will still wipe and free
the password, because I've also added a 'bool restartable' flag to
cmdline_get_passwd_input to let it know when it _is_ allowed to do
that.
In the GUI tools, I don't see any way to get round that, because if
the session is restarted you _have_ to still have the password to use
again. (And you can't infer that that will never happen from the
CONF_close_on_exit setting, because that too could be changed in
mid-session.) On the other hand, I think it's not all that worrying,
because the use of either -pw or -pwfile means that a persistent copy
of your password is *already* stored somewhere, so another one isn't
too big a stretch.
(Due to the change of -pw policy in 0.77, the effect of this bug was
that an attempt to reconnect in a session set up this way would lead
to "Configured password was not accepted". In 0.76, the failure mode
was different: PuTTY would interactively prompt for the password,
having wiped it out of memory after it was used the first time round.)
If the user holds down Alt-> so that the key repeats, then a second
call to change_font_size can occur while the window resize from the
previous one has yet to complete. This leads to the new pixel size of
the window from resize #1 being interpreted in the light of the font
size from reesize #2, so that the two get out of step and the
_character_ size of the terminal changes as a result.
The simplest fix is to disallow starting a second font-size-based
window resize while the first is still in flight - which, now that the
'win_resize_pending' flag lives in window.c and not terminal.c, is
easy to achieve.
In the changes around commit 420fe75552afa94, I made the terminal
suspend output processing while it waited for a term_size() callback
in response to a resize request. Because on X11 there are unusual
circumstances in which you never receive that callback, I also added a
last-ditch 5-second timeout, so that eventually we'll resume terminal
output processing regardless.
But the timeout lives in terminal.c, in the cross-platform code. This
is pointless on Windows (where resize processing is synchronous, so we
always finish it before the timer code next gets called anyway), but I
decided it was easier to keep the whole mechanism in terminal.c in the
absence of a good reason not to.
Now I've found that reason. We _also_ generate window resizes locally
to the GTK front end, in response to the key combinations that change
the font size, and _those_ still have an asynchrony problem.
So, to begin with, I'm refactoring the request_resize system so that
now there's an explicit callback from the frontend to the terminal to
say 'Your resize request has now been processed, whether or not you've
received a term_size() call'. On Windows, this simplifies matters
greatly because we always know exactly when to call that, and don't
have to keep a 'have we called term_size() already?' flag. On GTK, the
timing complexity previously in terminal.c has moved into window.c.
No functional change (I hope). The payoff will be in the next commit.
I got a pterm into a stuck state this morning by an accidental mouse
action. I'd intended to press Ctrl + right-click to pop up the context
menu, but I accidentally pressed down the left button first, starting
a selection drag, and then while the left button was still held down,
pressed down the right button as well, triggering the menu.
The effect was that the context menu appeared while term->selstate was
set to DRAGGING, in which state terminal output is suppressed, and
which is only unset by a mouse-button release event. But then that
release event went to the popup menu, and the terminal window never
got it. So the terminal stayed stuck forever - or rather, until I
guessed the cause and did another selection drag to reset it.
This happened to me on GTK, but once I knew how I'd done it, I found I
could reproduce the same misbehaviour on Windows by the same method.
Added a simplistic fix, on both platforms, that cancels a selection
drag if the popup menu is summoned part way through it.
When the window can't be resized for any reason, there will be extra
space inside the drawing area that's not part of our standard
width*font_width+2*window_border. We should include that in the
backing surface and make sure we erase it to the background colour,
otherwise it can end up containing unwanted visual junk.
An example is the same case described in the previous commit: maximise
the window and then start playing about with the font size. If you do
this while running a full-screen application that displays text in the
bottom line, it's easy to see that part of the previous display is
left over and not cleared when the new font size leaves more space at
the bottom than the old one.
If you maximise the terminal window and then press Ctrl-> or Ctrl-< to
change the font size, then the maximised window can't change size, so
what _should_ happen instead is that the terminal adjusts the number
of character cells to whatever the new font size will now permit in
the same size of window as before.
But in fact, the terminal size wasn't changing at all, because the
call to gtkwin_request_resize (called from change_font_size) detected
the maximised window and went straight to gtkwin_deny_term_resize,
which immediately called term_size() to tell the terminal it still had
the same size as before.
This commit switches gtkwin_deny_term_resize so that instead it calls
drawing_area_setup_simple(), which re-runs drawing_area_setup with the
same size the drawing area already had. This should work out the same
in the case where we're _not_ changing the font size, but now also
does the right thing when we are.
Correcting a source file name in the docs just now reminded me that
I've seen a lot of outdated source file names elsewhere in the code,
due to all the reorganisation since we moved to cmake. Here's a giant
pass of trying to make them all accurate again.
