Fix journals not rendering Unicode text correctly (fixes #1403) #1405
Conversation
For the ST::string overloads of the text rendering methods, the firstClippedChar output parameter still counted in wchar_t units, not UTF-8 bytes as needed for indexing into ST::string.
| // Convert the firstClippedChar offset from wchar_t units to UTF-8 byte units. | ||
| // This is a bit inefficient, because it creates an actual UTF-8 string | ||
| // even though we just need the count, but string_theory has no better alternative. | ||
| *firstClippedChar = ST::string::from_wchar(wcharBuf.data(), firstClippedWchar).size(); |
There was a problem hiding this comment.
By the way, this conversion will probably go away again in the long term. I assume at some point we'll convert the rendering APIs to use ST::string all the way through, which makes it trivial to calculate firstClippedChar as an UTF-8 count.
There was a problem hiding this comment.
string_theory does have a private API (_ST_PRIVATE::utf8_measure_from_utf16/32) that could do this, but I don't think we really want to rely on that. Another option, since the font rendering code currently (unfortunately) assumes that all renderable characters fit into a single wchar_t, is to write a simplified length calculation that just ignores the possibility of UTF-16 surrogates. It could look something like (completely untested)
static uint32_t WcharToUtf8Position(const wchar_t *text, uint32_t position)
{
const wchar_t *end = text + position;
uint32_t u8pos = 0;
while (text < end) {
if (*text < 0x80)
u8pos += 1;
else if (*text < 0x800)
u8pos += 2;
else /* Assumes all input characters are 0x0000 - 0xffff */
u8pos += 3;
++text;
}
return u8pos;
}Then you could just call *firstClippedChar = WcharToUtf8Position(wcharBuf.data(), firstClippedWchar)
But yeah, at some point we'll need to make the whole font rendering code work better with ST types, and break some of its faulty assumptions around wchar_t
There was a problem hiding this comment.
Yeah, I wanted to avoid writing a manual UTF-8 length calculation here - seems easy to get wrong in some way :/ I don't think it's a big enough performance/memory issue to be worth the effort, especially if we're going to throw it out again soon.
(Actually, the previous line wrapping code did an identical conversion elsewhere, which I was able to remove now, so this change shouldn't even make a difference at all.)
For the
ST::stringoverloads of the text rendering methods, thefirstClippedCharoutput parameter still counted inwchar_tunits, not UTF-8 bytes as needed for indexing intoST::string.