On my test case, this reduces Xwayland's CPU usage by 5-10%, though
curiously in glamor_block_handler rather than the drawing routine
itself.
XDrawImageString16 isn't as helpful as it might be for two reasons.
First, "M" type fonts (e.g. Courier Oblique) can extend beyond the
rectangle erased by XDrawImageString16, and PuTTY tries to support such
fonts. And second, we really want to blank the pixmap after drawing
rather than before, because otherwise we have to muck around clipping
the mask operation. So avoinding the XFillRectangle calls is difficult,
at which point we may as well use XDrawString16.
Instead of creating a new pixmap for each text-rendering operation, we
now stash one (along with the various Cairo structures associated with
it) in the xfont_individual structure. This saves the 20% or so of
Xwayland's CPU time that was being spent on CreatePixmap requests.
Switching to server-side rendering of X fonts under Cairo turned out to
make text rendering much slower, at least on my laptop. This appears to
be because PuTTY was asking the X server to render text into a
1-bit-per-pixel (bpp) pixmap before having Cairo composite into the
terminal surface. On modern X servers 1bpp pixmaps are slow, being
largely un-accelerated.
Happily, it is possible to get Cairo to use an 8bpp pixmap instead, and
this is rather faster. It's a bit inconvenient, though, because first
we have to confirm that that the X Rendering Extensions is present and
find the correct picture format. That requires linking in libXrender,
which means a bit of CMake faff. For now, I've make libXrender
mandatory (for X11 builds), but it and the corresponding Cairo functions
could be made optional fairly simply.
This hasn't actually made text rendering as much faster as I'd like on
my laptop. While creating 1bpp pixmaps is nearly free, creating 8bpp
pixmaps takes significant time because they actually involve the GPU.
So I think now I need to rework my persistent-pixmap patch to work on
top of this one.
X fonts are indexed by 16-bit quantities, so even Unicode-capable
fonts can only have characters in the Basic Multilingual Plane (BMP).
PuTTY, however, tried to look up all Unicode characters in X fonts,
and did so by effectively ignoring all but the low-order 16 bits of
the character code. This meant that trying to display a non-BMP
character could get you the corresponding character from the BMP
instead, if that character was in the font.
Now, x11font_has_glyph() always returns false for glyphs outside the
BMP, which should mean that x11font_draw_text() never gets asked to
draw them.
Xlib and the X protocol provide text handling calls in versions that
take both 8-bit and 16-bit character indices. They are mostly
interchangable, except that of course the 8-bit calls can only access
characters with codes up to 255.
Heretofore, PuTTY used the 16-bit X calls when using a font in the
"iso10646-1" encoding and 8-bit calls otherwise. This led to a lot of
duplicate code, and in particular two large and almost identical
implementations of x11font_cairo_draw*().
Now, PuTTY uses 16-bit calls throughout, even when using an 8-bit font
encoding. This simplifies the code at the expense of needing an
extra bit of conversion to expand the char array that we get from
put_wc_to_mb() into an array of XChar2b when using an 8-bit font.
CreatePixmap returns a Pixmap with undefined contents, and ImageText16
doesn't quite erase the whole rectangle covered by the text (and hence
the whole Pixmap. So to be on the safe side we should make sure to
erase the entire Pixmap before drawing the text.
Conveniently, ImageText16 ignores the function specified in the GC, so
we can set that to GXclear and avoid needing to change the GC
thereafter.
This is a fairly radical change of how X bitmap fonts are handled when
using Cairo for rendering. Before, we would download each glyph to the
client on first use and then composite those glyphs into the terminal's
backing surface. This worked pretty well when we were keeping an image
of the whole screen on the client anyway, but once I'd pushed all the
other Cairo rendering onto the X server, it meant that the character
bitmaps had to be repeatedly pushed to the X server.
The new arrangement just renders each string into a temporary Pixmap
using the usual X text-drawing calls and then asks Cairo to paste it
into the main backing Pixmap. It's tempting to draw the text straight
into the backing Pixmap, but that would require dealing directly with
X colour management. This way, we get to leave colours in the hands
of Cairo (and hence the Render extension).
There are still fragments of the old system around. Those should go
in the next commit.
According to the X specs, WhitePixel and BlackPixel refer to permanent
entries in the default colourmap. This means that they're not
necessarily appropriate for use with a Drawable with a different depth
than the root window. When drawing to a Pixmap that will be used as a
1-bit alpha mask by Cairo, the correct values are simply 0
(transparent) and 1 (opaque).
This commit fixes a problem that Simon observed when using an X bitmap
font with Cairo and making a line double-width or double-size. When
using Cairo, PuTTY implements double-width and double-size by just
asking Cairo to scale all its drawing operations. This works fine
with outline fonts, but when using a bitmap font the results are a bit
fuzzy. This appears to be because Cairo's default is to use bilinear
interpolation when scaling an image, which is fine for photos but not
so good for fonts.
In this commit, I decompose PuTTY's cairo_mask_surface() call into its
component parts so that I can set the mask pattern's filter to
CAIRO_FILTER_NEAREST before using it. That solves the problem, but it
suggests that maybe we should be caching the pattern rather then the
surface.
When using an X server-side font with Cairo rendering, PuTTY takes the
rather horrible approach of rendering each glyph it uses into a depth-1
Pixmap and then copying the result into a Cairo surface that it uses
every time it wants to display that glyph.
Heretofore, the conversion of the Pixmap into a Cairo surface was done
by downloading it using XGetImage() and then manually re-arringing the
bits into a suitable form for Cairo. But Cairo has a way of turning an
X Drawable (including a Pixmap) into a surface, and then it's just a
case of copying one surface to another using cairo_paint(). So that's
what PuTTY does now and the process is a little less unpleasant than it
was.
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.
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 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.
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.)
marshal.h now provides a macro put_fmt() which allows you to write
arbitrary printf-formatted data to an arbitrary BinarySink.
We already had this facility for strbufs in particular, in the form of
strbuf_catf(). That was able to take advantage of knowing the inner
structure of a strbuf to minimise memory allocation (it would snprintf
directly into the strbuf's existing buffer if possible). For a general
black-box BinarySink we can't do that, so instead we dupvprintf into a
temporary buffer.
For consistency, I've removed strbuf_catf, and converted all uses of
it into the new put_fmt - and I've also added an extra vtable method
in the BinarySink API, so that put_fmt can still use strbuf_catf's
more efficient memory management when talking to a strbuf, and fall
back to the simpler strategy when that's not available.
This gets rid of all those annoying 'win', 'ux' and 'gtk' prefixes
which made filenames annoying to type and to tab-complete. Also, as
with my other recent renaming sprees, I've taken the opportunity to
expand and clarify some of the names so that they're not such cryptic
abbreviations.
