Can we remove volatile from skbitmap?

Change-Id: I0faa324b91f2c291be56f5126361ffe7fd54eb65
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/302037
Reviewed-by: Mike Reed <[email protected]>
Commit-Queue: Mike Reed <[email protected]>

Author: reed-at-google

bench/BitmapBench.cpp

@@ -73,7 +73,7 @@ class BitmapBench : public Benchmark {
         this->onDrawIntoBitmap(bm);
 
         fBitmap = bm;
-        fBitmap.setIsVolatile(fIsVolatile);
+    //    fBitmap.setIsVolatile(fIsVolatile);
     }
 
     void onDraw(int loops, SkCanvas* canvas) override {

docs/examples/Bitmap_isVolatile.cpp

@@ -7,10 +7,10 @@ void draw(SkCanvas* canvas) {
     SkBitmap original;
     SkImageInfo info = SkImageInfo::Make(25, 35, kRGBA_8888_SkColorType, kOpaque_SkAlphaType);
     if (original.tryAllocPixels(info)) {
-        original.setIsVolatile(true);
+    //    original.setIsVolatile(true);
         SkBitmap copy;
         original.extractSubset(&copy, {5, 10, 15, 20});
-        SkDebugf("original is " "%s" "volatile\n", original.isVolatile() ? "" : "not ");
+   //     SkDebugf("original is " "%s" "volatile\n", original.isVolatile() ? "" : "not ");
         SkDebugf("copy is " "%s" "volatile\n", copy.isImmutable() ? "" : "not ");
     }
 }

docs/examples/Bitmap_setIsVolatile.cpp

@@ -12,7 +12,7 @@ void draw(SkCanvas* canvas) {
     canvas->drawBitmap(bitmap, 0, 0);
     *(SkPMColor*) bitmap.getPixels() = SkPreMultiplyColor(SK_ColorBLUE);
     canvas->drawBitmap(bitmap, 2, 0);
-    bitmap.setIsVolatile(true);
+ //   bitmap.setIsVolatile(true);
     *(SkPMColor*) bitmap.getPixels() = SkPreMultiplyColor(SK_ColorGREEN);
     canvas->drawBitmap(bitmap, 4, 0);
 }

include/core/SkBitmap.h

@@ -303,30 +303,6 @@ class SK_API SkBitmap {
         return SkAlphaTypeIsOpaque(this->alphaType());
     }
 
-    /** Provides a hint to caller that pixels should not be cached. Only true if
-        setIsVolatile() has been called to mark as volatile.
-
-        Volatile state is not shared by other bitmaps sharing the same SkPixelRef.
-
-        @return  true if marked volatile
-
-        example: https://fiddle.skia.org/c/@Bitmap_isVolatile
-    */
-    bool isVolatile() const;
-
-    /** Sets if pixels should be read from SkPixelRef on every access. SkBitmap are not
-        volatile by default; a GPU back end may upload pixel values expecting them to be
-        accessed repeatedly. Marking temporary SkBitmap as volatile provides a hint to
-        SkBaseDevice that the SkBitmap pixels should not be cached. This can
-        improve performance by avoiding overhead and reducing resource
-        consumption on SkBaseDevice.
-
-        @param isVolatile  true if backing pixels are temporary
-
-        example: https://fiddle.skia.org/c/@Bitmap_setIsVolatile
-    */
-    void setIsVolatile(bool isVolatile);
-
     /** Resets to its initial state; all fields are set to zero, as if SkBitmap had
         been initialized by SkBitmap().
 
@@ -1188,13 +1164,8 @@ class SK_API SkBitmap {
     };
 
 private:
-    enum Flags {
-        kImageIsVolatile_Flag   = 0x02,
-    };
-
     sk_sp<SkPixelRef>   fPixelRef;
     SkPixmap            fPixmap;
-    uint8_t             fFlags;
 
     friend class SkReadBuffer;        // unflatten
 };

samplecode/SampleAAGeometry.cpp

@@ -1315,7 +1315,7 @@ class AAGeometryView : public Sample {
 
     static uint8_t* set_up_dist_map(const SkImageInfo& imageInfo, SkBitmap* distMap) {
         distMap->setInfo(imageInfo);
-        distMap->setIsVolatile(true);
+   //     distMap->setIsVolatile(true);
         SkAssertResult(distMap->tryAllocPixels());
         SkASSERT((int) distMap->rowBytes() == imageInfo.width());
         return distMap->getAddr8(0, 0);

src/core/SkBitmap.cpp

@@ -37,12 +37,11 @@ static bool reset_return_false(SkBitmap* bm) {
     return false;
 }
 
-SkBitmap::SkBitmap() : fFlags(0) {}
+SkBitmap::SkBitmap() {}
 
 SkBitmap::SkBitmap(const SkBitmap& src)
     : fPixelRef      (src.fPixelRef)
     , fPixmap        (src.fPixmap)
-    , fFlags         (src.fFlags)
 {
     SkDEBUGCODE(src.validate();)
     SkDEBUGCODE(this->validate();)
@@ -51,11 +50,9 @@ SkBitmap::SkBitmap(const SkBitmap& src)
 SkBitmap::SkBitmap(SkBitmap&& other)
     : fPixelRef      (std::move(other.fPixelRef))
     , fPixmap        (std::move(other.fPixmap))
-    , fFlags                   (other.fFlags)
 {
     SkASSERT(!other.fPixelRef);
     other.fPixmap.reset();
-    other.fFlags          = 0;
 }
 
 SkBitmap::~SkBitmap() {}
@@ -64,7 +61,6 @@ SkBitmap& SkBitmap::operator=(const SkBitmap& src) {
     if (this != &src) {
         fPixelRef       = src.fPixelRef;
         fPixmap         = src.fPixmap;
-        fFlags          = src.fFlags;
     }
     SkDEBUGCODE(this->validate();)
     return *this;
@@ -74,10 +70,8 @@ SkBitmap& SkBitmap::operator=(SkBitmap&& other) {
     if (this != &other) {
         fPixelRef       = std::move(other.fPixelRef);
         fPixmap         = std::move(other.fPixmap);
-        fFlags          = other.fFlags;
         SkASSERT(!other.fPixelRef);
         other.fPixmap.reset();
-        other.fFlags          = 0;
     }
     return *this;
 }
@@ -91,7 +85,6 @@ void SkBitmap::swap(SkBitmap& other) {
 void SkBitmap::reset() {
     fPixelRef = nullptr;  // Free pixels.
     fPixmap.reset();
-    fFlags = 0;
 }
 
 void SkBitmap::getBounds(SkRect* bounds) const {
@@ -381,18 +374,6 @@ void SkBitmap::setImmutable() {
     }
 }
 
-bool SkBitmap::isVolatile() const {
-    return (fFlags & kImageIsVolatile_Flag) != 0;
-}
-
-void SkBitmap::setIsVolatile(bool isVolatile) {
-    if (isVolatile) {
-        fFlags |= kImageIsVolatile_Flag;
-    } else {
-        fFlags &= ~kImageIsVolatile_Flag;
-    }
-}
-
 void* SkBitmap::getAddr(int x, int y) const {
     SkASSERT((unsigned)x < (unsigned)this->width());
     SkASSERT((unsigned)y < (unsigned)this->height());
@@ -452,7 +433,6 @@ bool SkBitmap::extractSubset(SkBitmap* result, const SkIRect& subset) const {
 
     SkBitmap dst;
     dst.setInfo(this->info().makeDimensions(r.size()), this->rowBytes());
-    dst.setIsVolatile(this->isVolatile());
 
     if (fPixelRef) {
         SkIPoint origin = this->pixelRefOrigin();
@@ -595,8 +575,6 @@ void SkBitmap::validate() const {
     this->info().validate();
 
     SkASSERT(this->info().validRowBytes(this->rowBytes()));
-    uint8_t allFlags = kImageIsVolatile_Flag;
-    SkASSERT((~allFlags & fFlags) == 0);
 
     if (fPixelRef && fPixelRef->pixels()) {
         SkASSERT(this->getPixels());

src/core/SkPixmap.cpp

@@ -232,7 +232,7 @@ bool SkPixmap::scalePixels(const SkPixmap& actualDst, SkFilterQuality quality) c
         return false;
     }
     bitmap.setImmutable();        // Don't copy when we create an image.
-    bitmap.setIsVolatile(true);   // Disable any caching.
+ //   bitmap.setIsVolatile(true);   // Disable any caching.
 
     SkMatrix scale = SkMatrix::MakeRectToRect(SkRect::Make(src.bounds()),
                                               SkRect::Make(dst.bounds()),

src/gpu/GrBitmapTextureMaker.cpp

@@ -46,7 +46,7 @@ GrBitmapTextureMaker::GrBitmapTextureMaker(GrRecordingContext* context,
         , fBudgeted(cachePolicy == GrImageTexGenPolicy::kNew_Uncached_Unbudgeted
                             ? SkBudgeted::kNo
                             : SkBudgeted::kYes) {
-    if (!bitmap.isVolatile() && cachePolicy == GrImageTexGenPolicy::kDraw) {
+    if (/*!bitmap.isVolatile() && */ cachePolicy == GrImageTexGenPolicy::kDraw) {
         SkIPoint origin = bitmap.pixelRefOrigin();
         SkIRect subset = SkIRect::MakeXYWH(origin.fX, origin.fY, bitmap.width(),
                                            bitmap.height());

tests/BitmapCopyTest.cpp

@@ -91,13 +91,13 @@ DEF_TEST(BitmapCopy_extractSubset, reporter) {
         // Extract a subset which has the same width as the original. This
         // catches a bug where we cloned the genID incorrectly.
         r.setLTRB(0, 1, W, 3);
-        bitmap.setIsVolatile(true);
+   //     bitmap.setIsVolatile(true);
         // Relies on old behavior of extractSubset failing if colortype is unknown
         if (kUnknown_SkColorType != bitmap.colorType() && bitmap.extractSubset(&subset, r)) {
             REPORTER_ASSERT(reporter, subset.width() == W);
             REPORTER_ASSERT(reporter, subset.height() == 2);
             REPORTER_ASSERT(reporter, subset.alphaType() == bitmap.alphaType());
-            REPORTER_ASSERT(reporter, subset.isVolatile() == true);
+    //        REPORTER_ASSERT(reporter, subset.isVolatile() == true);
 
             // Test copying an extracted subset.
             for (size_t j = 0; j < SK_ARRAY_COUNT(gPairs); j++) {
@@ -120,10 +120,10 @@ DEF_TEST(BitmapCopy_extractSubset, reporter) {
         }
 
         bitmap = srcPremul;
-        bitmap.setIsVolatile(false);
+  //      bitmap.setIsVolatile(false);
         if (bitmap.extractSubset(&subset, r)) {
             REPORTER_ASSERT(reporter, subset.alphaType() == bitmap.alphaType());
-            REPORTER_ASSERT(reporter, subset.isVolatile() == false);
+   //         REPORTER_ASSERT(reporter, subset.isVolatile() == false);
         }
     }
 }