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

Add basic language and script fallback for the Android backend. #29

Merged
merged 1 commit into from
Apr 19, 2024

Conversation

xorgy
Copy link
Member

@xorgy xorgy commented Apr 13, 2024

No description provided.

@xorgy
Copy link
Member Author

xorgy commented Apr 13, 2024

This suffers from limitations in the other fallback code, namely that Hans and Hant are not tried because everything gets collapsed into Hani. The specific way that this should be handled depends on the order of system locales on some platforms.

@xorgy
Copy link
Member Author

xorgy commented Apr 13, 2024

I went ahead and added a workaround that checks if some lang fallback includes the Hant script, and adds that also for Hani. This should be locale-dependent in practice but this makes ambiguous han characters at least show up for now.

@xorgy xorgy requested a review from dfrg April 13, 2024 05:39
@xorgy
Copy link
Member Author

xorgy commented Apr 13, 2024

One of the limitations of this is that it only functions when postScriptName is present on at least one of the <font... elements in a fallback family. This is most of the language and script fallback families on common devices, but not all of them; on my device this misses the fallback families for Hebrew.

@xorgy xorgy requested review from xStrom, DJMcNab and nicoburns April 18, 2024 16:29
Copy link
Contributor

@waywardmonkeys waywardmonkeys left a comment

Choose a reason for hiding this comment

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

I don't know enough about what you're actually doing here, but I know enough to spot a typo!

src/fontique/backend/android.rs Outdated Show resolved Hide resolved
@DJMcNab
Copy link
Member

DJMcNab commented Apr 19, 2024

I'd be happy to test this on my device if there's a procedure for that, but I don't have the expertise to work out what's happening very quickly, and so could only give a rubber stamping review.

In which case, it should probably be given by Chad.

Copy link
Member

@DJMcNab DJMcNab left a comment

Choose a reason for hiding this comment

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

Nothing obviously entirely broken in the Rust code. Hopefully this behaviour will be fully covered in tests once we add those...

src/fontique/backend/android.rs Show resolved Hide resolved
src/fontique/backend/android.rs Outdated Show resolved Hide resolved
src/fontique/backend/android.rs Outdated Show resolved Hide resolved
src/fontique/backend/android.rs Show resolved Hide resolved
Copy link
Collaborator

@dfrg dfrg left a comment

Choose a reason for hiding this comment

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

In general, I think most of the logic here can be vastly simplified by just using the existing FallbackMap type which already handles most of the locale mapping internally. I don’t mind landing this as-is as an intermediate step though if it keeps things moving.

@xorgy
Copy link
Member Author

xorgy commented Apr 19, 2024

I'd be happy to test this on my device if there's a procedure for that, but I don't have the expertise to work out what's happening very quickly, and so could only give a rubber stamping review.

In which case, it should probably be given by Chad.

There is https://github.com/xorgy/xilem/tree/andtaffy if you have a local parley hooked up in the Cargo.toml. I primarily test this stuff on desktop (you just need to swap the "fontconfig/mod.rs" path for "android.rs" in your target's cfg of mod system;... and then you need to set $ANDROID_ROOT to a directory which has the equivalent structure (adb pull /system/etc/fonts.xml and /system/fonts/ into a dir, and point $ANDROID_ROOT TO THE system dir that you create). On a real Android device, $ANDROID_ROOT is set in the environment.

@xorgy xorgy dismissed waywardmonkeys’s stale review April 19, 2024 22:55

typo is resolved.

@xorgy xorgy added this pull request to the merge queue Apr 19, 2024
@xorgy xorgy removed this pull request from the merge queue due to a manual request Apr 19, 2024
@xorgy xorgy force-pushed the basic-android-fallback branch from 9884d7b to c29ae9f Compare April 19, 2024 23:09
@xorgy xorgy removed request for xStrom and nicoburns April 19, 2024 23:10
@xorgy xorgy added this pull request to the merge queue Apr 19, 2024
Merged via the queue into linebender:main with commit 88200b6 Apr 19, 2024
19 checks passed
@xorgy xorgy deleted the basic-android-fallback branch April 19, 2024 23:13
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.

5 participants