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.

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

TETRAEDGE: Encode text before draw #4924

Merged
merged 1 commit into from
Apr 20, 2023
Merged

Conversation

BLooperZ
Copy link
Contributor

This fixes an issue in the text drawing where the glyphs to use are implicitly casted to uint32 and does not get the current glyph range in the font.
I understand that implicit conversion from 8-bit encoding as-is is similiar to Latin-1 encoding, please correct me if it's not the case... I didn't notice problem with these languages but I'm not a native speaker.

Specifically, it fixes russian translation and allow better integration in the hebrew fan translation...

before:
syberia-rus-daily

after:
syberia-rus

(Also not a native speaker, but it seems to match the voice over)

Thank you

@@ -145,9 160,9 @@ void TeFont3::draw(TeImage &destImage, const Common::String &str, int fontSize,

uint32 uintcol = ((uint32)col.a() << fmt.aShift) | ((uint32)(col.r()) << fmt.rShift)
| ((uint32)(col.g()) << fmt.gShift) | ((uint32)(col.b()) << fmt.bShift);
Common::String line(str);
Common::U32String line = str.decode(_codePage);
if (g_engine->getCore()->language() == "he")
Copy link
Contributor Author

@BLooperZ BLooperZ Apr 19, 2023

Choose a reason for hiding this comment

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

This check (that the language is "he") actually became redundant with the changes in this PR, but can be left in for the sake of making sure it cannot affect other languages

@mduggan
Copy link
Contributor

mduggan commented Apr 19, 2023

Nice, thanks for that - I'd been meaning to work out why the RU translation wasn't working. There is an ugly getUnicodeFromISO function in TeFont3 which I'd copied from the original thinking that I should get rid of it.. this should do that nicely.

@@ -86,7 86,9 @@ class TeFont3 : public TeResource {
private:
void init();
Graphics::Font *getAtSize(uint size);
Common::CodePage codePage();
Copy link
Contributor

Choose a reason for hiding this comment

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

as a nit, I guess this should be static or at least const

Copy link
Contributor Author

@BLooperZ BLooperZ Apr 20, 2023

Choose a reason for hiding this comment

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

Thank you, I think it can't be static because it uses g_engine.
const on the return type is a warning (Common::CodePage is enum)
added const that marks that the instance does not change, but it might be relevant to more methods here.

please note that this method is currently used once in the init.

Copy link
Contributor

Choose a reason for hiding this comment

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

I could have been more clear - yep, setting the function const is what I meant, which means it won't change the contents of the object, not return const.

I think it could also be static which in c has a bit of a different meaning to Java in that it just means "does not depend on which instance of this class it's called on".

Since g_engine is a global rather than a class member it's also be fine to make this function static, but I haven't been super consistent about that myself so const is also fine as it's not performance-critical code.

Thanks again for the fix!

@mduggan mduggan merged commit 818d57c into scummvm:master Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants