From 8e74cbcd6526a7542b9f704b9e40b0c60d475849 Mon Sep 17 00:00:00 2001 From: Mike Reed Date: Fri, 8 Dec 2017 13:20:01 -0500 Subject: [PATCH] Revert "Revert "use serialprocs for typefaces"" This reverts commit 1a104bce20adc47a343fa910899ca6c4f261be40. Change (from first version) is - only signal error in readbuffer for corrupt stream, not default fonts - change test to ensure a non-null typeface (i.e. MakeDefault()) Bug: skia: Change-Id: I325445b56b0a402e1b89a2439df06e92314c793f Reviewed-on: https://skia-review.googlesource.com/82687 Reviewed-by: Mike Reed Commit-Queue: Mike Reed --- include/core/SkTextBlob.h | 6 +++ src/core/SkReadBuffer.cpp | 22 ++++++++-- src/core/SkReadBuffer.h | 2 +- src/core/SkTextBlob.cpp | 89 +++++++++++++++++++++++--------------- src/core/SkWriteBuffer.cpp | 25 +++++++++-- tests/TextBlobTest.cpp | 10 ++--- 6 files changed, 106 insertions(+), 48 deletions(-) diff --git a/include/core/SkTextBlob.h b/include/core/SkTextBlob.h index a87c9a3d2e..228747a1c8 100644 --- a/include/core/SkTextBlob.h +++ b/include/core/SkTextBlob.h @@ -17,6 +17,9 @@ class SkReadBuffer; class SkWriteBuffer; +struct SkSerialProcs; +struct SkDeserialProcs; + typedef void (*SkTypefaceCatalogerProc)(SkTypeface*, void* ctx); typedef sk_sp (*SkTypefaceResolverProc)(uint32_t id, void* ctx); @@ -71,6 +74,9 @@ public: static sk_sp Deserialize(const void* data, size_t size, SkTypefaceResolverProc, void* ctx); + sk_sp serialize(const SkSerialProcs&) const; + static sk_sp Deserialize(const void* data, size_t size, const SkDeserialProcs&); + private: friend class SkNVRefCnt; class RunRecord; diff --git a/src/core/SkReadBuffer.cpp b/src/core/SkReadBuffer.cpp index e3651cc3a2..4a2c61be02 100644 --- a/src/core/SkReadBuffer.cpp +++ b/src/core/SkReadBuffer.cpp @@ -362,12 +362,26 @@ sk_sp SkReadBuffer::readTypeface() { return sk_ref_sp(fInflator->getTypeface(this->read32())); } - uint32_t index = this->readUInt(); - if (0 == index || index > (unsigned)fTFCount) { + // Read 32 bits (signed) + // 0 -- return null (default font) + // >0 -- index + // <0 -- custom (serial procs) : negative size in bytes + + int32_t index = this->readUInt(); + if (index == 0) { return nullptr; - } else { - SkASSERT(fTFArray); + } else if (index > 0) { + if (!this->validate(index <= fTFCount)) { + return nullptr; + } return sk_ref_sp(fTFArray[index - 1]); + } else { // custom + size_t size = -index; + const void* data = this->skip(size); + if (!this->validate(data != nullptr)) { + return nullptr; + } + return fProcs.fTypefaceProc(data, size, fProcs.fTypefaceCtx); } } diff --git a/src/core/SkReadBuffer.h b/src/core/SkReadBuffer.h index 6819c47904..61f1915e19 100644 --- a/src/core/SkReadBuffer.h +++ b/src/core/SkReadBuffer.h @@ -184,7 +184,7 @@ public: // be created (e.g. it was not originally encoded) then this returns an image that doesn't // draw. sk_sp readImage(); - virtual sk_sp readTypeface(); + sk_sp readTypeface(); void setTypefaceArray(SkTypeface* array[], int count) { fTFArray = array; diff --git a/src/core/SkTextBlob.cpp b/src/core/SkTextBlob.cpp index 193c82de66..f778c68f13 100644 --- a/src/core/SkTextBlob.cpp +++ b/src/core/SkTextBlob.cpp @@ -868,25 +868,9 @@ sk_sp SkTextBlob::MakeFromBuffer(SkReadBuffer& reader) { return blobBuilder.make(); } -class SkTypefaceCatalogerWriteBuffer : public SkBinaryWriteBuffer { -public: - SkTypefaceCatalogerWriteBuffer(SkTypefaceCatalogerProc proc, void* ctx) - : SkBinaryWriteBuffer(SkBinaryWriteBuffer::kCrossProcess_Flag) - , fCatalogerProc(proc) - , fCatalogerCtx(ctx) - {} - - void writeTypeface(SkTypeface* typeface) override { - fCatalogerProc(typeface, fCatalogerCtx); - this->write32(typeface ? typeface->uniqueID() : 0); - } - - SkTypefaceCatalogerProc fCatalogerProc; - void* fCatalogerCtx; -}; - -sk_sp SkTextBlob::serialize(SkTypefaceCatalogerProc proc, void* ctx) const { - SkTypefaceCatalogerWriteBuffer buffer(proc, ctx); +sk_sp SkTextBlob::serialize(const SkSerialProcs& procs) const { + SkBinaryWriteBuffer buffer; + buffer.setSerialProcs(procs); this->flatten(buffer); size_t total = buffer.bytesWritten(); @@ -895,26 +879,61 @@ sk_sp SkTextBlob::serialize(SkTypefaceCatalogerProc proc, void* ctx) con return data; } -class SkTypefaceResolverReadBuffer : public SkReadBuffer { -public: - SkTypefaceResolverReadBuffer(const void* data, size_t size, SkTypefaceResolverProc proc, - void* ctx) - : SkReadBuffer(data, size) - , fResolverProc(proc) - , fResolverCtx(ctx) - {} +sk_sp SkTextBlob::Deserialize(const void* data, size_t length, + const SkDeserialProcs& procs) { + SkReadBuffer buffer(data, length); + buffer.setDeserialProcs(procs); + return SkTextBlob::MakeFromBuffer(buffer); +} - sk_sp readTypeface() override { - auto id = this->readUInt(); - return this->isValid() ? fResolverProc(id, fResolverCtx) : nullptr; +/////////////////////////////////////////////////////////////////////////////////////////////////// + +namespace { + struct CatalogState { + SkTypefaceCatalogerProc fProc; + void* fCtx; + }; + + sk_sp catalog_typeface_proc(SkTypeface* face, void* ctx) { + CatalogState* state = static_cast(ctx); + state->fProc(face, state->fCtx); + uint32_t id = face->uniqueID(); + return SkData::MakeWithCopy(&id, sizeof(uint32_t)); } +} - SkTypefaceResolverProc fResolverProc; - void* fResolverCtx; -}; +sk_sp SkTextBlob::serialize(SkTypefaceCatalogerProc proc, void* ctx) const { + CatalogState state = { proc, ctx }; + SkSerialProcs procs; + procs.fTypefaceProc = catalog_typeface_proc; + procs.fTypefaceCtx = &state; + return this->serialize(procs); +} + +namespace { + struct ResolverState { + SkTypefaceResolverProc fProc; + void* fCtx; + }; + + sk_sp resolver_typeface_proc(const void* data, size_t length, void* ctx) { + SkASSERT(length == 4); + if (length != 4) { + return nullptr; + } + + ResolverState* state = static_cast(ctx); + uint32_t id; + memcpy(&id, data, length); + return state->fProc(id, state->fCtx); + } +} sk_sp SkTextBlob::Deserialize(const void* data, size_t length, SkTypefaceResolverProc proc, void* ctx) { - SkTypefaceResolverReadBuffer buffer(data, length, proc, ctx); - return SkTextBlob::MakeFromBuffer(buffer); + ResolverState state = { proc, ctx }; + SkDeserialProcs procs; + procs.fTypefaceProc = resolver_typeface_proc; + procs.fTypefaceCtx = &state; + return Deserialize(data, length, procs); } diff --git a/src/core/SkWriteBuffer.cpp b/src/core/SkWriteBuffer.cpp index bd1d64b76a..0e02a8b8cd 100644 --- a/src/core/SkWriteBuffer.cpp +++ b/src/core/SkWriteBuffer.cpp @@ -181,11 +181,30 @@ void SkBinaryWriteBuffer::writeTypeface(SkTypeface* obj) { return; } - if (nullptr == obj || nullptr == fTFSet) { + // Write 32 bits (signed) + // 0 -- default font + // >0 -- index + // <0 -- custom (serial procs) + + if (obj == nullptr) { fWriter.write32(0); - } else { - fWriter.write32(fTFSet->add(obj)); + } else if (fProcs.fTypefaceProc) { + auto data = fProcs.fTypefaceProc(obj, fProcs.fTypefaceCtx); + if (data) { + size_t size = data->size(); + if (!sk_64_isS32(size)) { + size = 0; // fall back to default font + } + int32_t ssize = SkToS32(size); + fWriter.write32(-ssize); // negative to signal custom + if (size) { + this->writePad32(data->data(), size); + } + return; + } + // no data means fall through for std behavior } + fWriter.write32(fTFSet ? fTFSet->add(obj) : 0); } void SkBinaryWriteBuffer::writePaint(const SkPaint& paint) { diff --git a/tests/TextBlobTest.cpp b/tests/TextBlobTest.cpp index 8e272a5869..fdc5c3826a 100644 --- a/tests/TextBlobTest.cpp +++ b/tests/TextBlobTest.cpp @@ -426,12 +426,11 @@ static sk_sp render(const SkTextBlob* blob) { */ DEF_TEST(TextBlob_serialize, reporter) { sk_sp blob0 = []() { - sk_sp tf0; - sk_sp tf1 = SkTypeface::MakeFromName("Times", SkFontStyle()); + sk_sp tf = SkTypeface::MakeDefault(); SkTextBlobBuilder builder; - add_run(&builder, "Hello", 10, 20, tf0); - add_run(&builder, "World", 10, 40, tf1); + add_run(&builder, "Hello", 10, 20, nullptr); // we don't flatten this in the paint + add_run(&builder, "World", 10, 40, tf); // we will flatten this in the paint return builder.make(); }(); @@ -442,7 +441,8 @@ DEF_TEST(TextBlob_serialize, reporter) { *array->append() = tf; } }, &array); - REPORTER_ASSERT(reporter, array.count() > 0); + // we only expect 1, since null would not have been serialized, but the default would + REPORTER_ASSERT(reporter, array.count() == 1); sk_sp blob1 = SkTextBlob::Deserialize(data->data(), data->size(), [](uint32_t uniqueID, void* ctx) {