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

Improve how we manage font ability to display charsets in translation #3312

Closed
jordimas opened this issue Mar 30, 2024 · 7 comments
Closed

Comments

@jordimas
Copy link
Contributor

jordimas commented Mar 30, 2024

Affected hardware version

Bangle 2

Your firmware version

2v12

The bug

Background

Currently when uploading the Bangle translations to the app, we remap any chars that we don't think we can display in Espruino's built in fonts. See:

https://github.com/espruino/EspruinoAppLoaderCore/blob/master/js/appinfo.js#L77

In the mappings:

https://github.com/espruino/EspruinoAppLoaderCore/blob/master/js/utils.js#L75

things like "á" are mapped to "a".

Problem

This has an impact in many languages. For example, in Spanish or Catalan "Sí" means "Yes" but "Si" (without accent)" means "If". Removing the accent has an impact on meaning. Additionally it is a spelling mistake.

Some Espurino fonts are able to display iso-8859-1 characters and others do NOT. By doing this we remove the possibility for ALL the fonts to show these characters. This provides a suboptimal experience since we remove chars that can be displayed in some fonts.

Proposed solution

The goal is to display the iso-8859-1 when the font allows so for a better internationalised experience.

An alternative approach that will allow to display characters when it is possible:

  1. When processing the translation do not do any mapping, just make sure that the chars are that are valid iso-8859-1.

  2. When we draw the string (e.g DrawString) do the mapping ONLY if the font does not support all iso-8859-1 chars.

@bobrippling
Copy link
Collaborator

Thanks for the detailed issue - a good offshoot of #3109, @gfwilliams tagging for visibility

@gfwilliams
Copy link
Member

Thanks - this was the idea actually. While perhaps not clear in naming, convertStringToISOLatin should convert to ISO8859-1 - so please feel free to remove any conversions that shouldn't be there (or add some that should, like 0x0400 to 0x00C8 and similar make a lot of sense).

Most Espruino fonts (apart from 4x6 where it doesn't make sense) should now support ISO8859-1 I believe (I made some changes recently to 6x15 where it didn't handle some chars correctly so I think that doing the first point you suggested would fix most things.

When we draw the string (e.g DrawString) do the mapping ONLY if the font does not support all iso-8859-1 chars.

This is something that would require the mapping to be done in the Espruino firmware itself. It's definitely possible, but if this were to be added it'd need to be done in a reasonably space efficient way so it could fit in Bangle.js 1. Even the ability to map non-capital ASCII to capitals in fonts that only support capitals would be a cool addition that I had wondered about before, so extending that to ISO8859-1 shouldn't be hard.

I've just added some (commented out) code to the Espruino repo to add a _jswrap_graphics_fontMapCharacter function which could make sense to use.

As @bobrippling linked, #3109 is the ideal endpoint of all this (allowing big fonts to be installed and used where needed) - but that's quite a big thing to sort out, and I think in this case just ensuring that we really do use ISO8859-1 would probably do us fine.

@jordimas
Copy link
Contributor Author

jordimas commented Apr 3, 2024

Thanks so much @gfwilliams

In terms of next steps, my view is:

  1. Still some work left in _jswrap_graphics_fontInfoMaxChar to call it for the fonts which need the mapping
  2. A new version of the Espruino firmware then needs to be released which supports any drawing any iso-8859-1 (or drawing it or doing mapping in the firmware)
  3. I can do a PR in https://github.com/espruino/EspruinoAppLoaderCore/blob/master/js/appinfo.js#L77 to remove the conversion since the Espruino firmware at that point will support any iso-8859-1 string to be displayed. I will like to test this carefully to make sure that we do not introduce any regression on the existing apps and languages.

If this makes sense, I can do 3) when 1) and 2) are completed. Does make sense?

@gfwilliams
Copy link
Member

Afraid not - we want to keep the conversion. ISO-8859-1 is still the char codes 0..255, whereas Unicode is potentially 10,000 or more characters. We still need to ensure we convert anything we come across that's not 0..255 into the ISO-8859-1 range.

What I meant was to remove any character conversions from convertStringToISOLatin that convert chars in the 0..255 range

@jordimas
Copy link
Contributor Author

jordimas commented Apr 3, 2024

Yes, right. I was not clear, sorry. To remove the conversion I meant to remove the mappings, we still need to make sure that only ISO-8859-1 gets and that is also normalized as much as possible.

@gfwilliams
Copy link
Member

Well, I think you could update https://github.com/espruino/EspruinoAppLoaderCore/blob/master/js/utils.js#L75 to remove any we don't want now and it should all work with the existing firmware apart from where the 4x6 font is used (which is almost nowhere). I think the following can go:

> Object.keys(CODEPAGE_CONVERSIONS).filter(d=>d.charCodeAt()<=255)
[
  'æ', 'å', 'à', 'ç', 'è',
  'é', 'í', 'ï', 'ó', 'ò',
  'ø', 'ú', 'ü', 'À', 'Ç',
  'È', 'É', 'Ï', 'Ó', 'Ò',
  'Ú', 'Ü'
]

@jordimas
Copy link
Contributor Author

jordimas commented Apr 9, 2024

I'm closing this since this changes have been merged. Thanks

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

No branches or pull requests

3 participants