add SkSafeRef / SkSafeUnref as inline static functions, to use in place of our
existing obj->safeRef(), which is unsafe since it can its 'if (this)' can be
optimized away by some compilers.
fix some overflows in mimpmap generation
git-svn-id: http://skia.googlecode.com/svn/trunk@181 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/include/core/SkRefCnt.h b/include/core/SkRefCnt.h
index adb59dd..7e325e0 100644
--- a/include/core/SkRefCnt.h
+++ b/include/core/SkRefCnt.h
@@ -130,5 +130,22 @@
dst = src; \
} while (0)
+
+/** Check if the argument is non-null, and if so, call obj->ref()
+ */
+template <typename T> static inline void SkSafeRef(T* obj) {
+ if (obj) {
+ obj->ref();
+ }
+}
+
+/** Check if the argument is non-null, and if so, call obj->unref()
+ */
+template <typename T> static inline void SkSafeUnref(T* obj) {
+ if (obj) {
+ obj->unref();
+ }
+}
+
#endif
diff --git a/samplecode/SampleLayers.cpp b/samplecode/SampleLayers.cpp
index 8666745..e6867bb 100644
--- a/samplecode/SampleLayers.cpp
+++ b/samplecode/SampleLayers.cpp
@@ -149,12 +149,26 @@
}
void drawBG(SkCanvas* canvas) {
- canvas->drawColor(SK_ColorWHITE);
+ canvas->drawColor(SK_ColorGRAY);
}
virtual void onDraw(SkCanvas* canvas) {
this->drawBG(canvas);
+ if (true) {
+ SkRect r;
+ r.set(SkIntToScalar(0), SkIntToScalar(0),
+ SkIntToScalar(220), SkIntToScalar(120));
+ SkPaint p;
+ canvas->saveLayer(&r, &p);
+ canvas->drawColor(0xFFFF0000);
+ p.setAlpha(1); // or 0
+ p.setPorterDuffXfermode(SkPorterDuff::kSrc_Mode);
+ canvas->drawOval(r, p);
+ canvas->restore();
+ return;
+ }
+
if (false) {
SkRect r;
r.set(SkIntToScalar(0), SkIntToScalar(0),
diff --git a/samplecode/SamplePicture.cpp b/samplecode/SamplePicture.cpp
index dfc1555..8ebb566 100644
--- a/samplecode/SamplePicture.cpp
+++ b/samplecode/SamplePicture.cpp
@@ -91,13 +91,13 @@
this->drawBG(canvas);
drawSomething(canvas);
-
+
SkPicture* pict = new SkPicture;
SkAutoUnref aur(pict);
drawSomething(pict->beginRecording(100, 100));
pict->endRecording();
-
+
canvas->save();
canvas->translate(SkIntToScalar(300), SkIntToScalar(50));
canvas->scale(-SK_Scalar1, -SK_Scalar1);
diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index 0d47508..d7a311e 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -26,6 +26,10 @@
#include "SkPackBits.h"
#include <new>
+static bool isPos32Bits(const Sk64& value) {
+ return !value.isNeg() && value.is32();
+}
+
#ifdef SK_SUPPORT_MIPMAP
struct MipLevel {
void* fPixels;
@@ -40,9 +44,17 @@
// Pixels[]
static MipMap* Alloc(int levelCount, size_t pixelSize) {
- MipMap* mm = (MipMap*)sk_malloc_throw(sizeof(MipMap) +
- levelCount * sizeof(MipLevel) +
- pixelSize);
+ if (levelCount < 0) {
+ return NULL;
+ }
+ Sk64 size;
+ size.setMul(levelCount + 1, sizeof(MipLevel));
+ size.add(sizeof(MipMap));
+ size.add(pixelSize);
+ if (!isPos32Bits(size)) {
+ return NULL;
+ }
+ MipMap* mm = (MipMap*)sk_malloc_throw(size.get32());
mm->fRefCnt = 1;
mm->fLevelCount = levelCount;
return mm;
@@ -54,18 +66,15 @@
const void* pixels() const { return levels() + fLevelCount; }
void* pixels() { return levels() + fLevelCount; }
- void safeRef() {
- if (this) {
- SkASSERT(fRefCnt > 0);
- sk_atomic_inc(&fRefCnt);
+ void ref() {
+ if (SK_MaxS32 == sk_atomic_inc(&fRefCnt)) {
+ sk_throw();
}
}
- void safeUnref() {
- if (this) {
- SkASSERT(fRefCnt > 0);
- if (sk_atomic_dec(&fRefCnt) == 1) {
- sk_free(this);
- }
+ void unref() {
+ SkASSERT(fRefCnt > 0);
+ if (sk_atomic_dec(&fRefCnt) == 1) {
+ sk_free(this);
}
}
};
@@ -98,7 +107,7 @@
// inc src reference counts
src.fPixelRef->safeRef();
#ifdef SK_SUPPORT_MIPMAP
- src.fMipMap->safeRef();
+ SkSafeRef(src.fMipMap);
#endif
// we reset our locks if we get blown away
@@ -175,33 +184,40 @@
}
int SkBitmap::ComputeRowBytes(Config c, int width) {
- int rowBytes = 0;
+ if (width < 0) {
+ return 0;
+ }
+
+ Sk64 rowBytes;
+ rowBytes.setZero();
switch (c) {
case kNo_Config:
case kRLE_Index8_Config:
- // assume that the bitmap has no pixels to draw to
- rowBytes = 0;
break;
case kA1_Config:
- rowBytes = (width + 7) >> 3;
+ rowBytes.set(width);
+ rowBytes.add(7);
+ rowBytes.shiftRight(3);
break;
case kA8_Config:
case kIndex8_Config:
- rowBytes = width;
+ rowBytes.set(width);
break;
case kRGB_565_Config:
case kARGB_4444_Config:
- rowBytes = width << 1;
+ rowBytes.set(width);
+ rowBytes.shiftLeft(1);
break;
case kARGB_8888_Config:
- rowBytes = width << 2;
+ rowBytes.set(width);
+ rowBytes.shiftLeft(2);
break;
default:
SkASSERT(!"unknown config");
break;
}
- return rowBytes;
+ return isPos32Bits(rowBytes) ? rowBytes.get32() : 0;
}
Sk64 SkBitmap::ComputeSize64(Config c, int width, int height) {
@@ -212,18 +228,25 @@
size_t SkBitmap::ComputeSize(Config c, int width, int height) {
Sk64 size = SkBitmap::ComputeSize64(c, width, height);
- if (size.isNeg() || !size.is32()) {
- return 0;
- }
- return size.get32();
+ return isPos32Bits(size) ? size.get32() : 0;
}
void SkBitmap::setConfig(Config c, int width, int height, int rowBytes) {
this->freePixels();
+ if ((width | height | rowBytes) < 0) {
+ ERROR:
+ this->reset();
+ return;
+ }
+
if (rowBytes == 0) {
rowBytes = SkBitmap::ComputeRowBytes(c, width);
+ if (0 == rowBytes && kNo_Config != c) {
+ goto ERROR;
+ }
}
+
fConfig = SkToU8(c);
fWidth = width;
fHeight = height;
@@ -248,8 +271,10 @@
} else {
SkASSERT(0 == fPixelLockCount);
fPixels = NULL;
- fColorTable->safeUnref();
- fColorTable = NULL;
+ if (fColorTable) {
+ fColorTable->unref();
+ fColorTable = NULL;
+ }
}
}
}
@@ -315,8 +340,10 @@
// if we're gonna free the pixels, we certainly need to free the mipmap
this->freeMipMap();
- fColorTable->safeUnref();
- fColorTable = NULL;
+ if (fColorTable) {
+ fColorTable->unref();
+ fColorTable = NULL;
+ }
if (NULL != fPixelRef) {
if (fPixelLockCount > 0) {
@@ -332,8 +359,10 @@
void SkBitmap::freeMipMap() {
#ifdef SK_SUPPORT_MIPMAP
- fMipMap->safeUnref();
- fMipMap = NULL;
+ if (fMipMap) {
+ fMipMap->unref();
+ fMipMap = NULL;
+ }
#endif
}
@@ -359,7 +388,7 @@
}
SkMallocPixelRef::~SkMallocPixelRef() {
- fCTable->safeUnref();
+ SkSafeUnref(fCTable);
sk_free(fStorage);
}
@@ -898,13 +927,18 @@
default:
return; // don't build mipmaps for these configs
}
+
+ SkAutoLockPixels alp(*this);
+ if (!this->readyToDraw()) {
+ return;
+ }
// whip through our loop to compute the exact size needed
size_t size = 0;
int maxLevels = 0;
{
- unsigned width = this->width();
- unsigned height = this->height();
+ int width = this->width();
+ int height = this->height();
for (;;) {
width >>= 1;
height >>= 1;
@@ -915,20 +949,29 @@
maxLevels += 1;
}
}
+
+ // nothing to build
if (0 == maxLevels) {
return;
}
- MipMap* mm = MipMap::Alloc(maxLevels, size);
+ SkBitmap srcBM(*this);
+ srcBM.lockPixels();
+ if (!srcBM.readyToDraw()) {
+ return;
+ }
+
+ MipMap* mm = MipMap::Alloc(maxLevels, size);
+ if (NULL == mm) {
+ return;
+ }
+
MipLevel* level = mm->levels();
uint8_t* addr = (uint8_t*)mm->pixels();
-
- unsigned width = this->width();
- unsigned height = this->height();
+ int width = this->width();
+ int height = this->height();
unsigned rowBytes = this->rowBytes();
- SkBitmap srcBM(*this), dstBM;
-
- srcBM.lockPixels();
+ SkBitmap dstBM;
for (int i = 0; i < maxLevels; i++) {
width >>= 1;
@@ -943,8 +986,8 @@
dstBM.setConfig(config, width, height, rowBytes);
dstBM.setPixels(addr);
- for (unsigned y = 0; y < height; y++) {
- for (unsigned x = 0; x < width; x++) {
+ for (int y = 0; y < height; y++) {
+ for (int x = 0; x < width; x++) {
proc(&dstBM, x, y, srcBM);
}
}
@@ -1251,7 +1294,7 @@
} else {
buffer.skip(size);
}
- ctable->safeUnref();
+ SkSafeUnref(ctable);
break;
}
case SERIALIZE_PIXELTYPE_NONE:
diff --git a/xcode/sampleapp/SampleApp.xcodeproj/project.pbxproj b/xcode/sampleapp/SampleApp.xcodeproj/project.pbxproj
index 218a039..0c53fca 100644
--- a/xcode/sampleapp/SampleApp.xcodeproj/project.pbxproj
+++ b/xcode/sampleapp/SampleApp.xcodeproj/project.pbxproj
@@ -73,8 +73,8 @@
009490320FB0A5B90063C792 /* SampleLayerMask.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 009490310FB0A5B90063C792 /* SampleLayerMask.cpp */; };
009CC9190F65918A002185BE /* SampleFontScalerTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 009CC9180F65918A002185BE /* SampleFontScalerTest.cpp */; };
00A41E4B0EFC312F00C9CBEB /* SampleArc.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 00A41E4A0EFC312F00C9CBEB /* SampleArc.cpp */; };
- 00A8A3EE0FC25993006ED638 /* SampleEffects.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 00A8A3ED0FC25993006ED638 /* SampleEffects.cpp */; };
00C55DA10F8552DC000CAC09 /* SampleGradients.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 00C55DA00F8552DC000CAC09 /* SampleGradients.cpp */; };
+ 00FF39140FC6ED2C00915187 /* SampleEffects.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 00FF39130FC6ED2C00915187 /* SampleEffects.cpp */; };
0156F80407C56A3000C6122B /* Foundation.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 0156F80307C56A3000C6122B /* Foundation.framework */; };
01FC44D507BD3BB800D228F4 /* Quartz.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 01FC44D407BD3BB800D228F4 /* Quartz.framework */; };
8D0C4E8D0486CD37000505A6 /* InfoPlist.strings in Resources */ = {isa = PBXBuildFile; fileRef = 0867D6AAFE840B52C02AAC07 /* InfoPlist.strings */; };
@@ -198,9 +198,9 @@
009490310FB0A5B90063C792 /* SampleLayerMask.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SampleLayerMask.cpp; path = ../../samplecode/SampleLayerMask.cpp; sourceTree = SOURCE_ROOT; };
009CC9180F65918A002185BE /* SampleFontScalerTest.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SampleFontScalerTest.cpp; path = ../../samplecode/SampleFontScalerTest.cpp; sourceTree = SOURCE_ROOT; };
00A41E4A0EFC312F00C9CBEB /* SampleArc.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SampleArc.cpp; path = ../../samplecode/SampleArc.cpp; sourceTree = SOURCE_ROOT; };
- 00A8A3ED0FC25993006ED638 /* SampleEffects.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SampleEffects.cpp; path = ../../samplecode/SampleEffects.cpp; sourceTree = SOURCE_ROOT; };
00C55DA00F8552DC000CAC09 /* SampleGradients.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SampleGradients.cpp; path = ../../samplecode/SampleGradients.cpp; sourceTree = SOURCE_ROOT; };
00D6B5CB0F72DC4300C466B9 /* SampleFuzz.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SampleFuzz.cpp; path = ../../samplecode/SampleFuzz.cpp; sourceTree = SOURCE_ROOT; };
+ 00FF39130FC6ED2C00915187 /* SampleEffects.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SampleEffects.cpp; path = ../../samplecode/SampleEffects.cpp; sourceTree = SOURCE_ROOT; };
0156F80307C56A3000C6122B /* Foundation.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = Foundation.framework; path = /System/Library/Frameworks/Foundation.framework; sourceTree = "<absolute>"; };
01FC44D407BD3BB800D228F4 /* Quartz.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = Quartz.framework; path = /System/Library/Frameworks/Quartz.framework; sourceTree = "<absolute>"; };
0867D6ABFE840B52C02AAC07 /* English */ = {isa = PBXFileReference; fileEncoding = 10; lastKnownFileType = text.plist.strings; name = English; path = English.lproj/InfoPlist.strings; sourceTree = "<group>"; };
@@ -233,11 +233,11 @@
00003C610EFC2287000FF73A /* samples */ = {
isa = PBXGroup;
children = (
+ 00FF39130FC6ED2C00915187 /* SampleEffects.cpp */,
007A7CA40F01658C00A2D6EE /* SamplePicture.cpp */,
00D6B5CB0F72DC4300C466B9 /* SampleFuzz.cpp */,
00C55DA00F8552DC000CAC09 /* SampleGradients.cpp */,
009490310FB0A5B90063C792 /* SampleLayerMask.cpp */,
- 00A8A3ED0FC25993006ED638 /* SampleEffects.cpp */,
003145310FB9B48F00B10956 /* SampleShapes.cpp */,
009CC9180F65918A002185BE /* SampleFontScalerTest.cpp */,
007A7CA50F01658C00A2D6EE /* SamplePoints.cpp */,
@@ -490,6 +490,7 @@
files = (
00003C660EFC22A8000FF73A /* SampleApp.cpp in Sources */,
00003C680EFC22A8000FF73A /* SamplePath.cpp in Sources */,
+ 00003C690EFC22A8000FF73A /* SamplePathEffects.cpp in Sources */,
00003C740EFC22CE000FF73A /* SkEvent.cpp in Sources */,
00003C750EFC22CE000FF73A /* SkEventSink.cpp in Sources */,
00003C760EFC22CE000FF73A /* SkMetaData.cpp in Sources */,
@@ -532,6 +533,7 @@
007A7CB80F01658C00A2D6EE /* SampleStrokeText.cpp in Sources */,
007A7CBA0F01658C00A2D6EE /* SampleText.cpp in Sources */,
007A7CBB0F01658C00A2D6EE /* SampleTextAlpha.cpp in Sources */,
+ 007A7CBC0F01658C00A2D6EE /* SampleTextEffects.cpp in Sources */,
007A7CBD0F01658C00A2D6EE /* SampleTextOnPath.cpp in Sources */,
007A7CBE0F01658C00A2D6EE /* SampleTiling.cpp in Sources */,
007A7CBF0F01658C00A2D6EE /* SampleTypeface.cpp in Sources */,
@@ -550,9 +552,7 @@
003145200FB99CCE00B10956 /* SkRectShape.cpp in Sources */,
003145320FB9B48F00B10956 /* SampleShapes.cpp in Sources */,
003145370FB9BA4000B10956 /* SkGroupShape.cpp in Sources */,
- 007A7CBC0F01658C00A2D6EE /* SampleTextEffects.cpp in Sources */,
- 00003C690EFC22A8000FF73A /* SamplePathEffects.cpp in Sources */,
- 00A8A3EE0FC25993006ED638 /* SampleEffects.cpp in Sources */,
+ 00FF39140FC6ED2C00915187 /* SampleEffects.cpp in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};