Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix journals not rendering Unicode text correctly (fixes #1403) #1405

Merged
merged 1 commit into from
Jun 25, 2023
Merged

Fix journals not rendering Unicode text correctly (fixes #1403) #1405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions Sources/Plasma/FeatureLib/pfJournalBook/pfJournalBook.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2364,22 +2364,19 @@ void pfJournalBook::IRenderPage( uint32_t page, uint32_t whichDTMap, bool sup
idx--;
break;
}
if (lastChar < chunk->fText.size() && chunk->fText[lastChar] != 0)
if (lastChar < chunk->fText.size())
{
// Didn't get to render the whole paragraph in this go, so we're going to cheat
// and split the paragraph up into two so that we can handle it properly. Note:
// this changes the chunk array beyond this point, so we need to invalidate the
// cache, but that's ok 'cause if we're doing this, it's probably invalid (or empty)
// anyway
ST::wchar_buffer s = chunk->fText.to_wchar();

// Note: Makes a copy of the string
pfEsHTMLChunk *c2 = new pfEsHTMLChunk( &s[ lastChar ] );
pfEsHTMLChunk *c2 = new pfEsHTMLChunk(chunk->fText.substr(lastChar));
c2->fFlags = chunk->fFlags;
fHTMLSource.emplace(fHTMLSource.begin() + idx + 1, c2);

// Clip and reallocate so we don't have two copies laying around
chunk->fText = ST::string::from_wchar(s.c_str(), lastChar);
chunk->fText = chunk->fText.left(lastChar);

// Invalidate our cache starting with the next page
if (fPageStarts.size() > page + 1)
Expand Down
11 changes: 10 additions & 1 deletion Sources/Plasma/PubUtilLib/plGImage/plDynamicTextMap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,16 @@ void plDynamicTextMap::SetFirstLineIndent( int16_t indent )
void plDynamicTextMap::CalcWrappedStringSize( const ST::string &text, uint16_t *width, uint16_t *height, uint32_t *firstClippedChar, uint16_t *maxAscent, uint16_t *lastX, uint16_t *lastY )
{
// TEMP
CalcWrappedStringSize(text.to_wchar().data(), width, height, firstClippedChar, maxAscent, lastX, lastY);
ST::wchar_buffer wcharBuf = text.to_wchar();
uint32_t firstClippedWchar;
CalcWrappedStringSize(wcharBuf.data(), width, height, &firstClippedWchar, maxAscent, lastX, lastY);

if (firstClippedChar != nullptr) {
// 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();
Comment on lines +600 to +603
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any thoughts about this, @zrax?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

}
}

void plDynamicTextMap::CalcWrappedStringSize( const wchar_t *text, uint16_t *width, uint16_t *height, uint32_t *firstClippedChar, uint16_t *maxAscent, uint16_t *lastX, uint16_t *lastY )
Expand Down
9 changes: 6 additions & 3 deletions Sources/Plasma/PubUtilLib/plGImage/plFont.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1176,8 +1176,7 @@ void plFont::IRenderCharNull( const plCharacter &c )
uint16_t plFont::CalcStringWidth( const ST::string &string )
{
uint16_t w, h, a, lX, lY;
uint32_t s;
CalcStringExtents( string, w, h, a, s, lX, lY );
CalcStringExtents( string, w, h, a, lX, lY );
return w;
}

Expand All @@ -1189,9 +1188,13 @@ uint16_t plFont::CalcStringWidth( const wchar_t *string )
return w;
}

void plFont::CalcStringExtents( const ST::string &string, uint16_t &width, uint16_t &height, uint16_t &ascent, uint32_t &firstClippedChar, uint16_t &lastX, uint16_t &lastY )
void plFont::CalcStringExtents( const ST::string &string, uint16_t &width, uint16_t &height, uint16_t &ascent, uint16_t &lastX, uint16_t &lastY )
{
// convert the char string to a wchar_t string
// We don't expose firstClippedChar as an out parameter in the ST::string overload,
// because converting it to a correct UTF-8-based count is a bit complicated
// and currently nothing uses this information.
uint32_t firstClippedChar;
CalcStringExtents(string.to_wchar().data(), width, height, ascent, firstClippedChar, lastX, lastY);
}

Expand Down
2 changes: 1 addition & 1 deletion Sources/Plasma/PubUtilLib/plGImage/plFont.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ class plFont : public hsKeyedObject

uint16_t CalcStringWidth( const ST::string &string );
uint16_t CalcStringWidth( const wchar_t *string );
void CalcStringExtents( const ST::string &string, uint16_t &width, uint16_t &height, uint16_t &ascent, uint32_t &firstClippedChar, uint16_t &lastX, uint16_t &lastY );
void CalcStringExtents( const ST::string &string, uint16_t &width, uint16_t &height, uint16_t &ascent, uint16_t &lastX, uint16_t &lastY );
void CalcStringExtents( const wchar_t *string, uint16_t &width, uint16_t &height, uint16_t &ascent, uint32_t &firstClippedChar, uint16_t &lastX, uint16_t &lastY );

bool LoadFromFNT( const plFileName &path );
Expand Down
3 changes: 1 addition & 2 deletions Sources/Tools/plFontConverter/plFontPreview.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,7 @@ void plFontPreview::Update(plFont *font, const QString &text)
fFont->SetRenderFlag(plFont::kRenderClip, true);
fFont->SetRenderClipRect(0, 0, (int16_t)width(), (int16_t)height());
uint16_t w, h, a, lastX, lastY;
uint32_t firstCC;
fFont->CalcStringExtents(testString, w, h, a, firstCC, lastX, lastY);
fFont->CalcStringExtents(testString, w, h, a, lastX, lastY);

int cY = ((height() - h) >> 1) + a;

Expand Down