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

android: make system fonts toggable #5670

Merged
merged 2 commits into from Dec 8, 2019
Merged

Conversation

@pazos
Copy link
Contributor

pazos commented Dec 6, 2019

like on desktop linux, but without the menu entry to open fonts dir.

Wip: works fine on fresh installation (without the koreader folder or with the folder without further modifications) but crashes on my main device, which is actually is/was heavily modified.

I'll update the crash logs when I have a bit of time.

Edit: the main reason for this PR is to avoid 15 pages of noto fonts on my huawei tablet. Others will find this useful too, specially on generic, modern, phones/tablets.


This change is Reviewable

@NiLuJe

This comment has been minimized.

Copy link
Member

NiLuJe commented Dec 6, 2019

Oh, yeah, +1, I also have a bazillion Noto variants on my OnePlus phone ;).

@pazos

This comment has been minimized.

Copy link
Contributor Author

pazos commented Dec 7, 2019

okey, the error happens if/when a system font was used for a document and now is disabled:

12-07 11:32:44.131 12016 12036 E KOReader: [NativeThread]: failed to run script: frontend/ui/widget/textwidget.lua:68: attempt to index field 'face' (a nil value)
12-07 11:32:44.131 12016 12036 E KOReader: [NativeThread]: Stopping due to previous errors

It should affect desktop linux too, but I didn't test.

@poire-z

This comment has been minimized.

Copy link
Contributor

poire-z commented Dec 7, 2019

a system font was used for a document
textwidget.lua:68: attempt to index field 'face' (a nil value)

That's not for a document, but for text in the UI elements - which should use only the NotoSans & Free* fonts we define in font.lua - which we still ship on Android.
So, the question is whether disabling system fonts make KOReader not use its own shipped fonts - or if, in spite of this disabling, it would still look for them, see them before the shipped ones, and ignore the shipped ones, and end up not using them because they are disabled (haven't look at the code, just some naive thoughts).

(Just checked how crengine behave when the main and the fallback fonts previously set disappear: it does not crash, and seem to use some Noto font.)

@pazos

This comment has been minimized.

Copy link
Contributor Author

pazos commented Dec 7, 2019

@poire-z: thanks for the info. I was stupid enough and didn't notice the error. This time with a debug build. As you can see the error is not related to noto, but with DroidSansFallback:

12-07 15:34:28.887 14143 14163 V KOReader:  Found font: NotoSans-Regular.ttf in ./fonts/noto/NotoSans-Regular.ttf
12-07 15:34:28.899 14143 14163 V KOReader:  Found font: NotoSans-Regular.ttf in ./fonts/noto/NotoSans-Regular.ttf
12-07 15:34:28.899 14143 14163 V KOReader:  Found font: DroidSansMono.ttf in ./fonts/droid/DroidSansMono.ttf
12-07 15:34:28.901 14143 14163 V KOReader:  Found font: NotoSansCJKsc-Regular.otf in ./fonts/noto/NotoSansCJKsc-Regular.otf
12-07 15:34:28.905 14143 14163 E KOReader:  #! Font  DroidSansFallback.ttf  ( DroidSansFallback.ttf ) not supported:  ffi/freetype.lua:142: Failed to load font './fonts/DroidSansFallback.ttf', freetype error code: 1

Reverting 503a57b fixes the error

@pazos pazos requested a review from Frenzie as a code owner Dec 7, 2019
@pazos pazos changed the title [wip] android: make system fonts toggable android: make system fonts toggable Dec 7, 2019
@pazos pazos added this to the 2019.12 milestone Dec 7, 2019
@poire-z

This comment has been minimized.

Copy link
Contributor

poire-z commented Dec 7, 2019

The alternative to ressurecting all the fonts we got rid of :) would be to still pick "our no more shipped" fonts from system fonts, even when system fonts are disabled - which might be a bit tedious to code :|
But as we ressurected all of them except that DroidSans (did we?), I guess it's fine just getting it back and forget about that tricky font picking stuff.

@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Dec 7, 2019

Alright, let's just revert it. :-/ The Droid Sans one is pretty small anyway; it's big fat Noto I wanted to get rid of.

@poire-z poire-z merged commit a5069f1 into koreader:master Dec 8, 2019
1 check passed
1 check passed
ci/circleci: build Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.