From 0d4ff6c601f2ce404a97068718caad5a2bb3d594 Mon Sep 17 00:00:00 2001 From: Brian Osman Date: Tue, 17 Jan 2017 16:10:07 -0500 Subject: [PATCH] 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 Reviewed-by: Brian Salomon Commit-Queue: Brian Osman --- gm/gamut.cpp | 24 +++++++++++++----------- samplecode/SampleApp.cpp | 38 +++++++++++++++++++++----------------- src/core/SkImagePriv.h | 7 +++++++ src/image/SkImage.cpp | 27 +++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 28 deletions(-) diff --git a/gm/gamut.cpp b/gm/gamut.cpp index 21312ce..d513bc6 100644 --- 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 srgbGamutSurface = canvas->makeSurface(srgbGamutInfo); sk_sp wideGamutSurface = canvas->makeSurface(wideGamutInfo); if (!srgbGamutSurface || !wideGamutSurface) { @@ -179,16 +179,18 @@ static void draw_gamut_grid(SkCanvas* canvas, SkTArraydrawText(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 --- 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 --- 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 SkImageMakeRasterCopyAndAssignColorSpace(const SkImage*, SkColorSpace*); + #endif diff --git a/src/image/SkImage.cpp b/src/image/SkImage.cpp index d2fc845..86d1e19 100644 --- 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 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); +} -- 2.7.4