Skip to content

Commit bfb9a9a

Browse files
Fabrice Di MeglioAndroid (Google) Code Review
authored andcommitted
Merge "Revert "Fix bug #5332081 TextLayoutCache needs to be able to have more cache hits""
2 parents 4a5c14d + 9c418db commit bfb9a9a

File tree

7 files changed

+240
-306
lines changed

7 files changed

+240
-306
lines changed

core/jni/android/graphics/Canvas.cpp

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -758,7 +758,21 @@ class SkCanvasGlue {
758758
jfloat x, jfloat y, int flags, SkPaint* paint) {
759759

760760
jint count = end - start;
761-
drawTextWithGlyphs(canvas, textArray + start, 0, count, count, x, y, flags, paint);
761+
sp<TextLayoutCacheValue> value;
762+
#if USE_TEXT_LAYOUT_CACHE
763+
value = TextLayoutCache::getInstance().getValue(paint, textArray, start, count,
764+
end, flags);
765+
if (value == NULL) {
766+
LOGE("Cannot get TextLayoutCache value");
767+
return ;
768+
}
769+
#else
770+
value = new TextLayoutCacheValue();
771+
value->computeValues(paint, textArray, start, count, end, flags);
772+
#endif
773+
774+
doDrawGlyphs(canvas, value->getGlyphs(), 0, value->getGlyphsCount(),
775+
x, y, flags, paint);
762776
}
763777

764778
static void drawTextWithGlyphs(SkCanvas* canvas, const jchar* textArray,
@@ -767,23 +781,19 @@ class SkCanvasGlue {
767781

768782
sp<TextLayoutCacheValue> value;
769783
#if USE_TEXT_LAYOUT_CACHE
770-
value = TextLayoutCache::getInstance().getValue(paint, textArray, contextCount, flags);
784+
value = TextLayoutCache::getInstance().getValue(paint, textArray, start, count,
785+
contextCount, flags);
771786
if (value == NULL) {
772787
LOGE("Cannot get TextLayoutCache value");
773788
return ;
774789
}
775790
#else
776791
value = new TextLayoutCacheValue();
777-
value->computeValues(paint, textArray, contextCount, flags);
792+
value->computeValues(paint, textArray, start, count, contextCount, flags);
778793
#endif
779794

780-
size_t startIndex = 0;
781-
size_t glyphsCount = 0;
782-
value->getGlyphsIndexAndCount(start, count, &startIndex, &glyphsCount);
783-
jchar* glyphs = new jchar[glyphsCount];
784-
value->getGlyphs(startIndex, glyphsCount, glyphs);
785-
doDrawGlyphs(canvas, glyphs, 0, glyphsCount, x, y, flags, paint);
786-
delete[] glyphs;
795+
doDrawGlyphs(canvas, value->getGlyphs(), 0, value->getGlyphsCount(),
796+
x, y, flags, paint);
787797
}
788798

789799
static void doDrawGlyphs(SkCanvas* canvas, const jchar* glyphArray, int index, int count,

core/jni/android/graphics/Paint.cpp

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ class SkPaintGlue {
352352
jfloat result = 0;
353353
#if RTL_USE_HARFBUZZ
354354
TextLayout::getTextRunAdvances(paint, textArray, index, count, textLength,
355-
paint->getFlags(), NULL /* dont need all advances */, &result);
355+
paint->getFlags(), NULL /* dont need all advances */, result);
356356
#else
357357
// we double count, since measureText wants a byteLength
358358
SkScalar width = paint->measureText(textArray + index, count << 1);
@@ -382,7 +382,7 @@ class SkPaintGlue {
382382

383383
#if RTL_USE_HARFBUZZ
384384
TextLayout::getTextRunAdvances(paint, textArray, start, count, textLength,
385-
paint->getFlags(), NULL /* dont need all advances */, &width);
385+
paint->getFlags(), NULL /* dont need all advances */, width);
386386
#else
387387

388388
width = SkScalarToFloat(paint->measureText(textArray + start, count << 1));
@@ -406,7 +406,7 @@ class SkPaintGlue {
406406

407407
#if RTL_USE_HARFBUZZ
408408
TextLayout::getTextRunAdvances(paint, textArray, 0, textLength, textLength,
409-
paint->getFlags(), NULL /* dont need all advances */, &width);
409+
paint->getFlags(), NULL /* dont need all advances */, width);
410410
#else
411411
width = SkScalarToFloat(paint->measureText(textArray, textLength << 1));
412412
#endif
@@ -435,8 +435,10 @@ class SkPaintGlue {
435435
jfloat* widthsArray = autoWidths.ptr();
436436

437437
#if RTL_USE_HARFBUZZ
438+
jfloat totalAdvance;
439+
438440
TextLayout::getTextRunAdvances(paint, text, 0, count, count,
439-
paint->getFlags(), widthsArray, NULL);
441+
paint->getFlags(), widthsArray, totalAdvance);
440442
#else
441443
SkScalar* scalarArray = (SkScalar*)widthsArray;
442444

@@ -487,7 +489,7 @@ class SkPaintGlue {
487489
HB_FontRec font;
488490
FontData fontData;
489491
TextLayoutCacheValue::shapeWithHarfbuzz(&shaperItem, &font, &fontData, paint, text,
490-
contextCount, flags);
492+
start, count, contextCount, flags);
491493

492494
int glyphCount = shaperItem.num_glyphs;
493495
for (int i = 0; i < glyphCount; i++) {
@@ -531,7 +533,7 @@ class SkPaintGlue {
531533
jfloat totalAdvance = 0;
532534

533535
TextLayout::getTextRunAdvances(paint, text, start, count, contextCount, flags,
534-
advancesArray, &totalAdvance);
536+
advancesArray, totalAdvance);
535537

536538
if (advances != NULL) {
537539
env->SetFloatArrayRegion(advances, advancesIndex, count, advancesArray);
@@ -602,8 +604,10 @@ class SkPaintGlue {
602604
jint count, jint flags, jint offset, jint opt) {
603605
#if RTL_USE_HARFBUZZ
604606
jfloat scalarArray[count];
607+
jfloat totalAdvance = 0;
608+
605609
TextLayout::getTextRunAdvances(paint, text, start, count, count, flags,
606-
scalarArray, NULL);
610+
scalarArray, totalAdvance);
607611
#else
608612
SkScalar scalarArray[count];
609613
jchar buffer[count];

core/jni/android/graphics/TextLayout.cpp

Lines changed: 10 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -253,110 +253,40 @@ void TextLayout::drawTextRun(SkPaint* paint, const jchar* chars,
253253

254254
void TextLayout::getTextRunAdvances(SkPaint* paint, const jchar* chars, jint start,
255255
jint count, jint contextCount, jint dirFlags,
256-
jfloat* resultAdvances, jfloat* resultTotalAdvance) {
256+
jfloat* resultAdvances, jfloat& resultTotalAdvance) {
257257
sp<TextLayoutCacheValue> value;
258258
#if USE_TEXT_LAYOUT_CACHE
259259
// Return advances from the cache. Compute them if needed
260-
value = TextLayoutCache::getInstance().getValue(paint, chars, contextCount, dirFlags);
260+
value = TextLayoutCache::getInstance().getValue(
261+
paint, chars, start, count, contextCount, dirFlags);
261262
#else
262263
value = new TextLayoutCacheValue();
263-
value->computeValues(paint, chars, contextCount, dirFlags);
264+
value->computeValues(paint, chars, start, count, contextCount, dirFlags);
264265
#endif
265266
if (value != NULL) {
266-
if (resultAdvances) {
267-
value->getAdvances(start, count, resultAdvances);
268-
}
269-
if (resultTotalAdvance) {
270-
*resultTotalAdvance = value->getTotalAdvance(start, count);
267+
if (resultAdvances != NULL) {
268+
memcpy(resultAdvances, value->getAdvances(), value->getAdvancesCount() * sizeof(jfloat));
271269
}
270+
resultTotalAdvance = value->getTotalAdvance();
272271
}
273272
}
274273

275274
void TextLayout::getTextRunAdvancesHB(SkPaint* paint, const jchar* chars, jint start,
276275
jint count, jint contextCount, jint dirFlags,
277276
jfloat* resultAdvances, jfloat& resultTotalAdvance) {
278277
// Compute advances and return them
279-
TextLayoutCacheValue::computeValuesWithHarfbuzz(paint, chars, contextCount,
280-
dirFlags, resultAdvances, &resultTotalAdvance, NULL, NULL, NULL);
278+
TextLayoutCacheValue::computeValuesWithHarfbuzz(paint, chars, start, count, contextCount,
279+
dirFlags, resultAdvances, &resultTotalAdvance, NULL, NULL);
281280
}
282281

283282
void TextLayout::getTextRunAdvancesICU(SkPaint* paint, const jchar* chars, jint start,
284283
jint count, jint contextCount, jint dirFlags,
285284
jfloat* resultAdvances, jfloat& resultTotalAdvance) {
286285
// Compute advances and return them
287-
computeAdvancesWithICU(paint, chars, start, count, contextCount, dirFlags,
286+
TextLayoutCacheValue::computeAdvancesWithICU(paint, chars, start, count, contextCount, dirFlags,
288287
resultAdvances, &resultTotalAdvance);
289288
}
290289

291-
void TextLayout::computeAdvancesWithICU(SkPaint* paint, const UChar* chars,
292-
size_t start, size_t count, size_t contextCount, int dirFlags,
293-
jfloat* outAdvances, jfloat* outTotalAdvance) {
294-
SkAutoSTMalloc<CHAR_BUFFER_SIZE, jchar> tempBuffer(contextCount);
295-
jchar* buffer = tempBuffer.get();
296-
SkScalar* scalarArray = (SkScalar*)outAdvances;
297-
298-
// this is where we'd call harfbuzz
299-
// for now we just use ushape.c
300-
size_t widths;
301-
const jchar* text;
302-
if (dirFlags & 0x1) { // rtl, call arabic shaping in case
303-
UErrorCode status = U_ZERO_ERROR;
304-
// Use fixed length since we need to keep start and count valid
305-
u_shapeArabic(chars, contextCount, buffer, contextCount,
306-
U_SHAPE_LENGTH_FIXED_SPACES_NEAR |
307-
U_SHAPE_TEXT_DIRECTION_LOGICAL | U_SHAPE_LETTERS_SHAPE |
308-
U_SHAPE_X_LAMALEF_SUB_ALTERNATE, &status);
309-
// we shouldn't fail unless there's an out of memory condition,
310-
// in which case we're hosed anyway
311-
for (int i = start, e = i + count; i < e; ++i) {
312-
if (buffer[i] == UNICODE_NOT_A_CHAR) {
313-
buffer[i] = UNICODE_ZWSP; // zero-width-space for skia
314-
}
315-
}
316-
text = buffer + start;
317-
widths = paint->getTextWidths(text, count << 1, scalarArray);
318-
} else {
319-
text = chars + start;
320-
widths = paint->getTextWidths(text, count << 1, scalarArray);
321-
}
322-
323-
jfloat totalAdvance = 0;
324-
if (widths < count) {
325-
#if DEBUG_ADVANCES
326-
LOGD("ICU -- count=%d", widths);
327-
#endif
328-
// Skia operates on code points, not code units, so surrogate pairs return only
329-
// one value. Expand the result so we have one value per UTF-16 code unit.
330-
331-
// Note, skia's getTextWidth gets confused if it encounters a surrogate pair,
332-
// leaving the remaining widths zero. Not nice.
333-
for (size_t i = 0, p = 0; i < widths; ++i) {
334-
totalAdvance += outAdvances[p++] = SkScalarToFloat(scalarArray[i]);
335-
if (p < count &&
336-
text[p] >= UNICODE_FIRST_LOW_SURROGATE &&
337-
text[p] < UNICODE_FIRST_PRIVATE_USE &&
338-
text[p-1] >= UNICODE_FIRST_HIGH_SURROGATE &&
339-
text[p-1] < UNICODE_FIRST_LOW_SURROGATE) {
340-
outAdvances[p++] = 0;
341-
}
342-
#if DEBUG_ADVANCES
343-
LOGD("icu-adv = %f - total = %f", outAdvances[i], totalAdvance);
344-
#endif
345-
}
346-
} else {
347-
#if DEBUG_ADVANCES
348-
LOGD("ICU -- count=%d", count);
349-
#endif
350-
for (size_t i = 0; i < count; i++) {
351-
totalAdvance += outAdvances[i] = SkScalarToFloat(scalarArray[i]);
352-
#if DEBUG_ADVANCES
353-
LOGD("icu-adv = %f - total = %f", outAdvances[i], totalAdvance);
354-
#endif
355-
}
356-
}
357-
*outTotalAdvance = totalAdvance;
358-
}
359-
360290
// Draws a paragraph of text on a single line, running bidi and shaping
361291
void TextLayout::drawText(SkPaint* paint, const jchar* text, jsize len,
362292
int bidiFlags, jfloat x, jfloat y, SkCanvas* canvas) {

core/jni/android/graphics/TextLayout.h

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ class TextLayout {
7171

7272
static void getTextRunAdvances(SkPaint* paint, const jchar* chars, jint start,
7373
jint count, jint contextCount, jint dirFlags,
74-
jfloat* resultAdvances, jfloat* resultTotalAdvance);
74+
jfloat* resultAdvances, jfloat& resultTotalAdvance);
7575

7676
static void getTextRunAdvancesICU(SkPaint* paint, const jchar* chars, jint start,
7777
jint count, jint contextCount, jint dirFlags,
@@ -106,9 +106,5 @@ class TextLayout {
106106
UErrorCode &status);
107107
static void handleText(SkPaint* paint, const jchar* text, jsize len,
108108
int bidiFlags, jfloat x, jfloat y, SkCanvas* canvas, SkPath* path);
109-
110-
static void computeAdvancesWithICU(SkPaint* paint, const UChar* chars,
111-
size_t start, size_t count, size_t contextCount, int dirFlags,
112-
jfloat* outAdvances, jfloat* outTotalAdvance);
113109
};
114110
} // namespace android

0 commit comments

Comments
 (0)