AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Goal: ensure we always balance lock/unlock pixels calls.

Browse Source 
A big caller of lockPixels is setContext in the bitmapshader.

This change replaces beginSession/endSession with adding endContext(), and
adds debugging code to ensure that
1. setContext calls are never nested
2. endContext is always called after each setContext call.
Review URL: https://codereview.appspot.com/6937046

git-svn-id: http://skia.googlecode.com/svn/trunk@6798 2bbb7eff-a529-9590-31e7-b0007b416f81
This commit is contained in:
reed@google.com 2012-12-13 21:39:56 +00:00
parent 9f13174da5
commit 1adcf8859c
13 changed files with 136 additions and 115 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
include/core/SkComposeShader.h
View File

@ -34,11 +34,10 @@ public:
SkComposeShader(SkShader* sA, SkShader* sB, SkXfermode* mode = NULL);
virtual ~SkComposeShader();
// override
virtual bool setContext(const SkBitmap& device, const SkPaint& paint, const SkMatrix& matrix);
virtual void shadeSpan(int x, int y, SkPMColor result[], int count);
virtual void beginSession();
virtual void endSession();
virtual bool setContext(const SkBitmap&, const SkPaint&,
const SkMatrix&) SK_OVERRIDE;
virtual void endContext() SK_OVERRIDE;
virtual void shadeSpan(int x, int y, SkPMColor[], int count) SK_OVERRIDE;
SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkComposeShader)

29
include/core/SkShader.h
View File

@ -139,11 +139,28 @@ public:
/**
* Called once before drawing, with the current paint and device matrix.
* Return true if your shader supports these parameters, or false if not.
* If false is returned, nothing will be drawn.
* If false is returned, nothing will be drawn. If true is returned, then
* a balancing call to endContext() will be made before the next call to
* setContext.
*
* Subclasses should be sure to call their INHERITED::setContext() if they
* override this method.
*/
virtual bool setContext(const SkBitmap& device, const SkPaint& paint,
const SkMatrix& matrix);
/**
* Assuming setContext returned true, endContext() will be called when
* the draw using the shader has completed. It is an error for setContext
* to be called twice w/o an intervening call to endContext().
*
* Subclasses should be sure to call their INHERITED::endContext() if they
* override this method.
*/
virtual void endContext();
SkDEBUGCODE(bool setContextHasBeenCalled() const { return fInSetContext; })
/**
* Called for each span of the object being drawn. Your subclass should
* set the appropriate colors (with premultiplied alpha) that correspond
@ -182,14 +199,6 @@ public:
return (flags & kHasSpan16_Flag) != 0;
}
/**
* Called before a session using the shader begins. Some shaders override
* this to defer some of their work (like calling bitmap.lockPixels()).
* Must be balanced by a call to endSession.
*/
virtual void beginSession();
virtual void endSession();
/**
Gives method bitmap should be read to implement a shader.
Also determines number and interpretation of "extra" parameters returned
@ -355,7 +364,7 @@ private:
uint8_t fPaintAlpha;
uint8_t fDeviceConfig;
uint8_t fTotalInverseClass;
SkDEBUGCODE(SkBool8 fInSession;)
SkDEBUGCODE(SkBool8 fInSetContext;)
static SkShader* CreateBitmapShader(const SkBitmap& src,
TileMode, TileMode,

18
src/core/SkBitmapProcShader.cpp
View File

@ -41,18 +41,6 @@ SkBitmapProcShader::SkBitmapProcShader(SkFlattenableReadBuffer& buffer)
fFlags = 0; // computed in setContext
}
void SkBitmapProcShader::beginSession() {
this->INHERITED::beginSession();
fRawBitmap.lockPixels();
}
void SkBitmapProcShader::endSession() {
fRawBitmap.unlockPixels();
this->INHERITED::endSession();
}
SkShader::BitmapType SkBitmapProcShader::asABitmap(SkBitmap* texture,
SkMatrix* texM,
TileMode xy[]) const {
@ -102,6 +90,7 @@ bool SkBitmapProcShader::setContext(const SkBitmap& device,
}
if (!fState.chooseProcs(this->getTotalInverse(), paint)) {
fState.fOrigBitmap.unlockPixels();
return false;
}
@ -149,6 +138,11 @@ bool SkBitmapProcShader::setContext(const SkBitmap& device,
return true;
}
void SkBitmapProcShader::endContext() {
fState.fOrigBitmap.unlockPixels();
this->INHERITED::endContext();
}
#define BUF_MAX 128
#define TEST_BUFFER_OVERRITEx

3
src/core/SkBitmapProcShader.h
View File

@ -20,12 +20,11 @@ public:
// overrides from SkShader
virtual bool isOpaque() const SK_OVERRIDE;
virtual bool setContext(const SkBitmap&, const SkPaint&, const SkMatrix&);
virtual void endContext();
virtual uint32_t getFlags() { return fFlags; }
virtual void shadeSpan(int x, int y, SkPMColor dstC[], int count);
virtual ShadeProc asAShadeProc(void** ctx) SK_OVERRIDE;
virtual void shadeSpan16(int x, int y, uint16_t dstC[], int count);
virtual void beginSession();
virtual void endSession();
virtual BitmapType asABitmap(SkBitmap*, SkMatrix*, TileMode*) const;
static bool CanDo(const SkBitmap&, TileMode tx, TileMode ty);

52
src/core/SkBlitter.cpp
View File

@ -572,16 +572,30 @@ public:
void setMask(const SkMask* mask) { fMask = mask; }
virtual bool setContext(const SkBitmap& device, const SkPaint& paint,
const SkMatrix& matrix) {
const SkMatrix& matrix) SK_OVERRIDE {
if (!this->INHERITED::setContext(device, paint, matrix)) {
return false;
}
if (fProxy) {
return fProxy->setContext(device, paint, matrix);
if (!fProxy->setContext(device, paint, matrix)) {
// must keep our set/end context calls balanced
this->INHERITED::endContext();
return false;
}
} else {
fPMColor = SkPreMultiplyColor(paint.getColor());
return this->INHERITED::setContext(device, paint, matrix);
}
return true;
}
virtual void endContext() SK_OVERRIDE {
if (fProxy) {
fProxy->endContext();
}
this->INHERITED::endContext();
}
virtual void shadeSpan(int x, int y, SkPMColor span[], int count) {
virtual void shadeSpan(int x, int y, SkPMColor span[], int count) SK_OVERRIDE {
if (fProxy) {
fProxy->shadeSpan(x, y, span, count);
}
@ -645,20 +659,6 @@ public:
}
}
virtual void beginSession() {
this->INHERITED::beginSession();
if (fProxy) {
fProxy->beginSession();
}
}
virtual void endSession() {
if (fProxy) {
fProxy->endSession();
}
this->INHERITED::endSession();
}
SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(Sk3DShader)
protected:
@ -907,8 +907,17 @@ SkBlitter* SkBlitter::Choose(const SkBitmap& device,
// if there is one, the shader will take care of it.
}
/*
* We need to have balanced calls to the shader:
* setContext
* endContext
* We make the first call here, in case it fails we can abort the draw.
* The endContext() call is made by the blitter (assuming setContext did
* not fail) in its destructor.
*/
if (shader && !shader->setContext(device, *paint, matrix)) {
return SkNEW(SkNullBlitter);
SK_PLACEMENT_NEW(blitter, SkNullBlitter, storage, storageSize);
return blitter;
}
switch (device.getConfig()) {
@ -978,14 +987,15 @@ SkShaderBlitter::SkShaderBlitter(const SkBitmap& device, const SkPaint& paint)
: INHERITED(device) {
fShader = paint.getShader();
SkASSERT(fShader);
SkASSERT(fShader->setContextHasBeenCalled());
fShader->ref();
fShader->beginSession();
fShaderFlags = fShader->getFlags();
}
SkShaderBlitter::~SkShaderBlitter() {
fShader->endSession();
SkASSERT(fShader->setContextHasBeenCalled());
fShader->endContext();
fShader->unref();
}

28
src/core/SkColorFilter.cpp
View File

@ -62,16 +62,6 @@ SkFilterShader::~SkFilterShader() {
fShader->unref();
}
void SkFilterShader::beginSession() {
this->INHERITED::beginSession();
fShader->beginSession();
}
void SkFilterShader::endSession() {
fShader->endSession();
this->INHERITED::endSession();
}
void SkFilterShader::flatten(SkFlattenableWriteBuffer& buffer) const {
this->INHERITED::flatten(buffer);
buffer.writeFlattenable(fShader);
@ -96,8 +86,22 @@ uint32_t SkFilterShader::getFlags() {
bool SkFilterShader::setContext(const SkBitmap& device,
const SkPaint& paint,
const SkMatrix& matrix) {
return this->INHERITED::setContext(device, paint, matrix) &&
fShader->setContext(device, paint, matrix);
// we need to keep the setContext/endContext calls balanced. If we return
// false, our endContext() will not be called.
if (!this->INHERITED::setContext(device, paint, matrix)) {
return false;
}
if (!fShader->setContext(device, paint, matrix)) {
this->INHERITED::endContext();
return false;
}
return true;
}
void SkFilterShader::endContext() {
fShader->endContext();
this->INHERITED::endContext();
}
void SkFilterShader::shadeSpan(int x, int y, SkPMColor result[], int count) {

38
src/core/SkComposeShader.cpp
View File

@ -43,18 +43,6 @@ SkComposeShader::~SkComposeShader() {
fShaderA->unref();
}
void SkComposeShader::beginSession() {
this->INHERITED::beginSession();
fShaderA->beginSession();
fShaderB->beginSession();
}
void SkComposeShader::endSession() {
fShaderA->endSession();
fShaderB->endSession();
this->INHERITED::endSession();
}
class SkAutoAlphaRestore {
public:
SkAutoAlphaRestore(SkPaint* paint, uint8_t newAlpha) {
@ -81,7 +69,10 @@ void SkComposeShader::flatten(SkFlattenableWriteBuffer& buffer) const {
/* We call setContext on our two worker shaders. However, we
always let them see opaque alpha, and if the paint really
is translucent, then we apply that after the fact.
*/
We need to keep the calls to setContext/endContext balanced, since if we
return false, our endContext() will not be called.
*/
bool SkComposeShader::setContext(const SkBitmap& device,
const SkPaint& paint,
const SkMatrix& matrix) {
@ -98,8 +89,25 @@ bool SkComposeShader::setContext(const SkBitmap& device,
SkAutoAlphaRestore restore(const_cast<SkPaint*>(&paint), 0xFF);
return fShaderA->setContext(device, paint, tmpM) &&
fShaderB->setContext(device, paint, tmpM);
bool setContextA = fShaderA->setContext(device, paint, tmpM);
bool setContextB = fShaderB->setContext(device, paint, tmpM);
if (!setContextA || !setContextB) {
if (setContextB) {
fShaderB->endContext();
}
else if (setContextA) {
fShaderA->endContext();
}
this->INHERITED::endContext();
return false;
}
return true;
}
void SkComposeShader::endContext() {
fShaderB->endContext();
fShaderA->endContext();
this->INHERITED::endContext();
}
// larger is better (fewer times we have to loop), but we shouldn't

20
src/core/SkDraw.cpp
View File

@ -1228,13 +1228,6 @@ void SkDraw::drawBitmap(const SkBitmap& bitmap, const SkMatrix& prematrix,
}
}
// only lock the pixels if we passed the clip and bounder tests
SkAutoLockPixels alp(bitmap);
// after the lock, check if we are valid
if (!bitmap.readyToDraw()) {
return;
}
if (bitmap.getConfig() != SkBitmap::kA8_Config &&
just_translate(matrix, bitmap)) {
int ix = SkScalarRound(matrix.getTranslateX());
@ -2279,7 +2272,7 @@ public:
bool setup(const SkPoint pts[], const SkColor colors[], int, int, int);
virtual void shadeSpan(int x, int y, SkPMColor dstC[], int count);
virtual void shadeSpan(int x, int y, SkPMColor dstC[], int count) SK_OVERRIDE;
SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkTriColorShader)
@ -2406,7 +2399,7 @@ void SkDraw::drawVertices(SkCanvas::VertexMode vmode, int count,
if (NULL != colors) {
if (NULL == textures) {
// just colors (no texture)
p.setShader(&triShader);
shader = p.setShader(&triShader);
} else {
// colors * texture
SkASSERT(shader);
@ -2421,6 +2414,7 @@ void SkDraw::drawVertices(SkCanvas::VertexMode vmode, int count,
if (releaseMode) {
xmode->unref();
}
shader = compose;
}
}
@ -2436,10 +2430,17 @@ void SkDraw::drawVertices(SkCanvas::VertexMode vmode, int count,
savedLocalM = shader->getLocalMatrix();
}
// do we need this? triShader should have been installed in p, either
// directly or indirectly (using compose shader), so its setContext
// should have already been called.
if (NULL != colors) {
SkASSERT(triShader.setContextHasBeenCalled());
#if 0
if (!triShader.setContext(*fBitmap, p, *fMatrix)) {
colors = NULL;
}
#endif
}
while (vertProc(&state)) {
@ -2448,6 +2449,7 @@ void SkDraw::drawVertices(SkCanvas::VertexMode vmode, int count,
tempM.postConcat(savedLocalM);
shader->setLocalMatrix(tempM);
// need to recal setContext since we changed the local matrix
shader->endContext();
if (!shader->setContext(*fBitmap, p, *fMatrix)) {
continue;
}

14
src/core/SkFilterShader.h
View File

@ -17,14 +17,12 @@ public:
SkFilterShader(SkShader* shader, SkColorFilter* filter);
virtual ~SkFilterShader();
// override
virtual uint32_t getFlags();
virtual bool setContext(const SkBitmap& device, const SkPaint& paint,
const SkMatrix& matrix);
virtual void shadeSpan(int x, int y, SkPMColor result[], int count);
virtual void shadeSpan16(int x, int y, uint16_t result[], int count);
virtual void beginSession();
virtual void endSession();
virtual uint32_t getFlags() SK_OVERRIDE;
virtual bool setContext(const SkBitmap&, const SkPaint&,
const SkMatrix&) SK_OVERRIDE;
virtual void endContext() SK_OVERRIDE;
virtual void shadeSpan(int x, int y, SkPMColor[], int count) SK_OVERRIDE;
virtual void shadeSpan16(int x, int y, uint16_t[], int count) SK_OVERRIDE;
SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkFilterShader)

24
src/core/SkShader.cpp
View File

@ -17,7 +17,7 @@ SK_DEFINE_INST_COUNT(SkShader)
SkShader::SkShader() {
fLocalMatrix.reset();
SkDEBUGCODE(fInSession = false;)
SkDEBUGCODE(fInSetContext = false;)
}
SkShader::SkShader(SkFlattenableReadBuffer& buffer)
@ -28,21 +28,11 @@ SkShader::SkShader(SkFlattenableReadBuffer& buffer)
fLocalMatrix.reset();
}
SkDEBUGCODE(fInSession = false;)
SkDEBUGCODE(fInSetContext = false;)
}
SkShader::~SkShader() {
SkASSERT(!fInSession);
}
void SkShader::beginSession() {
SkASSERT(!fInSession);
SkDEBUGCODE(fInSession = true;)
}
void SkShader::endSession() {
SkASSERT(fInSession);
SkDEBUGCODE(fInSession = false;)
SkASSERT(!fInSetContext);
}
void SkShader::flatten(SkFlattenableWriteBuffer& buffer) const {
@ -57,6 +47,8 @@ void SkShader::flatten(SkFlattenableWriteBuffer& buffer) const {
bool SkShader::setContext(const SkBitmap& device,
const SkPaint& paint,
const SkMatrix& matrix) {
SkASSERT(!this->setContextHasBeenCalled());
const SkMatrix* m = &matrix;
SkMatrix total;
@ -68,11 +60,17 @@ bool SkShader::setContext(const SkBitmap& device,
}
if (m->invert(&fTotalInverse)) {
fTotalInverseClass = (uint8_t)ComputeMatrixClass(fTotalInverse);
SkDEBUGCODE(fInSetContext = true;)
return true;
}
return false;
}
void SkShader::endContext() {
SkASSERT(fInSetContext);
SkDEBUGCODE(fInSetContext = false;)
}
SkShader::ShadeProc SkShader::asAShadeProc(void** ctx) {
return NULL;
}

2
src/effects/gradients/SkGradientShader.cpp
View File

@ -213,6 +213,8 @@ bool SkGradientShaderBase::setContext(const SkBitmap& device,
const SkMatrix& inverse = this->getTotalInverse();
if (!fDstToIndex.setConcat(fPtsToUnit, inverse)) {
// need to keep our set/end context calls balanced.
this->INHERITED::endContext();
return false;
}

1
src/effects/gradients/SkGradientShaderPriv.h
View File

@ -91,7 +91,6 @@ public:
int colorCount, SkShader::TileMode mode, SkUnitMapper* mapper);
virtual ~SkGradientShaderBase();
// overrides
virtual bool setContext(const SkBitmap&, const SkPaint&, const SkMatrix&) SK_OVERRIDE;
virtual uint32_t getFlags() SK_OVERRIDE { return fFlags; }
virtual bool isOpaque() const SK_OVERRIDE;

13
src/effects/gradients/SkTwoPointRadialGradient.cpp
View File

@ -292,16 +292,15 @@ void SkTwoPointRadialGradient::shadeSpan(int x, int y, SkPMColor* dstCParam,
}
}
bool SkTwoPointRadialGradient::setContext(
const SkBitmap& device,
const SkPaint& paint,
const SkMatrix& matrix){
if (!this->INHERITED::setContext(device, paint, matrix)) {
bool SkTwoPointRadialGradient::setContext( const SkBitmap& device,
const SkPaint& paint,
const SkMatrix& matrix){
// For now, we might have divided by zero, so detect that
if (0 == fDiffRadius) {
return false;
}
// For now, we might have divided by zero, so detect that
if (0 == fDiffRadius) {
if (!this->INHERITED::setContext(device, paint, matrix)) {
return false;
}