This CL improves saveLayer handling in the SkMatrixClipStateMgr by:

1) no longer storing the clip skip offsets in the stack (since saveLayers can force multiple clip states to be open at one time)

2) writing out only the clips that are relative to the saveLayer's clip state

3) updates the testing harness to accept a save/restore bracketing a saveLayer/restore (since clips have to be applied to the saveLayer's result upon restore)

R=bsalomon@google.com, epoger@google.com

Author: robertphillips@google.com

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

git-svn-id: http://skia.googlecode.com/svn/trunk@13497 2bbb7eff-a529-9590-31e7-b0007b416f81
This commit is contained in:
commit-bot@chromium.org 2014-02-19 15:11:23 +00:00
parent cf52f5b726
commit 92da60cd63
4 changed files with 165 additions and 128 deletions

View File

@ -21,7 +21,6 @@ bool SkMatrixClipStateMgr::MatrixClipState::ClipInfo::clipPath(SkPictureRecord*
newClip->fOp = op;
newClip->fDoAA = doAA;
newClip->fMatrixID = matrixID;
newClip->fOffset = kInvalidJumpOffset;
return false;
}
@ -36,7 +35,6 @@ bool SkMatrixClipStateMgr::MatrixClipState::ClipInfo::clipRegion(SkPictureRecord
newClip->fOp = op;
newClip->fDoAA = true; // not necessary but sanity preserving
newClip->fMatrixID = matrixID;
newClip->fOffset = kInvalidJumpOffset;
return false;
}
@ -55,17 +53,10 @@ void SkMatrixClipStateMgr::writeDeltaMat(int currentMatID, int desiredMatID) {
// Note: this only writes out the clips for the current save state. To get the
// entire clip stack requires iterating of the entire matrix/clip stack.
void SkMatrixClipStateMgr::MatrixClipState::ClipInfo::writeClip(int* curMatID,
SkMatrixClipStateMgr* mgr,
bool* overrideFirstOp) {
SkMatrixClipStateMgr* mgr) {
for (int i = 0; i < fClips.count(); ++i) {
ClipOp& curClip = fClips[i];
SkRegion::Op op = curClip.fOp;
if (*overrideFirstOp) {
op = SkRegion::kReplace_Op;
*overrideFirstOp = false;
}
// TODO: use the matrix ID to skip writing the identity matrix
// over and over, i.e.:
// if (*curMatID != curClip.fMatrixID) {
@ -79,43 +70,31 @@ void SkMatrixClipStateMgr::MatrixClipState::ClipInfo::writeClip(int* curMatID,
mgr->writeDeltaMat(*curMatID, curClip.fMatrixID);
*curMatID = curClip.fMatrixID;
int offset = 0;
switch (curClip.fClipType) {
case kRect_ClipType:
curClip.fOffset = mgr->getPicRecord()->recordClipRect(curClip.fGeom.fRRect.rect(),
op, curClip.fDoAA);
offset = mgr->getPicRecord()->recordClipRect(curClip.fGeom.fRRect.rect(),
curClip.fOp, curClip.fDoAA);
break;
case kRRect_ClipType:
curClip.fOffset = mgr->getPicRecord()->recordClipRRect(curClip.fGeom.fRRect, op,
curClip.fDoAA);
offset = mgr->getPicRecord()->recordClipRRect(curClip.fGeom.fRRect, curClip.fOp,
curClip.fDoAA);
break;
case kPath_ClipType:
curClip.fOffset = mgr->getPicRecord()->recordClipPath(curClip.fGeom.fPathID, op,
curClip.fDoAA);
offset = mgr->getPicRecord()->recordClipPath(curClip.fGeom.fPathID, curClip.fOp,
curClip.fDoAA);
break;
case kRegion_ClipType: {
const SkRegion* region = mgr->lookupRegion(curClip.fGeom.fRegionID);
curClip.fOffset = mgr->getPicRecord()->recordClipRegion(*region, op);
offset = mgr->getPicRecord()->recordClipRegion(*region, curClip.fOp);
break;
}
default:
SkASSERT(0);
}
}
}
// Fill in the skip offsets for all the clips written in the current block
void SkMatrixClipStateMgr::MatrixClipState::ClipInfo::fillInSkips(SkWriter32* writer,
int32_t restoreOffset) {
for (int i = 0; i < fClips.count(); ++i) {
ClipOp& curClip = fClips[i];
if (-1 == curClip.fOffset) {
continue;
}
// SkDEBUGCODE(uint32_t peek = writer->read32At(curClip.fOffset);)
// SkASSERT(-1 == peek);
writer->overwriteTAt(curClip.fOffset, restoreOffset);
SkDEBUGCODE(curClip.fOffset = -1;)
mgr->addClipOffset(offset);
}
}
@ -126,6 +105,8 @@ SkMatrixClipStateMgr::SkMatrixClipStateMgr()
sizeof(fMatrixClipStackStorage))
, fCurOpenStateID(kIdentityWideOpenStateID) {
fSkipOffsets = SkNEW(SkTDArray<int>);
// The first slot in the matrix dictionary is reserved for the identity matrix
fMatrixDict.append()->reset();
@ -137,12 +118,12 @@ SkMatrixClipStateMgr::~SkMatrixClipStateMgr() {
for (int i = 0; i < fRegionDict.count(); ++i) {
SkDELETE(fRegionDict[i]);
}
SkDELETE(fSkipOffsets);
}
int SkMatrixClipStateMgr::save(SkCanvas::SaveFlags flags) {
SkDEBUGCODE(this->validate();)
int SkMatrixClipStateMgr::MCStackPush(SkCanvas::SaveFlags flags) {
MatrixClipState* newTop = (MatrixClipState*)fMatrixClipStack.push_back();
new (newTop) MatrixClipState(fCurMCState, flags); // balanced in restore()
fCurMCState = newTop;
@ -152,14 +133,29 @@ int SkMatrixClipStateMgr::save(SkCanvas::SaveFlags flags) {
return fMatrixClipStack.count();
}
int SkMatrixClipStateMgr::save(SkCanvas::SaveFlags flags) {
SkDEBUGCODE(this->validate();)
return this->MCStackPush(flags);
}
int SkMatrixClipStateMgr::saveLayer(const SkRect* bounds, const SkPaint* paint,
SkCanvas::SaveFlags flags) {
int result = this->save(flags);
// Since the saveLayer call draws something we need to potentially dump
// out the MC state
this->call(kOther_CallType);
int result = this->MCStackPush(flags);
++fCurMCState->fLayerID;
fCurMCState->fIsSaveLayer = true;
fCurMCState->fSaveLayerBracketed = this->call(kOther_CallType);
fCurMCState->fSaveLayerBaseStateID = fCurOpenStateID;
fCurMCState->fSavedSkipOffsets = fSkipOffsets;
// TODO: recycle these rather then new & deleting them on every saveLayer/
// restore
fSkipOffsets = SkNEW(SkTDArray<int>);
fPicRecord->recordSaveLayer(bounds, paint,
(SkCanvas::SaveFlags)(flags| SkCanvas::kMatrixClip_SaveFlag));
return result;
@ -170,21 +166,20 @@ void SkMatrixClipStateMgr::restore() {
if (fCurMCState->fIsSaveLayer) {
if (fCurMCState->fSaveLayerBaseStateID != fCurOpenStateID) {
fPicRecord->recordRestore(); // Close the open block
fPicRecord->recordRestore(); // Close the open block inside the saveLayer
}
// The saveLayer's don't carry any matrix or clip state in the
// new scheme so make sure the saveLayer's recordRestore doesn't
// try to finalize them (i.e., fill in their skip offsets).
fPicRecord->recordRestore(false); // close of saveLayer
// Close the Save that brackets the saveLayer. TODO: this doesn't handle
// the skip offsets correctly
if (fCurMCState->fSaveLayerBracketed) {
fPicRecord->recordRestore(false);
}
fCurOpenStateID = fCurMCState->fSaveLayerBaseStateID;
// MC states can be allowed to fuse across saveLayer/restore boundaries
fCurOpenStateID = kIdentityWideOpenStateID;
SkASSERT(0 == fSkipOffsets->count());
SkASSERT(NULL != fCurMCState->fSavedSkipOffsets);
SkDELETE(fSkipOffsets);
fSkipOffsets = fCurMCState->fSavedSkipOffsets;
}
fCurMCState->~MatrixClipState(); // balanced in save()
@ -220,7 +215,11 @@ bool SkMatrixClipStateMgr::call(CallType callType) {
return false;
}
if (kIdentityWideOpenStateID != fCurOpenStateID) {
if (kIdentityWideOpenStateID != fCurOpenStateID &&
(!fCurMCState->fIsSaveLayer ||
fCurMCState->fSaveLayerBaseStateID != fCurOpenStateID)) {
// Don't write a restore if the open state is one in which a saveLayer
// is nested. The save after the saveLayer's restore will close it.
fPicRecord->recordRestore(); // Close the open block
}
@ -230,17 +229,40 @@ bool SkMatrixClipStateMgr::call(CallType callType) {
fPicRecord->recordSave(SkCanvas::kMatrixClip_SaveFlag);
// write out clips
SkDeque::F2BIter iter(fMatrixClipStack);
bool firstClip = true;
int curMatID = kIdentityMatID;
for (const MatrixClipState* state = (const MatrixClipState*) iter.next();
SkDeque::Iter iter(fMatrixClipStack, SkDeque::Iter::kBack_IterStart);
const MatrixClipState* state;
// Loop back across the MC states until the last saveLayer. The MC
// state in front of the saveLayer has already been written out.
for (state = (const MatrixClipState*) iter.prev();
state != NULL;
state = (const MatrixClipState*) iter.next()) {
state->fClipInfo->writeClip(&curMatID, this, &firstClip);
state = (const MatrixClipState*) iter.prev()) {
if (state->fIsSaveLayer) {
break;
}
}
if (NULL == state) {
// There was no saveLayer in the MC stack so we need to output them all
iter.reset(fMatrixClipStack, SkDeque::Iter::kFront_IterStart);
state = (const MatrixClipState*) iter.next();
} else {
// SkDeque's iterators actually return the previous location so we
// need to reverse and go forward one to get back on track.
iter.next();
SkDEBUGCODE(const MatrixClipState* test = (const MatrixClipState*)) iter.next();
SkASSERT(test == state);
}
int curMatID = NULL != state ? state->fMatrixInfo->getID(this) : kIdentityMatID;
for ( ; state != NULL; state = (const MatrixClipState*) iter.next()) {
state->fClipInfo->writeClip(&curMatID, this);
}
// write out matrix
// TODO: this test isn't quite right. It should be:
// if (curMatID != fCurMCState->fMatrixInfo->getID(this)) {
// but right now the testing harness always expects a matrix if
// the matrices are non-I
if (kIdentityMatID != fCurMCState->fMatrixInfo->getID(this)) {
// TODO: writing out the delta matrix here is an artifact of the writing
// out of the entire clip stack (with its matrices). Ultimately we will
@ -253,6 +275,17 @@ bool SkMatrixClipStateMgr::call(CallType callType) {
return true;
}
// Fill in the skip offsets for all the clips written in the current block
void SkMatrixClipStateMgr::fillInSkips(SkWriter32* writer, int32_t restoreOffset) {
for (int i = 0; i < fSkipOffsets->count(); ++i) {
SkDEBUGCODE(int32_t peek = writer->readTAt<int32_t>((*fSkipOffsets)[i]);)
SkASSERT(-1 == peek);
writer->overwriteTAt<int32_t>((*fSkipOffsets)[i], restoreOffset);
}
fSkipOffsets->rewind();
}
void SkMatrixClipStateMgr::finish() {
if (kIdentityWideOpenStateID != fCurOpenStateID) {
fPicRecord->recordRestore(); // Close the open block
@ -262,16 +295,23 @@ void SkMatrixClipStateMgr::finish() {
#ifdef SK_DEBUG
void SkMatrixClipStateMgr::validate() {
if (fCurOpenStateID == fCurMCState->fMCStateID) {
// The current state is the active one so all its skip offsets should
// still be -1
SkDeque::F2BIter iter(fMatrixClipStack);
for (const MatrixClipState* state = (const MatrixClipState*) iter.next();
if (fCurOpenStateID == fCurMCState->fMCStateID &&
(!fCurMCState->fIsSaveLayer ||
fCurOpenStateID != fCurMCState->fSaveLayerBaseStateID)) {
// The current state is the active one so it should have a skip
// offset for each clip
SkDeque::Iter iter(fMatrixClipStack, SkDeque::Iter::kBack_IterStart);
int clipCount = 0;
for (const MatrixClipState* state = (const MatrixClipState*) iter.prev();
state != NULL;
state = (const MatrixClipState*) iter.next()) {
state->fClipInfo->checkOffsetNotEqual(-1);
state = (const MatrixClipState*) iter.prev()) {
clipCount += state->fClipInfo->numClips();
if (state->fIsSaveLayer) {
break;
}
}
SkASSERT(fSkipOffsets->count() == clipCount);
}
}
#endif

View File

@ -120,7 +120,6 @@ public:
newClip->fOp = op;
newClip->fDoAA = doAA;
newClip->fMatrixID = matrixID;
newClip->fOffset = kInvalidJumpOffset;
return false;
}
@ -134,7 +133,6 @@ public:
newClip->fOp = op;
newClip->fDoAA = doAA;
newClip->fMatrixID = matrixID;
newClip->fOffset = kInvalidJumpOffset;
return false;
}
@ -147,19 +145,10 @@ public:
int regionID,
SkRegion::Op op,
int matrixID);
void writeClip(int* curMatID,
SkMatrixClipStateMgr* mgr,
bool* overrideFirstOp);
void fillInSkips(SkWriter32* writer, int32_t restoreOffset);
void writeClip(int* curMatID, SkMatrixClipStateMgr* mgr);
SkDEBUGCODE(int numClips() const { return fClips.count(); })
#ifdef SK_DEBUG
void checkOffsetNotEqual(int32_t offset) {
for (int i = 0; i < fClips.count(); ++i) {
ClipOp& curClip = fClips[i];
SkASSERT(offset != curClip.fOffset);
}
}
#endif
private:
enum ClipType {
kRect_ClipType,
@ -168,8 +157,6 @@ public:
kRegion_ClipType
};
static const int kInvalidJumpOffset = -1;
class ClipOp {
public:
ClipType fClipType;
@ -185,10 +172,6 @@ public:
// The CTM in effect when this clip call was issued
int fMatrixID;
// The offset of this clipOp's "jump-to-offset" location in the skp.
// -1 means the offset hasn't been written.
int32_t fOffset;
};
SkTDArray<ClipOp> fClips;
@ -249,7 +232,7 @@ public:
// The next two fields are only valid when fIsSaveLayer is set.
int32_t fSaveLayerBaseStateID;
bool fSaveLayerBracketed;
SkTDArray<int>* fSavedSkipOffsets;
#ifdef SK_DEBUG
MatrixClipState* fPrev; // debugging aid
@ -347,17 +330,7 @@ public:
bool call(CallType callType);
void fillInSkips(SkWriter32* writer, int32_t restoreOffset) {
// Since we write out the entire clip stack at each block start we
// need to update the skips for the entire stack each time too.
SkDeque::F2BIter iter(fMatrixClipStack);
for (const MatrixClipState* state = (const MatrixClipState*) iter.next();
state != NULL;
state = (const MatrixClipState*) iter.next()) {
state->fClipInfo->fillInSkips(writer, restoreOffset);
}
}
void fillInSkips(SkWriter32* writer, int32_t restoreOffset);
void finish();
@ -379,9 +352,23 @@ protected:
// The MCStateID of the state currently in effect in the byte stream. 0 if none.
int32_t fCurOpenStateID;
// The skip offsets for the current open state. These are the locations in the
// skp that must be filled in when the current open state is closed. These are
// here rather then distributed across the MatrixClipState's because saveLayers
// can cause MC states to be nested.
SkTDArray<int32_t> *fSkipOffsets;
SkDEBUGCODE(void validate();)
int MCStackPush(SkCanvas::SaveFlags flags);
void addClipOffset(int offset) {
SkASSERT(NULL != fSkipOffsets);
SkASSERT(kIdentityWideOpenStateID != fCurOpenStateID);
*fSkipOffsets->append() = offset;
}
void writeDeltaMat(int currentMatID, int desiredMatID);
static int32_t NewMCStateID();

View File

@ -598,7 +598,6 @@ void SkPictureRecord::restore() {
return;
}
// TODO: don't write the restore to the op stream for normal saves
fMCMgr.restore();
#else
// check for underflow

View File

@ -129,10 +129,14 @@ enum DrawOpType {
kDrawVertices_DrawOpType,
#endif
kLast_DrawOpType = kRect_DrawOpType
kLastNonSaveLayer_DrawOpType = kRect_DrawOpType,
// saveLayer's have to handled apart from the other draw operations
// since they also alter the save/restore structure.
kSaveLayer_DrawOpType,
};
static const int kDrawOpTypeCount = kLast_DrawOpType + 1;
static const int kNonSaveLayerDrawOpTypeCount = kLastNonSaveLayer_DrawOpType + 1;
typedef void (*PFEmitMC)(SkCanvas* canvas, MatType mat, ClipType clip,
DrawOpType draw, SkTDArray<DrawType>* expected,
@ -321,13 +325,13 @@ static void emit_draw(SkCanvas* canvas, DrawOpType draw, SkTDArray<DrawType>* ex
static void emit_clip_and_mat(SkCanvas* canvas, MatType mat, ClipType clip,
DrawOpType draw, SkTDArray<DrawType>* expected,
int accumulatedClips) {
emit_clip(canvas, clip);
emit_mat(canvas, mat);
if (kNone_DrawOpType == draw) {
return;
}
emit_clip(canvas, clip);
emit_mat(canvas, mat);
for (int i = 0; i < accumulatedClips; ++i) {
add_clip(clip, mat, expected);
}
@ -342,13 +346,13 @@ static void emit_clip_and_mat(SkCanvas* canvas, MatType mat, ClipType clip,
static void emit_mat_and_clip(SkCanvas* canvas, MatType mat, ClipType clip,
DrawOpType draw, SkTDArray<DrawType>* expected,
int accumulatedClips) {
emit_mat(canvas, mat);
emit_clip(canvas, clip);
if (kNone_DrawOpType == draw) {
return;
}
emit_mat(canvas, mat);
emit_clip(canvas, clip);
// the matrix & clip order will be reversed once collapsed!
for (int i = 0; i < accumulatedClips; ++i) {
add_clip(clip, mat, expected);
@ -365,15 +369,15 @@ static void emit_mat_and_clip(SkCanvas* canvas, MatType mat, ClipType clip,
static void emit_double_mat_and_clip(SkCanvas* canvas, MatType mat, ClipType clip,
DrawOpType draw, SkTDArray<DrawType>* expected,
int accumulatedClips) {
emit_mat(canvas, mat);
emit_clip(canvas, clip);
emit_mat(canvas, mat);
emit_clip(canvas, clip);
if (kNone_DrawOpType == draw) {
return;
}
emit_mat(canvas, mat);
emit_clip(canvas, clip);
emit_mat(canvas, mat);
emit_clip(canvas, clip);
for (int i = 0; i < accumulatedClips; ++i) {
add_clip(clip, mat, expected);
add_clip(clip, mat, expected);
@ -390,14 +394,14 @@ static void emit_double_mat_and_clip(SkCanvas* canvas, MatType mat, ClipType cli
static void emit_mat_clip_clip(SkCanvas* canvas, MatType mat, ClipType clip,
DrawOpType draw, SkTDArray<DrawType>* expected,
int accumulatedClips) {
if (kNone_DrawOpType == draw) {
return;
}
emit_mat(canvas, mat);
emit_clip(canvas, clip);
emit_clip(canvas, clip);
if (kNone_DrawOpType == draw) {
return;
}
for (int i = 0; i < accumulatedClips; ++i) {
add_clip(clip, mat, expected);
add_clip(clip, mat, expected);
@ -465,22 +469,24 @@ static void emit_body2(SkCanvas* canvas, PFEmitMC emitMC, MatType mat,
bool needsSaveRestore = kNone_DrawOpType != draw &&
(kNone_MatType != mat || kNone_ClipType != clip);
if (needsSaveRestore) {
*expected->append() = SAVE_LAYER;
if (kNone_MatType != mat || kNone_ClipType != clip) {
*expected->append() = SAVE;
}
(*emitMC)(canvas, mat, clip, draw, NULL, 0); // these get fused into later ops
(*emitMC)(canvas, mat, clip, kSaveLayer_DrawOpType, expected, accumulatedClips+1);
*expected->append() = SAVE_LAYER;
// TODO: widen testing to exercise saveLayer's parameters
canvas->saveLayer(NULL, NULL);
if (needsSaveRestore) {
*expected->append() = SAVE;
}
(*emitMC)(canvas, mat, clip, draw, expected, accumulatedClips+2);
(*emitMC)(canvas, mat, clip, draw, expected, 1);
emit_draw(canvas, draw, expected);
if (needsSaveRestore) {
*expected->append() = RESTORE;
}
canvas->restore();
if (needsSaveRestore) {
*expected->append() = RESTORE;
if (kNone_MatType != mat || kNone_ClipType != clip) {
*expected->append() = RESTORE;
}
}
@ -501,35 +507,39 @@ static void emit_body3(SkCanvas* canvas, PFEmitMC emitMC, MatType mat,
bool needsSaveRestore = kNone_DrawOpType != draw &&
(kNone_MatType != mat || kNone_ClipType != clip);
// This saveLayer will always be forced b.c. we currently can't tell
// ahead of time if it will be empty (see comment in SkMatrixClipStateMgr::save)
if (kNone_MatType != mat || kNone_ClipType != clip) {
*expected->append() = SAVE;
}
(*emitMC)(canvas, mat, clip, kSaveLayer_DrawOpType, expected, accumulatedClips+1);
*expected->append() = SAVE_LAYER;
(*emitMC)(canvas, mat, clip, draw, NULL, 0); // these get fused into later ops
// TODO: widen testing to exercise saveLayer's parameters
canvas->saveLayer(NULL, NULL);
(*emitMC)(canvas, mat, clip, draw, NULL, 0); // these get fused into later ops
if (needsSaveRestore) {
*expected->append() = SAVE_LAYER;
(*emitMC)(canvas, mat, clip, kSaveLayer_DrawOpType, expected, 1);
if (kNone_MatType != mat || kNone_ClipType != clip) {
*expected->append() = SAVE;
}
*expected->append() = SAVE_LAYER;
// TODO: widen testing to exercise saveLayer's parameters
canvas->saveLayer(NULL, NULL);
if (needsSaveRestore) {
*expected->append() = SAVE;
}
(*emitMC)(canvas, mat, clip, draw, expected, accumulatedClips+3);
(*emitMC)(canvas, mat, clip, draw, expected, 1);
emit_draw(canvas, draw, expected);
if (needsSaveRestore) {
*expected->append() = RESTORE;
}
canvas->restore();
if (needsSaveRestore) {
canvas->restore(); // for saveLayer
*expected->append() = RESTORE; // for saveLayer
if (kNone_MatType != mat || kNone_ClipType != clip) {
*expected->append() = RESTORE;
}
canvas->restore();
// required to match forced SAVE_LAYER
*expected->append() = RESTORE;
if (kNone_MatType != mat || kNone_ClipType != clip) {
*expected->append() = RESTORE;
}
}
//////////////////////////////////////////////////////////////////////////////
@ -660,13 +670,14 @@ static void test_collapse(skiatest::Reporter* reporter) {
for (size_t k = 0; k < SK_ARRAY_COUNT(gMCs); ++k) {
for (int l = 0; l < kMatTypeCount; ++l) {
for (int m = 0; m < kClipTypeCount; ++m) {
for (int n = 0; n < kDrawOpTypeCount; ++n) {
for (int n = 0; n < kNonSaveLayerDrawOpTypeCount; ++n) {
#ifdef TEST_COLLAPSE_MATRIX_CLIP_STATE
static int testID = -1;
++testID;
if (testID < -1) {
continue;
}
SkDebugf("test: %d\n", testID);
#endif
SkTDArray<DrawType> expected, actual;