Only store width and height on SkPixelRef (part 2)

Relanding https://skia-review.googlesource.com/c/14105/
in pieces to try to diagnose problems with the Chrome
roll.

Bug: skia:6535
Change-Id: Iada034fc41ef315f7f00984d8de9d9cc2f361ad2
Reviewed-on: https://skia-review.googlesource.com/14657
Commit-Queue: Matt Sarett <msarett@google.com>
Reviewed-by: Mike Reed <reed@google.com>

diff --git a/include/core/SkMallocPixelRef.h b/include/core/SkMallocPixelRef.h
index 45ab654..e6d9727 100644 (file)
--- 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 05b0fbc..3c002dc 100644 (file)
--- a/include/core/SkPixelRef.h
+++ b/include/core/SkPixelRef.h
@@ -33,12 +33,37 @@ class SkDiscardableMemory;
 class SK_API SkPixelRef : public SkRefCnt {
 public:
     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; }
+
+    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; }
@@ -70,13 +95,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.
     */
@@ -116,27 +134,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/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index 1ba2968..5969a73 100644 (file)
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -198,32 +198,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() + dx <= pr->width());
+            SkASSERT(fInfo.height() + dy <= 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
@@ -843,7 +826,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;
 }
@@ -883,8 +866,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 cdc555b..36b790f 100644 (file)
--- a/src/core/SkMallocPixelRef.cpp
+++ b/src/core/SkMallocPixelRef.cpp
@@ -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 c6d59ea..143abbc 100644 (file)
--- a/src/core/SkPixelRef.cpp
+++ b/src/core/SkPixelRef.cpp
@@ -26,6 +26,10 @@ uint32_t SkNextID::ImageID() {
 
 ///////////////////////////////////////////////////////////////////////////////
 
+#ifdef SK_TRACE_PIXELREF_LIFETIME
+    static int32_t gInstCounter;
+#endif
+
 static SkImageInfo validate_info(const SkImageInfo& info) {
     SkAlphaType newAlphaType = info.alphaType();
     SkAssertResult(SkColorTypeValidateAlphaType(info.colorType(), info.alphaType(), &newAlphaType));
@@ -43,10 +47,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 +68,25 @@ SkPixelRef::SkPixelRef(const SkImageInfo& info, void* pixels, size_t rowBytes,
     fAddedToCache.store(false);
 }
 
+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 +95,10 @@ SkPixelRef::~SkPixelRef() {
 }
 
 #ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
+
 // 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);
@@ -89,6 +107,19 @@ void SkPixelRef::android_only_reset(const SkImageInfo& info, size_t rowBytes,
     // conservative, since its possible the "new" settings are the same as the old.
     this->notifyPixelsChanged();
 }
+
+// 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() {
@@ -179,7 +210,3 @@ void SkPixelRef::restoreMutability() {
 }
 
 void SkPixelRef::onNotifyPixelsChanged() { }
-
-size_t SkPixelRef::getAllocatedSizeInBytes() const {
-    return 0;
-}