From d1c65d6708de536a5971575809d7172fa4f54b37 Mon Sep 17 00:00:00 2001 From: Mike Reed Date: Wed, 3 Jan 2018 11:32:13 -0500 Subject: [PATCH] remove unneeded readbuffer flags - buffers are always 'cross-process' - readbuffer is always validating Bug:796107 Change-Id: I59614e9c29490c0b029c60d2aafe2806671bc9e1 Reviewed-on: https://skia-review.googlesource.com/90560 Reviewed-by: Mike Klein Commit-Queue: Mike Reed --- include/core/SkPicture.h | 3 ++- src/core/SkPaint.cpp | 20 ++++++++------------ src/core/SkPicture.cpp | 16 ++++++---------- src/core/SkPictureData.cpp | 26 +------------------------- src/core/SkPictureData.h | 7 ------- src/core/SkReadBuffer.cpp | 11 ----------- src/core/SkReadBuffer.h | 19 +------------------ src/core/SkRecordedDrawable.cpp | 1 - src/core/SkWriteBuffer.cpp | 10 ++++------ src/core/SkWriteBuffer.h | 15 ++------------- src/effects/SkPictureImageFilter.cpp | 4 ++-- src/shaders/SkPictureShader.cpp | 6 ++---- tools/debugger/SkJsonWriteBuffer.h | 2 -- tools/skpinfo.cpp | 22 ---------------------- 14 files changed, 28 insertions(+), 134 deletions(-) diff --git a/include/core/SkPicture.h b/include/core/SkPicture.h index 5ca60ca82c..34b9f30eb5 100644 --- a/include/core/SkPicture.h +++ b/include/core/SkPicture.h @@ -160,10 +160,11 @@ private: // V57: Sweep tiling info. // V58: No more 2pt conical flipping. // V59: No more LocalSpace option on PictureImageFilter + // V60: Remove flags in picture header // Only SKPs within the min/current picture version range (inclusive) can be read. static const uint32_t MIN_PICTURE_VERSION = 51; // Produced by Chrome ~M56. - static const uint32_t CURRENT_PICTURE_VERSION = 59; + static const uint32_t CURRENT_PICTURE_VERSION = 60; static bool IsValidPictInfo(const SkPictInfo& info); static sk_sp Forwardport(const SkPictInfo&, diff --git a/src/core/SkPaint.cpp b/src/core/SkPaint.cpp index f8e5c11efd..0987c640c1 100644 --- a/src/core/SkPaint.cpp +++ b/src/core/SkPaint.cpp @@ -1865,17 +1865,16 @@ static FlatFlags unpack_paint_flags(SkPaint* paint, uint32_t packed) { it if there are not tricky elements like shaders, etc. */ void SkPaint::flatten(SkWriteBuffer& buffer) const { - // If the writer is xprocess, then we force recording our typeface, even if its "default" - // since the other process may have a different notion of default. SkTypeface* tf = this->getTypeface(); - if (!tf && buffer.isCrossProcess()) { + if (!tf) { + // We force recording our typeface, even if its "default" since the receiver process + // may have a different notion of default. tf = SkTypeface::GetDefaultTypeface(); + SkASSERT(tf); } - uint8_t flatFlags = 0; - if (tf) { - flatFlags |= kHasTypeface_FlatFlag; - } + uint8_t flatFlags = kHasTypeface_FlatFlag; + if (asint(this->getPathEffect()) | asint(this->getShader()) | asint(this->getMaskFilter()) | @@ -1899,11 +1898,8 @@ void SkPaint::flatten(SkWriteBuffer& buffer) const { (this->getStyle() << 4) | this->getTextEncoding(), fBlendMode)); - // now we're done with ptr and the (pre)reserved space. If we need to write - // additional fields, use the buffer directly - if (flatFlags & kHasTypeface_FlatFlag) { - buffer.writeTypeface(tf); - } + buffer.writeTypeface(tf); + if (flatFlags & kHasEffects_FlatFlag) { buffer.writeFlattenable(this->getPathEffect()); buffer.writeFlattenable(this->getShader()); diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp index 2afa4605b5..05bad67397 100644 --- a/src/core/SkPicture.cpp +++ b/src/core/SkPicture.cpp @@ -66,13 +66,6 @@ SkPictInfo SkPicture::createHeader() const { // Set picture info after magic bytes in the header info.setVersion(CURRENT_PICTURE_VERSION); info.fCullRect = this->cullRect(); - info.fFlags = SkPictInfo::kCrossProcess_Flag; - // TODO: remove this flag, since we're always float (now) - info.fFlags |= SkPictInfo::kScalarIsFloat_Flag; - - if (8 == sizeof(void*)) { - info.fFlags |= SkPictInfo::kPtrIs64Bit_Flag; - } return info; } @@ -102,7 +95,9 @@ bool SkPicture::StreamIsSKP(SkStream* stream, SkPictInfo* pInfo) { info.fCullRect.fTop = stream->readScalar(); info.fCullRect.fRight = stream->readScalar(); info.fCullRect.fBottom = stream->readScalar(); - info.fFlags = stream->readU32(); + if (info.getVersion() < SkReadBuffer::kRemoveHeaderFlags_Version) { + (void)stream->readU32(); // used to be flags + } if (IsValidPictInfo(info)) { if (pInfo) { *pInfo = info; } @@ -123,7 +118,9 @@ bool SkPicture::BufferIsSKP(SkReadBuffer* buffer, SkPictInfo* pInfo) { info.setVersion(buffer->readUInt()); buffer->readRect(&info.fCullRect); - info.fFlags = buffer->readUInt(); + if (info.getVersion() < SkReadBuffer::kRemoveHeaderFlags_Version) { + (void)buffer->readUInt(); // used to be flags + } if (IsValidPictInfo(info)) { if (pInfo) { *pInfo = info; } @@ -306,7 +303,6 @@ void SkPicture::flatten(SkWriteBuffer& buffer) const { buffer.writeByteArray(&info.fMagic, sizeof(info.fMagic)); buffer.writeUInt(info.getVersion()); buffer.writeRect(info.fCullRect); - buffer.writeUInt(info.fFlags); if (auto custom = custom_serialize(this, buffer.fProcs)) { int32_t size = SkToS32(custom->size()); diff --git a/src/core/SkPictureData.cpp b/src/core/SkPictureData.cpp index 7c51bd936a..d0b206e90a 100644 --- a/src/core/SkPictureData.cpp +++ b/src/core/SkPictureData.cpp @@ -279,7 +279,7 @@ void SkPictureData::serialize(SkWStream* stream, const SkSerialProcs& procs, // We delay serializing the bulk of our data until after we've serialized // factories and typefaces by first serializing to an in-memory write buffer. SkFactorySet factSet; // buffer refs factSet, so factSet must come first. - SkBinaryWriteBuffer buffer(SkBinaryWriteBuffer::kCrossProcess_Flag); + SkBinaryWriteBuffer buffer; buffer.setFactoryRecorder(&factSet); buffer.setSerialProcs(procs); buffer.setTypefaceRecorder(typefaceSet); @@ -344,29 +344,6 @@ void SkPictureData::flatten(SkWriteBuffer& buffer) const { /////////////////////////////////////////////////////////////////////////////// -/** - * Return the corresponding SkReadBuffer flags, given a set of - * SkPictInfo flags. - */ -static uint32_t pictInfoFlagsToReadBufferFlags(uint32_t pictInfoFlags) { - static const struct { - uint32_t fSrc; - uint32_t fDst; - } gSD[] = { - { SkPictInfo::kCrossProcess_Flag, SkReadBuffer::kCrossProcess_Flag }, - { SkPictInfo::kScalarIsFloat_Flag, SkReadBuffer::kScalarIsFloat_Flag }, - { SkPictInfo::kPtrIs64Bit_Flag, SkReadBuffer::kPtrIs64Bit_Flag }, - }; - - uint32_t rbMask = 0; - for (size_t i = 0; i < SK_ARRAY_COUNT(gSD); ++i) { - if (pictInfoFlags & gSD[i].fSrc) { - rbMask |= gSD[i].fDst; - } - } - return rbMask; -} - bool SkPictureData::parseStreamTag(SkStream* stream, uint32_t tag, uint32_t size, @@ -437,7 +414,6 @@ bool SkPictureData::parseStreamTag(SkStream* stream, } SkReadBuffer buffer(storage.get(), size); - buffer.setFlags(pictInfoFlagsToReadBufferFlags(fInfo.fFlags)); buffer.setVersion(fInfo.getVersion()); if (!fFactoryPlayback) { diff --git a/src/core/SkPictureData.h b/src/core/SkPictureData.h index db3639a34b..89ddcfaabe 100644 --- a/src/core/SkPictureData.h +++ b/src/core/SkPictureData.h @@ -28,12 +28,6 @@ class SkReadBuffer; class SkTextBlob; struct SkPictInfo { - enum Flags { - kCrossProcess_Flag = 1 << 0, - kScalarIsFloat_Flag = 1 << 1, - kPtrIs64Bit_Flag = 1 << 2, - }; - SkPictInfo() : fVersion(~0U) {} uint32_t getVersion() const { @@ -52,7 +46,6 @@ private: uint32_t fVersion; public: SkRect fCullRect; - uint32_t fFlags; }; #define SK_PICT_READER_TAG SkSetFourByteTag('r', 'e', 'a', 'd') diff --git a/src/core/SkReadBuffer.cpp b/src/core/SkReadBuffer.cpp index 407408baa8..7e4c0fb8a0 100644 --- a/src/core/SkReadBuffer.cpp +++ b/src/core/SkReadBuffer.cpp @@ -36,17 +36,7 @@ namespace { } // anonymous namespace -static uint32_t default_flags() { - uint32_t flags = 0; - flags |= SkReadBuffer::kScalarIsFloat_Flag; - if (8 == sizeof(void*)) { - flags |= SkReadBuffer::kPtrIs64Bit_Flag; - } - return flags; -} - SkReadBuffer::SkReadBuffer() { - fFlags = default_flags(); fVersion = 0; fMemoryPtr = nullptr; @@ -61,7 +51,6 @@ SkReadBuffer::SkReadBuffer() { } SkReadBuffer::SkReadBuffer(const void* data, size_t size) { - fFlags = default_flags(); fVersion = 0; this->setMemory(data, size); fMemoryPtr = nullptr; diff --git a/src/core/SkReadBuffer.h b/src/core/SkReadBuffer.h index 8eb48692a0..3c021582ed 100644 --- a/src/core/SkReadBuffer.h +++ b/src/core/SkReadBuffer.h @@ -76,6 +76,7 @@ public: kTileInfoInSweepGradient_Version = 57, k2PtConicalNoFlip_Version = 58, kRemovePictureImageFilterLocalSpace = 59, + kRemoveHeaderFlags_Version = 60, }; /** @@ -94,23 +95,6 @@ public: fVersion = version; } - enum Flags { - kCrossProcess_Flag = 1 << 0, - kScalarIsFloat_Flag = 1 << 1, - kPtrIs64Bit_Flag = 1 << 2, - kValidation_Flag = 1 << 3, - }; - - void setFlags(uint32_t flags) { fFlags = flags; } - uint32_t getFlags() const { return fFlags; } - - bool isCrossProcess() const { - return this->isValidating() || SkToBool(fFlags & kCrossProcess_Flag); - } - bool isScalarFloat() const { return SkToBool(fFlags & kScalarIsFloat_Flag); } - bool isPtr64Bit() const { return SkToBool(fFlags & kPtrIs64Bit_Flag); } - bool isValidating() const { return SkToBool(fFlags & kValidation_Flag); } - size_t size() { return fReader.size(); } size_t offset() { return fReader.offset(); } bool eof() { return fReader.eof(); } @@ -281,7 +265,6 @@ private: bool readArray(void* value, size_t size, size_t elementSize); void setMemory(const void*, size_t); - uint32_t fFlags; int fVersion; void* fMemoryPtr; diff --git a/src/core/SkRecordedDrawable.cpp b/src/core/SkRecordedDrawable.cpp index 342701a365..be8a2cb07e 100644 --- a/src/core/SkRecordedDrawable.cpp +++ b/src/core/SkRecordedDrawable.cpp @@ -77,7 +77,6 @@ sk_sp SkRecordedDrawable::CreateProc(SkReadBuffer& buffer) { SkPictInfo info; info.setVersion(buffer.getVersion()); info.fCullRect = bounds; - info.fFlags = 0; // ??? std::unique_ptr pictureData(SkPictureData::CreateFromBuffer(buffer, info)); if (!pictureData) { return nullptr; diff --git a/src/core/SkWriteBuffer.cpp b/src/core/SkWriteBuffer.cpp index 0e02a8b8cd..278cdcb87a 100644 --- a/src/core/SkWriteBuffer.cpp +++ b/src/core/SkWriteBuffer.cpp @@ -16,15 +16,13 @@ /////////////////////////////////////////////////////////////////////////////////////////////////// -SkBinaryWriteBuffer::SkBinaryWriteBuffer(uint32_t flags) - : fFlags(flags) - , fFactorySet(nullptr) +SkBinaryWriteBuffer::SkBinaryWriteBuffer() + : fFactorySet(nullptr) , fTFSet(nullptr) { } -SkBinaryWriteBuffer::SkBinaryWriteBuffer(void* storage, size_t storageSize, uint32_t flags) - : fFlags(flags) - , fFactorySet(nullptr) +SkBinaryWriteBuffer::SkBinaryWriteBuffer(void* storage, size_t storageSize) + : fFactorySet(nullptr) , fWriter(storage, storageSize) , fTFSet(nullptr) { } diff --git a/src/core/SkWriteBuffer.h b/src/core/SkWriteBuffer.h index bd3864cda2..82691b3c49 100644 --- a/src/core/SkWriteBuffer.h +++ b/src/core/SkWriteBuffer.h @@ -25,8 +25,6 @@ public: SkWriteBuffer() {} virtual ~SkWriteBuffer() {} - virtual bool isCrossProcess() const = 0; - virtual void writePad32(const void* buffer, size_t bytes) = 0; virtual void writeByteArray(const void* data, size_t size) = 0; @@ -77,18 +75,10 @@ protected: */ class SkBinaryWriteBuffer : public SkWriteBuffer { public: - enum Flags { - kCrossProcess_Flag = 1 << 0, - }; - - SkBinaryWriteBuffer(uint32_t flags = 0); - SkBinaryWriteBuffer(void* initialStorage, size_t storageSize, uint32_t flags = 0); + SkBinaryWriteBuffer(); + SkBinaryWriteBuffer(void* initialStorage, size_t storageSize); ~SkBinaryWriteBuffer() override; - bool isCrossProcess() const override { - return SkToBool(fFlags & kCrossProcess_Flag); - } - void write(const void* buffer, size_t bytes) { fWriter.write(buffer, bytes); } @@ -135,7 +125,6 @@ public: SkRefCntSet* setTypefaceRecorder(SkRefCntSet*); private: - const uint32_t fFlags; SkFactorySet* fFactorySet; SkWriter32 fWriter; diff --git a/src/effects/SkPictureImageFilter.cpp b/src/effects/SkPictureImageFilter.cpp index 709a0901b6..2596f41426 100644 --- a/src/effects/SkPictureImageFilter.cpp +++ b/src/effects/SkPictureImageFilter.cpp @@ -56,7 +56,7 @@ sk_sp SkPictureImageFilter::CreateProc(SkReadBuffer& buffer) { sk_sp picture; SkRect cropRect; - if (buffer.isCrossProcess() && SkPicture::PictureIOSecurityPrecautionsEnabled()) { + if (SkPicture::PictureIOSecurityPrecautionsEnabled()) { buffer.validate(!buffer.readBool()); } else { if (buffer.readBool()) { @@ -77,7 +77,7 @@ sk_sp SkPictureImageFilter::CreateProc(SkReadBuffer& buffer) { } void SkPictureImageFilter::flatten(SkWriteBuffer& buffer) const { - if (buffer.isCrossProcess() && SkPicture::PictureIOSecurityPrecautionsEnabled()) { + if (SkPicture::PictureIOSecurityPrecautionsEnabled()) { buffer.writeBool(false); } else { bool hasPicture = (fPicture != nullptr); diff --git a/src/shaders/SkPictureShader.cpp b/src/shaders/SkPictureShader.cpp index 677c0e15dc..40724357e7 100644 --- a/src/shaders/SkPictureShader.cpp +++ b/src/shaders/SkPictureShader.cpp @@ -162,9 +162,7 @@ sk_sp SkPictureShader::CreateProc(SkReadBuffer& buffer) { sk_sp picture; - if (buffer.isCrossProcess() && SkPicture::PictureIOSecurityPrecautionsEnabled()) { - // Newer code won't serialize pictures in disallow-cross-process-picture mode. - // Assert that they didn't serialize anything except a false here. + if (SkPicture::PictureIOSecurityPrecautionsEnabled()) { buffer.validate(!buffer.readBool()); } else { bool didSerialize = buffer.readBool(); @@ -183,7 +181,7 @@ void SkPictureShader::flatten(SkWriteBuffer& buffer) const { // The deserialization code won't trust that our serialized picture is safe to deserialize. // So write a 'false' telling it that we're not serializing a picture. - if (buffer.isCrossProcess() && SkPicture::PictureIOSecurityPrecautionsEnabled()) { + if (SkPicture::PictureIOSecurityPrecautionsEnabled()) { buffer.writeBool(false); } else { buffer.writeBool(true); diff --git a/tools/debugger/SkJsonWriteBuffer.h b/tools/debugger/SkJsonWriteBuffer.h index efd896c989..0e0d5d60e1 100644 --- a/tools/debugger/SkJsonWriteBuffer.h +++ b/tools/debugger/SkJsonWriteBuffer.h @@ -21,8 +21,6 @@ public: : fUrlDataManager(urlDataManager) , fJson(Json::objectValue) {} - bool isCrossProcess() const override { return false; } - void writePad32(const void* buffer, size_t bytes) override; void writeByteArray(const void* data, size_t size) override; void writeBool(bool value) override; diff --git a/tools/skpinfo.cpp b/tools/skpinfo.cpp index 11b1bb6585..865492d58c 100644 --- a/tools/skpinfo.cpp +++ b/tools/skpinfo.cpp @@ -64,28 +64,6 @@ int main(int argc, char** argv) { info.fCullRect.fLeft, info.fCullRect.fTop, info.fCullRect.fRight, info.fCullRect.fBottom); } - if (FLAGS_flags && !FLAGS_quiet) { - SkDebugf("Flags: "); - bool needsSeparator = false; - if (info.fFlags & SkPictInfo::kCrossProcess_Flag) { - SkDebugf("kCrossProcess"); - needsSeparator = true; - } - if (info.fFlags & SkPictInfo::kScalarIsFloat_Flag) { - if (needsSeparator) { - SkDebugf("|"); - } - SkDebugf("kScalarIsFloat"); - needsSeparator = true; - } - if (info.fFlags & SkPictInfo::kPtrIs64Bit_Flag) { - if (needsSeparator) { - SkDebugf("|"); - } - SkDebugf("kPtrIs64Bit"); - } - SkDebugf("\n"); - } if (!stream.readBool()) { // If we read true there's a picture playback object flattened