From 5825d5c455505d522ee4be94a235ec1b6f4c8354 Mon Sep 17 00:00:00 2001 From: Pavel Date: Wed, 8 Nov 2023 18:15:57 +0100 Subject: [PATCH] don't crash an app if we got empty run during text shaping (#821) * don't crash an app if we got empty run during text shaping * on windows and linux it may produce some glyph --- .../cpp/common/include/TextLineRunHandler.hh | 46 ++++++++++--------- .../org/jetbrains/skia/shaper/Shaper.kt | 18 ++++---- .../kotlin/org/jetbrains/skia/TextLineTest.kt | 13 ++++-- 3 files changed, 42 insertions(+), 35 deletions(-) diff --git a/skiko/src/commonMain/cpp/common/include/TextLineRunHandler.hh b/skiko/src/commonMain/cpp/common/include/TextLineRunHandler.hh index 70a32ed18..adb747329 100644 --- a/skiko/src/commonMain/cpp/common/include/TextLineRunHandler.hh +++ b/skiko/src/commonMain/cpp/common/include/TextLineRunHandler.hh @@ -56,32 +56,34 @@ public: void commitRunBuffer(const RunInfo& info) override { TextLine::Run& run = fLine->fRuns.back(); - int32_t glyph = 0; - int32_t graphemesInGlyph = 1; - SkScalar glyphLeft = run.fPos[glyph].fX; - run.fBreakOffsets.reserve(info.utf8Range.size() + 1); - run.fBreakPositions.reserve(info.utf8Range.size() + 1); + if (0 < info.glyphCount) { + int32_t glyph = 0; + int32_t graphemesInGlyph = 1; + SkScalar glyphLeft = run.fPos[glyph].fX; + run.fBreakOffsets.reserve(info.utf8Range.size() + 1); + run.fBreakPositions.reserve(info.utf8Range.size() + 1); - // Only record grapheme clusters boundaries - for (int32_t offset = fGlyphOffsets[0]; offset <= info.utf8Range.end(); offset = ubrk_following(fGraphemeIter.get(), offset)) { - run.fBreakOffsets.push_back(conv.from8To16(offset)); + // Only record grapheme clusters boundaries + for (int32_t offset = fGlyphOffsets[0]; offset <= info.utf8Range.end(); offset = ubrk_following(fGraphemeIter.get(), offset)) { + run.fBreakOffsets.push_back(conv.from8To16(offset)); - // if grapheme clusters includes multiple glyphs, skip over them - while (glyph < info.glyphCount && fGlyphOffsets[glyph] < offset) - ++glyph; + // if grapheme clusters includes multiple glyphs, skip over them + while (glyph < info.glyphCount && fGlyphOffsets[glyph] < offset) + ++glyph; - // if one glyph includes multiple grapheme clusters (ligature, e.g. <->), accumulate - if ((glyph < info.glyphCount ? fGlyphOffsets[glyph] : info.utf8Range.end()) > offset) - ++graphemesInGlyph; + // if one glyph includes multiple grapheme clusters (ligature, e.g. <->), accumulate + if ((glyph < info.glyphCount ? fGlyphOffsets[glyph] : info.utf8Range.end()) > offset) + ++graphemesInGlyph; - // when boundaries meet, distribute break positions evenly inside glyph - else { - SkScalar glyphRight = glyph < info.glyphCount ? run.fPos[glyph].fX : fPosition + info.fAdvance.fX; - SkScalar step = (glyphRight - glyphLeft) / graphemesInGlyph; - for (int i = 0; i < graphemesInGlyph; ++i) - run.fBreakPositions.push_back(glyphLeft + step * (i + 1)); - graphemesInGlyph = 1; - glyphLeft = glyphRight; + // when boundaries meet, distribute break positions evenly inside glyph + else { + SkScalar glyphRight = glyph < info.glyphCount ? run.fPos[glyph].fX : fPosition + info.fAdvance.fX; + SkScalar step = (glyphRight - glyphLeft) / graphemesInGlyph; + for (int i = 0; i < graphemesInGlyph; ++i) + run.fBreakPositions.push_back(glyphLeft + step * (i + 1)); + graphemesInGlyph = 1; + glyphLeft = glyphRight; + } } } fPosition += info.fAdvance.fX; diff --git a/skiko/src/commonMain/kotlin/org/jetbrains/skia/shaper/Shaper.kt b/skiko/src/commonMain/kotlin/org/jetbrains/skia/shaper/Shaper.kt index 081882976..ce7547eae 100644 --- a/skiko/src/commonMain/kotlin/org/jetbrains/skia/shaper/Shaper.kt +++ b/skiko/src/commonMain/kotlin/org/jetbrains/skia/shaper/Shaper.kt @@ -195,16 +195,16 @@ class Shaper internal constructor(ptr: NativePointer) : Managed(ptr, _FinalizerH Stats.onNativeCall() ManagedString(text).use { managedString -> interopScope { - TextLine( - _nShapeLine( - _ptr, - managedString._ptr, - getPtr(font), - optsFeaturesLen = opts.features?.size ?: 0, - optsFeatures = arrayOfFontFeaturesToInterop(opts.features), - optsBooleanProps = opts._booleanPropsToInt() - ) + val linePtr = _nShapeLine( + _ptr, + managedString._ptr, + getPtr(font), + optsFeaturesLen = opts.features?.size ?: 0, + optsFeatures = arrayOfFontFeaturesToInterop(opts.features), + optsBooleanProps = opts._booleanPropsToInt() ) + require(linePtr != NullPointer) { "Shape line: $text returned nullptr" } + TextLine(linePtr) } } diff --git a/skiko/src/commonTest/kotlin/org/jetbrains/skia/TextLineTest.kt b/skiko/src/commonTest/kotlin/org/jetbrains/skia/TextLineTest.kt index e25abcdca..672597864 100644 --- a/skiko/src/commonTest/kotlin/org/jetbrains/skia/TextLineTest.kt +++ b/skiko/src/commonTest/kotlin/org/jetbrains/skia/TextLineTest.kt @@ -1,10 +1,8 @@ package org.jetbrains.skiko -import org.jetbrains.skia.Font -import org.jetbrains.skia.ManagedString -import org.jetbrains.skia.TextLine -import org.jetbrains.skia.Typeface +import org.jetbrains.skia.* import org.jetbrains.skia.impl.use +import org.jetbrains.skia.shaper.ShapingOptions import kotlin.test.Test import kotlin.test.assertContentEquals import kotlin.test.assertEquals @@ -227,4 +225,11 @@ class TextLineTest { assertCloseEnough(expected = 48.0f, actual = textLine.getCoordAtOffset(2), epsilon = eps) assertCloseEnough(expected = 69.0f, actual = textLine.getCoordAtOffset(3), epsilon = eps) } + + @Test + fun dontCrashOnStringsWithoutGlyphs() = runTest { + val failedOn = "\uFE0F" // Variation Selector-16 + val textLine = TextLine.make(failedOn, jbMono36(), ShapingOptions.DEFAULT) + assertCloseEnough(expected = 0f, actual = textLine.width) + } }