From 83a355d698abcc85420cad2cb32003dda86c1201 Mon Sep 17 00:00:00 2001 From: Herb Derby Date: Tue, 10 Jul 2018 22:59:23 +0000 Subject: [PATCH] Revert "Use new SkGlyphIDSet" This reverts commit 819f73c23cfd8471e1cbc77ee7c14d8150457765. Reason for revert: uninitialized memory - this is expected but Original change's description: > Use new SkGlyphIDSet > > Change-Id: I6b8080393a22a56577528f66630ad39372edf712 > Reviewed-on: https://skia-review.googlesource.com/140243 > Commit-Queue: Herb Derby > Reviewed-by: Mike Klein TBR=mtklein@google.com,herb@google.com Change-Id: I43e204520710738e9e8c84b0eb00260ca06fe6a2 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://skia-review.googlesource.com/140384 Reviewed-by: Herb Derby Commit-Queue: Herb Derby --- src/core/SkGlyphRun.cpp | 218 ++++++++++++++++++---------------------- src/core/SkGlyphRun.h | 51 +++++----- tests/GlyphRunTest.cpp | 39 +++---- 3 files changed, 142 insertions(+), 166 deletions(-) diff --git a/src/core/SkGlyphRun.cpp b/src/core/SkGlyphRun.cpp index a9ba900881..e7bab33ad8 100644 --- a/src/core/SkGlyphRun.cpp +++ b/src/core/SkGlyphRun.cpp @@ -35,6 +35,55 @@ static SkTypeface::Encoding convert_encoding(SkPaint::TextEncoding encoding) { } } // namespace +// -- SkGlyphSet ---------------------------------------------------------------------------------- +uint32_t SkGlyphSet::uniqueSize() { + // The size is how big the vector is grown since being passed into reuse. + return fUniqueGlyphIDs->size() - fStartOfUniqueIDs; +} + +uint16_t SkGlyphSet::add(SkGlyphID glyphID) { + static constexpr SkGlyphID kUndefGlyph{0}; + + if (glyphID >= fUniverseSize) { + glyphID = kUndefGlyph; + } + + if (glyphID >= fIndices.size()) { + fIndices.resize(glyphID + 1); + } + + auto index = fIndices[glyphID]; + + // Remember we start at the end of what ever was passed in. + if (index < this->uniqueSize() && (*fUniqueGlyphIDs)[fStartOfUniqueIDs + index] == glyphID) { + return index; + } + + uint16_t newIndex = SkTo(this->uniqueSize()); + fUniqueGlyphIDs->push_back(glyphID); + fIndices[glyphID] = newIndex; + return newIndex; +} + +void SkGlyphSet::reuse(uint32_t glyphUniverseSize, std::vector* uniqueGlyphIDs) { + SkASSERT(glyphUniverseSize <= (1 << 16)); + fUniverseSize = glyphUniverseSize; + fUniqueGlyphIDs = uniqueGlyphIDs; + + // Capture the vector end to act as the start of a new unique id vector. + fStartOfUniqueIDs = uniqueGlyphIDs->size(); + + // If we're hanging onto these arrays for a long time, we don't want their size to drift + // endlessly upwards. It's unusual to see a typeface with more than 4096 possible glyphs. + if (glyphUniverseSize < 4096 && fIndices.size() > 4096) { + fIndices.resize(4096); + fIndices.shrink_to_fit(); + } + + // No need to clear fIndices here... SkGlyphSet's set insertion algorithm is designed to work + // correctly even when the fIndexes buffer is uninitialized! +} + // -- SkGlyphRun ----------------------------------------------------------------------------------- SkGlyphRun::SkGlyphRun(SkPaint&& runPaint, SkSpan denseIndices, @@ -69,146 +118,83 @@ void SkGlyphRun::temporaryShuntToCallback(TemporaryShuntCallback callback) { callback(fTemporaryShuntGlyphIDs.size(), bytes, pos); } -// -- SkGlyphIDSet --------------------------------------------------------------------------------- -// A faster set implementation that does not need any initialization, and reading the set items -// is order the number of items, and not the size of the universe. -// This implementation is based on the paper by Briggs and Torczon, "An Efficient Representation -// for Sparse Sets" -// -// This implementation assumes that the unique glyphs added are appended to a vector that may -// already have unique glyph from a previous computation. This allows the packing of multiple -// UniqueID sequences in a single vector. -SkSpan SkGlyphIDSet::uniquifyGlyphIDs( - uint32_t universeSize, - SkSpan glyphIDs, - SkGlyphID* uniqueGlyphIDs, - uint16_t* denseIndices) { - static constexpr SkGlyphID kUndefGlyph{0}; - - if (universeSize > fUniverseToUniqueSize) { - fUniverseToUnique.reset(universeSize); - fUniverseToUniqueSize = universeSize; - } - - // No need to clear fUniverseToUnique here... the set insertion algorithm is designed to work - // correctly even when the fUniverseToUnique buffer is uninitialized! - - size_t uniqueSize = 0; - size_t denseIndicesCursor = 0; - for (auto glyphID : glyphIDs) { - - // If the glyphID is not in range then it is the undefined glyph. - if (glyphID >= universeSize) { - glyphID = kUndefGlyph; - } - - // The index into the unique ID vector. - auto uniqueIndex = fUniverseToUnique[glyphID]; - - if (uniqueIndex >= uniqueSize || uniqueGlyphIDs[uniqueIndex] != glyphID) { - uniqueIndex = SkTo(uniqueSize); - uniqueGlyphIDs[uniqueSize] = glyphID; - fUniverseToUnique[glyphID] = uniqueIndex; - uniqueSize += 1; - } - - denseIndices[denseIndicesCursor++] = uniqueIndex; - } - - // If we're hanging onto these arrays for a long time, we don't want their size to drift - // endlessly upwards. It's unusual to see a typeface with more than 4096 possible glyphs. - if (fUniverseToUniqueSize > 4096) { - fUniverseToUnique.reset(4096); - fUniverseToUniqueSize = 4096; - } - - return SkSpan(uniqueGlyphIDs, uniqueSize); -} - // -- SkGlyphRunBuilder ---------------------------------------------------------------------------- void SkGlyphRunBuilder::prepareDrawText( const SkPaint& paint, const void* bytes, size_t byteLength, SkPoint origin) { - auto glyphIDs = textToGlyphIDs(paint, bytes, byteLength); - this->initialize(glyphIDs.size()); + this->initialize(); SkSpan originalText((const char*)bytes, byteLength); if (paint.getTextEncoding() != SkPaint::kUTF8_TextEncoding) { originalText = SkSpan(); } - this->drawText(paint, glyphIDs, origin, originalText, SkSpan()); + this->drawText(paint, bytes, byteLength, origin, originalText, SkSpan()); } void SkGlyphRunBuilder::prepareDrawPosTextH(const SkPaint& paint, const void* bytes, size_t byteLength, const SkScalar* xpos, SkScalar constY) { - auto glyphIDs = textToGlyphIDs(paint, bytes, byteLength); - this->initialize(glyphIDs.size()); + this->initialize(); this->drawPosTextH( - paint, glyphIDs, xpos, constY, SkSpan(), SkSpan()); + paint, bytes, byteLength, xpos, constY, SkSpan(), SkSpan()); } void SkGlyphRunBuilder::prepareDrawPosText(const SkPaint& paint, const void* bytes, size_t byteLength, const SkPoint* pos) { - auto glyphIDs = textToGlyphIDs(paint, bytes, byteLength); - this->initialize(glyphIDs.size()); - this->drawPosText(paint, glyphIDs, pos, SkSpan(), SkSpan()); + this->initialize(); + this->drawPosText(paint, bytes, byteLength, pos, + SkSpan(), SkSpan()); } SkGlyphRun* SkGlyphRunBuilder::useGlyphRun() { return &fScratchGlyphRun; } -void SkGlyphRunBuilder::initialize(size_t totalRunSize) { +void SkGlyphRunBuilder::initialize() { fUniqueID = 0; - - // Using resize is temporary until simpler buffers are in place. - fDenseIndex.resize(totalRunSize); - fPositions.resize(totalRunSize); - fUniqueGlyphIDs.resize(totalRunSize); + fDenseIndex.clear(); + fPositions.clear(); + fUniqueGlyphIDs.clear(); // Be sure to clean up the last run before we reuse it. fScratchGlyphRun.~SkGlyphRun(); } -SkSpan SkGlyphRunBuilder::textToGlyphIDs( +void SkGlyphRunBuilder::addDenseAndUnique( const SkPaint& paint, const void* bytes, size_t byteLength) { + + size_t runSize = 0; + SkGlyphID* glyphIDs = nullptr; auto encoding = paint.getTextEncoding(); + auto typeface = SkPaintPriv::GetTypefaceOrDefault(paint); if (encoding != SkPaint::kGlyphID_TextEncoding) { auto tfEncoding = convert_encoding(encoding); int utfSize = SkUTFN_CountUnichars(tfEncoding, bytes, byteLength); if (utfSize > 0) { - size_t runSize = SkTo(utfSize); + runSize = SkTo(utfSize); fScratchGlyphIDs.resize(runSize); - auto typeface = SkPaintPriv::GetTypefaceOrDefault(paint); typeface->charsToGlyphs(bytes, tfEncoding, fScratchGlyphIDs.data(), runSize); - return SkSpan{fScratchGlyphIDs}; - } else { - return SkSpan(); + glyphIDs = fScratchGlyphIDs.data(); } } else { - return SkSpan((const SkGlyphID*)bytes, byteLength / 2); - } -} - -SkSpan SkGlyphRunBuilder::addDenseAndUnique( - const SkPaint& paint, - SkSpan glyphIDs) { - SkSpan uniquifiedGlyphIDs; - if (!glyphIDs.empty()) { - auto typeface = SkPaintPriv::GetTypefaceOrDefault(paint); - auto glyphUniverseSize = typeface->countGlyphs(); - uniquifiedGlyphIDs = fGlyphIDSet.uniquifyGlyphIDs( - glyphUniverseSize, glyphIDs, fUniqueGlyphIDs.data(), fDenseIndex.data()); + runSize = byteLength / 2; + glyphIDs = (SkGlyphID*)bytes; } - return uniquifiedGlyphIDs; + // TODO: Remove when glyphIds are passed back. + fGlyphIDs = glyphIDs; + + SkASSERT(glyphIDs != nullptr); + + if (runSize > 0) { + fGlyphSet.reuse(typeface->countGlyphs(), &fUniqueGlyphIDs); + for (size_t i = 0; i < runSize; i++) { + fDenseIndex.push_back(fGlyphSet.add(glyphIDs[i])); + } + } } void SkGlyphRunBuilder::makeGlyphRun( const SkPaint& runPaint, - SkSpan glyphIDs, - SkSpan positions, - SkSpan text, - SkSpan clusters) { + SkSpan text, SkSpan clusters) { // Ignore empty runs. if (!fDenseIndex.empty()) { @@ -219,8 +205,8 @@ void SkGlyphRunBuilder::makeGlyphRun( new ((void*)&fScratchGlyphRun) SkGlyphRun{ std::move(glyphRunPaint), SkSpan{fDenseIndex}, - positions, - glyphIDs, + SkSpan{fPositions}, + SkSpan{fGlyphIDs, SkTo(fDenseIndex.size())}, SkSpan{fUniqueGlyphIDs}, text, clusters @@ -229,21 +215,21 @@ void SkGlyphRunBuilder::makeGlyphRun( } void SkGlyphRunBuilder::drawText( - const SkPaint& paint, SkSpan glyphIDs, SkPoint origin, + const SkPaint& paint, const void* bytes, size_t byteLength, SkPoint origin, SkSpan text, SkSpan clusters) { - auto unqiueGlyphIDs = this->addDenseAndUnique(paint, glyphIDs); + this->addDenseAndUnique(paint, bytes, byteLength); fScratchAdvances.resize(fUniqueGlyphIDs.size()); { auto cache = SkStrikeCache::FindOrCreateStrikeExclusive(paint); - cache->getAdvances(unqiueGlyphIDs, fScratchAdvances.data()); + cache->getAdvances(SkSpan{fUniqueGlyphIDs}, fScratchAdvances.data()); } SkPoint endOfLastGlyph = origin; for (size_t i = 0; i < fDenseIndex.size(); i++) { - fPositions[i] = endOfLastGlyph; + fPositions.push_back(endOfLastGlyph); endOfLastGlyph += fScratchAdvances[fDenseIndex[i]]; } @@ -257,39 +243,33 @@ void SkGlyphRunBuilder::drawText( } } - this->makeGlyphRun(paint, glyphIDs, SkSpan{fPositions}, text, clusters); + this->makeGlyphRun(paint, text, clusters); } -void SkGlyphRunBuilder::drawPosTextH(const SkPaint& paint, SkSpan glyphIDs, - const SkScalar* xpos, SkScalar constY, +void SkGlyphRunBuilder::drawPosTextH(const SkPaint& paint, const void* bytes, + size_t byteLength, const SkScalar* xpos, + SkScalar constY, SkSpan text, SkSpan clusters) { - // The dense indices are not used by the rest of the stack yet. - #ifdef SK_DEBUG - this->addDenseAndUnique(paint, glyphIDs); - #endif + this->addDenseAndUnique(paint, bytes, byteLength); for (size_t i = 0; i < fDenseIndex.size(); i++) { - fPositions[i] = SkPoint::Make(xpos[i], constY); + fPositions.push_back(SkPoint::Make(xpos[i], constY)); } - this->makeGlyphRun(paint, glyphIDs, SkSpan{fPositions}, text, clusters); + this->makeGlyphRun(paint, text, clusters); } -void SkGlyphRunBuilder::drawPosText(const SkPaint& paint, SkSpan glyphIDs, - const SkPoint* pos, +void SkGlyphRunBuilder::drawPosText(const SkPaint& paint, const void* bytes, + size_t byteLength, const SkPoint* pos, SkSpan text, SkSpan clusters) { - - // The dense indices are not used by the rest of the stack yet. - #ifdef SK_DEBUG - this->addDenseAndUnique(paint, glyphIDs); - #endif + this->addDenseAndUnique(paint, bytes, byteLength); for (size_t i = 0; i < fDenseIndex.size(); i++) { - fPositions[i] = pos[i]; + fPositions.push_back(pos[i]); } - this->makeGlyphRun(paint, glyphIDs, SkSpan{fPositions}, text, clusters); + this->makeGlyphRun(paint, text, clusters); } diff --git a/src/core/SkGlyphRun.h b/src/core/SkGlyphRun.h index 7c3a74860c..7788376c70 100644 --- a/src/core/SkGlyphRun.h +++ b/src/core/SkGlyphRun.h @@ -83,14 +83,26 @@ private: const SkPaint fRunPaint; }; -class SkGlyphIDSet { +// A faster set implementation that does not need any initialization, and reading the set items +// is order the number of items, and not the size of the universe. +// This implementation is based on the paper by Briggs and Torczon, "An Efficient Representation +// for Sparse Sets" +// +// This implementation assumes that the unique glyphs added are appended to a vector that may +// already have unique glyph from a previous computation. This allows the packing of multiple +// UniqueID sequences in a single vector. +class SkGlyphSet { public: - SkSpan uniquifyGlyphIDs( - uint32_t universeSize, SkSpan glyphIDs, - SkGlyphID* uniqueGlyphIDs, uint16_t* denseindices); + SkGlyphSet() = default; + uint16_t add(SkGlyphID glyphID); + void reuse(uint32_t glyphUniverseSize, std::vector* uniqueGlyphIDs); + private: - size_t fUniverseToUniqueSize{0}; - SkAutoTMalloc fUniverseToUnique; + uint32_t uniqueSize(); + uint32_t fUniverseSize{0}; + size_t fStartOfUniqueIDs{0}; + std::vector fIndices; + std::vector* fUniqueGlyphIDs{nullptr}; }; class SkGlyphRunBuilder { @@ -103,35 +115,25 @@ public: const SkScalar xpos[], SkScalar constY); void prepareDrawPosText( const SkPaint& paint, const void* bytes, size_t byteLength, const SkPoint pos[]); + void prepareTextBlob(const SkPaint& paint, const SkTextBlob& blob, SkPoint origin); SkGlyphRun* useGlyphRun(); private: - void initialize(size_t totalRunSize); - SkSpan textToGlyphIDs( - const SkPaint& paint, const void* bytes, size_t byteLength); - - // Returns the span of unique glyph IDs. - SkSpan addDenseAndUnique( - const SkPaint& paint, - SkSpan glyphIDs); - + void initialize(); + void addDenseAndUnique(const SkPaint& paint, const void* bytes, size_t byteLength); void makeGlyphRun( - const SkPaint& runPaint, - SkSpan glyphIDs, - SkSpan positions, - SkSpan text, - SkSpan clusters); + const SkPaint& runPaint, SkSpan text, SkSpan clusters); void drawText( - const SkPaint& paint, SkSpan glyphIDs, SkPoint origin, + const SkPaint& paint, const void* bytes, size_t byteLength, SkPoint origin, SkSpan text, SkSpan clusters); void drawPosTextH( - const SkPaint& paint, SkSpan glyphIDs, + const SkPaint& paint, const void* bytes, size_t byteLength, const SkScalar* xpos, SkScalar constY, SkSpan text, SkSpan clusters); void drawPosText( - const SkPaint& paint, SkSpan glyphIDs, const SkPoint* pos, + const SkPaint& paint, const void* bytes, size_t byteLength, const SkPoint* pos, SkSpan text, SkSpan clusters); uint64_t fUniqueID{0}; @@ -139,6 +141,7 @@ private: std::vector fDenseIndex; std::vector fPositions; std::vector fUniqueGlyphIDs; + SkGlyphID* fGlyphIDs{nullptr}; // Used as a temporary for preparing using utfN text. This implies that only one run of // glyph ids will ever be needed because blobs are already glyph based. @@ -152,7 +155,7 @@ private: SkGlyphRun fScratchGlyphRun; // Used for collecting the set of unique glyphs. - SkGlyphIDSet fGlyphIDSet; + SkGlyphSet fGlyphSet; }; #endif // SkGlyphRunInfo_DEFINED diff --git a/tests/GlyphRunTest.cpp b/tests/GlyphRunTest.cpp index 0e0c441b75..46b4715c61 100644 --- a/tests/GlyphRunTest.cpp +++ b/tests/GlyphRunTest.cpp @@ -11,32 +11,25 @@ #include "Test.h" -DEF_TEST(GlyphRunGlyphIDSetBasic, reporter) { - SkGlyphID glyphs[] = {100, 3, 240, 3, 234}; - auto glyphIDs = SkSpan(glyphs, SK_ARRAY_COUNT(glyphs)); - int universeSize = 1000; - SkGlyphID uniqueGlyphs[SK_ARRAY_COUNT(glyphs)]; - uint16_t denseIndices[SK_ARRAY_COUNT(glyphs)]; +DEF_TEST(GlyphSetBasic, reporter) { + SkGlyphSet set; - SkGlyphIDSet gs; - auto uniqueGlyphIDs = gs.uniquifyGlyphIDs(universeSize, glyphIDs, uniqueGlyphs, denseIndices); + std::vector unique; - std::vector test{uniqueGlyphIDs.begin(), uniqueGlyphIDs.end()}; - std::sort(test.begin(), test.end()); - auto newEnd = std::unique(test.begin(), test.end()); - REPORTER_ASSERT(reporter, uniqueGlyphIDs.size() == newEnd - test.begin()); - REPORTER_ASSERT(reporter, uniqueGlyphIDs.size() == 4); - { - uint16_t answer[] = {0, 1, 2, 1, 3}; - REPORTER_ASSERT(reporter, - std::equal(answer, std::end(answer), denseIndices)); - } + set.reuse(10, &unique); + REPORTER_ASSERT(reporter, set.add(7) == 0); + REPORTER_ASSERT(reporter, set.add(3) == 1); + set.reuse(10, &unique); + REPORTER_ASSERT(reporter, set.add(5) == 0); + REPORTER_ASSERT(reporter, set.add(8) == 1); + REPORTER_ASSERT(reporter, set.add(3) == 2); - { - SkGlyphID answer[] = {100, 3, 240, 234}; - REPORTER_ASSERT(reporter, - std::equal(answer, std::end(answer), uniqueGlyphs)); - } + REPORTER_ASSERT(reporter, unique.size() == 5); + REPORTER_ASSERT(reporter, unique[0] == 7); + REPORTER_ASSERT(reporter, unique[1] == 3); + REPORTER_ASSERT(reporter, unique[2] == 5); + REPORTER_ASSERT(reporter, unique[3] == 8); + REPORTER_ASSERT(reporter, unique[4] == 3); } DEF_TEST(GlyphRunBasic, reporter) {