diff options
| -rw-r--r-- | gn/android_framework_defines.gni | 1 | ||||
| -rw-r--r-- | gn/tests.gni | 1 | ||||
| -rw-r--r-- | include/core/SkMallocPixelRef.h | 14 | ||||
| -rw-r--r-- | include/core/SkPixelRef.h | 17 | ||||
| -rw-r--r-- | public.bzl | 1 | ||||
| -rw-r--r-- | src/core/SkMallocPixelRef.cpp | 54 | ||||
| -rw-r--r-- | src/core/SkPixelRef.cpp | 50 | ||||
| -rw-r--r-- | tests/DrawBitmapRectTest.cpp | 41 | ||||
| -rw-r--r-- | tests/PDFInvalidBitmapTest.cpp | 63 |
9 files changed, 76 insertions, 166 deletions
diff --git a/gn/android_framework_defines.gni b/gn/android_framework_defines.gni index 9f37ed68b2..f4ebd1fbc8 100644 --- a/gn/android_framework_defines.gni +++ b/gn/android_framework_defines.gni @@ -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", ] diff --git a/gn/tests.gni b/gn/tests.gni index be99a9efd1..a7060b6272 100644 --- a/gn/tests.gni +++ b/gn/tests.gni @@ -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", diff --git a/include/core/SkMallocPixelRef.h b/include/core/SkMallocPixelRef.h index 2e4c4e69eb..a62da0e46b 100644 --- a/include/core/SkMallocPixelRef.h +++ b/include/core/SkMallocPixelRef.h @@ -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); diff --git a/include/core/SkPixelRef.h b/include/core/SkPixelRef.h index 5987ee59fb..9cad27979d 100644 --- a/include/core/SkPixelRef.h +++ b/include/core/SkPixelRef.h @@ -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; diff --git a/public.bzl b/public.bzl index 01d70f7bcd..fe27fdbec7 100644 --- a/public.bzl +++ b/public.bzl @@ -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", ] ################################################################################ diff --git a/src/core/SkMallocPixelRef.cpp b/src/core/SkMallocPixelRef.cpp index e42a15db21..ed3a97b7e0 100644 --- a/src/core/SkMallocPixelRef.cpp +++ b/src/core/SkMallocPixelRef.cpp @@ -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 diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp index 2dc0369f58..942f127ed3 100644 --- a/src/core/SkPixelRef.cpp +++ b/src/core/SkPixelRef.cpp @@ -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() { diff --git a/tests/DrawBitmapRectTest.cpp b/tests/DrawBitmapRectTest.cpp index 62da1c2af8..df410b2b00 100644 --- a/tests/DrawBitmapRectTest.cpp +++ b/tests/DrawBitmapRectTest.cpp @@ -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); } diff --git a/tests/PDFInvalidBitmapTest.cpp b/tests/PDFInvalidBitmapTest.cpp deleted file mode 100644 index 54acf02f40..0000000000 --- a/tests/PDFInvalidBitmapTest.cpp +++ /dev/null @@ -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. -} |