Fixing SkPicture serialization

Fixed a few issues while attempting to use the new
serialization path for SkPicture inside a fuzzer:
- SkReadBuffer and SkValidatingReadBuffer both had a fReader
member instead of sharing the same member, which leads to
problems if a base class function is used
- In SkPicture, a header is now written as a single chunk of
data, so it also has to be read as a single chunk of data
- In the SkPicturePlayback destructor, a bad deserialization
would lead to a crash if we don't safely unref fOpData
- Also in SkPicturePlayback, if we only use a ReadBuffer for
the whole deserialization, additional tags must be added to
parseBufferTag()
- SkValidatingReadBuffer::readBitmap() was broken, but this
path wasn't usen't since the only use case for
SkValidatingReadBuffer is currently image filters and
bitmaps are unflattened as part of the deserialization of
SkBitmapSource
- SkPictureImageFilter was not deserializable. Added it to
SkGlobalInitialization*
- Added a test that exercises the SkPicture serialization /
deserialization code

BUG=skia:
R=senorblanco@google.com, senorblanco@chromium.org, reed@google.com, robertphillips@google.com

Author: sugoi@chromium.org

Review URL: https://codereview.chromium.org/195223003

git-svn-id: http://skia.googlecode.com/svn/trunk@13764 2bbb7eff-a529-9590-31e7-b0007b416f81
This commit is contained in:
commit-bot@chromium.org 2014-03-12 14:46:41 +00:00
parent b6401863fb
commit 9e5f85e89d
10 changed files with 153 additions and 38 deletions

View File

@ -274,7 +274,8 @@ protected:
// SkBBoxHierarchy implementation
virtual SkBBoxHierarchy* createBBoxHierarchy() const;
private:
void createHeader(void* header) const;
void createHeader(SkPictInfo* info) const;
static bool IsValidPictInfo(const SkPictInfo& info);
friend class SkFlatPicture;
friend class SkPicturePlayback;

View File

@ -177,12 +177,14 @@ public:
virtual bool isValid() const { return true; }
virtual bool validateAvailable(size_t size) { return true; }
protected:
SkReader32 fReader;
private:
bool readArray(void* value, size_t size, size_t elementSize);
uint32_t fFlags;
SkReader32 fReader;
void* fMemoryPtr;
SkBitmapHeapReader* fBitmapStorage;

View File

@ -260,7 +260,19 @@ void SkPicture::draw(SkCanvas* surface, SkDrawPictureCallback* callback) {
#include "SkStream.h"
static const char kMagic[] = { 's', 'k', 'i', 'a', 'p', 'i', 'c', 't' };
static const size_t kHeaderSize = sizeof(kMagic) + sizeof(SkPictInfo);
bool SkPicture::IsValidPictInfo(const SkPictInfo& info) {
if (0 != memcmp(info.fMagic, kMagic, sizeof(kMagic))) {
return false;
}
if (info.fVersion < MIN_PICTURE_VERSION ||
info.fVersion > CURRENT_PICTURE_VERSION) {
return false;
}
return true;
}
bool SkPicture::InternalOnly_StreamIsSKP(SkStream* stream, SkPictInfo* pInfo) {
if (NULL == stream) {
@ -268,18 +280,9 @@ bool SkPicture::InternalOnly_StreamIsSKP(SkStream* stream, SkPictInfo* pInfo) {
}
// Check magic bytes.
char magic[sizeof(kMagic)];
if (!stream->read(magic, sizeof(kMagic)) ||
(0 != memcmp(magic, kMagic, sizeof(kMagic)))) {
return false;
}
SkPictInfo info;
if (!stream->read(&info, sizeof(SkPictInfo))) {
return false;
}
if (info.fVersion < MIN_PICTURE_VERSION || info.fVersion > CURRENT_PICTURE_VERSION) {
SkASSERT(sizeof(kMagic) == sizeof(info.fMagic));
if (!stream->read(&info, sizeof(info)) || !IsValidPictInfo(info)) {
return false;
}
@ -291,19 +294,9 @@ bool SkPicture::InternalOnly_StreamIsSKP(SkStream* stream, SkPictInfo* pInfo) {
bool SkPicture::InternalOnly_BufferIsSKP(SkReadBuffer& buffer, SkPictInfo* pInfo) {
// Check magic bytes.
char magic[sizeof(kMagic)];
if (!buffer.readByteArray(magic, sizeof(kMagic)) ||
(0 != memcmp(magic, kMagic, sizeof(kMagic)))) {
return false;
}
SkPictInfo info;
if (!buffer.readByteArray(&info, sizeof(SkPictInfo))) {
return false;
}
if (info.fVersion < MIN_PICTURE_VERSION || info.fVersion > CURRENT_PICTURE_VERSION) {
SkASSERT(sizeof(kMagic) == sizeof(info.fMagic));
if (!buffer.readByteArray(&info, sizeof(info)) || !IsValidPictInfo(info)) {
return false;
}
@ -361,13 +354,13 @@ SkPicture* SkPicture::CreateFromBuffer(SkReadBuffer& buffer) {
return SkNEW_ARGS(SkPicture, (playback, info.fWidth, info.fHeight));
}
void SkPicture::createHeader(void* header) const {
void SkPicture::createHeader(SkPictInfo* info) const {
// Copy magic bytes at the beginning of the header
SkASSERT(sizeof(kMagic) == 8);
memcpy(header, kMagic, sizeof(kMagic));
SkASSERT(sizeof(kMagic) == sizeof(info->fMagic));
memcpy(info->fMagic, kMagic, sizeof(kMagic));
// Set picture info after magic bytes in the header
SkPictInfo* info = (SkPictInfo*)(((char*)header) + sizeof(kMagic));
info->fVersion = CURRENT_PICTURE_VERSION;
info->fWidth = fWidth;
info->fHeight = fHeight;
@ -387,9 +380,9 @@ void SkPicture::serialize(SkWStream* stream, EncodeBitmap encoder) const {
playback = SkNEW_ARGS(SkPicturePlayback, (*fRecord));
}
char header[kHeaderSize];
SkPictInfo header;
this->createHeader(&header);
stream->write(header, kHeaderSize);
stream->write(&header, sizeof(header));
if (playback) {
stream->writeBool(true);
playback->serialize(stream, encoder);
@ -409,9 +402,9 @@ void SkPicture::flatten(SkWriteBuffer& buffer) const {
playback = SkNEW_ARGS(SkPicturePlayback, (*fRecord));
}
char header[kHeaderSize];
SkPictInfo header;
this->createHeader(&header);
buffer.writeByteArray(header, kHeaderSize);
buffer.writeByteArray(&header, sizeof(header));
if (playback) {
buffer.writeBool(true);
playback->flatten(buffer);

View File

@ -270,7 +270,7 @@ void SkPicturePlayback::init() {
}
SkPicturePlayback::~SkPicturePlayback() {
fOpData->unref();
SkSafeUnref(fOpData);
SkSafeUnref(fBitmaps);
SkSafeUnref(fPaints);
@ -612,6 +612,41 @@ bool SkPicturePlayback::parseBufferTag(SkReadBuffer& buffer,
fPathHeap.reset(SkNEW_ARGS(SkPathHeap, (buffer)));
}
break;
case SK_PICT_READER_TAG: {
SkAutoMalloc storage(size);
if (!buffer.readByteArray(storage.get(), size) ||
!buffer.validate(NULL == fOpData)) {
return false;
}
SkASSERT(NULL == fOpData);
fOpData = SkData::NewFromMalloc(storage.detach(), size);
} break;
case SK_PICT_PICTURE_TAG: {
if (!buffer.validate((0 == fPictureCount) && (NULL == fPictureRefs))) {
return false;
}
fPictureCount = size;
fPictureRefs = SkNEW_ARRAY(SkPicture*, fPictureCount);
bool success = true;
int i = 0;
for ( ; i < fPictureCount; i++) {
fPictureRefs[i] = SkPicture::CreateFromBuffer(buffer);
if (NULL == fPictureRefs[i]) {
success = false;
break;
}
}
if (!success) {
// Delete all of the pictures that were already created (up to but excluding i):
for (int j = 0; j < i; j++) {
fPictureRefs[j]->unref();
}
// Delete the array
SkDELETE_ARRAY(fPictureRefs);
fPictureCount = 0;
return false;
}
} break;
default:
// The tag was invalid.
return false;

View File

@ -40,6 +40,7 @@ struct SkPictInfo {
kPtrIs64Bit_Flag = 1 << 2,
};
char fMagic[8];
uint32_t fVersion;
uint32_t fWidth;
uint32_t fHeight;

View File

@ -213,10 +213,10 @@ uint32_t SkValidatingReadBuffer::getArrayCount() {
void SkValidatingReadBuffer::readBitmap(SkBitmap* bitmap) {
const int width = this->readInt();
const int height = this->readInt();
const bool useBitmapHeap = this->readBool();
const size_t length = this->readUInt();
// A size of zero means the SkBitmap was simply flattened.
this->validate(length == 0);
if (fError) {
if (!this->validate(!useBitmapHeap && (0 == length))) {
return;
}
bitmap->unflatten(*this);

View File

@ -75,7 +75,6 @@ private:
return SkIsAlign4((uintptr_t)ptr);
}
SkReader32 fReader;
bool fError;
typedef SkReadBuffer INHERITED;

View File

@ -50,6 +50,7 @@
#include "SkOffsetImageFilter.h"
#include "SkOnce.h"
#include "SkPerlinNoiseShader.h"
#include "SkPictureImageFilter.h"
#include "SkPixelXorXfermode.h"
#include "SkRectShaderImageFilter.h"
#include "SkResizeImageFilter.h"
@ -88,6 +89,7 @@ static void InitializeFlattenables(int*) {
SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkLine2DPathEffect)
SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkPath2DPathEffect)
SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkPerlinNoiseShader)
SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkPictureImageFilter)
SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkPixelXorXfermode)
SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkRectShaderImageFilter)
SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkResizeImageFilter)

View File

@ -50,6 +50,7 @@
#include "SkOffsetImageFilter.h"
#include "SkOnce.h"
#include "SkPerlinNoiseShader.h"
#include "SkPictureImageFilter.h"
#include "SkPixelXorXfermode.h"
#include "SkRectShaderImageFilter.h"
#include "SkResizeImageFilter.h"
@ -88,6 +89,7 @@ static void InitializeFlattenables(int*) {
SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkLine2DPathEffect)
SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkPath2DPathEffect)
SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkPerlinNoiseShader)
SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkPictureImageFilter)
SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkPixelXorXfermode)
SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkRectShaderImageFilter)
SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkResizeImageFilter)

View File

@ -15,6 +15,7 @@
#include "Test.h"
static const uint32_t kArraySize = 64;
static const int kBitmapSize = 256;
template<typename T>
static void TestAlignment(T* testObj, skiatest::Reporter* reporter) {
@ -229,6 +230,64 @@ static void TestBitmapSerialization(const SkBitmap& validBitmap,
}
}
static bool setup_bitmap_for_canvas(SkBitmap* bitmap) {
SkImageInfo info = SkImageInfo::Make(
kBitmapSize, kBitmapSize, kPMColor_SkColorType, kPremul_SkAlphaType);
return bitmap->allocPixels(info);
}
static bool make_checkerboard_bitmap(SkBitmap& bitmap) {
bool success = setup_bitmap_for_canvas(&bitmap);
SkCanvas canvas(bitmap);
canvas.clear(0x00000000);
SkPaint darkPaint;
darkPaint.setColor(0xFF804020);
SkPaint lightPaint;
lightPaint.setColor(0xFF244484);
const int i = kBitmapSize / 8;
const SkScalar f = SkIntToScalar(i);
for (int y = 0; y < kBitmapSize; y += i) {
for (int x = 0; x < kBitmapSize; x += i) {
canvas.save();
canvas.translate(SkIntToScalar(x), SkIntToScalar(y));
canvas.drawRect(SkRect::MakeXYWH(0, 0, f, f), darkPaint);
canvas.drawRect(SkRect::MakeXYWH(f, 0, f, f), lightPaint);
canvas.drawRect(SkRect::MakeXYWH(0, f, f, f), lightPaint);
canvas.drawRect(SkRect::MakeXYWH(f, f, f, f), darkPaint);
canvas.restore();
}
}
return success;
}
static bool drawSomething(SkCanvas* canvas) {
SkPaint paint;
SkBitmap bitmap;
bool success = make_checkerboard_bitmap(bitmap);
canvas->save();
canvas->scale(0.5f, 0.5f);
canvas->drawBitmap(bitmap, 0, 0, NULL);
canvas->restore();
const char beforeStr[] = "before circle";
const char afterStr[] = "after circle";
paint.setAntiAlias(true);
paint.setColor(SK_ColorRED);
canvas->drawData(beforeStr, sizeof(beforeStr));
canvas->drawCircle(SkIntToScalar(kBitmapSize/2), SkIntToScalar(kBitmapSize/2), SkIntToScalar(kBitmapSize/3), paint);
canvas->drawData(afterStr, sizeof(afterStr));
paint.setColor(SK_ColorBLACK);
paint.setTextSize(SkIntToScalar(kBitmapSize/3));
canvas->drawText("Picture", 7, SkIntToScalar(kBitmapSize/2), SkIntToScalar(kBitmapSize/4), paint);
return success;
}
DEF_TEST(Serialization, reporter) {
// Test matrix serialization
{
@ -292,7 +351,7 @@ DEF_TEST(Serialization, reporter) {
// Test invalid deserializations
{
SkImageInfo info = SkImageInfo::MakeN32Premul(256, 256);
SkImageInfo info = SkImageInfo::MakeN32Premul(kBitmapSize, kBitmapSize);
SkBitmap validBitmap;
validBitmap.setConfig(info);
@ -306,4 +365,25 @@ DEF_TEST(Serialization, reporter) {
// even when the device fails to initialize, due to its size
TestBitmapSerialization(validBitmap, invalidBitmap, true, reporter);
}
// Test simple SkPicture serialization
{
SkPicture* pict = new SkPicture;
SkAutoUnref aur(pict);
bool didDraw = drawSomething(pict->beginRecording(kBitmapSize, kBitmapSize));
REPORTER_ASSERT(reporter, didDraw);
pict->endRecording();
// Serialize picture
SkWriteBuffer writer(SkWriteBuffer::kValidation_Flag);
pict->flatten(writer);
size_t size = writer.bytesWritten();
void* data = sk_malloc_throw(size);
writer.writeToMemory(data);
// Deserialize picture
SkValidatingReadBuffer reader(data, size);
SkPicture* readPict(SkPicture::CreateFromBuffer(reader));
REPORTER_ASSERT(reporter, NULL != readPict);
}
}