Skip to content

Commit

Permalink
Fallback Fonts Fix
Browse files Browse the repository at this point in the history
Feedback from Duos [here](https://2dimensions.slack.com/archives/C029X99PETE/p1727461147306559?thread_ts=1725996757.536839&cid=C029X99PETE)

- Fixup trimming whitespace when parsing fonts xml and when trying to use a filename to access the file system
- Adds a simple test
- Update various Android dependencies and remove `kotlin-reflect` (which we validated was unnecessary)
- Try to also fix the Release build step by removing unnecessary pip install

Diffs=
942516344 Fallback Fonts Fix (#8251)

Co-authored-by: Umberto Sonnino <[email protected]>
  • Loading branch information
umberto-sonnino and umberto-sonnino committed Oct 2, 2024
1 parent e32ffa1 commit f3e4b39
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 32 deletions.
5 changes: 2 additions & 3 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ jobs:
with:
submodules: true
token: ${{ secrets.PAT_GITHUB }}

- name: Configure AWS Credentials
uses: aws-actions/configure-aws-credentials@v2
with:
Expand Down Expand Up @@ -66,7 +66,6 @@ jobs:
wget -q https://github.com/premake/premake-core/releases/download/v5.0.0-beta2/premake-5.0.0-beta2-linux.tar.gz
tar -xf premake-5.0.0-beta2-linux.tar.gz
sudo mv premake5 /usr/local/bin
pip3 install ply
# Base64 decodes and pipes the GPG key content into the secret file
- name: Prepare environment
Expand Down Expand Up @@ -120,7 +119,7 @@ jobs:
UAT_OSSRH_USERNAME: ${{ secrets.UAT_OSSRH_USERNAME }}
UAT_OSSRH_PASSWORD: ${{ secrets.UAT_OSSRH_PASSWORD }}
# TODO: remove these after UAT is confirmed working
OSSRH_USERNAME: ${{ secrets.OSSRH_USERNAME }}
OSSRH_USERNAME: ${{ secrets.OSSRH_USERNAME }}
OSSRH_PASSWORD: ${{ secrets.OSSRH_PASSWORD }}
# ====
SIGNING_KEY_ID: ${{ secrets.SIGNING_KEY_ID }}
Expand Down
2 changes: 1 addition & 1 deletion .rive_head
Original file line number Diff line number Diff line change
@@ -1 +1 @@
a6aef22e97dbd1d87a675f865169e9518c366bdb
942516344f34dc274ff7ba124b5fcde097937071
19 changes: 9 additions & 10 deletions app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -70,25 +70,24 @@ dependencies {
implementation 'androidx.core:core-ktx:1.13.1'
implementation 'androidx.appcompat:appcompat:1.7.0'
implementation 'androidx.constraintlayout:constraintlayout:2.1.4'
implementation 'androidx.fragment:fragment-ktx:1.8.0'
implementation 'androidx.activity:activity:1.9.0'
implementation 'androidx.fragment:fragment-ktx:1.8.3'
implementation 'androidx.activity:activity:1.9.2'
debugImplementation project(path: ':kotlin')
releaseImplementation project(path: ':kotlin')
previewImplementation 'app.rive:rive-android:+'
implementation 'androidx.lifecycle:lifecycle-runtime-ktx:2.8.2'
implementation 'androidx.lifecycle:lifecycle-runtime-ktx:2.8.6'
implementation 'com.google.android.material:material:1.12.0'
implementation 'androidx.activity:activity-compose:1.9.0'
implementation 'androidx.activity:activity-compose:1.9.2'
implementation "androidx.compose.ui:ui:$compose_version"
implementation "androidx.compose.ui:ui-tooling-preview:$compose_version"
implementation 'androidx.compose.material3:material3:1.2.1'
implementation 'androidx.startup:startup-runtime:1.1.1'
implementation 'androidx.compose.material3:material3:1.3.0'
implementation 'androidx.startup:startup-runtime:1.2.0'
implementation 'com.android.volley:volley:1.2.1'
implementation 'org.jetbrains.kotlin:kotlin-reflect:2.0.0'

testImplementation 'junit:junit:4.13.2'
androidTestImplementation 'androidx.test.ext:junit:1.1.5'
androidTestImplementation 'androidx.test.espresso:espresso-core:3.5.1'
androidTestImplementation 'androidx.compose.ui:ui-test-junit4:1.6.8'
androidTestImplementation 'androidx.test.ext:junit:1.2.1'
androidTestImplementation 'androidx.test.espresso:espresso-core:3.6.1'
androidTestImplementation 'androidx.compose.ui:ui-test-junit4:1.7.2'
androidTestImplementation project(path: ':kotlin')
debugImplementation "androidx.compose.ui:ui-tooling:$compose_version"
debugImplementation "androidx.compose.ui:ui-test-manifest:$compose_version"
Expand Down
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ ext {
}
buildscript {
ext {
gradle_version = '8.5.0'
gradle_version = '8.6.1'
kotlin_version = '1.9.22'
dokkaVersion = "1.4.30"
compose_version = '1.6.7'
Expand Down
9 changes: 4 additions & 5 deletions kotlin/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,14 @@ dependencies {
implementation 'androidx.core:core-ktx:1.13.1'
implementation 'androidx.appcompat:appcompat:1.7.0'
implementation 'com.getkeepsafe.relinker:relinker:1.4.5'
implementation 'androidx.startup:startup-runtime:1.1.1'
implementation 'androidx.startup:startup-runtime:1.2.0'

testImplementation 'junit:junit:4.13.2'
testImplementation 'androidx.test:core-ktx:1.5.0'
testImplementation 'androidx.test:core-ktx:1.6.1'


androidTestImplementation 'androidx.test.ext:junit:1.1.5'
androidTestImplementation 'androidx.test.espresso:espresso-core:3.5.1'
implementation "org.jetbrains.kotlin:kotlin-reflect:2.0.0"
androidTestImplementation 'androidx.test.ext:junit:1.2.1'
androidTestImplementation 'androidx.test.espresso:espresso-core:3.6.1'
}

// Commenting this out, this only really helps when you're doing local dev, but it means our git actions need abd!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import java.io.ByteArrayInputStream
import java.nio.charset.Charset


@RunWith(AndroidJUnit4::class)
Expand Down Expand Up @@ -586,6 +588,32 @@ class FontHelpersTest {
}
}

@Test
fun xmlWhitespace() {
val systemFontXml = """
<?xml version="1.0" encoding="utf-8"?>
<familyset version="21">
<family name="sans-serif">
<font weight="700" style="normal">Roboto-Regular.ttf
<axis tag="ital" stylevalue="0" />
<axis tag="wdth" stylevalue="100" />
<axis tag="wght" stylevalue="700" />
</font>
</family>
</familyset>
""".trimIndent()

val inputStream =
ByteArrayInputStream(Charset.forName("UTF-16").encode(systemFontXml).array())

val result = SystemFontsParser.parseFontsXML(inputStream)
assertEquals(1, result.size)
val font = result["sans-serif"]?.fonts?.get(Fonts.Weight.BOLD)?.first()
assertNotNull(font)
val fontFile = FontHelper.getFontFile(font!!)
assertNotNull(fontFile)
}

@Test
fun nativeFallbackFontRegistration() {
val families = FontHelper.getSystemFonts().values
Expand Down
24 changes: 12 additions & 12 deletions kotlin/src/main/java/app/rive/runtime/kotlin/fonts/FontHelpers.kt
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ class FontHelper {
*/
fun getFontFile(font: Fonts.Font): File? = SystemFontsParser.SYSTEM_FONTS_PATHS
.asSequence()
.map { basePath -> File(basePath, font.name) }
.map { basePath -> File(basePath, font.name.trim()) }
.firstOrNull { it.exists() }

/**
Expand Down Expand Up @@ -284,17 +284,17 @@ class SystemFontsParser {
while (keepReading(parser)) {
if (parser.eventType != XmlPullParser.START_TAG) continue

val tag = parser.name
val tag = parser.name.trim()
when (tag) {
"family" -> {
parser.getAttributeValue(null, "name")?.let { name ->
readFamily(name, parser)?.let {
readFamily(name.trim(), parser)?.let {
familiesMap[name] = it
}
} ?: run {
// null name - possibly a legacy family?
readLegacyFamily(parser)?.let { (family, aliases) ->
val familyName = family.name!!
val familyName = family.name!!.trim()
familiesMap[familyName] = family
aliases.forEach { alias ->
remapAlias(alias, familiesMap)?.let { remapped ->
Expand Down Expand Up @@ -350,7 +350,7 @@ class SystemFontsParser {
while (keepReading(parser)) {
if (parser.eventType != XmlPullParser.START_TAG) continue

val tag = parser.name
val tag = parser.name.trim()
when (tag) {
"font" -> {
val font = readFont(parser)
Expand Down Expand Up @@ -387,7 +387,7 @@ class SystemFontsParser {
while (keepReading(parser)) {
if (parser.eventType != XmlPullParser.START_TAG) continue

val tag = parser.name
val tag = parser.name.trim()
when (tag) {
"fileset" -> {
filesList.addAll(
Expand Down Expand Up @@ -510,14 +510,14 @@ class SystemFontsParser {

while (keepReading(parser)) {
if (parser.eventType == XmlPullParser.TEXT) {
filenameBuilder.append(parser.text)
filenameBuilder.append(parser.text.trim())
}
if (parser.eventType != XmlPullParser.START_TAG) {
continue
}


val tag = parser.name
val tag = parser.name.trim()
if (tag == "axis") {
axes.add(readAxis(parser))
} else {
Expand All @@ -530,7 +530,7 @@ class SystemFontsParser {

private fun readText(parser: XmlPullParser): String {
return if (parser.next() == XmlPullParser.TEXT) {
val result = parser.text
val result = parser.text.trim()
parser.nextTag()
return result
} else ""
Expand All @@ -541,7 +541,7 @@ class SystemFontsParser {
while (keepReading(parser)) {
if (parser.eventType != XmlPullParser.START_TAG) continue

val tag = parser.name
val tag = parser.name.trim()
if (tag != "name") continue

val nameset = readText(parser)
Expand All @@ -556,7 +556,7 @@ class SystemFontsParser {
while (keepReading(parser)) {
if (parser.eventType != XmlPullParser.START_TAG) continue

val tag = parser.name
val tag = parser.name.trim()
if (tag != "file") continue

val variant = parser.getAttributeValue(null, "variant")
Expand Down Expand Up @@ -591,7 +591,7 @@ class SystemFontsParser {

skip(parser) // move past empty tag.

return Fonts.Alias(name, to, weight)
return Fonts.Alias(name.trim(), to, weight)
}


Expand Down

0 comments on commit f3e4b39

Please sign in to comment.