search

natinusala / borealis

Hardware accelerated, controller and TV oriented UI library for PC and Nintendo Switch (libnx)
Apache License 2.0
260 stars 83 forks source link

Application: add Chinese shared font support #49

Closed wwwwwwzx closed 3 years ago

wwwwwwzx commented 4 years ago

also works for traditional Chinese characters. issue #26

natinusala commented 4 years ago

Thanks for the PR!

I have a concern about memory usage (that I already had for the Korean font at the time).

How large is the Chinese font? Does it contain latin characters as well?

My idea would be to only load Chinese font if the system language is Chinese (same for Korean). What do you think?

wwwwwwzx commented 4 years ago

Agree with loading the font if the system language is so. (But I am not familiar with the API) Have done this for Plutonium: https://github.com/zaksabeast/CaptureSight/pull/18/commits/aaccce5cb20282462d2c444bab48393aa1d3a4c3

natinusala commented 4 years ago

And can we avoid loading the Latin font as well? Or do you need both on Chinese systems?

Le 19 juillet 2020 11:18:54 wwwwwwzx notifications@github.com a écrit :

Agree with loading the font if the system language is so. (But I am not familiar with the API) Have done this for Plutonium: zaksabeast/CaptureSight@aaccce5 — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

WerWolv commented 4 years ago

Latin font should always be loaded. In EdiZon for example I always fall back to thr English translation if there's none in the currently selected language. That way you can have translations but don't make your app completely unusable if a few important translations are missing

natinusala commented 4 years ago

@WerWolv yeah but what if the Chinese font also contains [A-Za-z0-9]? We don't need to load both then, do we?

WerWolv commented 4 years ago

Does it have it? I never checked. If so then yeah. A way to still force load the fonts if needed would be nice though. I have a custom language switcher in my settings which really depends on all of them

natinusala commented 4 years ago

Well then I guess we can start by only loading Chinese and Korean if the system language needs it.

@wwwwwwzx can you make the change in your PR?

wwwwwwzx commented 4 years ago

Checked in CaptureSight. iPad Screen Shot 7-19-20, 3 50 AM

natinusala commented 4 years ago

That looks nice! Can you ensure that all latin characters are present, including accentued ones? It's for @WerWolv

Also I'm going to ask you to run./scripts/format --fix (you will need clang-format >= 10) and rebase + squash your PR before I can merge it.

wwwwwwzx commented 4 years ago

  1. Tested with string: "Å å Ǻ ǻ Ḁ ḁ ẚ Ă ă Ặ ặ Ắ ắ Ằ ằ Ẳ ẳ Ẵ ẵ Ȃ ȃ Â â Ậ ậ Ấ ấ Ầ ầ Ẫ ẫ Ẩ ẩ Ả ả Ǎ ǎ Ⱥ ⱥ Ȧ ȧ Ǡ ǡ Ạ ạ Ä ä Ǟ ǟ À à Ȁ ȁ Á á Ā ā Ā̀ ā̀ Ã ã Ą ą Ą́ ą́ Ą̃ ą̃ A̲ a̲ ᶏ" (copied from wiki) Only loaded Chinese font iPad Screen Shot 7-19-20, 4 10 PM Added regular font iPad Screen Shot 7-19-20, 4 09 PM So probably let's load the regular font in Chinese and Korean system as well.

  2. clang-format is automatically running in my IDE. I have 14 other changed files after running the script. Are you sure to squash them into this pull request?

natinusala commented 4 years ago

clang-format is automatically running in my IDE. I have 14 other changed files after running the script. Are you sure to squash them into this pull request?

Uh, no 😅 Please run it using the script we provide, it has our configuration with our code style in it.

Then we need to always load regular latin font as well, which is what you did in the commit.

However it seems like you catched junk from master while rebasing, can you fix that please?

wwwwwwzx commented 4 years ago

I did run your script. And at first, the error is the following (I have 10.0.0 installed): run-clang-format.py: error: Command 'clang-format-8 --version' failed to start: [Errno 2] No such file or directory After changing the --clang-format-executable to clang-format, there are 14 changed files (see the second commit)

natinusala commented 4 years ago

Oh shoot I misunderstood the first time. Let me run clang-format on the entirety of master, that way it doesn't interfere with your changes. Sorry about that.

wwwwwwzx commented 4 years ago

Uh, conflicts are still there. Sorry I am not familiar with git commands.

natinusala commented 4 years ago

You need to rebase on the master branch, they should resolve themselves

If not, git will prompt you to fix them and it should be fairly easy

natinusala commented 3 years ago

Hi, sorry for the delay! I do not accept contributions for master anymore so I will be closing this PR. #76 aims to reimplement what you did in the rework branch, if you want to have a look.