Skip to content

Commit

Permalink
Make SkTypefacePlayback use smart pointers.
Browse files Browse the repository at this point in the history
This clarifies the ownership of the typefaces as well as removes a use
of SkRefCnt_SafeAssign which is currently a code smell.

Change-Id: I8fec541f71f555c2182b77870979ece87b501901
Reviewed-on: https://skia-review.googlesource.com/140249
Reviewed-by: Herb Derby <[email protected]>
Commit-Queue: Ben Wagner <[email protected]>
  • Loading branch information
Ben Wagner authored and Skia Commit-Bot committed Jul 10, 2018
1 parent 1994f20 commit 5d94822
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 67 deletions.
21 changes: 3 additions & 18 deletions src/core/SkPictureData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -258,17 +258,6 @@ bool SkPictureData::parseStreamTag(SkStream* stream,
uint32_t size,
const SkDeserialProcs& procs,
SkTypefacePlayback* topLevelTFPlayback) {
/*
* By the time we encounter BUFFER_SIZE_TAG, we need to have already seen
* its dependents: FACTORY_TAG and TYPEFACE_TAG. These two are not required
* but if they are present, they need to have been seen before the buffer.
*
* We assert that if/when we see either of these, that we have not yet seen
* the buffer tag, because if we have, then its too-late to deal with the
* factories or typefaces.
*/
SkDEBUGCODE(bool haveBuffer = false;)

switch (tag) {
case SK_PICT_READER_TAG:
SkASSERT(nullptr == fOpData);
Expand All @@ -278,7 +267,6 @@ bool SkPictureData::parseStreamTag(SkStream* stream,
}
break;
case SK_PICT_FACTORY_TAG: {
SkASSERT(!haveBuffer);
if (!stream->readU32(&size)) { return false; }
fFactoryPlayback = skstd::make_unique<SkFactoryPlayback>(size);
for (size_t i = 0; i < size; i++) {
Expand All @@ -293,17 +281,15 @@ bool SkPictureData::parseStreamTag(SkStream* stream,
}
} break;
case SK_PICT_TYPEFACE_TAG: {
SkASSERT(!haveBuffer);
const int count = SkToInt(size);
fTFPlayback.setCount(count);
for (int i = 0; i < count; i++) {
fTFPlayback.setCount(size);
for (uint32_t i = 0; i < size; ++i) {
sk_sp<SkTypeface> tf(SkTypeface::MakeDeserialize(stream));
if (!tf.get()) { // failed to deserialize
// fTFPlayback asserts it never has a null, so we plop in
// the default here.
tf = SkTypeface::MakeDefault();
}
fTFPlayback.set(i, tf.get());
fTFPlayback[i] = std::move(tf);
}
} break;
case SK_PICT_PICTURE_TAG: {
Expand Down Expand Up @@ -349,7 +335,6 @@ bool SkPictureData::parseStreamTag(SkStream* stream,
if (!buffer.isValid()) {
return false;
}
SkDEBUGCODE(haveBuffer = true;)
} break;
}
return true; // success
Expand Down
39 changes: 3 additions & 36 deletions src/core/SkPictureFlat.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,42 +15,9 @@

///////////////////////////////////////////////////////////////////////////////

SkTypefacePlayback::SkTypefacePlayback() : fCount(0), fArray(nullptr) {}

SkTypefacePlayback::~SkTypefacePlayback() {
this->reset(nullptr);
}

void SkTypefacePlayback::reset(const SkRefCntSet* rec) {
for (int i = 0; i < fCount; i++) {
SkASSERT(fArray[i]);
fArray[i]->unref();
}
delete[] fArray;

if (rec!= nullptr && rec->count() > 0) {
fCount = rec->count();
fArray = new SkRefCnt* [fCount];
rec->copyToArray(fArray);
for (int i = 0; i < fCount; i++) {
fArray[i]->ref();
}
} else {
fCount = 0;
fArray = nullptr;
}
}

void SkTypefacePlayback::setCount(int count) {
this->reset(nullptr);
SkTypefacePlayback::~SkTypefacePlayback() {}

void SkTypefacePlayback::setCount(size_t count) {
fCount = count;
fArray = new SkRefCnt* [count];
sk_bzero(fArray, count * sizeof(SkRefCnt*));
}

SkRefCnt* SkTypefacePlayback::set(int index, SkRefCnt* obj) {
SkASSERT((unsigned)index < (unsigned)fCount);
SkRefCnt_SafeAssign(fArray[index], obj);
return obj;
fArray.reset(new sk_sp<SkTypeface>[count]);
}
20 changes: 11 additions & 9 deletions src/core/SkPictureFlat.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,23 +155,25 @@ static inline bool ClipParams_unpackDoAA(uint32_t packed) {

class SkTypefacePlayback {
public:
SkTypefacePlayback();
virtual ~SkTypefacePlayback();
SkTypefacePlayback() : fCount(0), fArray(nullptr) {}
~SkTypefacePlayback();

int count() const { return fCount; }
void setCount(size_t count);

void reset(const SkRefCntSet*);
size_t count() const { return fCount; }

void setCount(int count);
SkRefCnt* set(int index, SkRefCnt*);
sk_sp<SkTypeface>& operator[](size_t index) {
SkASSERT(index < fCount);
return fArray[index];
}

void setupBuffer(SkReadBuffer& buffer) const {
buffer.setTypefaceArray((SkTypeface**)fArray, fCount);
buffer.setTypefaceArray(fArray.get(), fCount);
}

protected:
int fCount;
SkRefCnt** fArray;
size_t fCount;
std::unique_ptr<sk_sp<SkTypeface>[]> fArray;
};

class SkFactoryPlayback {
Expand Down
2 changes: 1 addition & 1 deletion src/core/SkReadBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ sk_sp<SkTypeface> SkReadBuffer::readTypeface() {
if (!this->validate(index <= fTFCount)) {
return nullptr;
}
return sk_ref_sp(fTFArray[index - 1]);
return fTFArray[index - 1];
} else { // custom
size_t size = sk_negate_to_size_t(index);
const void* data = this->skip(size);
Expand Down
6 changes: 3 additions & 3 deletions src/core/SkReadBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ class SkReadBuffer {
sk_sp<SkImage> readImage();
sk_sp<SkTypeface> readTypeface();

void setTypefaceArray(SkTypeface* array[], int count) {
void setTypefaceArray(sk_sp<SkTypeface> array[], int count) {
fTFArray = array;
fTFCount = count;
}
Expand Down Expand Up @@ -282,8 +282,8 @@ class SkReadBuffer {

void* fMemoryPtr;

SkTypeface** fTFArray;
int fTFCount;
sk_sp<SkTypeface>* fTFArray;
int fTFCount;

SkFlattenable::Factory* fFactoryArray;
int fFactoryCount;
Expand Down

0 comments on commit 5d94822

Please sign in to comment.