diff --git a/include/core/SkImage.h b/include/core/SkImage.h index e2757b9af5..9d1c546ef5 100644 --- a/include/core/SkImage.h +++ b/include/core/SkImage.h @@ -44,10 +44,23 @@ public: SK_DECLARE_INST_COUNT(SkImage) typedef SkImageInfo Info; + typedef void* ReleaseContext; static SkImage* NewRasterCopy(const Info&, const void* pixels, size_t rowBytes); static SkImage* NewRasterData(const Info&, SkData* pixels, size_t rowBytes); + typedef void (*RasterReleaseProc)(const void* pixels, ReleaseContext); + + /** + * Return a new Image referencing the specified pixels. These must remain valid and unchanged + * until the specified release-proc is called, indicating that Skia no longer has a reference + * to the pixels. + * + * Returns NULL if the requested Info is unsupported. + */ + static SkImage* NewFromRaster(const Info&, const void* pixels, size_t rowBytes, + RasterReleaseProc, ReleaseContext); + /** * Construct a new SkImage based on the given ImageGenerator. * This function will always take ownership of the passed @@ -70,8 +83,25 @@ public: * * Will return NULL if the specified descriptor is unsupported. */ - static SkImage* NewFromTexture(GrContext*, const GrBackendTextureDesc&, - SkAlphaType = kPremul_SkAlphaType); + static SkImage* NewFromTexture(GrContext* ctx, const GrBackendTextureDesc& desc) { + return NewFromTexture(ctx, desc, kPremul_SkAlphaType, NULL, NULL); + } + + static SkImage* NewFromTexture(GrContext* ctx, const GrBackendTextureDesc& de, SkAlphaType at) { + return NewFromTexture(ctx, de, at, NULL, NULL); + } + + typedef void (*TextureReleaseProc)(ReleaseContext); + + /** + * Create a new image from the specified descriptor. The underlying platform texture must stay + * valid and unaltered until the specified release-proc is invoked, indicating that Skia + * nolonger is holding a reference to it. + * + * Will return NULL if the specified descriptor is unsupported. + */ + static SkImage* NewFromTexture(GrContext*, const GrBackendTextureDesc&, SkAlphaType, + TextureReleaseProc, ReleaseContext); /** * Create a new image from the specified descriptor. Note - Skia will delete or recycle the diff --git a/include/core/SkImageInfo.h b/include/core/SkImageInfo.h index daa50dcdb2..9a1ea5f5ee 100644 --- a/include/core/SkImageInfo.h +++ b/include/core/SkImageInfo.h @@ -282,7 +282,11 @@ public: } size_t getSafeSize(size_t rowBytes) const { - return (size_t)this->getSafeSize64(rowBytes); + int64_t size = this->getSafeSize64(rowBytes); + if (!sk_64_isS32(size)) { + return 0; + } + return sk_64_asS32(size); } bool validRowBytes(size_t rowBytes) const { diff --git a/include/gpu/GrSurface.h b/include/gpu/GrSurface.h index 2b40f9c1d7..b01b6928ac 100644 --- a/include/gpu/GrSurface.h +++ b/include/gpu/GrSurface.h @@ -127,6 +127,14 @@ public: inline GrSurfacePriv surfacePriv(); inline const GrSurfacePriv surfacePriv() const; + typedef void* ReleaseCtx; + typedef void (*ReleaseProc)(ReleaseCtx); + + void setRelease(ReleaseProc proc, ReleaseCtx ctx) { + fReleaseProc = proc; + fReleaseCtx = ctx; + } + protected: // Methods made available via GrSurfacePriv SkImageInfo info() const; @@ -140,12 +148,32 @@ protected: GrSurface(GrGpu* gpu, LifeCycle lifeCycle, const GrSurfaceDesc& desc) : INHERITED(gpu, lifeCycle) - , fDesc(desc) { + , fDesc(desc) + , fReleaseProc(NULL) + , fReleaseCtx(NULL) + {} + + ~GrSurface() override { + // check that invokeReleaseProc has been called (if needed) + SkASSERT(NULL == fReleaseProc); } GrSurfaceDesc fDesc; + void invokeReleaseProc() { + if (fReleaseProc) { + fReleaseProc(fReleaseCtx); + fReleaseProc = NULL; + } + } + + void onRelease() override; + void onAbandon() override; + private: + ReleaseProc fReleaseProc; + ReleaseCtx fReleaseCtx; + typedef GrGpuResource INHERITED; }; diff --git a/src/gpu/GrSurface.cpp b/src/gpu/GrSurface.cpp index 678755a58f..c052a235f0 100644 --- a/src/gpu/GrSurface.cpp +++ b/src/gpu/GrSurface.cpp @@ -125,3 +125,13 @@ bool GrSurface::hasPendingIO() const { } return false; } + +void GrSurface::onRelease() { + this->invokeReleaseProc(); + this->INHERITED::onRelease(); +} + +void GrSurface::onAbandon() { + this->invokeReleaseProc(); + this->INHERITED::onAbandon(); +} diff --git a/src/image/SkImage.cpp b/src/image/SkImage.cpp index 520255776f..66756cb512 100644 --- a/src/image/SkImage.cpp +++ b/src/image/SkImage.cpp @@ -205,7 +205,8 @@ SkImage* SkImage_Base::onNewImage(int newWidth, int newHeight, const SkIRect* su #if !SK_SUPPORT_GPU -SkImage* SkImage::NewFromTexture(GrContext*, const GrBackendTextureDesc&, SkAlphaType) { +SkImage* SkImage::NewFromTexture(GrContext*, const GrBackendTextureDesc&, SkAlphaType, + TextureReleaseProc, ReleaseContext) { return NULL; } diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp index 281f762dff..973f4ba304 100644 --- a/src/image/SkImage_Gpu.cpp +++ b/src/image/SkImage_Gpu.cpp @@ -115,7 +115,9 @@ bool SkImage_Gpu::onReadPixels(const SkImageInfo& info, void* pixels, size_t row /////////////////////////////////////////////////////////////////////////////////////////////////// static SkImage* new_wrapped_texture_common(GrContext* ctx, const GrBackendTextureDesc& desc, - SkAlphaType at, GrWrapOwnership ownership) { + SkAlphaType at, GrWrapOwnership ownership, + SkImage::TextureReleaseProc releaseProc, + SkImage::ReleaseContext releaseCtx) { if (desc.fWidth <= 0 || desc.fHeight <= 0) { return NULL; } @@ -123,18 +125,23 @@ static SkImage* new_wrapped_texture_common(GrContext* ctx, const GrBackendTextur if (!tex) { return NULL; } + if (releaseProc) { + tex->setRelease(releaseProc, releaseCtx); + } + const SkSurface::Budgeted budgeted = SkSurface::kNo_Budgeted; return SkNEW_ARGS(SkImage_Gpu, (desc.fWidth, desc.fHeight, at, tex, 0, budgeted)); } -SkImage* SkImage::NewFromTexture(GrContext* ctx, const GrBackendTextureDesc& desc, SkAlphaType at) { - return new_wrapped_texture_common(ctx, desc, at, kBorrow_GrWrapOwnership); +SkImage* SkImage::NewFromTexture(GrContext* ctx, const GrBackendTextureDesc& desc, SkAlphaType at, + TextureReleaseProc releaseP, ReleaseContext releaseC) { + return new_wrapped_texture_common(ctx, desc, at, kBorrow_GrWrapOwnership, releaseP, releaseC); } SkImage* SkImage::NewFromAdoptedTexture(GrContext* ctx, const GrBackendTextureDesc& desc, SkAlphaType at) { - return new_wrapped_texture_common(ctx, desc, at, kAdopt_GrWrapOwnership); + return new_wrapped_texture_common(ctx, desc, at, kAdopt_GrWrapOwnership, NULL, NULL); } SkImage* SkImage::NewFromTextureCopy(GrContext* ctx, const GrBackendTextureDesc& srcDesc, diff --git a/src/image/SkImage_Raster.cpp b/src/image/SkImage_Raster.cpp index 20ae62c9f7..2b64839bc5 100644 --- a/src/image/SkImage_Raster.cpp +++ b/src/image/SkImage_Raster.cpp @@ -16,9 +16,8 @@ class SkImage_Raster : public SkImage_Base { public: - static bool ValidArgs(const Info& info, size_t rowBytes) { + static bool ValidArgs(const Info& info, size_t rowBytes, size_t* minSize) { const int maxDimension = SK_MaxS32 >> 2; - const size_t kMaxPixelByteSize = SK_MaxS32; if (info.width() <= 0 || info.height() <= 0) { return false; @@ -43,10 +42,14 @@ public: return false; } - int64_t size = (int64_t)info.height() * rowBytes; - if (size > (int64_t)kMaxPixelByteSize) { + size_t size = info.getSafeSize(rowBytes); + if (0 == size) { return false; } + + if (minSize) { + *minSize = size; + } return true; } @@ -146,23 +149,24 @@ 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) || !pixels) { + size_t size; + if (!SkImage_Raster::ValidArgs(info, rowBytes, &size) || !pixels) { return NULL; } // Here we actually make a copy of the caller's pixel data - SkAutoDataUnref data(SkData::NewWithCopy(pixels, info.height() * rowBytes)); + SkAutoDataUnref data(SkData::NewWithCopy(pixels, size)); return SkNEW_ARGS(SkImage_Raster, (info, data, rowBytes, NULL)); } SkImage* SkImage::NewRasterData(const SkImageInfo& info, SkData* data, size_t rowBytes) { - if (!SkImage_Raster::ValidArgs(info, rowBytes) || !data) { + size_t size; + if (!SkImage_Raster::ValidArgs(info, rowBytes, &size) || !data) { return NULL; } // did they give us enough data? - size_t size = info.height() * rowBytes; if (data->size() < size) { return NULL; } @@ -170,6 +174,17 @@ SkImage* SkImage::NewRasterData(const SkImageInfo& info, SkData* data, size_t ro return SkNEW_ARGS(SkImage_Raster, (info, data, rowBytes, NULL)); } +SkImage* SkImage::NewFromRaster(const SkImageInfo& info, const void* pixels, size_t rowBytes, + RasterReleaseProc proc, ReleaseContext ctx) { + size_t size; + if (!SkImage_Raster::ValidArgs(info, rowBytes, &size) || !pixels) { + return NULL; + } + + SkAutoDataUnref data(SkData::NewWithProc(pixels, size, proc, ctx)); + return SkNEW_ARGS(SkImage_Raster, (info, data, rowBytes, NULL)); +} + SkImage* SkImage::NewFromGenerator(SkImageGenerator* generator) { SkBitmap bitmap; if (!SkInstallDiscardablePixelRef(generator, &bitmap)) { @@ -185,7 +200,7 @@ SkImage* SkImage::NewFromGenerator(SkImageGenerator* generator) { SkImage* SkNewImageFromPixelRef(const SkImageInfo& info, SkPixelRef* pr, const SkIPoint& pixelRefOrigin, size_t rowBytes, const SkSurfaceProps* props) { - if (!SkImage_Raster::ValidArgs(info, rowBytes)) { + if (!SkImage_Raster::ValidArgs(info, rowBytes, NULL)) { return NULL; } return SkNEW_ARGS(SkImage_Raster, (info, pr, pixelRefOrigin, rowBytes, props)); @@ -193,7 +208,7 @@ SkImage* SkNewImageFromPixelRef(const SkImageInfo& info, SkPixelRef* pr, SkImage* SkNewImageFromBitmap(const SkBitmap& bm, bool canSharePixelRef, const SkSurfaceProps* props) { - if (!SkImage_Raster::ValidArgs(bm.info(), bm.rowBytes())) { + if (!SkImage_Raster::ValidArgs(bm.info(), bm.rowBytes(), NULL)) { return NULL; } diff --git a/tests/SurfaceTest.cpp b/tests/SurfaceTest.cpp index 34e5ab524a..0eebfeb4ef 100644 --- a/tests/SurfaceTest.cpp +++ b/tests/SurfaceTest.cpp @@ -65,6 +65,7 @@ static SkSurface* createSurface(SurfaceType surfaceType, GrContext* context, enum ImageType { kRasterCopy_ImageType, kRasterData_ImageType, + kRasterProc_ImageType, kGpu_ImageType, kCodec_ImageType, }; @@ -81,6 +82,7 @@ static void test_empty_image(skiatest::Reporter* reporter) { REPORTER_ASSERT(reporter, NULL == SkImage::NewRasterCopy(info, NULL, 0)); REPORTER_ASSERT(reporter, NULL == SkImage::NewRasterData(info, NULL, 0)); + REPORTER_ASSERT(reporter, NULL == SkImage::NewFromRaster(info, NULL, 0, NULL, NULL)); REPORTER_ASSERT(reporter, NULL == SkImage::NewFromGenerator(SkNEW(EmptyGenerator))); } @@ -204,7 +206,21 @@ static void test_image(skiatest::Reporter* reporter) { data->unref(); } -static SkImage* createImage(ImageType imageType, GrContext* context, SkColor color) { +// Want to ensure that our Release is called when the owning image is destroyed +struct ReleaseDataContext { + skiatest::Reporter* fReporter; + SkData* fData; + + static void Release(const void* pixels, void* context) { + ReleaseDataContext* state = (ReleaseDataContext*)context; + REPORTER_ASSERT(state->fReporter, state->fData); + state->fData->unref(); + state->fData = NULL; + } +}; + +static SkImage* createImage(ImageType imageType, GrContext* context, SkColor color, + ReleaseDataContext* releaseContext) { const SkPMColor pmcolor = SkPreMultiplyColor(color); const SkImageInfo info = SkImageInfo::MakeN32Premul(10, 10); const size_t rowBytes = info.minRowBytes(); @@ -219,6 +235,11 @@ static SkImage* createImage(ImageType imageType, GrContext* context, SkColor col return SkImage::NewRasterCopy(info, addr, rowBytes); case kRasterData_ImageType: return SkImage::NewRasterData(info, data, rowBytes); + case kRasterProc_ImageType: + SkASSERT(releaseContext); + releaseContext->fData = SkRef(data.get()); + return SkImage::NewFromRaster(info, addr, rowBytes, + ReleaseDataContext::Release, releaseContext); case kGpu_ImageType: { SkAutoTUnref surf( SkSurface::NewRenderTarget(context, SkSurface::kNo_Budgeted, info, 0)); @@ -302,6 +323,7 @@ static void test_imagepeek(skiatest::Reporter* reporter, GrContextFactory* facto } gRec[] = { { kRasterCopy_ImageType, true, "RasterCopy" }, { kRasterData_ImageType, true, "RasterData" }, + { kRasterProc_ImageType, true, "RasterProc" }, { kGpu_ImageType, false, "Gpu" }, { kCodec_ImageType, false, "Codec" }, }; @@ -317,15 +339,25 @@ static void test_imagepeek(skiatest::Reporter* reporter, GrContextFactory* facto } #endif + ReleaseDataContext releaseCtx; + releaseCtx.fReporter = reporter; + for (size_t i = 0; i < SK_ARRAY_COUNT(gRec); ++i) { SkImageInfo info; size_t rowBytes; - SkAutoTUnref image(createImage(gRec[i].fType, ctx, color)); + releaseCtx.fData = NULL; + SkAutoTUnref image(createImage(gRec[i].fType, ctx, color, &releaseCtx)); if (!image.get()) { SkDebugf("failed to createImage[%d] %s\n", i, gRec[i].fName); continue; // gpu may not be enabled } + if (kRasterProc_ImageType == gRec[i].fType) { + REPORTER_ASSERT(reporter, NULL != releaseCtx.fData); // we are tracking the data + } else { + REPORTER_ASSERT(reporter, NULL == releaseCtx.fData); // we ignored the context + } + const void* addr = image->peekPixels(&info, &rowBytes); bool success = SkToBool(addr); REPORTER_ASSERT(reporter, gRec[i].fPeekShouldSucceed == success); @@ -341,6 +373,7 @@ static void test_imagepeek(skiatest::Reporter* reporter, GrContextFactory* facto test_image_readpixels(reporter, image, pmcolor); } + REPORTER_ASSERT(reporter, NULL == releaseCtx.fData); // we released the data } static void test_canvaspeek(skiatest::Reporter* reporter, @@ -739,7 +772,28 @@ DEF_GPUTEST(Surface, reporter, factory) { } #if SK_SUPPORT_GPU -static SkImage* make_desc_image(GrContext* ctx, int w, int h, GrBackendObject texID, bool doCopy) { + +struct ReleaseTextureContext { + ReleaseTextureContext(skiatest::Reporter* reporter) { + fReporter = reporter; + fIsReleased = false; + } + + skiatest::Reporter* fReporter; + bool fIsReleased; + + void doRelease() { + REPORTER_ASSERT(fReporter, false == fIsReleased); + fIsReleased = true; + } + + static void ReleaseProc(void* context) { + ((ReleaseTextureContext*)context)->doRelease(); + } +}; + +static SkImage* make_desc_image(GrContext* ctx, int w, int h, GrBackendObject texID, + ReleaseTextureContext* releaseContext) { GrBackendTextureDesc desc; desc.fConfig = kSkia8888_GrPixelConfig; // need to be a rendertarget for now... @@ -748,7 +802,10 @@ static SkImage* make_desc_image(GrContext* ctx, int w, int h, GrBackendObject te desc.fHeight = h; desc.fSampleCnt = 0; desc.fTextureHandle = texID; - return doCopy ? SkImage::NewFromTextureCopy(ctx, desc) : SkImage::NewFromTexture(ctx, desc); + return releaseContext + ? SkImage::NewFromTexture(ctx, desc, kPremul_SkAlphaType, + ReleaseTextureContext::ReleaseProc, releaseContext) + : SkImage::NewFromTextureCopy(ctx, desc, kPremul_SkAlphaType); } static void test_image_color(skiatest::Reporter* reporter, SkImage* image, SkPMColor expected) { @@ -785,10 +842,12 @@ DEF_GPUTEST(SkImage_NewFromTexture, reporter, factory) { REPORTER_ASSERT(reporter, false); return; } - + GrBackendObject srcTex = tex->getTextureHandle(); - SkAutoTUnref refImg(make_desc_image(ctx, w, h, srcTex, false)); - SkAutoTUnref cpyImg(make_desc_image(ctx, w, h, srcTex, true)); + ReleaseTextureContext releaseCtx(reporter); + + SkAutoTUnref refImg(make_desc_image(ctx, w, h, srcTex, &releaseCtx)); + SkAutoTUnref cpyImg(make_desc_image(ctx, w, h, srcTex, NULL)); test_image_color(reporter, refImg, expected0); test_image_color(reporter, cpyImg, expected0); @@ -801,5 +860,10 @@ DEF_GPUTEST(SkImage_NewFromTexture, reporter, factory) { // We expect the ref'd image to see the new color, but cpy'd one should still see the old color test_image_color(reporter, refImg, expected1); test_image_color(reporter, cpyImg, expected0); + + // Now exercise the release proc + REPORTER_ASSERT(reporter, !releaseCtx.fIsReleased); + refImg.reset(NULL); // force a release of the image + REPORTER_ASSERT(reporter, releaseCtx.fIsReleased); } #endif