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>

11 files changed, 108 insertions, 83 deletions

diff --git a/gn/android_framework_defines.gni b/gn/android_framework_defines.gni
index 67c6a2d5e2..bc49f4be45 100644
--- a/gn/android_framework_defines.gni
+++ b/gn/android_framework_defines.gni
@@ -9,6 +9,7 @@ android_framework_defines = [
   "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 45ab6547f4..e6d9727952 100644
--- a/include/core/SkMallocPixelRef.h
+++ b/include/core/SkMallocPixelRef.h
@@ -78,8 +78,6 @@ public:
 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 342863f74e..036fb6df13 100644
--- a/include/core/SkPixelRef.h
+++ b/include/core/SkPixelRef.h
@@ -32,13 +32,42 @@ class SkDiscardableMemory;
 */
 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; }
@@ -68,13 +97,6 @@ public:
      */
     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.
     */
@@ -114,27 +136,18 @@ protected:
     // 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(const SkImageInfo&, size_t rowBytes, sk_sp<SkColorTable>);
+    void android_only_reset(int width, int height, size_t rowBytes, sk_sp<SkColorTable>);
 #endif
 
 private:
-    // mostly const. fInfo.fAlpahType can be changed at runtime.
-    const SkImageInfo fInfo;
+    // TODO (msarett): After we remove legacy APIs, we should replace |fInfo| with just a width
+    //                 and height.
+    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 01d70f7bcd..bb27e99558 100644
--- a/public.bzl
+++ b/public.bzl
@@ -654,8 +654,10 @@ DEFINES_ALL = [
     # 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 8f131d15f5..0c1bc7d70b 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -171,9 +171,6 @@ bool SkBitmap::setAlphaType(SkAlphaType newAlphaType) {
     }
     if (fInfo.alphaType() != newAlphaType) {
         fInfo = fInfo.makeAlphaType(newAlphaType);
-        if (fPixelRef) {
-            fPixelRef->changeAlphaType(newAlphaType);
-        }
     }
     SkDEBUGCODE(this->validate();)
     return true;
@@ -198,32 +195,15 @@ void SkBitmap::setPixelRef(sk_sp<SkPixelRef> pr, int dx, int dy) {
 #ifdef SK_DEBUG
     if (pr) {
         if (kUnknown_SkColorType != fInfo.colorType()) {
-            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;
-            }
+            SkASSERT(fInfo.width() <= pr->width());
+            SkASSERT(fInfo.height() <= pr->height());
         }
     }
 #endif
 
     fPixelRef = std::move(pr);
     if (fPixelRef) {
-        const SkImageInfo& info = fPixelRef->info();
-        fPixelRefOrigin.set(SkTPin(dx, 0, info.width()), SkTPin(dy, 0, info.height()));
+        fPixelRefOrigin.set(SkTPin(dx, 0, fPixelRef->width()), SkTPin(dy, 0, fPixelRef->height()));
         this->updatePixelsFromRef();
     } else {
         // ignore dx,dy if there is no pixelref
@@ -677,9 +657,9 @@ bool SkBitmap::internalCopyTo(SkBitmap* dst, SkColorType dstColorType, Allocator
     //      if (src_pixelref->info == dst_pixelref->info)
     //
     if (srcPM.colorType() == dstColorType && tmpDst.getSize() == srcPM.getSize64()) {
-        SkPixelRef* dstPixelRef = tmpDst.pixelRef();
-        if (dstPixelRef->info() == fPixelRef->info()) {
-            dstPixelRef->cloneGenID(*fPixelRef);
+        if (tmpDst.info() == this->info() && tmpDst.pixelRef()->width() == fPixelRef->width() &&
+                tmpDst.pixelRef()->height() == fPixelRef->height()) {
+            tmpDst.pixelRef()->cloneGenID(*fPixelRef);
         }
     }
 
@@ -916,7 +896,7 @@ bool SkBitmap::ReadRawPixels(SkReadBuffer* buffer, SkBitmap* bitmap) {
     if (!pr) {
         return false;
     }
-    bitmap->setInfo(pr->info());
+    bitmap->setInfo(info);
     bitmap->setPixelRef(std::move(pr), 0, 0);
     return true;
 }
@@ -956,8 +936,8 @@ void SkBitmap::validate() const {
         SkASSERT(fPixelRef->rowBytes() == fRowBytes);
         SkASSERT(fPixelRefOrigin.fX >= 0);
         SkASSERT(fPixelRefOrigin.fY >= 0);
-        SkASSERT(fPixelRef->info().width() >= (int)this->width() + fPixelRefOrigin.fX);
-        SkASSERT(fPixelRef->info().height() >= (int)this->height() + fPixelRefOrigin.fY);
+        SkASSERT(fPixelRef->width() >= (int)this->width() + fPixelRefOrigin.fX);
+        SkASSERT(fPixelRef->height() >= (int)this->height() + fPixelRefOrigin.fY);
         SkASSERT(fPixelRef->rowBytes() >= fInfo.minRowBytes());
     }
 }
diff --git a/src/core/SkMallocPixelRef.cpp b/src/core/SkMallocPixelRef.cpp
index cdc555b748..cf4fac7929 100644
--- a/src/core/SkMallocPixelRef.cpp
+++ b/src/core/SkMallocPixelRef.cpp
@@ -156,7 +156,7 @@ SkMallocPixelRef::SkMallocPixelRef(const SkImageInfo& info, void* storage,
                                    size_t rowBytes, sk_sp<SkColorTable> ctable,
                                    SkMallocPixelRef::ReleaseProc proc,
                                    void* context)
-    : INHERITED(info, storage, rowBytes, sanitize(info, std::move(ctable)))
+    : INHERITED(info.width(), info.height(), storage, rowBytes, sanitize(info, std::move(ctable)))
     , fReleaseProc(proc)
     , fReleaseProcContext(context)
 {}
@@ -167,7 +167,3 @@ SkMallocPixelRef::~SkMallocPixelRef() {
         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 5834b66936..a32b153cd2 100644
--- a/src/core/SkPixelRef.cpp
+++ b/src/core/SkPixelRef.cpp
@@ -26,6 +26,12 @@ uint32_t SkNextID::ImageID() {
 
 ///////////////////////////////////////////////////////////////////////////////
 
+#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));
@@ -43,10 +49,6 @@ static void validate_pixels_ctable(const SkImageInfo& info, const SkColorTable*
     }
 }
 
-#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))
@@ -68,6 +70,27 @@ SkPixelRef::SkPixelRef(const SkImageInfo& info, void* pixels, size_t rowBytes,
     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);
@@ -76,11 +99,11 @@ SkPixelRef::~SkPixelRef() {
 }
 
 #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);
@@ -91,6 +114,20 @@ void SkPixelRef::android_only_reset(const SkImageInfo& info, size_t rowBytes,
 }
 #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.
@@ -163,9 +200,11 @@ void SkPixelRef::notifyPixelsChanged() {
     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;
@@ -193,7 +232,3 @@ void SkPixelRef::restoreMutability() {
 }
 
 void SkPixelRef::onNotifyPixelsChanged() { }
-
-size_t SkPixelRef::getAllocatedSizeInBytes() const {
-    return 0;
-}
diff --git a/src/core/SkSpecialSurface.cpp b/src/core/SkSpecialSurface.cpp
index 33ef97e5bd..9564a14f48 100644
--- a/src/core/SkSpecialSurface.cpp
+++ b/src/core/SkSpecialSurface.cpp
@@ -63,12 +63,11 @@ sk_sp<SkSpecialImage> SkSpecialSurface::makeImageSnapshot() {
 
 class SkSpecialSurface_Raster : public SkSpecialSurface_Base {
 public:
-    SkSpecialSurface_Raster(sk_sp<SkPixelRef> pr,
+    SkSpecialSurface_Raster(const SkImageInfo& info,
+                            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);
 
@@ -93,7 +92,7 @@ private:
 
 sk_sp<SkSpecialSurface> SkSpecialSurface::MakeFromBitmap(const SkIRect& subset, SkBitmap& bm,
                                                          const SkSurfaceProps* props) {
-    return sk_make_sp<SkSpecialSurface_Raster>(sk_ref_sp(bm.pixelRef()), subset, props);
+    return sk_make_sp<SkSpecialSurface_Raster>(bm.info(), sk_ref_sp(bm.pixelRef()), subset, props);
 }
 
 sk_sp<SkSpecialSurface> SkSpecialSurface::MakeRaster(const SkImageInfo& info,
@@ -103,9 +102,9 @@ sk_sp<SkSpecialSurface> SkSpecialSurface::MakeRaster(const SkImageInfo& info,
         return nullptr;
     }
 
-    const SkIRect subset = SkIRect::MakeWH(pr->info().width(), pr->info().height());
+    const SkIRect subset = SkIRect::MakeWH(info.width(), info.height());
 
-    return sk_make_sp<SkSpecialSurface_Raster>(std::move(pr), subset, props);
+    return sk_make_sp<SkSpecialSurface_Raster>(info, std::move(pr), subset, props);
 }
 
 #if SK_SUPPORT_GPU
diff --git a/src/image/SkImage_Raster.cpp b/src/image/SkImage_Raster.cpp
index 91de4c5b0b..88847551d1 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 = bm.pixelRef()->info().dimensions();
+    SkISize dim = SkISize::Make(bm.pixelRef()->width(), bm.pixelRef()->height());
     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 2de1ed70d3..c4796c402f 100644
--- a/src/image/SkSurface_Raster.cpp
+++ b/src/image/SkSurface_Raster.cpp
@@ -20,7 +20,7 @@ public:
     SkSurface_Raster(const SkImageInfo&, void*, size_t rb,
                      void (*releaseProc)(void* pixels, void* context), void* context,
                      const SkSurfaceProps*);
-    SkSurface_Raster(sk_sp<SkPixelRef>, const SkSurfaceProps*);
+    SkSurface_Raster(const SkImageInfo& info, sk_sp<SkPixelRef>, const SkSurfaceProps*);
 
     SkCanvas* onNewCanvas() override;
     sk_sp<SkSurface> onNewSurface(const SkImageInfo&) override;
@@ -108,11 +108,10 @@ SkSurface_Raster::SkSurface_Raster(const SkImageInfo& info, void* pixels, size_t
     fWeOwnThePixels = false;    // We are "Direct"
 }
 
-SkSurface_Raster::SkSurface_Raster(sk_sp<SkPixelRef> pr, const SkSurfaceProps* props)
-    : INHERITED(pr->info().width(), pr->info().height(), props)
+SkSurface_Raster::SkSurface_Raster(const SkImageInfo& info, sk_sp<SkPixelRef> pr,
+                                   const SkSurfaceProps* props)
+    : INHERITED(pr->width(), pr->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);
@@ -215,5 +214,5 @@ sk_sp<SkSurface> SkSurface::MakeRaster(const SkImageInfo& info, size_t rowBytes,
     if (rowBytes) {
         SkASSERT(pr->rowBytes() == rowBytes);
     }
-    return sk_make_sp<SkSurface_Raster>(std::move(pr), props);
+    return sk_make_sp<SkSurface_Raster>(info, std::move(pr), props);
 }
diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index 358ba1638f..768277a7b9 100644
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -106,7 +106,9 @@ static SkImageSubset make_image_subset(const SkBitmap& bitmap) {
     SkIRect subset = bitmap.getSubset();
     SkASSERT(bitmap.pixelRef());
     SkBitmap tmp;
-    tmp.setInfo(bitmap.pixelRef()->info(), bitmap.rowBytes());
+    SkImageInfo pixelRefInfo =
+            bitmap.info().makeWH(bitmap.pixelRef()->width(), bitmap.pixelRef()->height());
+    tmp.setInfo(pixelRefInfo, bitmap.rowBytes());
     tmp.setPixelRef(sk_ref_sp(bitmap.pixelRef()), 0, 0);
     auto img = SkImage::MakeFromBitmap(tmp);
     if (img) {