diff --git a/skiko/build.gradle.kts b/skiko/build.gradle.kts index 1a6c6bfe9..746022660 100644 --- a/skiko/build.gradle.kts +++ b/skiko/build.gradle.kts @@ -1217,6 +1217,7 @@ tasks.withType().configureEach { ) systemProperty("skiko.test.ui.enabled", System.getProperty("skiko.test.ui.enabled", canRunUiTests.toString())) systemProperty("skiko.test.ui.renderApi", System.getProperty("skiko.test.ui.renderApi", "all")) + systemProperty("skiko.test.debug", buildType == SkiaBuildType.DEBUG) // Tests should be deterministic, so disable scaling. // On MacOs we need the actual scale, otherwise we will have aliased screenshots because of scaling. diff --git a/skiko/buildSrc/src/main/kotlin/properties.kt b/skiko/buildSrc/src/main/kotlin/properties.kt index 23ca8101a..53a5c0c4c 100644 --- a/skiko/buildSrc/src/main/kotlin/properties.kt +++ b/skiko/buildSrc/src/main/kotlin/properties.kt @@ -75,7 +75,7 @@ enum class SkiaBuildType( DEBUG( "Debug", flags = arrayOf("-DSK_DEBUG"), - clangFlags = arrayOf("-std=c++17", "-g"), + clangFlags = arrayOf("-std=c++17", "-g", "-DSK_TRIVIAL_ABI=[[clang::trivial_abi]]"), msvcCompilerFlags = arrayOf("/Zi /std:c++17"), msvcLinkerFlags = arrayOf("/DEBUG"), ), diff --git a/skiko/src/commonMain/kotlin/org/jetbrains/skia/Font.kt b/skiko/src/commonMain/kotlin/org/jetbrains/skia/Font.kt index ab2bcc068..cbb75e388 100644 --- a/skiko/src/commonMain/kotlin/org/jetbrains/skia/Font.kt +++ b/skiko/src/commonMain/kotlin/org/jetbrains/skia/Font.kt @@ -585,7 +585,7 @@ class Font : Managed { val spacing: Float get() = try { Stats.onNativeCall() - _nGetSpacing(_ptr, NullPointer) + _nGetSpacing(_ptr) } finally { reachabilityBarrier(this) } @@ -729,4 +729,4 @@ private external fun _nGetPaths(ptr: NativePointer, glyphs: InteropPointer, coun private external fun _nGetMetrics(ptr: NativePointer, metrics: InteropPointer) @ExternalSymbolName("org_jetbrains_skia_Font__1nGetSpacing") -private external fun _nGetSpacing(ptr: NativePointer, glyphsArr: NativePointer): Float +private external fun _nGetSpacing(ptr: NativePointer): Float diff --git a/skiko/src/commonMain/kotlin/org/jetbrains/skia/paragraph/Paragraph.kt b/skiko/src/commonMain/kotlin/org/jetbrains/skia/paragraph/Paragraph.kt index 219e69eed..e1e6784ce 100644 --- a/skiko/src/commonMain/kotlin/org/jetbrains/skia/paragraph/Paragraph.kt +++ b/skiko/src/commonMain/kotlin/org/jetbrains/skia/paragraph/Paragraph.kt @@ -200,6 +200,12 @@ class Paragraph internal constructor(ptr: NativePointer, text: ManagedString?) : // // TODO: update _text // return this; // } + + /** + * `from` and `to` are ignored by skia + * and change applies to the whole paragraph text + * see: https://github.com/JetBrains/skia/blob/51072f3e6d263eeffed4c3038655ab1bf9cf8439/modules/skparagraph/src/ParagraphImpl.cpp#L1069 + */ fun updateFontSize(from: Int, to: Int, size: Float): Paragraph { return try { if (_text != null) { @@ -219,6 +225,10 @@ class Paragraph internal constructor(ptr: NativePointer, text: ManagedString?) : } } + /** + * `from` and `to` are ignored by skia + * and change applies to the whole paragraph text + */ fun updateForegroundPaint(from: Int, to: Int, paint: Paint?): Paragraph { return try { if (_text != null) { @@ -239,6 +249,10 @@ class Paragraph internal constructor(ptr: NativePointer, text: ManagedString?) : } } + /** + * `from` and `to` are ignored by skia + * and change applies to the whole paragraph text + */ fun updateBackgroundPaint(from: Int, to: Int, paint: Paint?): Paragraph { return try { if (_text != null) { diff --git a/skiko/src/commonTest/kotlin/org/jetbrains/skia/BreakIteratorTests.kt b/skiko/src/commonTest/kotlin/org/jetbrains/skia/BreakIteratorTests.kt index bcbbe1f54..c316a2537 100644 --- a/skiko/src/commonTest/kotlin/org/jetbrains/skia/BreakIteratorTests.kt +++ b/skiko/src/commonTest/kotlin/org/jetbrains/skia/BreakIteratorTests.kt @@ -1,15 +1,10 @@ package org.jetbrains.skia +import org.jetbrains.skia.impl.reachabilityBarrier import org.jetbrains.skiko.OS import org.jetbrains.skiko.hostOs -import org.jetbrains.skiko.tests.SkipJsTarget -import org.jetbrains.skiko.tests.SkipJvmTarget -import org.jetbrains.skiko.tests.SkipNativeTarget -import kotlin.test.Test -import kotlin.test.assertContentEquals -import kotlin.test.assertEquals -import kotlin.test.assertFailsWith -import kotlin.test.assertTrue +import org.jetbrains.skiko.tests.* +import kotlin.test.* private fun BreakIterator.asSequence() = generateSequence { next().let { n -> if (n == -1) null else n } } @@ -27,13 +22,38 @@ class BreakIteratorTests { val boundary = BreakIterator.makeWordInstance() boundary.setText("家捷克的软件开发公司 ,software development company") - assertContentEquals(listOf(1, 3, 4, 6, 8, 10, 11, 12, 20, 21, 32, 33, 40), boundary.asSequence().toList()) + val boundariesList = listOf(1, 3, 4, 6, 8, 10, 11, 12, 20, 21, 32, 33, 40) + assertContentEquals(boundariesList, boundary.asSequence().toList()) + assertEquals(-1, boundary.next()) + + //todo check what happens if we will continue with next() after -1 assertEquals(0, boundary.first()) assertEquals(40, boundary.last()) + } + + @Test + @SkipJsTarget + fun breakIteratorCloneTest() { + // Wasm and iOS builds of Skia do not include required data to implement those iterators, + // see `third_party/externals/icu/flutter/README.md`. + if (hostOs == OS.Ios) + return + + if (isDebugModeOnJvm) + throw Error("This test is usually crashes in DEBUG mode") + + val boundary = BreakIterator.makeWordInstance() val boundaryCloned = boundary.clone() - assertContentEquals(boundary.asSequence().toList(), boundaryCloned.asSequence().toList()) + + boundaryCloned.setText("家捷克的软件开发公司 ,software development company") + val boundariesList = listOf(1, 3, 4, 6, 8, 10, 11, 12, 20, 21, 32, 33, 40) + assertContentEquals(boundariesList, boundaryCloned.asSequence().toList()) + + assertEquals(0, boundaryCloned.first()) + assertEquals(40, boundaryCloned.last()) + reachabilityBarrier(boundary) } @Test diff --git a/skiko/src/commonTest/kotlin/org/jetbrains/skia/ParagraphTest.kt b/skiko/src/commonTest/kotlin/org/jetbrains/skia/ParagraphTest.kt index b7cc11911..50e284d94 100644 --- a/skiko/src/commonTest/kotlin/org/jetbrains/skia/ParagraphTest.kt +++ b/skiko/src/commonTest/kotlin/org/jetbrains/skia/ParagraphTest.kt @@ -104,9 +104,9 @@ class ParagraphTest { ), paragraph.getRectsForRange(8, 21, RectHeightMode.TIGHT, RectWidthMode.TIGHT),0.01f) paragraph = paragraph - .updateFontSize(8, 21, 48.0f) - .updateForegroundPaint(8, 21, Paint().apply { color = Color.RED }) - .updateBackgroundPaint(8, 21, Paint().apply { color = Color.BLACK }) + .updateFontSize(0, text.length, 48.0f) + .updateForegroundPaint(0, text.length, Paint().apply { color = Color.RED }) + .updateBackgroundPaint(0, text.length, Paint().apply { color = Color.BLACK }) .updateAlignment(Alignment.RIGHT) .markDirty() diff --git a/skiko/src/commonTest/kotlin/org/jetbrains/skia/TextLineTest.kt b/skiko/src/commonTest/kotlin/org/jetbrains/skia/TextLineTest.kt index 672597864..2e36cce78 100644 --- a/skiko/src/commonTest/kotlin/org/jetbrains/skia/TextLineTest.kt +++ b/skiko/src/commonTest/kotlin/org/jetbrains/skia/TextLineTest.kt @@ -1,15 +1,19 @@ package org.jetbrains.skiko -import org.jetbrains.skia.* +import org.jetbrains.skia.Font +import org.jetbrains.skia.ManagedString +import org.jetbrains.skia.TextLine +import org.jetbrains.skia.Typeface import org.jetbrains.skia.impl.use import org.jetbrains.skia.shaper.ShapingOptions +import org.jetbrains.skia.tests.assertCloseEnough +import org.jetbrains.skia.tests.makeFromResource +import org.jetbrains.skiko.tests.isDebugModeOnJvm +import org.jetbrains.skiko.tests.runTest import kotlin.test.Test import kotlin.test.assertContentEquals import kotlin.test.assertEquals import kotlin.test.assertNotEquals -import org.jetbrains.skia.tests.makeFromResource -import org.jetbrains.skia.tests.assertCloseEnough -import org.jetbrains.skiko.tests.runTest class TextLineTest { private val inter36: suspend () -> Font = suspend { @@ -228,6 +232,9 @@ class TextLineTest { @Test fun dontCrashOnStringsWithoutGlyphs() = runTest { + if (isDebugModeOnJvm) + throw Error("This test is usually crashes in DEBUG mode") + val failedOn = "\uFE0F" // Variation Selector-16 val textLine = TextLine.make(failedOn, jbMono36(), ShapingOptions.DEFAULT) assertCloseEnough(expected = 0f, actual = textLine.width) diff --git a/skiko/src/commonTest/kotlin/org/jetbrains/skiko/tests/TestUtils.kt b/skiko/src/commonTest/kotlin/org/jetbrains/skiko/tests/TestUtils.kt index b169e44fa..9fd4f33df 100644 --- a/skiko/src/commonTest/kotlin/org/jetbrains/skiko/tests/TestUtils.kt +++ b/skiko/src/commonTest/kotlin/org/jetbrains/skiko/tests/TestUtils.kt @@ -15,3 +15,5 @@ expect annotation class SkipJsTarget expect annotation class SkipJvmTarget expect fun makeFromFileName(path: String?): Data + +expect val isDebugModeOnJvm: Boolean diff --git a/skiko/src/jsTest/kotlin/TestUtils.js.kt b/skiko/src/jsTest/kotlin/TestUtils.js.kt index 2b496d6d6..50f99b98a 100644 --- a/skiko/src/jsTest/kotlin/TestUtils.js.kt +++ b/skiko/src/jsTest/kotlin/TestUtils.js.kt @@ -34,3 +34,5 @@ actual annotation class SkipJvmTarget actual annotation class SkipNativeTarget actual fun makeFromFileName(path: String?): Data = Data(0) + +actual val isDebugModeOnJvm: Boolean = false diff --git a/skiko/src/jvmMain/cpp/common/Font.cc b/skiko/src/jvmMain/cpp/common/Font.cc index 47e61965d..856fe2e9c 100644 --- a/skiko/src/jvmMain/cpp/common/Font.cc +++ b/skiko/src/jvmMain/cpp/common/Font.cc @@ -382,7 +382,7 @@ extern "C" JNIEXPORT void JNICALL Java_org_jetbrains_skia_FontKt__1nGetMetrics } extern "C" JNIEXPORT jfloat JNICALL Java_org_jetbrains_skia_FontKt__1nGetSpacing - (JNIEnv* env, jclass jclass, jlong ptr, jshortArray glyphsArr) { + (JNIEnv* env, jclass jclass, jlong ptr) { SkFont* instance = reinterpret_cast(static_cast(ptr)); return instance->getSpacing(); } diff --git a/skiko/src/jvmMain/cpp/common/Image.cc b/skiko/src/jvmMain/cpp/common/Image.cc index fdbda1f24..83fa9385d 100644 --- a/skiko/src/jvmMain/cpp/common/Image.cc +++ b/skiko/src/jvmMain/cpp/common/Image.cc @@ -80,7 +80,7 @@ extern "C" JNIEXPORT jlong JNICALL Java_org_jetbrains_skia_ImageKt__1nEncodeToDa switch (skFormat) { case SkEncodedImageFormat::kPNG: { SkPngEncoder::Options options = SkPngEncoder::Options(); - options.fZLibLevel = std::max((int)(quality / 10), 9); + options.fZLibLevel = std::max(0, std::min((int)(quality / 10), 9)); SkData* data = SkPngEncoder::Encode(nullptr, instance, options).release(); return reinterpret_cast(data); } diff --git a/skiko/src/jvmMain/cpp/common/RuntimeShaderBuilder.cc b/skiko/src/jvmMain/cpp/common/RuntimeShaderBuilder.cc index ce098ce75..0bd49fe33 100644 --- a/skiko/src/jvmMain/cpp/common/RuntimeShaderBuilder.cc +++ b/skiko/src/jvmMain/cpp/common/RuntimeShaderBuilder.cc @@ -97,7 +97,7 @@ Java_org_jetbrains_skia_RuntimeShaderBuilderKt__1nUniformFloatMatrix33 (JNIEnv* env, jclass jclass, jlong builderPtr, jstring uniformName, jfloatArray uniformMatrix33) { SkRuntimeShaderBuilder* runtimeShaderBuilder = jlongToPtr(builderPtr); std::unique_ptr matrix33 = skMatrix(env, uniformMatrix33); - runtimeShaderBuilder->uniform(skString(env, uniformName).c_str()) = matrix33.get(); + runtimeShaderBuilder->uniform(skString(env, uniformName).c_str()) = *matrix33; } extern "C" JNIEXPORT void JNICALL @@ -105,7 +105,7 @@ Java_org_jetbrains_skia_RuntimeShaderBuilderKt__1nUniformFloatMatrix44 (JNIEnv* env, jclass jclass, jlong builderPtr, jstring uniformName, jfloatArray uniformMatrix44) { SkRuntimeShaderBuilder* runtimeShaderBuilder = jlongToPtr(builderPtr); std::unique_ptr matrix44 = skM44(env, uniformMatrix44); - runtimeShaderBuilder->uniform(skString(env, uniformName).c_str()) = matrix44.get(); + runtimeShaderBuilder->uniform(skString(env, uniformName).c_str()) = *matrix44; } extern "C" JNIEXPORT void JNICALL diff --git a/skiko/src/jvmMain/cpp/common/interop.cc b/skiko/src/jvmMain/cpp/common/interop.cc index 6ff1a8acd..5b7317731 100644 --- a/skiko/src/jvmMain/cpp/common/interop.cc +++ b/skiko/src/jvmMain/cpp/common/interop.cc @@ -521,19 +521,6 @@ namespace skija { return java::lang::Throwable::exceptionThrown(env) ? nullptr : res; } - std::unique_ptr toSkIRect(JNIEnv* env, jobject obj) { - if (obj == nullptr) - return std::unique_ptr(nullptr); - else { - return std::unique_ptr(new SkIRect{ - env->GetIntField(obj, left), - env->GetIntField(obj, top), - env->GetIntField(obj, right), - env->GetIntField(obj, bottom) - }); - } - } - std::unique_ptr toSkIRect(JNIEnv* env, jintArray rectInts) { if (rectInts == nullptr) return std::unique_ptr(nullptr); diff --git a/skiko/src/jvmMain/cpp/common/interop.hh b/skiko/src/jvmMain/cpp/common/interop.hh index e5f25f7bf..96e030236 100644 --- a/skiko/src/jvmMain/cpp/common/interop.hh +++ b/skiko/src/jvmMain/cpp/common/interop.hh @@ -226,7 +226,7 @@ namespace skija { void onLoad(JNIEnv* env); void onUnload(JNIEnv* env); jobject fromSkIRect(JNIEnv* env, const SkIRect& rect); - std::unique_ptr toSkIRect(JNIEnv* env, jobject obj); + std::unique_ptr toSkIRect(JNIEnv* env, jintArray obj); } namespace Path { diff --git a/skiko/src/jvmTest/kotlin/TestUtils.jvm.kt b/skiko/src/jvmTest/kotlin/TestUtils.jvm.kt index 912566370..5d2f2b94b 100644 --- a/skiko/src/jvmTest/kotlin/TestUtils.jvm.kt +++ b/skiko/src/jvmTest/kotlin/TestUtils.jvm.kt @@ -22,4 +22,9 @@ actual typealias SkipJvmTarget = org.junit.Ignore actual annotation class SkipNativeTarget -actual fun makeFromFileName(path: String?): Data = Data.Companion.makeFromFileName(path) \ No newline at end of file +actual fun makeFromFileName(path: String?): Data = Data.Companion.makeFromFileName(path) + +actual val isDebugModeOnJvm: Boolean + get() { + return System.getProperty("skiko.test.debug") == "true" + } \ No newline at end of file diff --git a/skiko/src/nativeJsMain/cpp/Font.cc b/skiko/src/nativeJsMain/cpp/Font.cc index 8eb1f4a91..0a06545b9 100644 --- a/skiko/src/nativeJsMain/cpp/Font.cc +++ b/skiko/src/nativeJsMain/cpp/Font.cc @@ -360,7 +360,7 @@ SKIKO_EXPORT void org_jetbrains_skia_Font__1nGetMetrics SKIKO_EXPORT KFloat org_jetbrains_skia_Font__1nGetSpacing - (KNativePointer ptr, KShort* glyphsArr) { + (KNativePointer ptr) { SkFont* instance = reinterpret_cast(ptr); return instance->getSpacing(); } diff --git a/skiko/src/nativeJsMain/cpp/Image.cc b/skiko/src/nativeJsMain/cpp/Image.cc index 7ec5f06c2..10a3460ca 100644 --- a/skiko/src/nativeJsMain/cpp/Image.cc +++ b/skiko/src/nativeJsMain/cpp/Image.cc @@ -74,7 +74,7 @@ SKIKO_EXPORT KNativePointer org_jetbrains_skia_Image__1nEncodeToData switch (skFormat) { case SkEncodedImageFormat::kPNG: { SkPngEncoder::Options options = SkPngEncoder::Options(); - options.fZLibLevel = std::max(quality / 10, 9); + options.fZLibLevel = std::max(0, std::min(quality / 10, 9)); SkData* data = SkPngEncoder::Encode(nullptr, instance, options).release(); return reinterpret_cast(data); } diff --git a/skiko/src/nativeJsMain/cpp/RuntimeShaderBuilder.cc b/skiko/src/nativeJsMain/cpp/RuntimeShaderBuilder.cc index 7ecf2f6a6..bb7baab96 100644 --- a/skiko/src/nativeJsMain/cpp/RuntimeShaderBuilder.cc +++ b/skiko/src/nativeJsMain/cpp/RuntimeShaderBuilder.cc @@ -1,5 +1,6 @@ #include #include "SkRuntimeEffect.h" +#include "SkMatrix.h" #include "common.h" static void deleteRuntimeShaderBuilder(SkRuntimeShaderBuilder* builder) { @@ -81,13 +82,15 @@ SKIKO_EXPORT void org_jetbrains_skia_RuntimeShaderBuilder__1nUniformFloatMatrix2 SKIKO_EXPORT void org_jetbrains_skia_RuntimeShaderBuilder__1nUniformFloatMatrix33 (KNativePointer builderPtr, KInteropPointer uniformName, KFloat* uniformMatrix33) { SkRuntimeShaderBuilder* runtimeShaderBuilder = reinterpret_cast(builderPtr); - runtimeShaderBuilder->uniform(skString(uniformName).c_str()) = uniformMatrix33; + std::unique_ptr matrix33 = skMatrix(uniformMatrix33); + runtimeShaderBuilder->uniform(skString(uniformName).c_str()) = *matrix33; } SKIKO_EXPORT void org_jetbrains_skia_RuntimeShaderBuilder__1nUniformFloatMatrix44 (KNativePointer builderPtr, KInteropPointer uniformName, KFloat* uniformMatrix44) { SkRuntimeShaderBuilder* runtimeShaderBuilder = reinterpret_cast(builderPtr); - runtimeShaderBuilder->uniform(skString(uniformName).c_str()) = uniformMatrix44; + std::unique_ptr matrix44 = skM44(uniformMatrix44); + runtimeShaderBuilder->uniform(skString(uniformName).c_str()) = *matrix44; } SKIKO_EXPORT void org_jetbrains_skia_RuntimeShaderBuilder__1nChildShader diff --git a/skiko/src/nativeTest/kotlin/org/jetbrains/skiko/tests/TestUtils.native.kt b/skiko/src/nativeTest/kotlin/org/jetbrains/skiko/tests/TestUtils.native.kt index f750db4b2..1763a74f4 100644 --- a/skiko/src/nativeTest/kotlin/org/jetbrains/skiko/tests/TestUtils.native.kt +++ b/skiko/src/nativeTest/kotlin/org/jetbrains/skiko/tests/TestUtils.native.kt @@ -20,4 +20,6 @@ actual annotation class SkipJvmTarget actual typealias SkipNativeTarget = kotlin.test.Ignore -actual fun makeFromFileName(path: String?): Data = Data.Companion.makeFromFileName(path) \ No newline at end of file +actual fun makeFromFileName(path: String?): Data = Data.Companion.makeFromFileName(path) + +actual val isDebugModeOnJvm: Boolean = false \ No newline at end of file