Deprecate SkDevice::accessBitmap method

Relies on https://codereview.chromium.org/2162423003/ (Add SK_SUPPORT_LEGACY_ACCESSBITMAP Skia guard) landing in Chromium first.

Calved off: https://codereview.chromium.org/2163323002/ (Add desired width & height to drawContext (as opposed to using the width & height of the RT))

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2168483003

Review-Url: https://codereview.chromium.org/2168483003
This commit is contained in:
robertphillips 2016-07-21 07:17:54 -07:00 committed by Commit bot
parent afbf71dd92
commit 1f3923e4e5
11 changed files with 33 additions and 114 deletions

View File

@ -135,7 +135,11 @@ protected:
altered. The config/width/height/rowbytes must remain unchanged.
@return the device contents as a bitmap
*/
#ifdef SK_SUPPORT_LEGACY_ACCESSBITMAP
const SkBitmap& onAccessBitmap() override;
#else
const SkBitmap& onAccessBitmap();
#endif
SkPixelRef* getPixelRef() const { return fBitmap.pixelRef(); }
// just for subclasses, to assign a custom pixelref

View File

@ -77,6 +77,7 @@ public:
return this->imageInfo().isOpaque();
}
#ifdef SK_SUPPORT_LEGACY_ACCESSBITMAP
/** Return the bitmap associated with this device. Call this each time you need
to access the bitmap, as it notifies the subclass to perform any flushing
etc. before you examine the pixels.
@ -84,6 +85,7 @@ public:
@return the device's bitmap
*/
const SkBitmap& accessBitmap(bool changePixels);
#endif
bool writePixels(const SkImageInfo&, const void*, size_t rowBytes, int x, int y);
@ -276,11 +278,16 @@ protected:
///////////////////////////////////////////////////////////////////////////
#ifdef SK_SUPPORT_LEGACY_ACCESSBITMAP
/** Update as needed the pixel value in the bitmap, so that the caller can
access the pixels directly.
@return The device contents as a bitmap
*/
virtual const SkBitmap& onAccessBitmap() = 0;
virtual const SkBitmap& onAccessBitmap() {
SkASSERT(0);
return fLegacyBitmap;
}
#endif
virtual GrContext* context() const { return nullptr; }
@ -387,6 +394,10 @@ private:
SkMetaData* fMetaData;
SkSurfaceProps fSurfaceProps;
#ifdef SK_SUPPORT_LEGACY_ACCESSBITMAP
SkBitmap fLegacyBitmap;
#endif
#ifdef SK_DEBUG
bool fAttachedToCanvas;
#endif

View File

@ -48,6 +48,7 @@ SkImageInfo SkBaseDevice::imageInfo() const {
return SkImageInfo::MakeUnknown();
}
#ifdef SK_SUPPORT_LEGACY_ACCESSBITMAP
const SkBitmap& SkBaseDevice::accessBitmap(bool changePixels) {
const SkBitmap& bitmap = this->onAccessBitmap();
if (changePixels) {
@ -55,6 +56,7 @@ const SkBitmap& SkBaseDevice::accessBitmap(bool changePixels) {
}
return bitmap;
}
#endif
SkPixelGeometry SkBaseDevice::CreateInfo::AdjustGeometry(const SkImageInfo& info,
TileUsage tileUsage,

View File

@ -182,14 +182,9 @@ SkGpuDevice::SkGpuDevice(sk_sp<GrDrawContext> drawContext, int width, int height
, fContext(SkRef(drawContext->accessRenderTarget()->getContext()))
, fRenderTarget(drawContext->renderTarget())
, fDrawContext(std::move(drawContext)) {
fSize.set(width, height);
fOpaque = SkToBool(flags & kIsOpaque_Flag);
SkAlphaType at = fOpaque ? kOpaque_SkAlphaType : kPremul_SkAlphaType;
SkImageInfo info = fRenderTarget->surfacePriv().info(at).makeWH(width, height);
SkPixelRef* pr = new SkGrPixelRef(info, fRenderTarget.get());
fLegacyBitmap.setInfo(info);
fLegacyBitmap.setPixelRef(pr)->unref();
if (flags & kNeedClear_Flag) {
this->clearAll();
}
@ -284,23 +279,11 @@ bool SkGpuDevice::onWritePixels(const SkImageInfo& info, const void* pixels, siz
}
fRenderTarget->writePixels(x, y, info.width(), info.height(), config, pixels, rowBytes, flags);
// need to bump our genID for compatibility with clients that "know" we have a bitmap
fLegacyBitmap.notifyPixelsChanged();
return true;
}
const SkBitmap& SkGpuDevice::onAccessBitmap() {
ASSERT_SINGLE_OWNER
return fLegacyBitmap;
}
bool SkGpuDevice::onAccessPixels(SkPixmap* pmap) {
ASSERT_SINGLE_OWNER
// For compatibility with clients the know we're backed w/ a bitmap, and want to inspect its
// genID. When we can hide/remove that fact, we can eliminate this call to notify.
// ... ugh.
fLegacyBitmap.notifyPixelsChanged();
return false;
}
@ -370,14 +353,6 @@ void SkGpuDevice::replaceDrawContext(bool shouldRetainContent) {
fRenderTarget = newDC->renderTarget();
#ifdef SK_DEBUG
SkImageInfo info = fRenderTarget->surfacePriv().info(fOpaque ? kOpaque_SkAlphaType :
kPremul_SkAlphaType);
SkASSERT(info == fLegacyBitmap.info());
#endif
SkPixelRef* pr = new SkGrPixelRef(fLegacyBitmap.info(), fRenderTarget.get());
fLegacyBitmap.setPixelRef(pr)->unref();
fDrawContext = newDC;
}

View File

@ -81,7 +81,10 @@ public:
GrDrawContext* accessDrawContext() override;
SkImageInfo imageInfo() const override {
return fLegacyBitmap.info();
SkAlphaType at = fOpaque ? kOpaque_SkAlphaType : kPremul_SkAlphaType;
SkImageInfo info = fRenderTarget->surfacePriv().info(at).makeWH(fSize.fWidth,
fSize.fHeight);
return info;
}
void drawPaint(const SkDraw&, const SkPaint& paint) override;
@ -141,7 +144,6 @@ public:
void onAttachToCanvas(SkCanvas* canvas) override;
void onDetachFromCanvas() override;
const SkBitmap& onAccessBitmap() override;
bool onAccessPixels(SkPixmap*) override;
// for debugging purposes only
@ -161,8 +163,7 @@ private:
SkAutoTUnref<const SkClipStack> fClipStack;
SkIPoint fClipOrigin;
GrClipStackClip fClip;
// remove when our clients don't rely on accessBitmap()
SkBitmap fLegacyBitmap;
SkISize fSize;
bool fOpaque;
enum Flags {

View File

@ -34,8 +34,6 @@ static GrRenderTarget* prepare_rt_for_external_access(SkSurface_Gpu* surface,
case SkSurface::kDiscardWrite_BackendHandleAccess:
// for now we don't special-case on Discard, but we may in the future.
surface->notifyContentWillChange(SkSurface::kRetain_ContentChangeMode);
// legacy: need to dirty the bitmap's genID in our device (curse it)
surface->getDevice()->accessBitmap(false).notifyPixelsChanged();
break;
}

View File

@ -658,8 +658,7 @@ SkPDFDevice::SkPDFDevice(SkISize pageSize, SkScalar rasterDpi, SkPDFDocument* do
, fDocument(doc) {
SkASSERT(pageSize.width() > 0);
SkASSERT(pageSize.height() > 0);
fLegacyBitmap.setInfo(
SkImageInfo::MakeUnknown(pageSize.width(), pageSize.height()));
if (flip) {
// Skia generally uses the top left as the origin but PDF
// natively has the origin at the bottom left. This matrix
@ -1402,7 +1401,8 @@ void SkPDFDevice::drawDevice(const SkDraw& d, SkBaseDevice* device,
}
SkImageInfo SkPDFDevice::imageInfo() const {
return fLegacyBitmap.info();
SkImageInfo info = SkImageInfo::MakeUnknown(fPageSize.width(), fPageSize.height());
return info;
}
void SkPDFDevice::onAttachToCanvas(SkCanvas* canvas) {

View File

@ -193,10 +193,6 @@ public:
};
protected:
const SkBitmap& onAccessBitmap() override {
return fLegacyBitmap;
}
sk_sp<SkSurface> makeSurface(const SkImageInfo&, const SkSurfaceProps&) override;
void drawAnnotation(const SkDraw&, const SkRect&, const char key[], SkData* value) override;
@ -264,8 +260,6 @@ private:
SkScalar fRasterDpi;
SkBitmap fLegacyBitmap;
SkPDFDocument* fDocument;
////////////////////////////////////////////////////////////////////////////

View File

@ -572,11 +572,10 @@ SkBaseDevice* SkSVGDevice::Create(const SkISize& size, SkXMLWriter* writer) {
SkSVGDevice::SkSVGDevice(const SkISize& size, SkXMLWriter* writer)
: INHERITED(SkSurfaceProps(0, kUnknown_SkPixelGeometry))
, fWriter(writer)
, fResourceBucket(new ResourceBucket) {
, fResourceBucket(new ResourceBucket)
, fSize(size) {
SkASSERT(writer);
fLegacyBitmap.setInfo(SkImageInfo::MakeUnknown(size.width(), size.height()));
fWriter->writeHeader();
// The root <svg> tag gets closed by the destructor.
@ -592,11 +591,8 @@ SkSVGDevice::~SkSVGDevice() {
}
SkImageInfo SkSVGDevice::imageInfo() const {
return fLegacyBitmap.info();
}
const SkBitmap& SkSVGDevice::onAccessBitmap() {
return fLegacyBitmap;
SkImageInfo info = SkImageInfo::MakeUnknown(fSize.fWidth, fSize.fHeight);
return info;
}
void SkSVGDevice::drawPaint(const SkDraw& draw, const SkPaint& paint) {

View File

@ -55,7 +55,6 @@ protected:
void drawDevice(const SkDraw&, SkBaseDevice*, int x, int y,
const SkPaint&) override;
const SkBitmap& onAccessBitmap() override;
private:
SkSVGDevice(const SkISize& size, SkXMLWriter* writer);
@ -69,7 +68,7 @@ private:
SkXMLWriter* fWriter;
SkAutoTDelete<AutoElement> fRootElement;
SkAutoTDelete<ResourceBucket> fResourceBucket;
SkBitmap fLegacyBitmap;
SkISize fSize;
typedef SkBaseDevice INHERITED;
};

View File

@ -128,38 +128,6 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SurfaceCanvasPeek_Gpu, reporter, ctxInfo) {
}
#endif
// For compatibility with clients that still call accessBitmap(), we need to ensure that we bump
// the bitmap's genID when we draw to it, else they won't know it has new values. When they are
// exclusively using surface/image, and we can hide accessBitmap from device, we can remove this
// test.
void test_access_pixels(skiatest::Reporter* reporter, const sk_sp<SkSurface>& surface) {
SkCanvas* canvas = surface->getCanvas();
canvas->clear(0);
SkBaseDevice* device = canvas->getDevice_just_for_deprecated_compatibility_testing();
SkBitmap bm = device->accessBitmap(false);
uint32_t genID0 = bm.getGenerationID();
// Now we draw something, which needs to "dirty" the genID (sorta like copy-on-write)
canvas->drawColor(SK_ColorBLUE);
// Now check that we get a different genID
uint32_t genID1 = bm.getGenerationID();
REPORTER_ASSERT(reporter, genID0 != genID1);
}
DEF_TEST(SurfaceAccessPixels, reporter) {
for (auto& surface_func : { &create_surface, &create_direct_surface }) {
auto surface(surface_func(kPremul_SkAlphaType, nullptr));
test_access_pixels(reporter, surface);
}
}
#if SK_SUPPORT_GPU
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SurfaceAccessPixels_Gpu, reporter, ctxInfo) {
for (auto& surface_func : { &create_gpu_surface, &create_gpu_scratch_surface }) {
auto surface(surface_func(ctxInfo.grContext(), kPremul_SkAlphaType, nullptr));
test_access_pixels(reporter, surface);
}
}
#endif
static void test_snapshot_alphatype(skiatest::Reporter* reporter, const sk_sp<SkSurface>& surface,
bool expectOpaque) {
REPORTER_ASSERT(reporter, surface);
@ -380,36 +348,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(UniqueImageSnapshot_Gpu, reporter, ctxInfo) {
#endif
#if SK_SUPPORT_GPU
// May we (soon) eliminate the need to keep testing this, by hiding the bloody device!
static uint32_t get_legacy_gen_id(SkSurface* surface) {
SkBaseDevice* device =
surface->getCanvas()->getDevice_just_for_deprecated_compatibility_testing();
return device->accessBitmap(false).getGenerationID();
}
/*
* Test legacy behavor of bumping the surface's device's bitmap's genID when we access its
* texture handle for writing.
*
* Note: this needs to be tested separately from checking makeImageSnapshot, as calling that
* can also incidentally bump the genID (when a new backing surface is created).
*/
static void test_backend_handle_gen_id(
skiatest::Reporter* reporter, SkSurface* surface,
GrBackendObject (*func)(SkSurface*, SkSurface::BackendHandleAccess)) {
const uint32_t gen0 = get_legacy_gen_id(surface);
func(surface, SkSurface::kFlushRead_BackendHandleAccess);
const uint32_t gen1 = get_legacy_gen_id(surface);
REPORTER_ASSERT(reporter, gen0 == gen1);
func(surface, SkSurface::kFlushWrite_BackendHandleAccess);
const uint32_t gen2 = get_legacy_gen_id(surface);
REPORTER_ASSERT(reporter, gen0 != gen2);
func(surface, SkSurface::kDiscardWrite_BackendHandleAccess);
const uint32_t gen3 = get_legacy_gen_id(surface);
REPORTER_ASSERT(reporter, gen0 != gen3);
REPORTER_ASSERT(reporter, gen2 != gen3);
}
static void test_backend_handle_unique_id(
skiatest::Reporter* reporter, SkSurface* surface,
GrBackendObject (*func)(SkSurface*, SkSurface::BackendHandleAccess)) {
@ -436,7 +375,7 @@ static void test_backend_handle_unique_id(
// No CPU test.
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SurfaceBackendHandleAccessIDs_Gpu, reporter, ctxInfo) {
for (auto& surface_func : { &create_gpu_surface, &create_gpu_scratch_surface }) {
for (auto& test_func : { &test_backend_handle_unique_id, &test_backend_handle_gen_id }) {
for (auto& test_func : { &test_backend_handle_unique_id }) {
for (auto& handle_access_func :
{ &get_surface_backend_texture_handle, &get_surface_backend_render_target_handle}) {
auto surface(surface_func(ctxInfo.grContext(), kPremul_SkAlphaType, nullptr));