change API contract: disallow zero-sized images or surfaces

BUG=skia:

Review URL: https://codereview.chromium.org/830033003
This commit is contained in:
reed 2014-12-31 12:31:43 -08:00 committed by Commit bot
parent 55374058b3
commit b2497c2d94
7 changed files with 54 additions and 35 deletions

View File

@ -32,6 +32,10 @@ class GrTexture;
* The content of SkImage is always immutable, though the actual storage may
* change, if for example that image can be re-created via encoded data or
* other means.
*
* SkImage always has a non-zero dimensions. If there is a request to create a new image, either
* directly or via SkSurface, and either of the requested dimensions are zero, then NULL will be
* returned.
*/
class SK_API SkImage : public SkRefCnt {
public:
@ -130,8 +134,8 @@ protected:
fHeight(height),
fUniqueID(NextUniqueID()) {
SkASSERT(width >= 0);
SkASSERT(height >= 0);
SkASSERT(width > 0);
SkASSERT(height > 0);
}
private:

View File

@ -24,6 +24,9 @@ class GrRenderTarget;
*
* To draw into a canvas, first create the appropriate type of Surface, and
* then request the canvas from the surface.
*
* SkSurface always has non-zero dimensions. If there is a request for a new surface, and either
* of the requested dimensions are zero, then NULL will be returned.
*/
class SK_API SkSurface : public SkRefCnt {
public:

View File

@ -19,7 +19,7 @@ public:
const int maxDimension = SK_MaxS32 >> 2;
const size_t kMaxPixelByteSize = SK_MaxS32;
if (info.width() < 0 || info.height() < 0) {
if (info.width() <= 0 || info.height() <= 0) {
return false;
}
if (info.width() > maxDimension || info.height() > maxDimension) {
@ -49,8 +49,6 @@ public:
return true;
}
static SkImage* NewEmpty();
SkImage_Raster(const SkImageInfo&, SkData*, size_t rb, const SkSurfaceProps*);
virtual ~SkImage_Raster();
@ -86,16 +84,6 @@ private:
///////////////////////////////////////////////////////////////////////////////
SkImage* SkImage_Raster::NewEmpty() {
// Returns lazily created singleton
static SkImage* gEmpty;
if (NULL == gEmpty) {
gEmpty = SkNEW(SkImage_Raster);
}
gEmpty->ref();
return gEmpty;
}
static void release_data(void* addr, void* context) {
SkData* data = static_cast<SkData*>(context);
data->unref();
@ -169,14 +157,7 @@ bool SkImage_Raster::getROPixels(SkBitmap* dst) const {
///////////////////////////////////////////////////////////////////////////////
SkImage* SkImage::NewRasterCopy(const SkImageInfo& info, const void* pixels, size_t rowBytes) {
if (!SkImage_Raster::ValidArgs(info, rowBytes)) {
return NULL;
}
if (0 == info.width() && 0 == info.height()) {
return SkImage_Raster::NewEmpty();
}
// check this after empty-check
if (NULL == pixels) {
if (!SkImage_Raster::ValidArgs(info, rowBytes) || !pixels) {
return NULL;
}
@ -187,14 +168,7 @@ SkImage* SkImage::NewRasterCopy(const SkImageInfo& info, const void* pixels, siz
SkImage* SkImage::NewRasterData(const SkImageInfo& info, SkData* data, size_t rowBytes) {
if (!SkImage_Raster::ValidArgs(info, rowBytes)) {
return NULL;
}
if (0 == info.width() && 0 == info.height()) {
return SkImage_Raster::NewEmpty();
}
// check this after empty-check
if (NULL == data) {
if (!SkImage_Raster::ValidArgs(info, rowBytes) || !data) {
return NULL;
}

View File

@ -123,16 +123,16 @@ static SkSurface_Base* asSB(SkSurface* surface) {
SkSurface::SkSurface(int width, int height, const SkSurfaceProps* props)
: fProps(SkSurfacePropsCopyOrDefault(props)), fWidth(width), fHeight(height)
{
SkASSERT(fWidth >= 0);
SkASSERT(fHeight >= 0);
SkASSERT(fWidth > 0);
SkASSERT(fHeight > 0);
fGenerationID = 0;
}
SkSurface::SkSurface(const SkImageInfo& info, const SkSurfaceProps* props)
: fProps(SkSurfacePropsCopyOrDefault(props)), fWidth(info.width()), fHeight(info.height())
{
SkASSERT(fWidth >= 0);
SkASSERT(fHeight >= 0);
SkASSERT(fWidth > 0);
SkASSERT(fHeight > 0);
fGenerationID = 0;
}

View File

@ -39,6 +39,10 @@ private:
///////////////////////////////////////////////////////////////////////////////
bool SkSurface_Raster::Valid(const SkImageInfo& info, size_t rowBytes) {
if (info.isEmpty()) {
return false;
}
static const size_t kMaxTotalSize = SK_MaxS32;
int shift = 0;

View File

@ -98,6 +98,7 @@ bool SkInstallDiscardablePixelRef(SkImageGenerator* generator, SkBitmap* dst,
SkAutoTDelete<SkImageGenerator> autoGenerator(generator);
if ((NULL == autoGenerator.get())
|| (!autoGenerator->getInfo(&info))
|| info.isEmpty()
|| (!dst->setInfo(info))) {
return false;
}

View File

@ -71,6 +71,35 @@ enum ImageType {
kCodec_ImageType,
};
#include "SkImageGenerator.h"
class EmptyGenerator : public SkImageGenerator {
protected:
bool onGetInfo(SkImageInfo* info) SK_OVERRIDE {
*info = SkImageInfo::Make(0, 0, kN32_SkColorType, kPremul_SkAlphaType);
return true;
}
};
static void test_empty_image(skiatest::Reporter* reporter) {
const SkImageInfo info = SkImageInfo::Make(0, 0, kN32_SkColorType, kPremul_SkAlphaType);
REPORTER_ASSERT(reporter, NULL == SkImage::NewRasterCopy(info, NULL, 0));
REPORTER_ASSERT(reporter, NULL == SkImage::NewRasterData(info, NULL, 0));
REPORTER_ASSERT(reporter, NULL == SkImage::NewFromGenerator(SkNEW(EmptyGenerator)));
}
static void test_empty_surface(skiatest::Reporter* reporter, GrContext* ctx) {
const SkImageInfo info = SkImageInfo::Make(0, 0, kN32_SkColorType, kPremul_SkAlphaType);
REPORTER_ASSERT(reporter, NULL == SkSurface::NewRaster(info));
REPORTER_ASSERT(reporter, NULL == SkSurface::NewRasterDirect(info, NULL, 0));
if (ctx) {
REPORTER_ASSERT(reporter, NULL == SkSurface::NewRenderTarget(ctx, info, 0, NULL));
REPORTER_ASSERT(reporter, NULL == SkSurface::NewScratchRenderTarget(ctx, info, 0, NULL));
}
}
static void test_image(skiatest::Reporter* reporter) {
SkImageInfo info = SkImageInfo::MakeN32Premul(1, 1);
size_t rowBytes = info.minRowBytes();
@ -475,6 +504,9 @@ DEF_GPUTEST(Surface, reporter, factory) {
TestSurfaceNoCanvas(reporter, kRaster_SurfaceType, NULL, SkSurface::kDiscard_ContentChangeMode);
TestSurfaceNoCanvas(reporter, kRaster_SurfaceType, NULL, SkSurface::kRetain_ContentChangeMode);
test_empty_image(reporter);
test_empty_surface(reporter, NULL);
test_imagepeek(reporter, factory);
test_canvaspeek(reporter, factory);
@ -500,6 +532,7 @@ DEF_GPUTEST(Surface, reporter, factory) {
TestSurfaceNoCanvas(reporter, kGpuScratch_SurfaceType, context, SkSurface::kRetain_ContentChangeMode);
TestGetTexture(reporter, kGpu_SurfaceType, context);
TestGetTexture(reporter, kGpuScratch_SurfaceType, context);
test_empty_surface(reporter, context);
}
}
}