Skip to content

Commit a90d652

Browse files
Fabrice Di MeglioAndroid (Google) Code Review
authored andcommitted
Merge "Improve TextLayoutCache logging"
2 parents f11fd27 + 56e6e54 commit a90d652

File tree

2 files changed

+80
-62
lines changed

2 files changed

+80
-62
lines changed

core/jni/android/graphics/TextLayoutCache.cpp

Lines changed: 78 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,12 @@ void TextLayoutCache::init() {
5656

5757
mDebugLevel = readRtlDebugLevel();
5858
mDebugEnabled = mDebugLevel & kRtlDebugCaches;
59-
LOGD("Using debug level: %d - Debug Enabled: %d", mDebugLevel, mDebugEnabled);
59+
LOGD("Using debug level = %d - Debug Enabled = %d", mDebugLevel, mDebugEnabled);
6060

6161
mCacheStartTime = systemTime(SYSTEM_TIME_MONOTONIC);
6262

6363
if (mDebugEnabled) {
64-
LOGD("Initialization is done - Start time: %lld", mCacheStartTime);
64+
LOGD("Initialization is done - Start time = %lld", mCacheStartTime);
6565
}
6666

6767
mInitialized = true;
@@ -132,7 +132,7 @@ sp<TextLayoutCacheValue> TextLayoutCache::getValue(SkPaint* paint,
132132
bool removedOne = mCache.removeOldest();
133133
LOG_ALWAYS_FATAL_IF(!removedOne, "The cache is non-empty but we "
134134
"failed to remove the oldest entry. "
135-
"mSize=%u, size=%u, mMaxSize=%u, mCache.size()=%u",
135+
"mSize = %u, size = %u, mMaxSize = %u, mCache.size() = %u",
136136
mSize, size, mMaxSize, mCache.size());
137137
}
138138
}
@@ -147,15 +147,15 @@ sp<TextLayoutCacheValue> TextLayoutCache::getValue(SkPaint* paint,
147147
LOG_ALWAYS_FATAL_IF(!putOne, "Failed to put an entry into the cache. "
148148
"This indicates that the cache already has an entry with the "
149149
"same key but it should not since we checked earlier!"
150-
" - start=%d count=%d contextCount=%d - Text='%s'",
150+
" - start = %d, count = %d, contextCount = %d - Text = '%s'",
151151
start, count, contextCount, String8(text + start, count).string());
152152

153153
if (mDebugEnabled) {
154154
nsecs_t totalTime = systemTime(SYSTEM_TIME_MONOTONIC) - startTime;
155155
LOGD("CACHE MISS: Added entry %p "
156-
"with start=%d count=%d contextCount=%d, "
156+
"with start = %d, count = %d, contextCount = %d, "
157157
"entry size %d bytes, remaining space %d bytes"
158-
" - Compute time %0.6f ms - Put time %0.6f ms - Text='%s'",
158+
" - Compute time %0.6f ms - Put time %0.6f ms - Text = '%s'",
159159
value.get(), start, count, contextCount, size, mMaxSize - mSize,
160160
value->getElapsedTime() * 0.000001f,
161161
(totalTime - value->getElapsedTime()) * 0.000001f,
@@ -164,9 +164,9 @@ sp<TextLayoutCacheValue> TextLayoutCache::getValue(SkPaint* paint,
164164
} else {
165165
if (mDebugEnabled) {
166166
LOGD("CACHE MISS: Calculated but not storing entry because it is too big "
167-
"with start=%d count=%d contextCount=%d, "
167+
"with start = %d, count = %d, contextCount = %d, "
168168
"entry size %d bytes, remaining space %d bytes"
169-
" - Compute time %0.6f ms - Text='%s'",
169+
" - Compute time %0.6f ms - Text = '%s'",
170170
start, count, contextCount, size, mMaxSize - mSize,
171171
value->getElapsedTime() * 0.000001f,
172172
String8(text + start, count).string());
@@ -183,9 +183,9 @@ sp<TextLayoutCacheValue> TextLayoutCache::getValue(SkPaint* paint,
183183
if (value->getElapsedTime() > 0) {
184184
float deltaPercent = 100 * ((value->getElapsedTime() - elapsedTimeThruCacheGet)
185185
/ ((float)value->getElapsedTime()));
186-
LOGD("CACHE HIT #%d with start=%d count=%d contextCount=%d"
186+
LOGD("CACHE HIT #%d with start = %d, count = %d, contextCount = %d"
187187
"- Compute time %0.6f ms - "
188-
"Cache get time %0.6f ms - Gain in percent: %2.2f - Text='%s' ",
188+
"Cache get time %0.6f ms - Gain in percent: %2.2f - Text = '%s'",
189189
mCacheHitCount, start, count, contextCount,
190190
value->getElapsedTime() * 0.000001f,
191191
elapsedTimeThruCacheGet * 0.000001f,
@@ -364,15 +364,15 @@ TextLayoutEngine::~TextLayoutEngine() {
364364
void TextLayoutEngine::computeValues(TextLayoutCacheValue* value, SkPaint* paint, const UChar* chars,
365365
size_t start, size_t count, size_t contextCount, int dirFlags) {
366366

367-
computeValuesWithHarfbuzz(paint, chars, start, count, contextCount, dirFlags,
367+
computeValues(paint, chars, start, count, contextCount, dirFlags,
368368
&value->mAdvances, &value->mTotalAdvance, &value->mGlyphs);
369369
#if DEBUG_ADVANCES
370-
LOGD("Advances - start=%d, count=%d, contextCount=%d, totalAdvance=%f", start, count,
370+
LOGD("Advances - start = %d, count = %d, contextCount = %d, totalAdvance = %f", start, count,
371371
contextCount, mTotalAdvance);
372372
#endif
373373
}
374374

375-
void TextLayoutEngine::computeValuesWithHarfbuzz(SkPaint* paint, const UChar* chars,
375+
void TextLayoutEngine::computeValues(SkPaint* paint, const UChar* chars,
376376
size_t start, size_t count, size_t contextCount, int dirFlags,
377377
Vector<jfloat>* const outAdvances, jfloat* outTotalAdvance,
378378
Vector<jchar>* const outGlyphs) {
@@ -399,22 +399,30 @@ void TextLayoutEngine::computeValuesWithHarfbuzz(SkPaint* paint, const UChar* ch
399399
if (bidi) {
400400
UErrorCode status = U_ZERO_ERROR;
401401
#if DEBUG_GLYPHS
402-
LOGD("computeValuesWithHarfbuzz -- bidiReq=%d", bidiReq);
402+
LOGD("******** ComputeValues -- start");
403+
LOGD(" -- string = '%s'", String8(chars + start, count).string());
404+
LOGD(" -- start = %d", start);
405+
LOGD(" -- count = %d", count);
406+
LOGD(" -- contextCount = %d", contextCount);
407+
LOGD(" -- bidiReq = %d", bidiReq);
403408
#endif
404409
ubidi_setPara(bidi, chars, contextCount, bidiReq, NULL, &status);
405410
if (U_SUCCESS(status)) {
406411
int paraDir = ubidi_getParaLevel(bidi) & kDirection_Mask; // 0 if ltr, 1 if rtl
407412
ssize_t rc = ubidi_countRuns(bidi, &status);
408413
#if DEBUG_GLYPHS
409-
LOGD("computeValuesWithHarfbuzz -- dirFlags=%d run-count=%d paraDir=%d",
410-
dirFlags, rc, paraDir);
414+
LOGD(" -- dirFlags = %d", dirFlags);
415+
LOGD(" -- paraDir = %d", paraDir);
416+
LOGD(" -- run-count = %d", rc);
411417
#endif
412418
if (U_SUCCESS(status) && rc == 1) {
413419
// Normal case: one run, status is ok
414420
isRTL = (paraDir == 1);
415421
useSingleRun = true;
416422
} else if (!U_SUCCESS(status) || rc < 1) {
417-
LOGW("computeValuesWithHarfbuzz -- need to force to single run");
423+
LOGW("Need to force to single run -- string = '%s',"
424+
" status = %d, rc = %d",
425+
String8(chars + start, count).string(), status, rc);
418426
isRTL = (paraDir == 1);
419427
useSingleRun = true;
420428
} else {
@@ -427,7 +435,7 @@ void TextLayoutEngine::computeValuesWithHarfbuzz(SkPaint* paint, const UChar* ch
427435
if (startRun == -1 || lengthRun == -1) {
428436
// Something went wrong when getting the visual run, need to clear
429437
// already computed data before doing a single run pass
430-
LOGW("computeValuesWithHarfbuzz -- visual run is not valid");
438+
LOGW("Visual run is not valid");
431439
outGlyphs->clear();
432440
outAdvances->clear();
433441
*outTotalAdvance = 0;
@@ -454,23 +462,23 @@ void TextLayoutEngine::computeValuesWithHarfbuzz(SkPaint* paint, const UChar* ch
454462
isRTL = (runDir == UBIDI_RTL);
455463
jfloat runTotalAdvance = 0;
456464
#if DEBUG_GLYPHS
457-
LOGD("computeValuesWithHarfbuzz -- run-start=%d run-len=%d isRTL=%d",
458-
startRun, lengthRun, isRTL);
465+
LOGD("Processing Bidi Run = %d -- run-start = %d, run-len = %d, isRTL = %d",
466+
i, startRun, lengthRun, isRTL);
459467
#endif
460-
computeRunValuesWithHarfbuzz(paint, chars + startRun, lengthRun, isRTL,
468+
computeRunValues(paint, chars + startRun, lengthRun, isRTL,
461469
outAdvances, &runTotalAdvance, outGlyphs);
462470

463471
*outTotalAdvance += runTotalAdvance;
464472
}
465473
}
466474
} else {
467-
LOGW("computeValuesWithHarfbuzz -- cannot set Para");
475+
LOGW("Cannot set Para");
468476
useSingleRun = true;
469477
isRTL = (bidiReq = 1) || (bidiReq = UBIDI_DEFAULT_RTL);
470478
}
471479
ubidi_close(bidi);
472480
} else {
473-
LOGW("computeValuesWithHarfbuzz -- cannot ubidi_open()");
481+
LOGW("Cannot ubidi_open()");
474482
useSingleRun = true;
475483
isRTL = (bidiReq = 1) || (bidiReq = UBIDI_DEFAULT_RTL);
476484
}
@@ -479,28 +487,29 @@ void TextLayoutEngine::computeValuesWithHarfbuzz(SkPaint* paint, const UChar* ch
479487
// Default single run case
480488
if (useSingleRun){
481489
#if DEBUG_GLYPHS
482-
LOGD("computeValuesWithHarfbuzz -- Using a SINGLE Run "
483-
"-- run-start=%d run-len=%d isRTL=%d", start, count, isRTL);
490+
LOGD("Using a SINGLE ICURun "
491+
"-- run-start = %d, run-len = %d, isRTL = %d", start, count, isRTL);
484492
#endif
485-
computeRunValuesWithHarfbuzz(paint, chars + start, count, isRTL,
493+
computeRunValues(paint, chars + start, count, isRTL,
486494
outAdvances, outTotalAdvance, outGlyphs);
487495
}
488496

489497
#if DEBUG_GLYPHS
490-
LOGD("computeValuesWithHarfbuzz -- total-glyphs-count=%d", outGlyphs->size());
498+
LOGD(" -- Total returned glyphs-count = %d", outGlyphs->size());
499+
LOGD("******** ComputeValues -- end");
491500
#endif
492501
}
493502

494503
static void logGlyphs(HB_ShaperItem shaperItem) {
495-
LOGD("Got glyphs - count=%d", shaperItem.num_glyphs);
504+
LOGD(" -- glyphs count=%d", shaperItem.num_glyphs);
496505
for (size_t i = 0; i < shaperItem.num_glyphs; i++) {
497-
LOGD(" glyph[%d]=%d - offset.x=%f offset.y=%f", i, shaperItem.glyphs[i],
506+
LOGD(" -- glyph[%d] = %d, offset.x = %f, offset.y = %f", i, shaperItem.glyphs[i],
498507
HBFixedToFloat(shaperItem.offsets[i].x),
499508
HBFixedToFloat(shaperItem.offsets[i].y));
500509
}
501510
}
502511

503-
void TextLayoutEngine::computeRunValuesWithHarfbuzz(SkPaint* paint, const UChar* chars,
512+
void TextLayoutEngine::computeRunValues(SkPaint* paint, const UChar* chars,
504513
size_t count, bool isRTL,
505514
Vector<jfloat>* const outAdvances, jfloat* outTotalAdvance,
506515
Vector<jchar>* const outGlyphs) {
@@ -529,58 +538,60 @@ void TextLayoutEngine::computeRunValuesWithHarfbuzz(SkPaint* paint, const UChar*
529538
hb_utf16_script_run_next(&numCodePoints, &mShaperItem.item, chars,
530539
count, &indexFontRun)) {
531540

532-
ssize_t startFontRun = mShaperItem.item.pos;
533-
size_t countFontRun = mShaperItem.item.length;
534-
ssize_t endFontRun = startFontRun + countFontRun;
541+
ssize_t startScriptRun = mShaperItem.item.pos;
542+
size_t countScriptRun = mShaperItem.item.length;
543+
ssize_t endScriptRun = startScriptRun + countScriptRun;
535544

536545
#if DEBUG_GLYPHS
537-
LOGD("Shaping Font Run with");
538-
LOGD(" -- isRTL=%d", isRTL);
539-
LOGD(" -- HB script=%d", mShaperItem.item.script);
540-
LOGD(" -- startFontRun=%d", startFontRun);
541-
LOGD(" -- endFontRun=%d", endFontRun);
542-
LOGD(" -- countFontRun=%d", countFontRun);
543-
LOGD(" -- run='%s'", String8(chars + startFontRun, countFontRun).string());
544-
LOGD(" -- string='%s'", String8(chars, count).string());
546+
LOGD("-------- Start of Script Run --------");
547+
LOGD("Shaping Script Run with");
548+
LOGD(" -- isRTL = %d", isRTL);
549+
LOGD(" -- HB script = %d", mShaperItem.item.script);
550+
LOGD(" -- startFontRun = %d", startScriptRun);
551+
LOGD(" -- endFontRun = %d", endScriptRun);
552+
LOGD(" -- countFontRun = %d", countScriptRun);
553+
LOGD(" -- run = '%s'", String8(chars + startScriptRun, countScriptRun).string());
554+
LOGD(" -- string = '%s'", String8(chars, count).string());
545555
#endif
546556

547557
// Initialize Harfbuzz Shaper and get the base glyph count for offsetting the glyphIDs
548558
// and shape the Font run
549559
size_t glyphBaseCount = shapeFontRun(paint, isRTL);
550560

551561
#if DEBUG_GLYPHS
552-
LOGD("HARFBUZZ -- num_glypth=%d - kerning_applied=%d", mShaperItem.num_glyphs,
553-
mShaperItem.kerning_applied);
554-
LOGD(" -- isDevKernText=%d", paint->isDevKernText());
555-
LOGD(" -- glyphBaseCount=%d", glyphBaseCount);
562+
LOGD("Got from Harfbuzz");
563+
LOGD(" -- glyphBaseCount = %d", glyphBaseCount);
564+
LOGD(" -- num_glypth = %d", mShaperItem.num_glyphs);
565+
LOGD(" -- kerning_applied = %d", mShaperItem.kerning_applied);
566+
LOGD(" -- isDevKernText = %d", paint->isDevKernText());
556567

557568
logGlyphs(mShaperItem);
558569
#endif
559570
if (isRTL) {
560-
endFontRun = startFontRun;
571+
endScriptRun = startScriptRun;
561572
#if DEBUG_GLYPHS
562-
LOGD(" -- updated endFontRun=%d", endFontRun);
573+
LOGD("Updated endScriptRun = %d", endScriptRun);
563574
#endif
564575
} else {
565-
startFontRun = endFontRun;
576+
startScriptRun = endScriptRun;
566577
#if DEBUG_GLYPHS
567-
LOGD(" -- updated startFontRun=%d", startFontRun);
578+
LOGD("Updated startScriptRun = %d", startScriptRun);
568579
#endif
569580
}
570581

571582
if (mShaperItem.advances == NULL || mShaperItem.num_glyphs == 0) {
572583
#if DEBUG_GLYPHS
573-
LOGD("HARFBUZZ -- advances array is empty or num_glypth = 0");
584+
LOGD("Advances array is empty or num_glypth = 0");
574585
#endif
575-
outAdvances->insertAt(0, outAdvances->size(), countFontRun);
586+
outAdvances->insertAt(0, outAdvances->size(), countScriptRun);
576587
continue;
577588
}
578589

579590
// Get Advances and their total
580591
jfloat currentAdvance = HBFixedToFloat(mShaperItem.advances[mShaperItem.log_clusters[0]]);
581592
jfloat totalFontRunAdvance = currentAdvance;
582593
outAdvances->add(currentAdvance);
583-
for (size_t i = 1; i < countFontRun; i++) {
594+
for (size_t i = 1; i < countScriptRun; i++) {
584595
size_t clusterPrevious = mShaperItem.log_clusters[i - 1];
585596
size_t cluster = mShaperItem.log_clusters[i];
586597
if (cluster == clusterPrevious) {
@@ -594,26 +605,33 @@ void TextLayoutEngine::computeRunValuesWithHarfbuzz(SkPaint* paint, const UChar*
594605
totalAdvance += totalFontRunAdvance;
595606

596607
#if DEBUG_ADVANCES
597-
for (size_t i = 0; i < countFontRun; i++) {
598-
LOGD("hb-adv[%d] = %f - log_clusters = %d - total = %f", i,
608+
LOGD("Returned advances");
609+
for (size_t i = 0; i < countScriptRun; i++) {
610+
LOGD(" -- hb-adv[%d] = %f, log_clusters = %d, total = %f", i,
599611
(*outAdvances)[i], shaperItem.log_clusters[i], totalFontRunAdvance);
600612
}
601613
#endif
602614

603615
// Get Glyphs and reverse them in place if RTL
604616
if (outGlyphs) {
605617
size_t countGlyphs = mShaperItem.num_glyphs;
618+
#if DEBUG_GLYPHS
619+
LOGD("Returned script run glyphs -- count = %d", countGlyphs);
620+
#endif
606621
for (size_t i = 0; i < countGlyphs; i++) {
607622
jchar glyph = glyphBaseCount +
608623
(jchar) mShaperItem.glyphs[(!isRTL) ? i : countGlyphs - 1 - i];
609624
#if DEBUG_GLYPHS
610-
LOGD("HARFBUZZ -- glyph[%d]=%d", i, glyph);
625+
LOGD(" -- glyph[%d] = %d", i, glyph);
611626
#endif
612627
outGlyphs->add(glyph);
613628
}
614629
}
615630
}
616631
*outTotalAdvance = totalAdvance;
632+
#if DEBUG_GLYPHS
633+
LOGD("-------- End of Script Run --------");
634+
#endif
617635
}
618636

619637

@@ -681,7 +699,7 @@ size_t TextLayoutEngine::shapeFontRun(SkPaint* paint, bool isRTL) {
681699
mShaperItem.face = getCachedHBFace(typeface);
682700

683701
#if DEBUG_GLYPHS
684-
LOGD("Run typeFace = %p, uniqueID = %d, hb_face = %p",
702+
LOGD("Run typeface = %p, uniqueID = %d, hb_face = %p",
685703
typeface, typeface->uniqueID(), mShaperItem.face);
686704
#endif
687705

@@ -724,7 +742,7 @@ void TextLayoutEngine::ensureShaperItemGlyphArrays(size_t size) {
724742

725743
void TextLayoutEngine::createShaperItemGlyphArrays(size_t size) {
726744
#if DEBUG_GLYPHS
727-
LOGD("createGlyphArrays -- size=%d", size);
745+
LOGD("Creating Glyph Arrays with size = %d", size);
728746
#endif
729747
mShaperItemGlyphArraySize = size;
730748
mShaperItem.glyphs = new HB_Glyph[size];
@@ -749,7 +767,7 @@ void TextLayoutEngine::ensureShaperItemLogClustersArray(size_t size) {
749767

750768
void TextLayoutEngine::createShaperItemLogClustersArray(size_t size) {
751769
#if DEBUG_GLYPHS
752-
LOGD("createLogClustersArray -- size=%d", size);
770+
LOGD("Creating LogClusters Array with size = %d", size);
753771
#endif
754772
mShaperItemLogClustersArraySize = size;
755773
mShaperItem.log_clusters = new unsigned short[size];
@@ -764,7 +782,7 @@ SkTypeface* TextLayoutEngine::getCachedTypeface(SkTypeface** typeface, const cha
764782
*typeface = SkTypeface::CreateFromFile(path);
765783
(*typeface)->ref();
766784
#if DEBUG_GLYPHS
767-
LOGD("Created SkTypeface from file: %s", path);
785+
LOGD("Created SkTypeface from file '%s' with uniqueID = %d", path, (*typeface)->uniqueID());
768786
#endif
769787
}
770788
return *typeface;
@@ -779,7 +797,7 @@ HB_Face TextLayoutEngine::getCachedHBFace(SkTypeface* typeface) {
779797
HB_Face face = HB_NewFace(typeface, harfbuzzSkiaGetTable);
780798
if (face) {
781799
#if DEBUG_GLYPHS
782-
LOGD("Created HB_NewFace %p from paint typeface: %p", face, typeface);
800+
LOGD("Created HB_NewFace %p from paint typeface = %p", face, typeface);
783801
#endif
784802
mCachedHBFaces.add(fontId, face);
785803
}

core/jni/android/graphics/TextLayoutCache.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,12 +256,12 @@ class TextLayoutEngine : public Singleton<TextLayoutEngine> {
256256

257257
size_t shapeFontRun(SkPaint* paint, bool isRTL);
258258

259-
void computeValuesWithHarfbuzz(SkPaint* paint, const UChar* chars,
259+
void computeValues(SkPaint* paint, const UChar* chars,
260260
size_t start, size_t count, size_t contextCount, int dirFlags,
261261
Vector<jfloat>* const outAdvances, jfloat* outTotalAdvance,
262262
Vector<jchar>* const outGlyphs);
263263

264-
void computeRunValuesWithHarfbuzz(SkPaint* paint, const UChar* chars,
264+
void computeRunValues(SkPaint* paint, const UChar* chars,
265265
size_t count, bool isRTL,
266266
Vector<jfloat>* const outAdvances, jfloat* outTotalAdvance,
267267
Vector<jchar>* const outGlyphs);

0 commit comments

Comments
 (0)