switch over to no lockPixels in pixelref

Bug: skia:6481
Change-Id: I7c32d2e6dcd4c9cd8aa761ac5c4794c916eb650a
Reviewed-on: https://skia-review.googlesource.com/13193
Reviewed-by: Florin Malita <fmalita@chromium.org>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Mike Reed <reed@google.com>
This commit is contained in:
Mike Reed 2017-04-11 15:50:02 -04:00 committed by Skia Commit-Bot
parent 921ebe5b73
commit a00f347747
9 changed files with 76 additions and 166 deletions

View File

@ -15,4 +15,5 @@ android_framework_defines = [
"SK_SUPPORT_LEGACY_SHADER_ISABITMAP",
"SK_SUPPORT_LEGACY_EMBOSSMASKFILTER",
"SK_SUPPORT_OBSOLETE_REPLAYCLIP",
"SK_SUPPORT_LEGACY_NO_ADDR_PIXELREF",
]

View File

@ -148,7 +148,6 @@ tests_sources = [
"$_tests/PDFDeflateWStreamTest.cpp",
"$_tests/PDFDocumentTest.cpp",
"$_tests/PDFGlyphsToUnicodeTest.cpp",
"$_tests/PDFInvalidBitmapTest.cpp",
"$_tests/PDFJpegEmbedTest.cpp",
"$_tests/PDFMetadataAttributeTest.cpp",
"$_tests/PDFOpaqueSrcModeToSrcOverTest.cpp",

View File

@ -98,16 +98,13 @@ public:
SkData* data);
#endif
void* getAddr() const { return fStorage; }
protected:
// The ownPixels version of this constructor is deprecated.
SkMallocPixelRef(const SkImageInfo&, void* addr, size_t rb, SkColorTable*,
bool ownPixels);
~SkMallocPixelRef() override;
#ifdef SK_SUPPORT_LEGACY_NO_ADDR_PIXELREF
bool onNewLockPixels(LockRec*) override;
void onUnlockPixels() override;
#endif
size_t getAllocatedSizeInBytes() const override;
private:
@ -117,11 +114,8 @@ private:
size_t rowBytes,
sk_sp<SkColorTable>);
void* fStorage;
sk_sp<SkColorTable> fCTable;
size_t fRB;
ReleaseProc fReleaseProc;
void* fReleaseProcContext;
ReleaseProc fReleaseProc;
void* fReleaseProcContext;
SkMallocPixelRef(const SkImageInfo&, void* addr, size_t rb, sk_sp<SkColorTable>,
ReleaseProc proc, void* context);

View File

@ -35,7 +35,11 @@ class SkDiscardableMemory;
*/
class SK_API SkPixelRef : public SkRefCnt {
public:
#ifdef SK_SUPPORT_LEGACY_NO_ADDR_PIXELREF
explicit SkPixelRef(const SkImageInfo&);
#endif
explicit SkPixelRef(const SkImageInfo&, void* addr, size_t rowBytes,
sk_sp<SkColorTable> = nullptr);
virtual ~SkPixelRef();
const SkImageInfo& info() const {
@ -209,6 +213,7 @@ public:
virtual SkDiscardableMemory* diagnostic_only_getDiscardable() const { return NULL; }
protected:
#ifdef SK_SUPPORT_LEGACY_NO_ADDR_PIXELREF
/**
* On success, returns true and fills out the LockRec for the pixels. On
* failure returns false and ignores the LockRec parameter.
@ -227,6 +232,15 @@ protected:
* method need not do that.
*/
virtual void onUnlockPixels() = 0;
#else
bool onNewLockPixels(LockRec*) {
SkASSERT(false); // should never be called
return true;
}
void onUnlockPixels() {
SkASSERT(false); // should never be called
}
#endif
// default impl does nothing.
virtual void onNotifyPixelsChanged();
@ -246,16 +260,19 @@ protected:
*/
SkBaseMutex* mutex() const { return &fMutex; }
#ifdef SK_SUPPORT_LEGACY_NO_ADDR_PIXELREF
// only call from constructor. Flags this to always be locked, removing
// the need to grab the mutex and call onLockPixels/onUnlockPixels.
// Performance tweak to avoid those calls (esp. in multi-thread use case).
void setPreLocked(void*, size_t rowBytes, SkColorTable*);
#endif
private:
mutable SkMutex fMutex;
// mostly const. fInfo.fAlpahType can be changed at runtime.
const SkImageInfo fInfo;
sk_sp<SkColorTable> fCTable; // duplicated in LockRec, will unify later
// LockRec is only valid if we're in a locked state (isLocked())
LockRec fRec;

View File

@ -656,6 +656,7 @@ DEFINES_ALL = [
# Staging flags for API changes
# Temporarily Disable analytic AA for Google3
"SK_NO_ANALYTIC_AA",
"SK_SUPPORT_LEGACY_NO_ADDR_PIXELREF",
]
################################################################################

View File

@ -130,8 +130,7 @@ sk_sp<SkPixelRef> SkMallocPixelRef::MakeWithData(const SkImageInfo& info,
if (!is_valid(info, ctable.get())) {
return nullptr;
}
if ((rowBytes < info.minRowBytes())
|| (data->size() < info.getSafeSize(rowBytes))) {
if ((rowBytes < info.minRowBytes()) || (data->size() < info.getSafeSize(rowBytes))) {
return nullptr;
}
// must get this address before we call release
@ -144,69 +143,44 @@ sk_sp<SkPixelRef> SkMallocPixelRef::MakeWithData(const SkImageInfo& info,
///////////////////////////////////////////////////////////////////////////////
SkMallocPixelRef::SkMallocPixelRef(const SkImageInfo& info, void* storage,
size_t rowBytes, SkColorTable* ctable,
bool ownsPixels)
: INHERITED(info)
, fCTable(sk_ref_sp(ctable))
, fReleaseProc(ownsPixels ? sk_free_releaseproc : nullptr)
, fReleaseProcContext(nullptr) {
// This constructor is now DEPRICATED.
SkASSERT(is_valid(info, fCTable.get()));
SkASSERT(rowBytes >= info.minRowBytes());
if (kIndex_8_SkColorType != info.colorType()) {
fCTable = nullptr;
static sk_sp<SkColorTable> sanitize(const SkImageInfo& info, sk_sp<SkColorTable> ctable) {
if (kIndex_8_SkColorType == info.colorType()) {
SkASSERT(ctable);
} else {
ctable.reset(nullptr);
}
fStorage = storage;
fRB = rowBytes;
this->setPreLocked(fStorage, rowBytes, fCTable.get());
return ctable;
}
SkMallocPixelRef::SkMallocPixelRef(const SkImageInfo& info, void* storage,
size_t rowBytes, sk_sp<SkColorTable> ctable,
SkMallocPixelRef::ReleaseProc proc,
void* context)
: INHERITED(info)
: INHERITED(info, storage, rowBytes, sanitize(info, std::move(ctable)))
, fReleaseProc(proc)
, fReleaseProcContext(context)
{
SkASSERT(is_valid(info, ctable.get()));
SkASSERT(rowBytes >= info.minRowBytes());
if (kIndex_8_SkColorType != info.colorType()) {
ctable.reset(nullptr);
}
fStorage = storage;
fCTable = std::move(ctable);
fRB = rowBytes;
this->setPreLocked(fStorage, rowBytes, fCTable.get());
}
{}
SkMallocPixelRef::~SkMallocPixelRef() {
if (fReleaseProc != nullptr) {
fReleaseProc(fStorage, fReleaseProcContext);
fReleaseProc(this->pixels(), fReleaseProcContext);
}
}
#ifdef SK_SUPPORT_LEGACY_NO_ADDR_PIXELREF
bool SkMallocPixelRef::onNewLockPixels(LockRec* rec) {
rec->fPixels = fStorage;
rec->fRowBytes = fRB;
rec->fColorTable = fCTable.get();
sk_throw(); // should never get here
return true;
}
void SkMallocPixelRef::onUnlockPixels() {
// nothing to do
}
#endif
size_t SkMallocPixelRef::getAllocatedSizeInBytes() const {
return this->info().getSafeSize(fRB);
return this->info().getSafeSize(this->rowBytes());
}
#ifdef SK_SUPPORT_LEGACY_PIXELREFFACTORY

View File

@ -36,10 +36,22 @@ static SkImageInfo validate_info(const SkImageInfo& info) {
return info.makeAlphaType(newAlphaType);
}
static void validate_pixels_ctable(const SkImageInfo& info, const SkColorTable* ctable) {
if (info.isEmpty()) {
return; // can't require ctable if the dimensions are empty
}
if (kIndex_8_SkColorType == info.colorType()) {
SkASSERT(ctable);
} else {
SkASSERT(nullptr == ctable);
}
}
#ifdef SK_TRACE_PIXELREF_LIFETIME
static int32_t gInstCounter;
#endif
#ifdef SK_SUPPORT_LEGACY_NO_ADDR_PIXELREF
SkPixelRef::SkPixelRef(const SkImageInfo& info)
: fInfo(validate_info(info))
#ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
@ -57,6 +69,31 @@ SkPixelRef::SkPixelRef(const SkImageInfo& info)
fPreLocked = false;
fAddedToCache.store(false);
}
#endif
SkPixelRef::SkPixelRef(const SkImageInfo& info, void* pixels, size_t rowBytes,
sk_sp<SkColorTable> ctable)
: fInfo(validate_info(info))
, fCTable(std::move(ctable))
#ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
, fStableID(SkNextID::ImageID())
#endif
{
validate_pixels_ctable(fInfo, fCTable.get());
SkASSERT(rowBytes >= info.minRowBytes());
#ifdef SK_TRACE_PIXELREF_LIFETIME
SkDebugf(" pixelref %d\n", sk_atomic_inc(&gInstCounter));
#endif
fRec.fPixels = pixels;
fRec.fRowBytes = rowBytes;
fRec.fColorTable = fCTable.get();
fLockCount = SKPIXELREF_PRELOCKED_LOCKCOUNT;
this->needsNewGenID();
fMutability = kMutable;
fPreLocked = true;
fAddedToCache.store(false);
}
SkPixelRef::~SkPixelRef() {
#ifndef SK_SUPPORT_LEGACY_UNBALANCED_PIXELREF_LOCKCOUNT
@ -88,17 +125,7 @@ void SkPixelRef::cloneGenID(const SkPixelRef& that) {
SkASSERT(!that. genIDIsUnique());
}
static void validate_pixels_ctable(const SkImageInfo& info, const SkColorTable* ctable) {
if (info.isEmpty()) {
return; // can't require ctable if the dimensions are empty
}
if (kIndex_8_SkColorType == info.colorType()) {
SkASSERT(ctable);
} else {
SkASSERT(nullptr == ctable);
}
}
#ifdef SK_SUPPORT_LEGACY_NO_ADDR_PIXELREF
void SkPixelRef::setPreLocked(void* pixels, size_t rowBytes, SkColorTable* ctable) {
SkASSERT(pixels);
validate_pixels_ctable(fInfo, ctable);
@ -110,6 +137,7 @@ void SkPixelRef::setPreLocked(void* pixels, size_t rowBytes, SkColorTable* ctabl
fLockCount = SKPIXELREF_PRELOCKED_LOCKCOUNT;
fPreLocked = true;
}
#endif
// Increments fLockCount only on success
bool SkPixelRef::lockPixelsInsideMutex() {

View File

@ -19,46 +19,6 @@
#include "SkSurface.h"
#include "Test.h"
class FailurePixelRef : public SkPixelRef {
public:
FailurePixelRef(const SkImageInfo& info) : SkPixelRef(info) {}
protected:
bool onNewLockPixels(LockRec*) override { return false; }
void onUnlockPixels() override {}
};
// crbug.com/295895
// Crashing in skia when a pixelref fails in lockPixels
//
static void test_faulty_pixelref(skiatest::Reporter* reporter) {
// need a cache, but don't expect to use it, so the budget is not critical
sk_sp<SkDiscardableMemoryPool> pool(
SkDiscardableMemoryPool::Create(10 * 1000, nullptr));
SkBitmap bm;
const SkImageInfo info = SkImageInfo::MakeN32Premul(100, 100);
bm.setInfo(info);
bm.setPixelRef(sk_make_sp<FailurePixelRef>(info), 0, 0);
// now our bitmap has a pixelref, but we know it will fail to lock
auto surface(SkSurface::MakeRasterN32Premul(200, 200));
SkCanvas* canvas = surface->getCanvas();
const SkFilterQuality levels[] = {
kNone_SkFilterQuality,
kLow_SkFilterQuality,
kMedium_SkFilterQuality,
kHigh_SkFilterQuality,
};
SkPaint paint;
canvas->scale(2, 2); // need a scale, otherwise we may ignore filtering
for (size_t i = 0; i < SK_ARRAY_COUNT(levels); ++i) {
paint.setFilterQuality(levels[i]);
canvas->drawBitmap(bm, 0, 0, &paint);
}
}
///////////////////////////////////////////////////////////////////////////////
static void rand_matrix(SkMatrix* mat, SkRandom& rand, unsigned mask) {
@ -308,5 +268,4 @@ DEF_TEST(DrawBitmapRect, reporter) {
test_giantrepeat_crbug118018(reporter);
test_treatAsSprite(reporter);
test_faulty_pixelref(reporter);
}

View File

@ -1,63 +0,0 @@
/*
* Copyright 2013 Google Inc.
*
* Use of this source code is governed by a BSD-style license that can be
* found in the LICENSE file.
*/
#include "SkBitmap.h"
#include "SkCanvas.h"
#include "SkDocument.h"
#include "SkImageInfo.h"
#include "SkPixelRef.h"
#include "SkRefCnt.h"
#include "SkStream.h"
#include "Test.h"
namespace {
// SkPixelRef which fails to lock, as a lazy pixel ref might if its pixels
// cannot be generated.
class InvalidPixelRef : public SkPixelRef {
public:
InvalidPixelRef(const SkImageInfo& info) : SkPixelRef(info) {}
private:
bool onNewLockPixels(LockRec*) override { return false; }
void onUnlockPixels() override {
SkDEBUGFAIL("InvalidPixelRef can't be locked");
}
};
SkBitmap make_invalid_bitmap(const SkImageInfo& imageInfo) {
SkBitmap bitmap;
bitmap.setInfo(imageInfo);
bitmap.setPixelRef(sk_make_sp<InvalidPixelRef>(imageInfo), 0 ,0);
return bitmap;
}
SkBitmap make_invalid_bitmap(SkColorType colorType) {
return make_invalid_bitmap(
SkImageInfo::Make(100, 100, colorType, kPremul_SkAlphaType));
}
} // namespace
DEF_TEST(SkPDF_InvalidBitmap, reporter) {
SkDynamicMemoryWStream stream;
sk_sp<SkDocument> document(SkDocument::MakePDF(&stream));
if (!document) {
return;
}
SkCanvas* canvas = document->beginPage(100, 100);
canvas->drawBitmap(SkBitmap(), 0, 0);
canvas->drawBitmap(make_invalid_bitmap(SkImageInfo()), 0, 0);
canvas->drawBitmap(make_invalid_bitmap(kN32_SkColorType), 0, 0);
canvas->drawBitmap(make_invalid_bitmap(kIndex_8_SkColorType), 0, 0);
canvas->drawBitmap(make_invalid_bitmap(kARGB_4444_SkColorType), 0, 0);
canvas->drawBitmap(make_invalid_bitmap(kRGB_565_SkColorType), 0, 0);
canvas->drawBitmap(make_invalid_bitmap(kAlpha_8_SkColorType), 0, 0);
// This test passes if it does not crash.
}