Convert SkPicture's generation ID to a unique ID

This CL addresses linger code review comments on r14037 (Add generation ID to SkPicture https://codereview.chromium.org/222683002/)

R=reed@google.com

Author: robertphillips@google.com

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

git-svn-id: http://skia.googlecode.com/svn/trunk@14079 2bbb7eff-a529-9590-31e7-b0007b416f81
This commit is contained in:
commit-bot@chromium.org 2014-04-07 18:26:22 +00:00
parent c96268d792
commit 2b4e370a2f
6 changed files with 43 additions and 41 deletions

View File

@ -201,14 +201,12 @@ public:
*/
int height() const { return fHeight; }
static const uint32_t kInvalidGenID = 0;
/** Return a non-zero, unique value representing the picture. This call is
only valid when not recording. Between a beginRecording/endRecording
pair it will just return 0 (the invalid gen ID). Each beginRecording/
pair it will just return 0 (the invalid ID). Each beginRecording/
endRecording pair will cause a different generation ID to be returned.
*/
uint32_t getGenerationID() const;
uint32_t uniqueID() const;
/**
* Function to encode an SkBitmap to an SkData. A function with this
@ -308,7 +306,7 @@ protected:
static const uint32_t MIN_PICTURE_VERSION = 19;
static const uint32_t CURRENT_PICTURE_VERSION = 22;
mutable uint32_t fGenerationID;
mutable uint32_t fUniqueID;
// fPlayback, fRecord, fWidth & fHeight are protected to allow derived classes to
// install their own SkPicturePlayback-derived players,SkPictureRecord-derived
@ -318,7 +316,7 @@ protected:
int fWidth, fHeight;
const AccelData* fAccelData;
void needsNewGenID() { fGenerationID = kInvalidGenID; }
void needsNewGenID() { fUniqueID = SK_InvalidGenID; }
// Create a new SkPicture from an existing SkPicturePlayback. Ref count of
// playback is unchanged.

View File

@ -315,6 +315,10 @@ typedef uint32_t SkMSec;
*/
#define SkMSec_LE(a, b) ((int32_t)(a) - (int32_t)(b) <= 0)
/** The generation IDs in Skia reserve 0 has an invalid marker.
*/
#define SK_InvalidGenID 0
/****************************************************************************
The rest of these only build with C++
*/

View File

@ -140,7 +140,7 @@ SkPicture::SkPicture(const SkPicture& src)
if (src.fPlayback) {
fPlayback = SkNEW_ARGS(SkPicturePlayback, (*src.fPlayback));
SkASSERT(NULL == src.fRecord);
fGenerationID = src.getGenerationID(); // need to call method to ensure != 0
fUniqueID = src.uniqueID(); // need to call method to ensure != 0
} else if (src.fRecord) {
SkPictInfo info;
this->createHeader(&info);
@ -164,7 +164,7 @@ void SkPicture::internalOnly_EnableOpts(bool enableOpts) {
}
void SkPicture::swap(SkPicture& other) {
SkTSwap(fGenerationID, other.fGenerationID);
SkTSwap(fUniqueID, other.fUniqueID);
SkTSwap(fRecord, other.fRecord);
SkTSwap(fPlayback, other.fPlayback);
SkTSwap(fAccelData, other.fAccelData);
@ -199,7 +199,7 @@ void SkPicture::clone(SkPicture* pictures, int count) const {
if (fPlayback) {
clone->fPlayback = SkNEW_ARGS(SkPicturePlayback, (*fPlayback, &copyInfo));
SkASSERT(NULL == fRecord);
clone->fGenerationID = this->getGenerationID(); // need to call method to ensure != 0
clone->fUniqueID = this->uniqueID(); // need to call method to ensure != 0
} else if (fRecord) {
// here we do a fake src.endRecording()
clone->fPlayback = SkNEW_ARGS(SkPicturePlayback, (*fRecord, info, true));
@ -506,18 +506,18 @@ static int32_t next_picture_generation_id() {
int32_t genID;
do {
genID = sk_atomic_inc(&gPictureGenerationID) + 1;
} while (((int32_t)SkPicture::kInvalidGenID) == genID);
} while (SK_InvalidGenID == genID);
return genID;
}
uint32_t SkPicture::getGenerationID() const {
uint32_t SkPicture::uniqueID() const {
if (NULL != fRecord) {
SkASSERT(NULL == fPlayback);
return kInvalidGenID;
return SK_InvalidGenID;
}
if (kInvalidGenID == fGenerationID) {
fGenerationID = next_picture_generation_id();
if (SK_InvalidGenID == fUniqueID) {
fUniqueID = next_picture_generation_id();
}
return fGenerationID;
return fUniqueID;
}

View File

@ -69,16 +69,16 @@ void GrLayerCache::freeAll() {
GrAtlasedLayer* GrLayerCache::createLayer(SkPicture* picture, int layerID) {
GrAtlasedLayer* layer = fLayerPool.alloc();
SkASSERT(picture->getGenerationID() != SkPicture::kInvalidGenID);
layer->init(picture->getGenerationID(), layerID);
fLayerHash.insert(PictureLayerKey(picture->getGenerationID(), layerID), layer);
SkASSERT(picture->uniqueID() != SK_InvalidGenID);
layer->init(picture->uniqueID(), layerID);
fLayerHash.insert(PictureLayerKey(picture->uniqueID(), layerID), layer);
return layer;
}
const GrAtlasedLayer* GrLayerCache::findLayerOrCreate(SkPicture* picture, int layerID) {
SkASSERT(picture->getGenerationID() != SkPicture::kInvalidGenID);
GrAtlasedLayer* layer = fLayerHash.find(PictureLayerKey(picture->getGenerationID(), layerID));
SkASSERT(picture->uniqueID() != SK_InvalidGenID);
GrAtlasedLayer* layer = fLayerHash.find(PictureLayerKey(picture->uniqueID(), layerID));
if (NULL == layer) {
layer = this->createLayer(picture, layerID);
}

View File

@ -47,7 +47,7 @@ private:
// It is roughly equivalent to a GrGlyph in the font caching system
class GrAtlasedLayer {
public:
GrAtlasedLayer() : fPictureID(SkPicture::kInvalidGenID) { }
GrAtlasedLayer() : fPictureID(SK_InvalidGenID) { }
uint32_t pictureID() const { return fPictureID; }
int layerID() const { return fLayerID; }

View File

@ -1146,52 +1146,52 @@ static void test_gen_id(skiatest::Reporter* reporter) {
SkPicture hasData, empty, midRecord;
uint32_t beforeID = hasData.getGenerationID();
REPORTER_ASSERT(reporter, SkPicture::kInvalidGenID != beforeID);
uint32_t beforeID = hasData.uniqueID();
REPORTER_ASSERT(reporter, SK_InvalidGenID != beforeID);
// all 3 pictures should have different ids
REPORTER_ASSERT(reporter, beforeID != empty.getGenerationID());
REPORTER_ASSERT(reporter, beforeID != midRecord.getGenerationID());
REPORTER_ASSERT(reporter, empty.getGenerationID() != midRecord.getGenerationID());
REPORTER_ASSERT(reporter, beforeID != empty.uniqueID());
REPORTER_ASSERT(reporter, beforeID != midRecord.uniqueID());
REPORTER_ASSERT(reporter, empty.uniqueID() != midRecord.uniqueID());
hasData.beginRecording(1, 1);
// gen ID should be invalid mid-record
REPORTER_ASSERT(reporter, SkPicture::kInvalidGenID == hasData.getGenerationID());
REPORTER_ASSERT(reporter, SK_InvalidGenID == hasData.uniqueID());
hasData.endRecording();
// picture should get a new (non-zero) id after recording
REPORTER_ASSERT(reporter, hasData.getGenerationID() != beforeID);
REPORTER_ASSERT(reporter, hasData.getGenerationID() != SkPicture::kInvalidGenID);
REPORTER_ASSERT(reporter, hasData.uniqueID() != beforeID);
REPORTER_ASSERT(reporter, hasData.uniqueID() != SK_InvalidGenID);
midRecord.beginRecording(1, 1);
REPORTER_ASSERT(reporter, SkPicture::kInvalidGenID == midRecord.getGenerationID());
REPORTER_ASSERT(reporter, SK_InvalidGenID == midRecord.uniqueID());
// test out copy constructor
SkPicture copyWithData(hasData);
REPORTER_ASSERT(reporter, hasData.getGenerationID() == copyWithData.getGenerationID());
REPORTER_ASSERT(reporter, hasData.uniqueID() == copyWithData.uniqueID());
SkPicture emptyCopy(empty);
REPORTER_ASSERT(reporter, empty.getGenerationID() != emptyCopy.getGenerationID());
REPORTER_ASSERT(reporter, empty.uniqueID() != emptyCopy.uniqueID());
SkPicture copyMidRecord(midRecord);
REPORTER_ASSERT(reporter, midRecord.getGenerationID() != copyMidRecord.getGenerationID());
REPORTER_ASSERT(reporter, copyMidRecord.getGenerationID() != SkPicture::kInvalidGenID);
REPORTER_ASSERT(reporter, midRecord.uniqueID() != copyMidRecord.uniqueID());
REPORTER_ASSERT(reporter, copyMidRecord.uniqueID() != SK_InvalidGenID);
// test out swap
beforeID = copyMidRecord.getGenerationID();
beforeID = copyMidRecord.uniqueID();
copyWithData.swap(copyMidRecord);
REPORTER_ASSERT(reporter, copyWithData.getGenerationID() == beforeID);
REPORTER_ASSERT(reporter, copyMidRecord.getGenerationID() == hasData.getGenerationID());
REPORTER_ASSERT(reporter, copyWithData.uniqueID() == beforeID);
REPORTER_ASSERT(reporter, copyMidRecord.uniqueID() == hasData.uniqueID());
// test out clone
SkAutoTUnref<SkPicture> cloneWithData(hasData.clone());
REPORTER_ASSERT(reporter, hasData.getGenerationID() == cloneWithData->getGenerationID());
REPORTER_ASSERT(reporter, hasData.uniqueID() == cloneWithData->uniqueID());
SkAutoTUnref<SkPicture> emptyClone(empty.clone());
REPORTER_ASSERT(reporter, empty.getGenerationID() != emptyClone->getGenerationID());
REPORTER_ASSERT(reporter, empty.uniqueID() != emptyClone->uniqueID());
SkAutoTUnref<SkPicture> cloneMidRecord(midRecord.clone());
REPORTER_ASSERT(reporter, midRecord.getGenerationID() != cloneMidRecord->getGenerationID());
REPORTER_ASSERT(reporter, cloneMidRecord->getGenerationID() != SkPicture::kInvalidGenID);
REPORTER_ASSERT(reporter, midRecord.uniqueID() != cloneMidRecord->uniqueID());
REPORTER_ASSERT(reporter, cloneMidRecord->uniqueID() != SK_InvalidGenID);
}
DEF_TEST(Picture, reporter) {