Switching between variable fonts with different weights bug [372044241]

Fixed

Bug

Status Update

No update yet.

Description

ad...@gmail.com

created issue #1

Oct 7, 2024 07:56PM

- Jetpack Compose version: BOM 2024.09.03
- Jetpack Compose component used: FontVariation
- Android Studio Build: Build #AI-241.18034.62.2412.12266719, built on August 23, 2024
- Kotlin version: 1.9.0, 2.0.20
- Devices/Android versions reproduced on: Pixel 8 Pro API 35 emulator
- Keyboard (i.e. Gboard, Samsung, etc): Gboard

- Sample project to trigger the issue:

https://github.com/adrcotfas/VariableFontBug
A screen recording was attached.

Description of the minimal example:
I'm using two variable fonts: Azeret Mono and Roboto Mono.
For each font I created a map containing 4 different font weights(100, 200, 300, 400). I'm persisting this weight info along with the font index in a DataStore.
Using a slider and a menu I'm changing the style of a Text.

Observed behavior:
- negative case:
When I open the app and change the font from Azeret(200) to Roboto(200) the displayed style is Roboto(400) instead. Similar behavior can be observed with other configurations too.

- positive case:
When I open the app and play around with the slider first and then change the font, everything looks fine.

Screen_recording_20241007_224659.webm

7.4 MB

Download

Comments

gr...@google.com <gr...@google.com> #2Oct 11, 2024 12:17AM

Reassigned to se...@google.com.

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

https://android-review.googlesource.com/1360099

gr...@google.com <gr...@google.com> #3Oct 11, 2024 05:53PM

Forgot one part of the repro, you will need the font resources from the app to be present

se...@google.com <se...@google.com> Oct 15, 2024 07:53PM

Reassigned to an...@google.com.

se...@google.com <se...@google.com> #4Oct 15, 2024 09:14PM

Clearly caching bug, minimal repro:

Screenshot 2024-10-15 at 2.12.50 PM.png

104 KB

View

Download

Screenshot 2024-10-15 at 2.12.50 PM.png

104 KB

View

Download

Screenshot 2024-10-15 at 2.13.58 PM.png

84 KB

View

Download

se...@google.com <se...@google.com> #5Oct 15, 2024 09:14PM

private val fontWeights = listOf(100, 200, 300, 400)
private val fonts = listOf(
    "Azeret" to R.font.azeret_mono,
    "Roboto Mono" to R.font.roboto_mono,
)
@OptIn(ExperimentalTextApi::class)
private val fontFamilies = fonts.map { (_, resId) ->
    FontFamily(
        fontWeights.map { weight ->
            Font(
                resId = resId,
                weight = FontWeight(weight),
                variationSettings = FontVariation.Settings(FontVariation.weight(weight))
            )
        }
    )
}

@Composable
fun Demo() {
    Column {
        FullListRepro()
//        FullListOK()
    }
}

@Composable
fun FullListRepro() {
    Column {
        fontWeights.forEach { weight ->
            fonts.forEachIndexed { i, (familyName, _) ->
                val fontFamily = fontFamilies[i]
                val style = TextStyle(
                    fontFamily = fontFamily,
                    fontWeight = FontWeight(weight),
                    fontSize = 40.sp
                )
                BasicText("$weight $familyName", style=style)
            }
        }
    }
}

@Composable
fun FullListOK() {
    Column {
        fonts.forEachIndexed { i, (familyName, _) ->
            fontWeights.forEach { weight ->
                val fontFamily = fontFamilies[i]
                val style = TextStyle(
                    fontFamily = fontFamily,
                    fontWeight = FontWeight(weight),
                    fontSize = 40.sp
                )
                BasicText("$weight $familyName", style=style)
            }
        }
    }
}

OK composable is fine, but if you switch the order you see the incorrect behavior coming from resource font loading.

se...@google.com <se...@google.com> Oct 15, 2024 11:29PM

Reassigned to se...@google.com.

mo...@gmail.com <mo...@gmail.com> #6Nov 19, 2024 09:29AM

We had a similar kind of issue...

When setting the fontWeight, either in the TextStyle, or in the TextField, ... 100-300 shows same weight, 400-600 shows same weight etc ...

When setting it as FontVariation.Setting it is working correctly ...

Is this need to explicitly mention for variable fonts ?

se...@google.com <se...@google.com> #7Nov 19, 2024 09:32AM

That issue is a little bit different

We don't do any draw time istantiation of font axes.

You can get the behavior you expect by defining multiple instances of the font. One for each font weight that you want to support. You can put them in a FontFamily, and then matching is done by (FontWeight, Style)

Message last modified on Nov 19, 2024 09:38AM

se...@google.com <se...@google.com> Jan 7, 2025 09:59PM

Accepted by se...@google.com.

ap...@google.com <ap...@google.com> #8Jan 8, 2025 12:53AM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-main
Author: Sean McQuillan <seanmcq@google.com>
Link: https://android-review.googlesource.com/3308947

Don't allow Paint to cache variation settings of resource fonts

Expand for full commit details

Don't allow Paint to cache variation settings of resource fonts 
 
Prev: Loading two resource fonts with the same variation settings would cause the variation settings to not be applied to the second font. 
 
New: Paint cache is invalidated during font load path. 
 
Fixes: b/372044241 
Test: manual of variable font demo 
 
Relnote: "Resource fonts with the same variation settings will now avoid over-caching causing the incorrect variation settings to be applied." 
Change-Id: If3dff4ea44e33b1ed80bc7fb5057923a088f1678

Files:

M compose/ui/ui-text/src/androidMain/kotlin/androidx/compose/ui/text/font/PlatformTypefaces.android.kt

Hash: 8ad5f10469c5a044be4fd8a90fc19c5614f61734
Date: Tue Oct 15 16:25:26 2024

pr...@google.com <pr...@google.com> #9Jan 15, 2025 06:31PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.compose.ui:ui-text:1.8.0-alpha08
androidx.compose.ui:ui-text-android:1.8.0-alpha08
androidx.compose.ui:ui-text-jvmstubs:1.8.0-alpha08
androidx.compose.ui:ui-text-linuxx64stubs:1.8.0-alpha08

Issue 372044241

Description

Issue summary

Comments

gr...@google.com <gr...@google.com> #2Oct 11, 2024 12:17AM

gr...@google.com <gr...@google.com> #3Oct 11, 2024 05:53PM

se...@google.com <se...@google.com> Oct 15, 2024 07:53PM

se...@google.com <se...@google.com> #4Oct 15, 2024 09:14PM

se...@google.com <se...@google.com> #5Oct 15, 2024 09:14PM

se...@google.com <se...@google.com> Oct 15, 2024 11:29PM

mo...@gmail.com <mo...@gmail.com> #6Nov 19, 2024 09:29AM

se...@google.com <se...@google.com> #7Nov 19, 2024 09:32AM

se...@google.com <se...@google.com> Jan 7, 2025 09:59PM

ap...@google.com <ap...@google.com> #8Jan 8, 2025 12:53AM

pr...@google.com <pr...@google.com> #9Jan 15, 2025 06:31PM

Add comment

Issue metadata