Nan vals in textstyle (#870)

* Include <cstdint> is required because of uintptr_t

* move `generateVersion` to common code

* on Linux with Chinese language chosen in interface this test fails

* this combination leads to NaN values when height is overridden

* test that empty font still have a meaningful metrics, because FontStyle may rely on it

* simplify test

* check that we don't put NaN in skia

this value might be used as part of key in paragraph cache, which may lead to UB atm

skiko/buildSrc/src/main/kotlin/tasks/configuration/CommonTasksConfiguration.kt

 package tasks.configuration
 
+import Arch
 import OS
 import SkiaBuildType
+import SkikoProperties
+import org.gradle.api.DefaultTask
 import org.gradle.api.Project
+import org.gradle.kotlin.dsl.get
+import org.jetbrains.kotlin.gradle.plugin.KotlinTarget
+import org.jetbrains.kotlin.gradle.tasks.KotlinCompileTool
+import registerSkikoTask
 import supportAndroid
 import supportWasm
+import toTitleCase
 import java.io.File
 
 fun skiaHeadersDirs(skiaDir: File): List<File> =
@@ -134,4 +142,46 @@ fun Project.configureSignAndPublishDependencies() {
             }
         }
     }
+}
+
+fun KotlinTarget.generateVersion(
+    targetOs: OS,
+    targetArch: Arch,
+    skikoProperties: SkikoProperties
+) {
+    val targetName = this.name
+    val isArm64Simulator = isIosSimArm64()
+    val generatedDir = project.layout.buildDirectory.dir("generated/$targetName")
+    val generateVersionTask = project.registerSkikoTask<DefaultTask>(
+        "generateVersion${toTitleCase(platformType.name)}".withSuffix(isIosSim = isArm64Simulator),
+        targetOs,
+        targetArch
+    ) {
+        inputs.property("buildType", skikoProperties.buildType.id)
+        outputs.dir(generatedDir)
+        doFirst {
+            val outDir = generatedDir.get().asFile
+            outDir.deleteRecursively()
+            outDir.mkdirs()
+            val out = "$outDir/Version.kt"
+
+            val target = "${targetOs.id}-${targetArch.id}"
+            val skiaTag = project.property("dependencies.skia.$target") as String
+            File(out).writeText(
+                """
+                package org.jetbrains.skiko
+                object Version {
+                val skiko = "${skikoProperties.deployVersion}"
+                val skia = "$skiaTag"
+                }
+                """.trimIndent()
+            )
+        }
+    }
+
+    val compilation = compilations["main"] ?: error("Could not find 'main' compilation for target '$this'")
+    compilation.compileKotlinTaskProvider.configure {
+        dependsOn(generateVersionTask)
+        (this as KotlinCompileTool).source(generatedDir.get().asFile)
+    }
 }
\ No newline at end of file

skiko/buildSrc/src/main/kotlin/tasks/configuration/NativeTasksConfiguration.kt

@@ -36,48 +36,6 @@ fun String.withSuffix(isIosSim: Boolean = false) =
 fun KotlinTarget.isIosSimArm64() =
     name.contains("iosSimulatorArm64", ignoreCase = true)
 
-fun KotlinTarget.generateVersion(
-    targetOs: OS,
-    targetArch: Arch,
-    skikoProperties: SkikoProperties
-) {
-    val targetName = this.name
-    val isArm64Simulator = isIosSimArm64()
-    val generatedDir = project.layout.buildDirectory.dir("generated/$targetName")
-    val generateVersionTask = project.registerSkikoTask<DefaultTask>(
-        "generateVersion${toTitleCase(platformType.name)}".withSuffix(isIosSim = isArm64Simulator),
-        targetOs,
-        targetArch
-    ) {
-        inputs.property("buildType", skikoProperties.buildType.id)
-        outputs.dir(generatedDir)
-        doFirst {
-            val outDir = generatedDir.get().asFile
-            outDir.deleteRecursively()
-            outDir.mkdirs()
-            val out = "$outDir/Version.kt"
-
-            val target = "${targetOs.id}-${targetArch.id}"
-            val skiaTag = project.property("dependencies.skia.$target") as String
-            File(out).writeText(
-                """
-                package org.jetbrains.skiko
-                object Version {
-                  val skiko = "${skikoProperties.deployVersion}"
-                  val skia = "$skiaTag"
-                }
-                """.trimIndent()
-            )
-        }
-    }
-
-    val compilation = compilations["main"] ?: error("Could not find 'main' compilation for target '$this'")
-    compilation.compileKotlinTaskProvider.configure {
-        dependsOn(generateVersionTask)
-        (this as KotlinCompileTool).source(generatedDir.get().asFile)
-    }
-}
-
 fun SkikoProjectContext.compileNativeBridgesTask(
     os: OS, arch: Arch, isArm64Simulator: Boolean
 ): TaskProvider<CompileSkikoCppTask> = with (this.project) {

skiko/src/awtMain/cpp/include/jni_helpers.h

 #pragma once
 
+#include <cstdint>
 #include <jni.h>
 
 template<typename T>

skiko/src/commonMain/kotlin/org/jetbrains/skia/paragraph/ParagraphBuilder.kt

@@ -49,15 +49,22 @@ class ParagraphBuilder(style: ParagraphStyle?, fc: FontCollection?) :
     }
 
     fun addPlaceholder(style: PlaceholderStyle): ParagraphBuilder {
-        Stats.onNativeCall()
-        _nAddPlaceholder(
-            _ptr,
-            style.width,
-            style.height,
-            style.alignment.ordinal,
-            style.baselineMode.ordinal,
-            style.baseline
-        )
+        check(!style.width.isNaN())
+        check(!style.height.isNaN())
+        check(!style.baseline.isNaN())
+        try {
+            Stats.onNativeCall()
+            _nAddPlaceholder(
+                _ptr,
+                style.width,
+                style.height,
+                style.alignment.ordinal,
+                style.baselineMode.ordinal,
+                style.baseline
+            )
+        } finally {
+            reachabilityBarrier(this)
+        }
         return this
     }
 

skiko/src/commonMain/kotlin/org/jetbrains/skia/paragraph/StrutStyle.kt

@@ -47,9 +47,13 @@ class StrutStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finali
         }
 
     fun setFontFamilies(families: Array<String>): StrutStyle {
-        Stats.onNativeCall()
-        interopScope {
-            StrutStyle_nSetFontFamilies(_ptr, toInterop(families), families.size)
+        try {
+            Stats.onNativeCall()
+            interopScope {
+                StrutStyle_nSetFontFamilies(_ptr, toInterop(families), families.size)
+            }
+        } finally {
+            reachabilityBarrier(this)
         }
         return this
     }
@@ -69,8 +73,12 @@ class StrutStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finali
         }
     
     fun setFontStyle(style: FontStyle): StrutStyle {
-        Stats.onNativeCall()
-        _nSetFontStyle(_ptr, style._value)
+        try {
+            Stats.onNativeCall()
+            _nSetFontStyle(_ptr, style._value)
+        } finally {
+            reachabilityBarrier(this)
+        }
         return this
     }
 
@@ -86,8 +94,13 @@ class StrutStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finali
         }
 
     fun setFontSize(value: Float): StrutStyle {
-        Stats.onNativeCall()
-        _nSetFontSize(_ptr, value)
+        check(!value.isNaN())
+        try {
+            Stats.onNativeCall()
+            _nSetFontSize(_ptr, value)
+        } finally {
+            reachabilityBarrier(this)
+        }
         return this
     }
 
@@ -103,8 +116,13 @@ class StrutStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finali
         }
     
     fun setHeight(value: Float): StrutStyle {
-        Stats.onNativeCall()
-        StrutStyle_nSetHeight(_ptr, value)
+        check(!value.isNaN())
+        try {
+            Stats.onNativeCall()
+            StrutStyle_nSetHeight(_ptr, value)
+        } finally {
+            reachabilityBarrier(this)
+        }
         return this
     }
 
@@ -120,8 +138,13 @@ class StrutStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finali
         }
     
     fun setLeading(value: Float): StrutStyle {
-        Stats.onNativeCall()
-        _nSetLeading(_ptr, value)
+        check(!value.isNaN())
+        try {
+            Stats.onNativeCall()
+            _nSetLeading(_ptr, value)
+        } finally {
+            reachabilityBarrier(this)
+        }
         return this
     }
 
@@ -137,8 +160,12 @@ class StrutStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finali
         }
     
     fun setEnabled(value: Boolean): StrutStyle {
-        Stats.onNativeCall()
-        StrutStyle_nSetEnabled(_ptr, value)
+        try {
+            Stats.onNativeCall()
+            StrutStyle_nSetEnabled(_ptr, value)
+        } finally {
+            reachabilityBarrier(this)
+        }
         return this
     }
 
@@ -154,8 +181,12 @@ class StrutStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finali
         }
     
     fun setHeightForced(value: Boolean): StrutStyle {
-        Stats.onNativeCall()
-        _nSetHeightForced(_ptr, value)
+        try {
+            Stats.onNativeCall()
+            _nSetHeightForced(_ptr, value)
+        } finally {
+            reachabilityBarrier(this)
+        }
         return this
     }
 
@@ -171,8 +202,12 @@ class StrutStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finali
         }
     
     fun setHeightOverridden(value: Boolean): StrutStyle {
-        Stats.onNativeCall()
-        _nSetHeightOverridden(_ptr, value)
+        try {
+            Stats.onNativeCall()
+            _nSetHeightOverridden(_ptr, value)
+        } finally {
+            reachabilityBarrier(this)
+        }
         return this
     }
 
@@ -188,8 +223,12 @@ class StrutStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finali
         }
 
     fun setHalfLeading(value: Boolean): StrutStyle {
-        Stats.onNativeCall()
-        _nSetHalfLeading(_ptr, value)
+        try {
+            Stats.onNativeCall()
+            _nSetHalfLeading(_ptr, value)
+        } finally {
+            reachabilityBarrier(this)
+        }
         return this
     }
 

skiko/src/commonMain/kotlin/org/jetbrains/skia/paragraph/TextStyle.kt

@@ -54,8 +54,12 @@ class TextStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finaliz
         }
 
     fun setColor(color: Int): TextStyle {
-        Stats.onNativeCall()
-        _nSetColor(_ptr, color)
+        try {
+            Stats.onNativeCall()
+            _nSetColor(_ptr, color)
+        } finally {
+            reachabilityBarrier(this)
+        }
         return this
     }
 
@@ -125,17 +129,21 @@ class TextStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finaliz
         }
 
     fun setDecorationStyle(d: DecorationStyle): TextStyle {
-        Stats.onNativeCall()
-        _nSetDecorationStyle(
-            _ptr,
-            d._underline,
-            d._overline,
-            d._lineThrough,
-            d._gaps,
-            d.color,
-            d._lineStyle.ordinal,
-            d.thicknessMultiplier
-        )
+        try {
+            Stats.onNativeCall()
+            _nSetDecorationStyle(
+                _ptr,
+                d._underline,
+                d._overline,
+                d._lineThrough,
+                d._gaps,
+                d.color,
+                d._lineStyle.ordinal,
+                d.thicknessMultiplier
+            )
+        } finally {
+            reachabilityBarrier(this)
+        }
         return this
     }
 
@@ -151,8 +159,12 @@ class TextStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finaliz
         }
 
     fun setFontStyle(s: FontStyle): TextStyle {
-        Stats.onNativeCall()
-        TextStyle_nSetFontStyle(_ptr, s._value)
+        try {
+            Stats.onNativeCall()
+            TextStyle_nSetFontStyle(_ptr, s._value)
+        } finally {
+            reachabilityBarrier(this)
+        }
         return this
     }
 
@@ -167,8 +179,12 @@ class TextStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finaliz
         }
 
     fun addShadow(s: Shadow): TextStyle {
-        Stats.onNativeCall()
-        _nAddShadow(_ptr, s.color, s.offsetX, s.offsetY, s.blurSigma)
+        try {
+            Stats.onNativeCall()
+            _nAddShadow(_ptr, s.color, s.offsetX, s.offsetY, s.blurSigma)
+        } finally {
+            reachabilityBarrier(this)
+        }
         return this
     }
 
@@ -178,17 +194,21 @@ class TextStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finaliz
     }
 
     fun clearShadows(): TextStyle {
-        Stats.onNativeCall()
-        _nClearShadows(_ptr)
+        try {
+            Stats.onNativeCall()
+            _nClearShadows(_ptr)
+        } finally {
+            reachabilityBarrier(this)
+        }
         return this
     }
 
     val fontFeatures: Array<FontFeature>
         get() = try {
             Stats.onNativeCall()
-
             val size = _nGetFontFeaturesSize(_ptr)
             withResult(IntArray(size * 2)) {
+                Stats.onNativeCall()
                 _nGetFontFeatures(_ptr, it)
             }.let {
                 FontFeature.fromInteropEncodedBy2Ints(it)
@@ -198,9 +218,13 @@ class TextStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finaliz
         }
 
     fun addFontFeature(f: FontFeature): TextStyle {
-        Stats.onNativeCall()
-        interopScope {
-            _nAddFontFeature(_ptr, toInterop(f.tag), f.value)
+        try {
+            Stats.onNativeCall()
+            interopScope {
+                _nAddFontFeature(_ptr, toInterop(f.tag), f.value)
+            }
+        } finally {
+            reachabilityBarrier(this)
         }
         return this
     }
@@ -211,8 +235,12 @@ class TextStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finaliz
     }
 
     fun clearFontFeatures(): TextStyle {
-        Stats.onNativeCall()
-        _nClearFontFeatures(_ptr)
+        try {
+            Stats.onNativeCall()
+            _nClearFontFeatures(_ptr)
+        } finally {
+            reachabilityBarrier(this)
+        }
         return this
     }
 
@@ -228,8 +256,13 @@ class TextStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finaliz
         }
 
     fun setFontSize(size: Float): TextStyle {
-        Stats.onNativeCall()
-        TextStyle_nSetFontSize(_ptr, size)
+        check(!size.isNaN())
+        try {
+            Stats.onNativeCall()
+            TextStyle_nSetFontSize(_ptr, size)
+        } finally {
+            reachabilityBarrier(this)
+        }
         return this
     }
 
@@ -271,8 +304,22 @@ class TextStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finaliz
         }
 
     fun setHeight(height: Float?): TextStyle {
-        Stats.onNativeCall()
-        if (height == null) TextStyle_nSetHeight(_ptr, false, 0f) else TextStyle_nSetHeight(_ptr, true, height)
+        if (height == null) {
+            try {
+                Stats.onNativeCall()
+                TextStyle_nSetHeight(_ptr, false, 0f)
+            } finally {
+                reachabilityBarrier(this)
+            }
+        } else {
+            check(!height.isNaN())
+            try {
+                Stats.onNativeCall()
+                TextStyle_nSetHeight(_ptr, true, height)
+            } finally {
+                reachabilityBarrier(this)
+            }
+        }
         return this
     }
 
@@ -288,8 +335,12 @@ class TextStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finaliz
         }
 
     fun setHalfLeading(value: Boolean): TextStyle {
-        Stats.onNativeCall()
-        TextStyle_nSetHalfLeading(_ptr, value)
+        try {
+            Stats.onNativeCall()
+            TextStyle_nSetHalfLeading(_ptr, value)
+        } finally {
+            reachabilityBarrier(this)
+        }
         return this
     }
 
@@ -305,8 +356,13 @@ class TextStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finaliz
         }
 
     fun setLetterSpacing(letterSpacing: Float): TextStyle {
-        Stats.onNativeCall()
-        _nSetLetterSpacing(_ptr, letterSpacing)
+        check(!letterSpacing.isNaN())
+        try {
+            Stats.onNativeCall()
+            _nSetLetterSpacing(_ptr, letterSpacing)
+        } finally {
+            reachabilityBarrier(this)
+        }
         return this
     }
 
@@ -322,8 +378,13 @@ class TextStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finaliz
         }
 
     fun setBaselineShift(baselineShift: Float): TextStyle {
-        Stats.onNativeCall()
-        TextStyle_nSetBaselineShift(_ptr, baselineShift)
+        check(!baselineShift.isNaN())
+        try {
+            Stats.onNativeCall()
+            TextStyle_nSetBaselineShift(_ptr, baselineShift)
+        } finally {
+            reachabilityBarrier(this)
+        }
         return this
     }
 
@@ -339,8 +400,13 @@ class TextStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finaliz
         }
 
     fun setWordSpacing(wordSpacing: Float): TextStyle {
-        Stats.onNativeCall()
-        _nSetWordSpacing(_ptr, wordSpacing)
+        check(!wordSpacing.isNaN())
+        try {
+            Stats.onNativeCall()
+            _nSetWordSpacing(_ptr, wordSpacing)
+        } finally {
+            reachabilityBarrier(this)
+        }
         return this
     }
 
@@ -384,9 +450,13 @@ class TextStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finaliz
         }
 
     fun setLocale(locale: String?): TextStyle {
-        Stats.onNativeCall()
-        interopScope {
-            _nSetLocale(_ptr, toInterop(locale))
+        try {
+            Stats.onNativeCall()
+            interopScope {
+                _nSetLocale(_ptr, toInterop(locale))
+            }
+        } finally {
+            reachabilityBarrier(this)
         }
         return this
     }
@@ -394,7 +464,7 @@ class TextStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finaliz
     var baselineMode: BaselineMode
         get() = try {
             Stats.onNativeCall()
-            BaselineMode.values().get(_nGetBaselineMode(_ptr))
+            BaselineMode.entries[_nGetBaselineMode(_ptr)]
         } finally {
             reachabilityBarrier(this)
         }
@@ -403,8 +473,12 @@ class TextStyle internal constructor(ptr: NativePointer) : Managed(ptr, _Finaliz
         }
 
     fun setBaselineMode(baseline: BaselineMode): TextStyle {
-        Stats.onNativeCall()
-        _nSetBaselineMode(_ptr, baseline.ordinal)
+        try {
+            Stats.onNativeCall()
+            _nSetBaselineMode(_ptr, baseline.ordinal)
+        } finally {
+            reachabilityBarrier(this)
+        }
         return this
     }
 

skiko/src/commonTest/kotlin/org/jetbrains/skia/FontTests.kt

@@ -12,6 +12,7 @@ import org.jetbrains.skiko.tests.runTest
 import kotlin.test.Test
 import kotlin.test.assertContentEquals
 import kotlin.test.assertEquals
+import kotlin.test.assertFalse
 
 private fun isMac() = (hostOs == OS.MacOS)
 private fun isIos() = (hostOs == OS.Ios)
@@ -119,4 +120,17 @@ class FontTests {
 
         }
     }
+
+    @Test
+    fun emptyFontMetrics() {
+        Font(null).use { font ->
+            val metrics = font.metrics
+            assertFalse(
+                metrics.top == 0f &&
+                        metrics.bottom == 0f &&
+                        metrics.ascent == 0f &&
+                        metrics.descent == 0f
+            )
+        }
+    }
 }

skiko/src/commonTest/kotlin/org/jetbrains/skiko/paragraph/TextStyleTest.kt

@@ -132,6 +132,7 @@ class TextStyleTest {
         }
     }
 
+    @Test
     fun textStyleBaselineTest() {
         TextStyle().use { textStyle ->
             assertEquals(0.0f, textStyle.baselineShift)
@@ -140,4 +141,20 @@ class TextStyleTest {
         }
     }
 
+    @Test
+    fun textStyleMetricsContainsMeaningfulValues() {
+        TextStyle().use { textStyle ->
+            val metrics = textStyle.fontMetrics
+            assertFalse(metrics.ascent == 0f && metrics.descent == 0f && metrics.leading == 0f)
+        }
+    }
+
+    @Test
+    fun textStyleNotContainNaNValues() {
+        TextStyle().use { textStyle ->
+            textStyle.height = 32f
+            assertTrue(!textStyle.fontMetrics.ascent.isNaN())
+        }
+    }
+
 }

skiko/src/jvmMain/cpp/common/impl/Managed.cc

 #include <iostream>
+#include <cstdint>
 #include <jni.h>
 
 typedef void (*FreeFunction)(void*);