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.
