Application: General i18n workaround for shared font loading #76

CaiMiao · 2021-01-20T00:11:37Z

This is similar to #49 but I made a more graceful implement (at least imo), and I didn't aware of #49 when I was writing this lol

For memory consuming concern, we do not load full fallback chain when we are not in title takeover mode.

Confirmed working for my WIP tiny tool.

I'm not really a programmer so let me know if cleanup/optimization is needed.

Still I have a question, I can't really figure out how nvgAddFallbackFontId push the fallback order so I may have reversed my intend.

For example:

nvgAddFallbackFontId(Application::vg, Application::fontStash.regular, Application::fontStash.korean);
nvgAddFallbackFontId(Application::vg, Application::fontStash.regular, Application::fontStash.sharedSymbols);

does make the order regular->korean->symbol (appending) or regular->symbol->korean (prepending)?

natinusala · 2021-01-20T12:48:43Z

library/include/borealis/i18n.hpp

@@ -64,6 +64,14 @@ void loadTranslations();
 */
 std::string getCurrentLocale();

+#ifdef __SWITCH__


Why put it here? Technically it's i18n related but it doesn't belong to the i18n system

I mean it depends on the Switch implementation, and i18n is supposed to be generic (and eventually will)

this id only make sense for libnx i think
removed #ifdef

natinusala · 2021-01-20T12:49:44Z

library/lib/application.cpp

+            Logger::warning("Non applet mode, font full fallback is enabled!");
+        }
+
+        if (locale == 6 || locale == 15 || isFullFallback)


Not a fan of magic values here, could you define them somewhere instead?

substituted with enforcing SetLanguage enum

natinusala · 2021-01-20T12:50:13Z

library/lib/application.cpp

+        if (at == AppletType_Application || at == AppletType_SystemApplication) // title takeover
+        {
+            isFullFallback = true;
+            Logger::warning("Non applet mode, font full fallback is enabled!");


Is this really a warning?

changed to INFO

natinusala · 2021-01-20T12:54:37Z

library/lib/i18n.cpp

+        }
+        else
+        {
+            brls::Logger::error("Unable to convert system language ID (error 0x{0:x}), using the default one: {1}", res, DEFAULT_LOCALE);


You can use {0:#x} instead of 0x{0x:}

natinusala · 2021-01-20T12:55:33Z

Thanks for the PR!

nvgAddFallbackFontId works by appending the fallback, so the order would be regular then Korean then shared symbols.

I am going to wait for you to add proper names to the locale magic values since I am having a real hard time to understand the second part of the code without it.

natinusala · 2021-01-20T12:56:04Z

library/lib/application.cpp

-            Logger::info("Using Switch shared font");
-            Application::fontStash.regular = Application::loadFontFromMemory("regular", font.address, font.size, false);
+            Logger::info("Adding Switch shared standard font");
+            Application::fontStash.standard = Application::loadFontFromMemory("regular", font.address, font.size, false);


Wouldn't that be standard as well in the loadFontFromMemory call?

natinusala · 2021-01-24T19:59:52Z

I am very sorry but I am currently rewriting the entirety of font handling for the yoga rework (yoga branch, my work is not pushed yet).

I am currently adapting Behemoth's work of splitting borealis in two "platforms", glfw and switch. That means the Switch will have its own font loading code, which is no longer in application.cpp. The loading logic will be different too - everything will be abstracted away from the Switch definitions.

Would you mind redoing what you did on the yoga branch once I push what I have? I will notify you here.

The core of what you did will not be altered, you will just need to put it in the new font code dedicated to the Switch platform.

CaiMiao · 2021-01-24T20:08:31Z

i'm okay with that

natinusala · 2021-02-06T19:57:22Z

Hey @CaiMiao, I finished working on the new font abstraction layer. It's on the yoga branch, feel free to PR to that branch if you want to.

Now fonts are defined in a fully platform agnostic file, and every platform implementation loads them as they wish. The fallback code is still in application.cpp but the font loading has been moved to glfw_font.cpp and switch_font.cpp.

CaiMiao · 2021-02-07T14:10:22Z

have problem with building demo
massive <command-line>: warning: missing terminating " character
and some errors deny me from building (not only from this file)

<command-line>: error: missing terminating " character
<command-line>: note: in definition of macro 'BRLS_RESOURCES'
C:/src/msys64/home/CaiMiao/borealis/library/lib/core/font.cpp:25:29: note: in expansion of macro 'BRLS_ASSET'
   25 | #define MATERIAL_ICONS_PATH BRLS_ASSET("material/MaterialIcons-Regular.ttf")
      |                             ^~~~~~~~~~
C:/src/msys64/home/CaiMiao/borealis/library/lib/core/font.cpp:51:56: note: in expansion of macro 'MATERIAL_ICONS_PATH'
   51 |     return this->loadFontFromFile(FONT_MATERIAL_ICONS, MATERIAL_ICONS_PATH);
      |                                                        ^~~~~~~~~~~~~~~~~~~
make[1]: *** [/opt/devkitpro/devkitA64/base_rules:14：font.o] 错误 1
make: *** [Makefile:186：all] 错误 2

natinusala · 2021-02-07T15:48:05Z

It looks like your BRLS_RESOURCES define is wrong, it's missing a quote

CaiMiao · 2021-02-07T18:42:34Z

It looks like your BRLS_RESOURCES define is wrong, it's missing a quote

I'm compiling the demo under the yoga branch, and everything is unmodified, so i don't really know where I did wrong.
can only see CXXFLAGS := $(CXXFLAGS) -DYG_ENABLE_EVENTS -fdata-sections -DBRLS_RESOURCES="\"romfs:/\"" in borealis.mk

natinusala · 2021-02-07T19:42:02Z

Can you try again with the changes of this commit please?

6ea9f96

CaiMiao · 2021-02-08T09:07:11Z

Can you try again with the changes of this commit please?

6ea9f96

this works for me, ty. once i tested the implement okay i will PR them again asap.

CaiMiao · 2021-02-08T10:13:05Z

replaced by #79

CaiMiao · 2021-02-08T15:28:47Z

well i think it is not bad to merge this to legacy though, in case anyone need to include the shared font workaround without need to update to yoga.

natinusala · 2021-02-09T17:49:00Z

The thing is, I don't want to maintain this version anymore. The yoga rework is too hard to rebase since so many things changed 😕

Load Chinese font workaround

6691a96

natinusala requested changes Jan 20, 2021

View reviewed changes

commit 2 regarding review

c03edbd

CaiMiao force-pushed the master branch from 38ac446 to c03edbd Compare January 20, 2021 14:41

CaiMiao mentioned this pull request Feb 8, 2021

switch/application : shared font workaround #79

Open

CaiMiao closed this Feb 8, 2021

CaiMiao deleted the master branch February 8, 2021 14:52

CaiMiao restored the master branch February 8, 2021 15:22

CaiMiao changed the title ~~Application: Workaround for S./T. Chinese font loading~~ Application: General i18n workaround for shared font loading Feb 8, 2021

CaiMiao reopened this Feb 8, 2021

CaiMiao closed this Feb 10, 2021

natinusala mentioned this pull request Mar 7, 2021

Application: add Chinese shared font support #49

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Application: General i18n workaround for shared font loading #76

Application: General i18n workaround for shared font loading #76

CaiMiao commented Jan 20, 2021 •

edited

Loading

natinusala Jan 20, 2021

natinusala Jan 20, 2021

CaiMiao Jan 20, 2021

natinusala Jan 20, 2021

CaiMiao Jan 20, 2021

natinusala Jan 20, 2021

CaiMiao Jan 20, 2021

natinusala Jan 20, 2021

CaiMiao Jan 20, 2021

natinusala commented Jan 20, 2021

natinusala Jan 20, 2021

CaiMiao Jan 20, 2021

natinusala commented Jan 24, 2021

CaiMiao commented Jan 24, 2021 via email

natinusala commented Feb 6, 2021

CaiMiao commented Feb 7, 2021 •

edited

Loading

natinusala commented Feb 7, 2021

CaiMiao commented Feb 7, 2021

natinusala commented Feb 7, 2021

CaiMiao commented Feb 8, 2021

CaiMiao commented Feb 8, 2021

CaiMiao commented Feb 8, 2021

natinusala commented Feb 9, 2021

Application: General i18n workaround for shared font loading #76

Application: General i18n workaround for shared font loading #76

Conversation

CaiMiao commented Jan 20, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

natinusala commented Jan 20, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

natinusala commented Jan 24, 2021

CaiMiao commented Jan 24, 2021 via email

natinusala commented Feb 6, 2021

CaiMiao commented Feb 7, 2021 • edited Loading

natinusala commented Feb 7, 2021

CaiMiao commented Feb 7, 2021

natinusala commented Feb 7, 2021

CaiMiao commented Feb 8, 2021

CaiMiao commented Feb 8, 2021

CaiMiao commented Feb 8, 2021

natinusala commented Feb 9, 2021

CaiMiao commented Jan 20, 2021 •

edited

Loading

CaiMiao commented Feb 7, 2021 •

edited

Loading