Fix code that relied on readPixels not doing color space conversion

SampleApp doesn't have (can't easily get) an image, so I couldn't use
the new helper function there. It's probably still worth having?

BUG=skia:

Change-Id: I60c208ff958076015a9539359921b9aff68f25c8
Reviewed-on: https://skia-review.googlesource.com/7129
Reviewed-by: Matt Sarett <msarett@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>

diff --git a/gm/gamut.cpp b/gm/gamut.cpp
index 21312ce..d513bc6 100644 (file)
--- a/gm/gamut.cpp
+++ b/gm/gamut.cpp
@@ -9,6 +9,7 @@
 
 #include "SkColorSpace_Base.h"
 #include "SkGradientShader.h"
+#include "SkImagePriv.h"
 #include "SkPM4fPriv.h"
 #include "SkSurface.h"
 
@@ -147,13 +148,12 @@ static void draw_gamut_grid(SkCanvas* canvas, SkTArray<std::unique_ptr<CellRende
     SkASSERT(srgbCS);
     SkASSERT(wideCS);
 
-    // Make our two working surfaces (one sRGB, one Adobe)
+    // Make our two working surfaces (one sRGB, one Wide)
     SkImageInfo srgbGamutInfo = SkImageInfo::Make(gRectSize, gRectSize, origInfo.colorType(),
                                                   kPremul_SkAlphaType, srgbCS);
     SkImageInfo wideGamutInfo = SkImageInfo::Make(gRectSize, gRectSize, origInfo.colorType(),
                                                   kPremul_SkAlphaType, wideCS);
-    // readPixels doesn't do color conversion (yet), so we can use it to see the raw (wide) data
-    SkImageInfo dstInfo = srgbGamutInfo.makeColorSpace(nullptr);
+
     sk_sp<SkSurface> srgbGamutSurface = canvas->makeSurface(srgbGamutInfo);
     sk_sp<SkSurface> wideGamutSurface = canvas->makeSurface(wideGamutInfo);
     if (!srgbGamutSurface || !wideGamutSurface) {
@@ -179,16 +179,18 @@ static void draw_gamut_grid(SkCanvas* canvas, SkTArray<std::unique_ptr<CellRende
         canvas->drawText(renderer->label(), strlen(renderer->label()), x, y + textHeight,
                          textPaint);
 
-        SkBitmap srgbBitmap;
-        srgbBitmap.setInfo(dstInfo);
-        srgbGamutCanvas->readPixels(&srgbBitmap, 0, 0);
-        canvas->drawBitmap(srgbBitmap, x, y + textHeight + 5);
+        // Re-interpret the off-screen images, so we can see the raw data (eg, Wide gamut squares
+        // will look desaturated, relative to sRGB).
+        auto srgbImage = srgbGamutSurface->makeImageSnapshot();
+        srgbImage = SkImageMakeRasterCopyAndAssignColorSpace(srgbImage.get(),
+                                                             origInfo.colorSpace());
+        canvas->drawImage(srgbImage, x, y + textHeight + 5);
         x += (gScalarSize + 1);
 
-        SkBitmap wideBitmap;
-        wideBitmap.setInfo(dstInfo);
-        wideGamutCanvas->readPixels(&wideBitmap, 0, 0);
-        canvas->drawBitmap(wideBitmap, x, y + textHeight + 5);
+        auto wideImage = wideGamutSurface->makeImageSnapshot();
+        wideImage = SkImageMakeRasterCopyAndAssignColorSpace(wideImage.get(),
+                                                             origInfo.colorSpace());
+        canvas->drawImage(wideImage, x, y + textHeight + 5);
         x += (gScalarSize + 10);
 
         if (x + (2 * gScalarSize + 1) > gTestWidth) {

diff --git a/samplecode/SampleApp.cpp b/samplecode/SampleApp.cpp
index 3919c24..bd298df 100644 (file)
--- a/samplecode/SampleApp.cpp
+++ b/samplecode/SampleApp.cpp
@@ -326,23 +326,27 @@ public:
         if (!IsGpuDeviceType(dType) ||
             kRGBA_F16_SkColorType == win->info().colorType() ||
             fActualColorBits > 24) {
-            // We made/have an off-screen surface. Get the contents as an SkImage:
-            SkImageInfo offscreenInfo = win->info();
-            if (kMonitor_OutputColorSpace == gConfig[win->getColorConfigIndex()].fColorSpace ||
-                kNarrow_OutputColorSpace == gConfig[win->getColorConfigIndex()].fColorSpace) {
-                // This is a big hack. We want our final output to be color "correct". If we snap
-                // an image in the gamut of the monitor, and then render to FBO0 (which we've tagged
-                // as sRGB), then we end up doing round-trip gamut conversion, and still seeing the
-                // same colors on-screen as if we weren't color managed at all.
-                // Instead, we readPixels into a buffer that we claim is sRGB (readPixels doesn't
-                // do gamut conversion), so these pixels then get thrown directly at the monitor,
-                // giving us the expected results (the output is adapted to the monitor's gamut).
-                auto srgb = SkColorSpace::MakeNamed(SkColorSpace::kSRGB_Named);
-                offscreenInfo = offscreenInfo.makeColorSpace(srgb);
+            // We made/have an off-screen surface. Extract the pixels exactly as we rendered them:
+            SkImageInfo info = win->info();
+            size_t rowBytes = info.minRowBytes();
+            size_t size = info.getSafeSize(rowBytes);
+            auto data = SkData::MakeUninitialized(size);
+            SkASSERT(data);
+
+            if (!renderingCanvas->readPixels(info, data->writable_data(), rowBytes, 0, 0)) {
+                SkDEBUGFAIL("Failed to read canvas pixels");
+                return;
             }
-            SkBitmap bm;
-            bm.allocPixels(offscreenInfo);
-            renderingCanvas->readPixels(&bm, 0, 0);
+
+            // Now, re-interpret those pixels as sRGB, so they won't be color converted when we
+            // draw then to FBO0. This ensures that if we rendered in any strange gamut, we'll see
+            // the "correct" output (because we generated the pixel values we wanted in the
+            // offscreen canvas).
+            auto colorSpace = kRGBA_F16_SkColorType == info.colorType()
+                ? SkColorSpace::MakeNamed(SkColorSpace::kSRGBLinear_Named)
+                : SkColorSpace::MakeNamed(SkColorSpace::kSRGB_Named);
+            auto offscreenImage = SkImage::MakeRasterData(info.makeColorSpace(colorSpace), data,
+                                                          rowBytes);
 
             SkCanvas* gpuCanvas = fGpuSurface->getCanvas();
 
@@ -357,7 +361,7 @@ public:
                 gammaPaint.setColorFilter(SkGammaColorFilter::Make(1.0f / 2.2f));
             }
 
-            gpuCanvas->drawBitmap(bm, 0, 0, &gammaPaint);
+            gpuCanvas->drawImage(offscreenImage, 0, 0, &gammaPaint);
         }
 
         fGpuSurface->prepareForExternalIO();

diff --git a/src/core/SkImagePriv.h b/src/core/SkImagePriv.h
index 1d19714..6011086 100644 (file)
--- a/src/core/SkImagePriv.h
+++ b/src/core/SkImagePriv.h
@@ -96,4 +96,11 @@ bool SkImage_pinAsTexture(const SkImage*, GrContext*);
  */
 void SkImage_unpinAsTexture(const SkImage*, GrContext*);
 
+/**
+ *  Returns a new image containing the same pixel values as the source, but with a different color
+ *  space assigned. This performs no color space conversion. Primarily used in tests, to visualize
+ *  the results of rendering in wide or narrow gamuts.
+ */
+sk_sp<SkImage> SkImageMakeRasterCopyAndAssignColorSpace(const SkImage*, SkColorSpace*);
+
 #endif

diff --git a/src/image/SkImage.cpp b/src/image/SkImage.cpp
index d2fc845..86d1e19 100644 (file)
--- a/src/image/SkImage.cpp
+++ b/src/image/SkImage.cpp
@@ -409,3 +409,30 @@ void SkImage_unpinAsTexture(const SkImage* image, GrContext* ctx) {
     SkASSERT(ctx);
     as_IB(image)->onUnpinAsTexture(ctx);
 }
+
+///////////////////////////////////////////////////////////////////////////////////////////////////
+
+sk_sp<SkImage> SkImageMakeRasterCopyAndAssignColorSpace(const SkImage* src,
+                                                        SkColorSpace* colorSpace) {
+    // Read the pixels out of the source image, with no conversion
+    SkImageInfo info = as_IB(src)->onImageInfo();
+    if (kUnknown_SkColorType == info.colorType()) {
+        SkDEBUGFAIL("Unexpected color type");
+        return nullptr;
+    }
+
+    size_t rowBytes = info.minRowBytes();
+    size_t size = info.getSafeSize(rowBytes);
+    auto data = SkData::MakeUninitialized(size);
+    if (!data) {
+        return nullptr;
+    }
+
+    SkPixmap pm(info, data->writable_data(), rowBytes);
+    if (!src->readPixels(pm, 0, 0, SkImage::kDisallow_CachingHint)) {
+        return nullptr;
+    }
+
+    // Wrap them in a new image with a different color space
+    return SkImage::MakeRasterData(info.makeColorSpace(sk_ref_sp(colorSpace)), data, rowBytes);
+}