Revert "Only store width and height on SkPixelRef"
This reverts commit 2cbb6662e329981840f90ef4edd62f70f69e6030.
Reason for revert: Likely cause of Chromium DEPS roll failure; speculative revert.
Original change's description:
> Only store width and height on SkPixelRef
>
> Bug: skia:6535
> Change-Id: Id91e8d1e82f593be7d4b23ca5abde752f2666a77
> Reviewed-on: https://skia-review.googlesource.com/14105
> Commit-Queue: Matt Sarett <msarett@google.com>
> Reviewed-by: Mike Reed <reed@google.com>
>
TBR=djsollen@google.com,msarett@google.com,reed@google.com,stani@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Change-Id: I12a024a71833f33432d5ea8cffdfc642b8b4240a
Bug: skia:
Reviewed-on: https://skia-review.googlesource.com/14644
Reviewed-by: Ben Wagner <benjaminwagner@google.com>
Commit-Queue: Ben Wagner <benjaminwagner@google.com>
diff --git a/gn/android_framework_defines.gni b/gn/android_framework_defines.gni
index bc49f4b..67c6a2d 100644
--- a/gn/android_framework_defines.gni
+++ b/gn/android_framework_defines.gni
@@ -9,7 +9,6 @@
"SK_IGNORE_LINEONLY_AA_CONVEX_PATH_OPTS",
"SK_SUPPORT_LEGACY_GRADIENT_DITHERING",
"SK_SUPPORT_LEGACY_DRAWFILTER",
- "SK_SUPPORT_LEGACY_PIXELREF_API",
"SK_IGNORE_GPU_DITHER",
"SK_SUPPORT_LEGACY_SHADER_ISABITMAP",
"SK_SUPPORT_LEGACY_EMBOSSMASKFILTER",
diff --git a/include/core/SkMallocPixelRef.h b/include/core/SkMallocPixelRef.h
index e6d9727..45ab654 100644
--- a/include/core/SkMallocPixelRef.h
+++ b/include/core/SkMallocPixelRef.h
@@ -78,6 +78,8 @@
protected:
~SkMallocPixelRef() override;
+ size_t getAllocatedSizeInBytes() const override;
+
private:
// Uses alloc to implement NewAllocate or NewZeroed.
static sk_sp<SkPixelRef> MakeUsing(void*(*alloc)(size_t),
diff --git a/include/core/SkPixelRef.h b/include/core/SkPixelRef.h
index 535c6e1..6a86e59 100644
--- a/include/core/SkPixelRef.h
+++ b/include/core/SkPixelRef.h
@@ -32,42 +32,13 @@
*/
class SK_API SkPixelRef : public SkRefCnt {
public:
-#ifdef SK_SUPPORT_LEGACY_PIXELREF_API
SkPixelRef(const SkImageInfo&, void* addr, size_t rowBytes, sk_sp<SkColorTable> = nullptr);
+ ~SkPixelRef() override;
const SkImageInfo& info() const {
return fInfo;
}
-#ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
- // This is undefined if there are clients in-flight trying to use us
- void android_only_reset(const SkImageInfo&, size_t rowBytes, sk_sp<SkColorTable>);
-#endif
-
- /**
- * Change the info's AlphaType. Note that this does not automatically
- * invalidate the generation ID. If the pixel values themselves have
- * changed, then you must explicitly call notifyPixelsChanged() as well.
- */
- void changeAlphaType(SkAlphaType at);
-
- /**
- * Returns the size (in bytes) of the internally allocated memory.
- * This should be implemented in all serializable SkPixelRef derived classes.
- * SkBitmap::fPixelRefOffset + SkBitmap::getSafeSize() should never overflow this value,
- * otherwise the rendering code may attempt to read memory out of bounds.
- *
- * @return default impl returns 0.
- */
- virtual size_t getAllocatedSizeInBytes() const { return 0; }
-
-#endif
- SkPixelRef(int width, int height, void* addr, size_t rowBytes, sk_sp<SkColorTable> = nullptr);
-
- ~SkPixelRef() override;
-
- int width() const { return fInfo.width(); }
- int height() const { return fInfo.height(); }
void* pixels() const { return fPixels; }
SkColorTable* colorTable() const { return fCTable.get(); }
size_t rowBytes() const { return fRowBytes; }
@@ -97,6 +68,13 @@
*/
void notifyPixelsChanged();
+ /**
+ * Change the info's AlphaType. Note that this does not automatically
+ * invalidate the generation ID. If the pixel values themselves have
+ * changed, then you must explicitly call notifyPixelsChanged() as well.
+ */
+ void changeAlphaType(SkAlphaType at);
+
/** Returns true if this pixelref is marked as immutable, meaning that the
contents of its pixels will not change for the lifetime of the pixelref.
*/
@@ -136,18 +114,27 @@
// default impl does nothing.
virtual void onNotifyPixelsChanged();
+ /**
+ * Returns the size (in bytes) of the internally allocated memory.
+ * This should be implemented in all serializable SkPixelRef derived classes.
+ * SkBitmap::fPixelRefOffset + SkBitmap::getSafeSize() should never overflow this value,
+ * otherwise the rendering code may attempt to read memory out of bounds.
+ *
+ * @return default impl returns 0.
+ */
+ virtual size_t getAllocatedSizeInBytes() const;
+
#ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
// This is undefined if there are clients in-flight trying to use us
- void android_only_reset(int width, int height, size_t rowBytes, sk_sp<SkColorTable>);
+ void android_only_reset(const SkImageInfo&, size_t rowBytes, sk_sp<SkColorTable>);
#endif
private:
- // TODO (msarett): After we remove legacy APIs, we should replace |fInfo| with just a width
- // and height.
- const SkImageInfo fInfo;
+ // mostly const. fInfo.fAlpahType can be changed at runtime.
+ const SkImageInfo fInfo;
sk_sp<SkColorTable> fCTable;
- void* fPixels;
- size_t fRowBytes;
+ void* fPixels;
+ size_t fRowBytes;
// Bottom bit indicates the Gen ID is unique.
bool genIDIsUnique() const { return SkToBool(fTaggedGenID.load() & 1); }
diff --git a/public.bzl b/public.bzl
index f0ac995..9c41cdb 100644
--- a/public.bzl
+++ b/public.bzl
@@ -655,10 +655,8 @@
# Turn on a few Google3-specific build fixes.
"GOOGLE3",
# Staging flags for API changes
- "SK_SUPPORT_LEGACY_PIXELREF_API",
# Temporarily Disable analytic AA for Google3
"SK_NO_ANALYTIC_AA",
-
]
################################################################################
diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index 8bd2684..1ba2968 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -171,6 +171,9 @@
}
if (fInfo.alphaType() != newAlphaType) {
fInfo = fInfo.makeAlphaType(newAlphaType);
+ if (fPixelRef) {
+ fPixelRef->changeAlphaType(newAlphaType);
+ }
}
SkDEBUGCODE(this->validate();)
return true;
@@ -195,15 +198,32 @@
#ifdef SK_DEBUG
if (pr) {
if (kUnknown_SkColorType != fInfo.colorType()) {
- SkASSERT(fInfo.width() <= pr->width());
- SkASSERT(fInfo.height() <= pr->height());
+ const SkImageInfo& prInfo = pr->info();
+ SkASSERT(fInfo.width() <= prInfo.width());
+ SkASSERT(fInfo.height() <= prInfo.height());
+ SkASSERT(fInfo.colorType() == prInfo.colorType());
+ switch (prInfo.alphaType()) {
+ case kUnknown_SkAlphaType:
+ SkASSERT(fInfo.alphaType() == kUnknown_SkAlphaType);
+ break;
+ case kOpaque_SkAlphaType:
+ case kPremul_SkAlphaType:
+ SkASSERT(fInfo.alphaType() == kOpaque_SkAlphaType ||
+ fInfo.alphaType() == kPremul_SkAlphaType);
+ break;
+ case kUnpremul_SkAlphaType:
+ SkASSERT(fInfo.alphaType() == kOpaque_SkAlphaType ||
+ fInfo.alphaType() == kUnpremul_SkAlphaType);
+ break;
+ }
}
}
#endif
fPixelRef = std::move(pr);
if (fPixelRef) {
- fPixelRefOrigin.set(SkTPin(dx, 0, fPixelRef->width()), SkTPin(dy, 0, fPixelRef->height()));
+ const SkImageInfo& info = fPixelRef->info();
+ fPixelRefOrigin.set(SkTPin(dx, 0, info.width()), SkTPin(dy, 0, info.height()));
this->updatePixelsFromRef();
} else {
// ignore dx,dy if there is no pixelref
@@ -823,7 +843,7 @@
if (!pr) {
return false;
}
- bitmap->setInfo(info);
+ bitmap->setInfo(pr->info());
bitmap->setPixelRef(std::move(pr), 0, 0);
return true;
}
@@ -863,8 +883,8 @@
SkASSERT(fPixelRef->rowBytes() == fRowBytes);
SkASSERT(fPixelRefOrigin.fX >= 0);
SkASSERT(fPixelRefOrigin.fY >= 0);
- SkASSERT(fPixelRef->width() >= (int)this->width() + fPixelRefOrigin.fX);
- SkASSERT(fPixelRef->height() >= (int)this->height() + fPixelRefOrigin.fY);
+ SkASSERT(fPixelRef->info().width() >= (int)this->width() + fPixelRefOrigin.fX);
+ SkASSERT(fPixelRef->info().height() >= (int)this->height() + fPixelRefOrigin.fY);
SkASSERT(fPixelRef->rowBytes() >= fInfo.minRowBytes());
}
}
diff --git a/src/core/SkMallocPixelRef.cpp b/src/core/SkMallocPixelRef.cpp
index cf4fac7..cdc555b 100644
--- a/src/core/SkMallocPixelRef.cpp
+++ b/src/core/SkMallocPixelRef.cpp
@@ -156,7 +156,7 @@
size_t rowBytes, sk_sp<SkColorTable> ctable,
SkMallocPixelRef::ReleaseProc proc,
void* context)
- : INHERITED(info.width(), info.height(), storage, rowBytes, sanitize(info, std::move(ctable)))
+ : INHERITED(info, storage, rowBytes, sanitize(info, std::move(ctable)))
, fReleaseProc(proc)
, fReleaseProcContext(context)
{}
@@ -167,3 +167,7 @@
fReleaseProc(this->pixels(), fReleaseProcContext);
}
}
+
+size_t SkMallocPixelRef::getAllocatedSizeInBytes() const {
+ return this->info().getSafeSize(this->rowBytes());
+}
diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp
index 487779c..c6d59ea 100644
--- a/src/core/SkPixelRef.cpp
+++ b/src/core/SkPixelRef.cpp
@@ -26,12 +26,6 @@
///////////////////////////////////////////////////////////////////////////////
-#ifdef SK_TRACE_PIXELREF_LIFETIME
- static int32_t gInstCounter;
-#endif
-
-#ifdef SK_SUPPORT_LEGACY_PIXELREF_API
-
static SkImageInfo validate_info(const SkImageInfo& info) {
SkAlphaType newAlphaType = info.alphaType();
SkAssertResult(SkColorTypeValidateAlphaType(info.colorType(), info.alphaType(), &newAlphaType));
@@ -49,6 +43,10 @@
}
}
+#ifdef SK_TRACE_PIXELREF_LIFETIME
+ static int32_t gInstCounter;
+#endif
+
SkPixelRef::SkPixelRef(const SkImageInfo& info, void* pixels, size_t rowBytes,
sk_sp<SkColorTable> ctable)
: fInfo(validate_info(info))
@@ -70,27 +68,6 @@
fAddedToCache.store(false);
}
-#endif
-
-SkPixelRef::SkPixelRef(int width, int height, void* pixels, size_t rowBytes,
- sk_sp<SkColorTable> ctable)
- : fInfo(SkImageInfo::MakeUnknown(width, height))
- , fCTable(std::move(ctable))
- , fPixels(pixels)
- , fRowBytes(rowBytes)
-#ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
- , fStableID(SkNextID::ImageID())
-#endif
-{
-#ifdef SK_TRACE_PIXELREF_LIFETIME
- SkDebugf(" pixelref %d\n", sk_atomic_inc(&gInstCounter));
-#endif
-
- this->needsNewGenID();
- fMutability = kMutable;
- fAddedToCache.store(false);
-}
-
SkPixelRef::~SkPixelRef() {
#ifdef SK_TRACE_PIXELREF_LIFETIME
SkDebugf("~pixelref %d\n", sk_atomic_dec(&gInstCounter) - 1);
@@ -99,11 +76,11 @@
}
#ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
-
-#ifdef SK_SUPPORT_LEGACY_PIXELREF_API
// This is undefined if there are clients in-flight trying to use us
void SkPixelRef::android_only_reset(const SkImageInfo& info, size_t rowBytes,
sk_sp<SkColorTable> ctable) {
+ validate_pixels_ctable(info, ctable.get());
+
*const_cast<SkImageInfo*>(&fInfo) = info;
fRowBytes = rowBytes;
fCTable = std::move(ctable);
@@ -114,20 +91,6 @@
}
#endif
-// This is undefined if there are clients in-flight trying to use us
-void SkPixelRef::android_only_reset(int width, int height, size_t rowBytes,
- sk_sp<SkColorTable> ctable) {
- *const_cast<SkImageInfo*>(&fInfo) = fInfo.makeWH(width, height);
- fRowBytes = rowBytes;
- fCTable = std::move(ctable);
- // note: we do not change fPixels
-
- // conservative, since its possible the "new" settings are the same as the old.
- this->notifyPixelsChanged();
-}
-
-#endif
-
void SkPixelRef::needsNewGenID() {
fTaggedGenID.store(0);
SkASSERT(!this->genIDIsUnique()); // This method isn't threadsafe, so the assert should be fine.
@@ -186,11 +149,9 @@
this->onNotifyPixelsChanged();
}
-#ifdef SK_SUPPORT_LEGACY_PIXELREF_API
void SkPixelRef::changeAlphaType(SkAlphaType at) {
*const_cast<SkImageInfo*>(&fInfo) = fInfo.makeAlphaType(at);
}
-#endif
void SkPixelRef::setImmutable() {
fMutability = kImmutable;
@@ -218,3 +179,7 @@
}
void SkPixelRef::onNotifyPixelsChanged() { }
+
+size_t SkPixelRef::getAllocatedSizeInBytes() const {
+ return 0;
+}
diff --git a/src/core/SkSpecialSurface.cpp b/src/core/SkSpecialSurface.cpp
index 9564a14..33ef97e 100644
--- a/src/core/SkSpecialSurface.cpp
+++ b/src/core/SkSpecialSurface.cpp
@@ -63,11 +63,12 @@
class SkSpecialSurface_Raster : public SkSpecialSurface_Base {
public:
- SkSpecialSurface_Raster(const SkImageInfo& info,
- sk_sp<SkPixelRef> pr,
+ SkSpecialSurface_Raster(sk_sp<SkPixelRef> pr,
const SkIRect& subset,
const SkSurfaceProps* props)
: INHERITED(subset, props) {
+ const SkImageInfo& info = pr->info();
+
fBitmap.setInfo(info, info.minRowBytes());
fBitmap.setPixelRef(std::move(pr), 0, 0);
@@ -92,7 +93,7 @@
sk_sp<SkSpecialSurface> SkSpecialSurface::MakeFromBitmap(const SkIRect& subset, SkBitmap& bm,
const SkSurfaceProps* props) {
- return sk_make_sp<SkSpecialSurface_Raster>(bm.info(), sk_ref_sp(bm.pixelRef()), subset, props);
+ return sk_make_sp<SkSpecialSurface_Raster>(sk_ref_sp(bm.pixelRef()), subset, props);
}
sk_sp<SkSpecialSurface> SkSpecialSurface::MakeRaster(const SkImageInfo& info,
@@ -102,9 +103,9 @@
return nullptr;
}
- const SkIRect subset = SkIRect::MakeWH(info.width(), info.height());
+ const SkIRect subset = SkIRect::MakeWH(pr->info().width(), pr->info().height());
- return sk_make_sp<SkSpecialSurface_Raster>(info, std::move(pr), subset, props);
+ return sk_make_sp<SkSpecialSurface_Raster>(std::move(pr), subset, props);
}
#if SK_SUPPORT_GPU
diff --git a/src/image/SkImage_Raster.cpp b/src/image/SkImage_Raster.cpp
index 8884755..91de4c5 100644
--- a/src/image/SkImage_Raster.cpp
+++ b/src/image/SkImage_Raster.cpp
@@ -28,7 +28,7 @@
// fixes https://bug.skia.org/5096
static bool is_not_subset(const SkBitmap& bm) {
SkASSERT(bm.pixelRef());
- SkISize dim = SkISize::Make(bm.pixelRef()->width(), bm.pixelRef()->height());
+ SkISize dim = bm.pixelRef()->info().dimensions();
SkASSERT(dim != bm.dimensions() || bm.pixelRefOrigin().isZero());
return dim == bm.dimensions();
}
diff --git a/src/image/SkSurface_Raster.cpp b/src/image/SkSurface_Raster.cpp
index c4796c4..2de1ed7 100644
--- a/src/image/SkSurface_Raster.cpp
+++ b/src/image/SkSurface_Raster.cpp
@@ -20,7 +20,7 @@
SkSurface_Raster(const SkImageInfo&, void*, size_t rb,
void (*releaseProc)(void* pixels, void* context), void* context,
const SkSurfaceProps*);
- SkSurface_Raster(const SkImageInfo& info, sk_sp<SkPixelRef>, const SkSurfaceProps*);
+ SkSurface_Raster(sk_sp<SkPixelRef>, const SkSurfaceProps*);
SkCanvas* onNewCanvas() override;
sk_sp<SkSurface> onNewSurface(const SkImageInfo&) override;
@@ -108,10 +108,11 @@
fWeOwnThePixels = false; // We are "Direct"
}
-SkSurface_Raster::SkSurface_Raster(const SkImageInfo& info, sk_sp<SkPixelRef> pr,
- const SkSurfaceProps* props)
- : INHERITED(pr->width(), pr->height(), props)
+SkSurface_Raster::SkSurface_Raster(sk_sp<SkPixelRef> pr, const SkSurfaceProps* props)
+ : INHERITED(pr->info().width(), pr->info().height(), props)
{
+ const SkImageInfo& info = pr->info();
+
fBitmap.setInfo(info, pr->rowBytes());
fRowBytes = pr->rowBytes(); // we track this, so that subsequent re-allocs will match
fBitmap.setPixelRef(std::move(pr), 0, 0);
@@ -214,5 +215,5 @@
if (rowBytes) {
SkASSERT(pr->rowBytes() == rowBytes);
}
- return sk_make_sp<SkSurface_Raster>(info, std::move(pr), props);
+ return sk_make_sp<SkSurface_Raster>(std::move(pr), props);
}
diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index 768277a..358ba16 100644
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -106,9 +106,7 @@
SkIRect subset = bitmap.getSubset();
SkASSERT(bitmap.pixelRef());
SkBitmap tmp;
- SkImageInfo pixelRefInfo =
- bitmap.info().makeWH(bitmap.pixelRef()->width(), bitmap.pixelRef()->height());
- tmp.setInfo(pixelRefInfo, bitmap.rowBytes());
+ tmp.setInfo(bitmap.pixelRef()->info(), bitmap.rowBytes());
tmp.setPixelRef(sk_ref_sp(bitmap.pixelRef()), 0, 0);
auto img = SkImage::MakeFromBitmap(tmp);
if (img) {