Skip to content

Commit

Permalink
Fix LineMetrics resolution for wasm and js targets (#846)
Browse files Browse the repository at this point in the history 
* Fix LineMetrics resolution for wasm and js targets

This contributes to fixing https://youtrack.jetbrains.com/issue/COMPOSE-108/CfW-fix-TextField-cursor

I had to introduce assertCloseEnough sensitive enought to path
throughout all our targets

* LineMetrics::layoutParagraph idiomatically returns runTest immediately

* Rename LineMetrics_nGetArrayElement from longArgs (misleading) to intArgs
  • Loading branch information
@Schahen
Schahen authored Dec 15, 2023
1 parent 5414141 commit dcbac74
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 30 deletions.
10 changes: 7 additions & 3 deletions skiko/src/commonMain/kotlin/org/jetbrains/skia/paragraph/LineMetrics.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,13 @@ class LineMetrics(
override fun getArrayElement(array: InteropPointer, index: Int): LineMetrics {
val intArray = IntArray(6)
val doubleArray = DoubleArray(7)
interopScope {
LineMetrics_nGetArrayElement(array, index, toInterop(intArray), toInterop(doubleArray))

withResult(intArray) { intArrayInterop ->
withResult(doubleArray) { doubleArrayInterop ->
LineMetrics_nGetArrayElement(array, index, intArrayInterop, doubleArrayInterop)
}
}

return LineMetrics(
intArray[0],
intArray[1],
Expand Down Expand Up @@ -175,4 +179,4 @@ private external fun LineMetrics_nGetArraySize(array: InteropPointer): Int
private external fun LineMetrics_nDisposeArray(array: InteropPointer)
@ExternalSymbolName("org_jetbrains_skia_paragraph_LineMetrics__1nGetArrayElement")
@ModuleImport("./skiko.mjs", "org_jetbrains_skia_paragraph_LineMetrics__1nGetArrayElement")
private external fun LineMetrics_nGetArrayElement(array: InteropPointer, index: Int, longArgs: InteropPointer, doubleArgs: InteropPointer)
private external fun LineMetrics_nGetArrayElement(array: InteropPointer, index: Int, intArgs: InteropPointer, doubleArgs: InteropPointer)
58 changes: 39 additions & 19 deletions skiko/src/commonTest/kotlin/org/jetbrains/skia/ParagraphTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@ import org.jetbrains.skia.paragraph.*
import org.jetbrains.skia.tests.assertCloseEnough
import org.jetbrains.skia.tests.assertContentCloseEnough
import org.jetbrains.skia.tests.makeFromResource
import org.jetbrains.skiko.tests.SkipJsTarget
import org.jetbrains.skiko.tests.SkipNativeTarget
import org.jetbrains.skiko.tests.SkipWasmTarget
import org.jetbrains.skiko.tests.runTest
import org.jetbrains.skiko.tests.*
import kotlin.test.Test
import kotlin.test.assertContentEquals
import kotlin.test.assertEquals
Expand Down Expand Up @@ -49,23 +46,46 @@ class ParagraphTest {
}

@Test
@SkipJsTarget
@SkipWasmTarget
@SkipNativeTarget
fun layoutParagraph() = runTest {
singleLineMetrics("aa").let { lineMetrics -> // latin
assertEquals(0, lineMetrics.startIndex)
assertEquals(2, lineMetrics.endIndex)
assertEquals(2, lineMetrics.endIncludingNewline)
assertEquals(2, lineMetrics.endExcludingWhitespaces)
}
singleLineMetrics("яя").let { lineMetrics -> // cyrillic
assertEquals(0, lineMetrics.startIndex)
assertEquals(2, lineMetrics.endIndex)
assertEquals(2, lineMetrics.endIncludingNewline)
assertEquals(2, lineMetrics.endExcludingWhitespaces)
}
val lineMetricsEpsilon = 0.0001f

assertCloseEnough(
singleLineMetrics("aa"), LineMetrics(
startIndex = 0,
endIndex = 2,
endExcludingWhitespaces = 2,
endIncludingNewline = 2,
isHardBreak = true,
ascent = 13.5625,
descent = 3.3806817531585693,
unscaledAscent = 13.5625,
height = 17.0,
width = 15.789999961853027,
left = 0.0,
baseline = 13.619318008422852,
lineNumber = 0
), epsilon = lineMetricsEpsilon
)

assertCloseEnough(
singleLineMetrics("яя"), LineMetrics(
startIndex = 0,
endIndex = 2,
endExcludingWhitespaces = 2,
endIncludingNewline = 2,
isHardBreak = true,
ascent = 13.5625,
descent = 3.3806817531585693,
unscaledAscent = 13.5625,
height = 17.0,
width = 15.710000038146973,
left = 0.0,
baseline = 13.619318008422852,
lineNumber = 0
), epsilon = lineMetricsEpsilon
)
}

@Test
@SkipJsTarget // FIXME Emscripten's stringToUTF8 function does not correctly handle invalid unicode symbols.
fun invalidUnicode() = runTest {
Expand Down
22 changes: 22 additions & 0 deletions skiko/src/commonTest/kotlin/org/jetbrains/skia/util/assertCloseEnough.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import org.jetbrains.skia.FontMetrics
import org.jetbrains.skia.Matrix33
import org.jetbrains.skia.Point
import org.jetbrains.skia.Rect
import org.jetbrains.skia.paragraph.LineMetrics
import org.jetbrains.skia.paragraph.Shadow
import org.jetbrains.skia.paragraph.TextBox
import org.jetbrains.skiko.KotlinBackend
Expand All @@ -17,6 +18,9 @@ private val EPSILON = if (kotlinBackend == KotlinBackend.JS) 0.00001f else 0.000
private inline fun Float?.isCloseEnoughTo(b: Float?, epsilon: Float) =
if (this == null) b == null else if (b == null) false else if (epsilon == 0f) this == b else abs(this - b) < epsilon

private inline fun Double?.isCloseEnoughTo(b: Double?, epsilon: Float) =
if (this == null) b == null else if (b == null) false else if (epsilon == 0f) this == b else abs(this - b) < epsilon

private inline fun Point.isCloseEnoughTo(b: Point, epsilon: Float) =
x.isCloseEnoughTo(b.x, epsilon) && y.isCloseEnoughTo(b.y, epsilon)

Expand Down Expand Up @@ -48,6 +52,20 @@ private inline fun FontMetrics.isCloseEnoughTo(b: FontMetrics, epsilon: Float) =
strikeoutThickness.isCloseEnoughTo(b.strikeoutThickness, epsilon) &&
strikeoutPosition.isCloseEnoughTo(b.strikeoutPosition, epsilon)

private inline fun LineMetrics.isCloseEnoughTo(b: LineMetrics, epsilon: Float): Boolean =
startIndex == b.startIndex &&
endIndex == b.endIndex &&
endExcludingWhitespaces == b.endExcludingWhitespaces &&
endIncludingNewline == b.endIncludingNewline &&
isHardBreak == b.isHardBreak &&
ascent.isCloseEnoughTo(b.ascent, epsilon) &&
descent.isCloseEnoughTo(b.descent, epsilon) &&
unscaledAscent.isCloseEnoughTo(b.unscaledAscent, epsilon) &&
height.isCloseEnoughTo(b.height, epsilon) &&
width.isCloseEnoughTo(b.width, epsilon) &&
baseline.isCloseEnoughTo(b.baseline, epsilon) &&
lineNumber == b.lineNumber

private inline fun Rect.isCloseEnoughTo(rect: Rect, epsilon: Float): Boolean =
left.isCloseEnoughTo(rect.left, epsilon) && right.isCloseEnoughTo(rect.right, epsilon)
&& top.isCloseEnoughTo(rect.top, epsilon) && bottom.isCloseEnoughTo(rect.bottom, epsilon)
Expand Down Expand Up @@ -87,6 +105,10 @@ internal fun assertCloseEnough(expected: Rect, actual: Rect, epsilon: Float = EP
assertTrue(expected.isCloseEnoughTo(actual, epsilon), message = "expected=$expected, actual=$actual, eps=$epsilon")
}

internal fun assertCloseEnough(expected: LineMetrics, actual: LineMetrics, epsilon: Float = EPSILON) {
assertTrue(expected.isCloseEnoughTo(actual, epsilon), message = "expected=$expected, actual=$actual, eps=$epsilon")
}

private fun fail(message: String) {
throw AssertionError(message)
}
Expand Down
16 changes: 8 additions & 8 deletions skiko/src/nativeJsMain/cpp/paragraph/LineMetrics.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,20 @@ SKIKO_EXPORT void org_jetbrains_skia_paragraph_LineMetrics__1nDisposeArray
}

SKIKO_EXPORT void org_jetbrains_skia_paragraph_LineMetrics__1nGetArrayElement
(KNativePointer blob, KInt index, KNativePointer longArgs, KNativePointer doubleArgs) {
(KNativePointer blob, KInt index, KNativePointer intArgs, KNativePointer doubleArgs) {

std::vector<LineMetrics>* vect = reinterpret_cast<std::vector<LineMetrics> *>(blob);
int* longs = reinterpret_cast<int *>(longArgs);
int* ints = reinterpret_cast<int *>(intArgs);
double* doubles = reinterpret_cast<double *>(doubleArgs);

LineMetrics lm = vect->at(index);

longs[0] = lm.fStartIndex;
longs[1] = lm.fEndIndex;
longs[2] = lm.fEndExcludingWhitespaces;
longs[3] = lm.fEndIncludingNewline;
longs[4] = lm.fHardBreak;
longs[5] = lm.fLineNumber;
ints[0] = lm.fStartIndex;
ints[1] = lm.fEndIndex;
ints[2] = lm.fEndExcludingWhitespaces;
ints[3] = lm.fEndIncludingNewline;
ints[4] = lm.fHardBreak;
ints[5] = lm.fLineNumber;

doubles[0] = lm.fAscent;
doubles[1] = lm.fDescent;
Expand Down

0 comments on commit dcbac74

Please sign in to comment.