commit b8a1392b021f480e292d66576e3da5198480845c
author Brian Osman <brianosman@google.com> Wed Apr 26 15:28:30 2017 -0400
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Wed Apr 26 20:59:10 2017 +0000
tree 863a6a90ff65969b3c702215f0cd2712d355b746
parent f6d7eb158e4148fb26349b12f4b7f7fd4d96e563

Fix uncached getROPixels from GPU images

We were allocating a local bitmap, but then attempting to read into an
uninitialized local pixmap. The only public API that funnels the caching
hint to this function is scalePixels, so I added a test (which previously
failed).

Bug: skia:
Change-Id: Ib4370350be664935b4c85e34c70b675e6d82ba64
Reviewed-on: https://skia-review.googlesource.com/14402
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>

src/image/SkImage_Gpu.cpp

diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp
index 96a81f5..0c5d4c7 100644
--- a/src/image/SkImage_Gpu.cpp
+++ b/src/image/SkImage_Gpu.cpp
@@ -86,7 +86,7 @@
             return false;
         }
     } else {
-        if (!dst->tryAllocPixels(ii)) {
+        if (!dst->tryAllocPixels(ii) || !dst->peekPixels(&pmap)) {
             return false;
         }
     }

tests/ImageTest.cpp

diff --git a/tests/ImageTest.cpp b/tests/ImageTest.cpp
index 382a624..2c4ba8f 100644
--- a/tests/ImageTest.cpp
+++ b/tests/ImageTest.cpp
@@ -1149,3 +1149,67 @@
 
     REPORTER_ASSERT(reporter, equal(bm0, bm2));
 }
+
+///////////////////////////////////////////////////////////////////////////////////////////////////
+
+static void check_scaled_pixels(skiatest::Reporter* reporter, SkPixmap* pmap, uint32_t expected) {
+    // Verify that all pixels contain the original test color
+    for (auto y = 0; y < pmap->height(); ++y) {
+        for (auto x = 0; x < pmap->width(); ++x) {
+            uint32_t pixel = *pmap->addr32(x, y);
+            if (pixel != expected) {
+                ERRORF(reporter, "Expected scaled pixels to be the same. At %d,%d 0x%08x != 0x%08x",
+                       x, y, pixel, expected);
+                return;
+            }
+        }
+    }
+}
+
+static void test_scale_pixels(skiatest::Reporter* reporter, const SkImage* image,
+                              uint32_t expected) {
+    SkImageInfo info = SkImageInfo::MakeN32Premul(image->width() * 2, image->height() * 2);
+
+    // Make sure to test kDisallow first, so we don't just get a cache hit in that case
+    for (auto chint : { SkImage::kDisallow_CachingHint, SkImage::kAllow_CachingHint }) {
+        SkAutoPixmapStorage scaled;
+        scaled.alloc(info);
+        if (!image->scalePixels(scaled, kLow_SkFilterQuality, chint)) {
+            ERRORF(reporter, "Failed to scale image");
+            continue;
+        }
+
+        check_scaled_pixels(reporter, &scaled, expected);
+    }
+}
+
+DEF_TEST(ImageScalePixels, reporter) {
+    const SkPMColor pmRed = SkPackARGB32(0xFF, 0xFF, 0, 0);
+    const SkColor red = SK_ColorRED;
+
+    // Test raster image
+    SkImageInfo info = SkImageInfo::MakeN32Premul(1, 1);
+    sk_sp<SkSurface> surface = SkSurface::MakeRaster(info);
+    surface->getCanvas()->clear(red);
+    sk_sp<SkImage> rasterImage = surface->makeImageSnapshot();
+    test_scale_pixels(reporter, rasterImage.get(), pmRed);
+
+    // Test encoded image
+    sk_sp<SkData> data(rasterImage->encode());
+    sk_sp<SkImage> codecImage = SkImage::MakeFromEncoded(data);
+    test_scale_pixels(reporter, codecImage.get(), pmRed);
+}
+
+#if SK_SUPPORT_GPU
+DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ImageScalePixels_Gpu, reporter, ctxInfo) {
+    const SkPMColor pmRed = SkPackARGB32(0xFF, 0xFF, 0, 0);
+    const SkColor red = SK_ColorRED;
+
+    SkImageInfo info = SkImageInfo::MakeN32Premul(16, 16);
+    sk_sp<SkSurface> surface = SkSurface::MakeRenderTarget(ctxInfo.grContext(), SkBudgeted::kNo,
+                                                           info);
+    surface->getCanvas()->clear(red);
+    sk_sp<SkImage> gpuImage = surface->makeImageSnapshot();
+    test_scale_pixels(reporter, gpuImage.get(), pmRed);
+}
+#endif