diff options
author | Matt Sarett <msarett@google.com> | 2017-03-15 15:48:19 -0400 |
---|---|---|
committer | Skia Commit-Bot <skia-commit-bot@chromium.org> | 2017-03-15 21:17:18 +0000 |
commit | cb01aec63bcb3dee52afcf3605bcd64166b873c0 (patch) | |
tree | 3feb4c702f66bec0245d1b9f960a2ae701b011d3 | |
parent | 8336e949b8aecc664a4f2690b56369d3821b2a1b (diff) |
Add color spin test for SkColorSpaceXformCanvas
Also changes behavior to treat nullptr srcs as sRGB.
Testing locally, it looks like 353 gms have no diffs from 8888.
There are 269 diffs - some are fine (gms that do color space stuff)
and some are bugs.
BUG=skia:
Change-Id: I55c2825f4f4b857e0b0a0ec050c6db82ac881492
Reviewed-on: https://skia-review.googlesource.com/9738
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Matt Sarett <msarett@google.com>
19 files changed, 167 insertions, 18 deletions
@@ -14,6 +14,7 @@ #include "SkCodec.h" #include "SkColorPriv.h" #include "SkColorSpace.h" +#include "SkColorSpacePriv.h" #include "SkCommonFlags.h" #include "SkCommonFlagsConfig.h" #include "SkCommonFlagsPathRenderer.h" @@ -895,9 +896,26 @@ static sk_sp<SkColorSpace> adobe_rgb() { SkColorSpace::kAdobeRGB_Gamut); } +static sk_sp<SkColorSpace> rgb_to_gbr() { + float gbr[9]; + gbr[0] = gSRGB_toXYZD50[1]; + gbr[1] = gSRGB_toXYZD50[2]; + gbr[2] = gSRGB_toXYZD50[0]; + gbr[3] = gSRGB_toXYZD50[4]; + gbr[4] = gSRGB_toXYZD50[5]; + gbr[5] = gSRGB_toXYZD50[3]; + gbr[6] = gSRGB_toXYZD50[7]; + gbr[7] = gSRGB_toXYZD50[8]; + gbr[8] = gSRGB_toXYZD50[6]; + SkMatrix44 toXYZD50(SkMatrix44::kUninitialized_Constructor); + toXYZD50.set3x3RowMajorf(gbr); + return SkColorSpace::MakeRGB(SkColorSpace::kSRGB_RenderTargetGamma, toXYZD50); +} + static Sink* create_via(const SkString& tag, Sink* wrapped) { #define VIA(t, via, ...) if (tag.equals(t)) { return new via(__VA_ARGS__); } - VIA("adobe", ViaCSXform, wrapped, adobe_rgb()); + VIA("adobe", ViaCSXform, wrapped, adobe_rgb(), false); + VIA("gbr", ViaCSXform, wrapped, rgb_to_gbr(), true); VIA("lite", ViaLite, wrapped); VIA("pipe", ViaPipe, wrapped); VIA("twice", ViaTwice, wrapped); diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp index 4f5cd43af8..14d5552789 100644 --- a/dm/DMSrcSink.cpp +++ b/dm/DMSrcSink.cpp @@ -1842,13 +1842,41 @@ Error ViaLite::draw(const Src& src, SkBitmap* bitmap, SkWStream* stream, SkStrin /*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/ -ViaCSXform::ViaCSXform(Sink* sink, sk_sp<SkColorSpace> cs) : Via(sink), fCS(std::move(cs)) {} +ViaCSXform::ViaCSXform(Sink* sink, sk_sp<SkColorSpace> cs, bool colorSpin) + : Via(sink) + , fCS(std::move(cs)) + , fColorSpin(colorSpin) {} Error ViaCSXform::draw(const Src& src, SkBitmap* bitmap, SkWStream* stream, SkString* log) const { return draw_to_canvas(fSink.get(), bitmap, stream, log, src.size(), [&](SkCanvas* canvas) -> Error { auto proxy = SkCreateColorSpaceXformCanvas(canvas, fCS); - return src.draw(proxy.get()); + Error err = src.draw(proxy.get()); + if (!err.isEmpty()) { + return err; + } + + // Undo the color spin, so we can look at the pixels in Gold. + if (fColorSpin) { + SkBitmap pixels; + pixels.allocPixels(canvas->imageInfo()); + canvas->readPixels(&pixels, 0, 0); + for (int y = 0; y < pixels.height(); y++) { + for (int x = 0; x < pixels.width(); x++) { + uint32_t pixel = *pixels.getAddr32(x, y); + uint8_t r = SkGetPackedR32(pixel); + uint8_t g = SkGetPackedG32(pixel); + uint8_t b = SkGetPackedB32(pixel); + uint8_t a = SkGetPackedA32(pixel); + *pixels.getAddr32(x, y) = + SkSwizzle_RGBA_to_PMColor(b << 0 | r << 8 | g << 16 | a << 24); + } + } + + canvas->writePixels(pixels, 0, 0); + } + + return ""; }); } diff --git a/dm/DMSrcSink.h b/dm/DMSrcSink.h index c973e3b393..b6e1b5955c 100644 --- a/dm/DMSrcSink.h +++ b/dm/DMSrcSink.h @@ -496,10 +496,11 @@ public: class ViaCSXform : public Via { public: - explicit ViaCSXform(Sink*, sk_sp<SkColorSpace>); + explicit ViaCSXform(Sink*, sk_sp<SkColorSpace>, bool colorSpin); Error draw(const Src&, SkBitmap*, SkWStream*, SkString*) const override; private: sk_sp<SkColorSpace> fCS; + bool fColorSpin; }; } // namespace DM diff --git a/gm/imagemasksubset.cpp b/gm/imagemasksubset.cpp index 80282787da..2d0d12038c 100644 --- a/gm/imagemasksubset.cpp +++ b/gm/imagemasksubset.cpp @@ -34,7 +34,12 @@ public: return false; } - make_mask(SkSurface::MakeRasterDirect(info, pixels, rowBytes)); + SkImageInfo surfaceInfo = info; + if (kAlpha_8_SkColorType == info.colorType()) { + surfaceInfo = surfaceInfo.makeColorSpace(nullptr); + } + + make_mask(SkSurface::MakeRasterDirect(surfaceInfo, pixels, rowBytes)); return true; } diff --git a/infra/bots/recipe_modules/sktest/api.py b/infra/bots/recipe_modules/sktest/api.py index d458183baa..d3916019cf 100644 --- a/infra/bots/recipe_modules/sktest/api.py +++ b/infra/bots/recipe_modules/sktest/api.py @@ -50,7 +50,7 @@ def dm_flags(bot): configs.extend(['f16']) configs.extend(['sp-8888', '2ndpic-8888']) # Test niche uses of SkPicture. configs.extend(['lite-8888']) # Experimental display list. - configs.extend(['srgbnl']) + configs.extend(['gbr-8888']) if '-TSAN' not in bot: if ('TegraK1' in bot or @@ -129,6 +129,10 @@ def dm_flags(bot): # of 8888. blacklist('8888 image _ _') + # Not any point to running these. + blacklist('gbr-8888 image _ _') + blacklist('gbr-8888 colorImage _ _') + if 'Valgrind' in bot: # These take 18+ hours to run. blacklist('pdf gm _ fontmgr_iter') diff --git a/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86-Debug.json b/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86-Debug.json index a618500c14..904da2eaa1 100644 --- a/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86-Debug.json +++ b/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86-Debug.json @@ -228,7 +228,7 @@ "sp-8888", "2ndpic-8888", "lite-8888", - "srgbnl", + "gbr-8888", "serialize-8888", "tiles_rt-8888", "pic-8888", @@ -251,6 +251,14 @@ "image", "_", "_", + "gbr-8888", + "image", + "_", + "_", + "gbr-8888", + "colorImage", + "_", + "_", "serialize-8888", "gm", "_", diff --git a/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-ASAN.json b/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-ASAN.json index 33f2c83ada..1b2eda0049 100644 --- a/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-ASAN.json +++ b/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-ASAN.json @@ -118,7 +118,7 @@ "sp-8888", "2ndpic-8888", "lite-8888", - "srgbnl", + "gbr-8888", "serialize-8888", "tiles_rt-8888", "pic-8888", @@ -141,6 +141,14 @@ "image", "_", "_", + "gbr-8888", + "image", + "_", + "_", + "gbr-8888", + "colorImage", + "_", + "_", "serialize-8888", "gm", "_", diff --git a/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-MSAN.json b/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-MSAN.json index ccc6ea30c7..f30ab68e2a 100644 --- a/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-MSAN.json +++ b/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-MSAN.json @@ -118,7 +118,7 @@ "sp-8888", "2ndpic-8888", "lite-8888", - "srgbnl", + "gbr-8888", "serialize-8888", "tiles_rt-8888", "pic-8888", @@ -141,6 +141,14 @@ "image", "_", "_", + "gbr-8888", + "image", + "_", + "_", + "gbr-8888", + "colorImage", + "_", + "_", "serialize-8888", "gm", "_", diff --git a/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug.json b/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug.json index 64a3497596..bc225af4a3 100644 --- a/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug.json +++ b/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug.json @@ -226,7 +226,7 @@ "sp-8888", "2ndpic-8888", "lite-8888", - "srgbnl", + "gbr-8888", "serialize-8888", "tiles_rt-8888", "pic-8888", @@ -249,6 +249,14 @@ "image", "_", "_", + "gbr-8888", + "image", + "_", + "_", + "gbr-8888", + "colorImage", + "_", + "_", "serialize-8888", "gm", "_", diff --git a/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared.json b/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared.json index 9a234e4d18..62e89a39f1 100644 --- a/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared.json +++ b/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared.json @@ -228,7 +228,7 @@ "sp-8888", "2ndpic-8888", "lite-8888", - "srgbnl", + "gbr-8888", "serialize-8888", "tiles_rt-8888", "pic-8888", @@ -251,6 +251,14 @@ "image", "_", "_", + "gbr-8888", + "image", + "_", + "_", + "gbr-8888", + "colorImage", + "_", + "_", "serialize-8888", "gm", "_", diff --git a/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-TSAN.json b/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-TSAN.json index 71b209897b..88cb4fc875 100644 --- a/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-TSAN.json +++ b/infra/bots/recipe_modules/sktest/example.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-TSAN.json @@ -118,7 +118,7 @@ "sp-8888", "2ndpic-8888", "lite-8888", - "srgbnl", + "gbr-8888", "serialize-8888", "tiles_rt-8888", "pic-8888", @@ -141,6 +141,14 @@ "image", "_", "_", + "gbr-8888", + "image", + "_", + "_", + "gbr-8888", + "colorImage", + "_", + "_", "serialize-8888", "gm", "_", diff --git a/infra/bots/recipe_modules/sktest/example.expected/failed_dm.json b/infra/bots/recipe_modules/sktest/example.expected/failed_dm.json index dea19a0a9b..d84da51608 100644 --- a/infra/bots/recipe_modules/sktest/example.expected/failed_dm.json +++ b/infra/bots/recipe_modules/sktest/example.expected/failed_dm.json @@ -226,7 +226,7 @@ "sp-8888", "2ndpic-8888", "lite-8888", - "srgbnl", + "gbr-8888", "serialize-8888", "tiles_rt-8888", "pic-8888", @@ -249,6 +249,14 @@ "image", "_", "_", + "gbr-8888", + "image", + "_", + "_", + "gbr-8888", + "colorImage", + "_", + "_", "serialize-8888", "gm", "_", diff --git a/infra/bots/recipe_modules/sktest/example.expected/nobuildbot.json b/infra/bots/recipe_modules/sktest/example.expected/nobuildbot.json index 5d166e8bd9..b8f8c0f622 100644 --- a/infra/bots/recipe_modules/sktest/example.expected/nobuildbot.json +++ b/infra/bots/recipe_modules/sktest/example.expected/nobuildbot.json @@ -266,7 +266,7 @@ "sp-8888", "2ndpic-8888", "lite-8888", - "srgbnl", + "gbr-8888", "serialize-8888", "tiles_rt-8888", "pic-8888", @@ -289,6 +289,14 @@ "image", "_", "_", + "gbr-8888", + "image", + "_", + "_", + "gbr-8888", + "colorImage", + "_", + "_", "serialize-8888", "gm", "_", diff --git a/infra/bots/recipe_modules/sktest/example.expected/recipe_with_gerrit_patch.json b/infra/bots/recipe_modules/sktest/example.expected/recipe_with_gerrit_patch.json index b55dcd9104..64cc6a8b81 100644 --- a/infra/bots/recipe_modules/sktest/example.expected/recipe_with_gerrit_patch.json +++ b/infra/bots/recipe_modules/sktest/example.expected/recipe_with_gerrit_patch.json @@ -232,7 +232,7 @@ "sp-8888", "2ndpic-8888", "lite-8888", - "srgbnl", + "gbr-8888", "serialize-8888", "tiles_rt-8888", "pic-8888", @@ -255,6 +255,14 @@ "image", "_", "_", + "gbr-8888", + "image", + "_", + "_", + "gbr-8888", + "colorImage", + "_", + "_", "serialize-8888", "gm", "_", diff --git a/infra/bots/recipes/swarm_test.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86-Debug.json b/infra/bots/recipes/swarm_test.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86-Debug.json index 4171ec83c5..2b18bc67f4 100644 --- a/infra/bots/recipes/swarm_test.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86-Debug.json +++ b/infra/bots/recipes/swarm_test.expected/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86-Debug.json @@ -228,7 +228,7 @@ "sp-8888", "2ndpic-8888", "lite-8888", - "srgbnl", + "gbr-8888", "serialize-8888", "tiles_rt-8888", "pic-8888", @@ -251,6 +251,14 @@ "image", "_", "_", + "gbr-8888", + "image", + "_", + "_", + "gbr-8888", + "colorImage", + "_", + "_", "serialize-8888", "gm", "_", diff --git a/src/core/SkColorSpace.cpp b/src/core/SkColorSpace.cpp index 8ffff5edf3..b953ffa53d 100644 --- a/src/core/SkColorSpace.cpp +++ b/src/core/SkColorSpace.cpp @@ -578,6 +578,10 @@ sk_sp<SkColorSpace> SkColorSpace::Deserialize(const void* data, size_t length) { } bool SkColorSpace_Base::EqualsIgnoreFlags(SkColorSpace* src, SkColorSpace* dst) { + if (!src || !dst) { + return src == dst; + } + return SkColorSpace::Equals(as_CSB(src)->makeWithoutFlags().get(), as_CSB(dst)->makeWithoutFlags().get()); } diff --git a/src/image/SkImage.cpp b/src/image/SkImage.cpp index 2a83eaf0ed..99b0be161b 100644 --- a/src/image/SkImage.cpp +++ b/src/image/SkImage.cpp @@ -8,6 +8,7 @@ #include "SkBitmap.h" #include "SkBitmapCache.h" #include "SkCanvas.h" +#include "SkColorSpace_Base.h" #include "SkCrossContextImageData.h" #include "SkData.h" #include "SkImageEncoder.h" @@ -318,7 +319,9 @@ sk_sp<SkImage> SkImage_Base::makeColorSpace(sk_sp<SkColorSpace> target) const { return nullptr; } - if (!this->colorSpace() || SkColorSpace::Equals(this->colorSpace(), target.get())) { + // Be sure to treat nullptr srcs as "equal to" sRGB. + if ((!this->colorSpace() && target->isSRGB()) || + SkColorSpace_Base::EqualsIgnoreFlags(this->colorSpace(), target.get())) { return sk_ref_sp(const_cast<SkImage_Base*>(this)); } diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp index 44d793842e..f548797ebe 100644 --- a/src/image/SkImage_Gpu.cpp +++ b/src/image/SkImage_Gpu.cpp @@ -848,7 +848,8 @@ sk_sp<SkImage> SkImage::MakeTextureFromMipMap(GrContext* ctx, const SkImageInfo& } sk_sp<SkImage> SkImage_Gpu::onMakeColorSpace(sk_sp<SkColorSpace> colorSpace) const { - auto xform = GrNonlinearColorSpaceXformEffect::Make(fColorSpace.get(), colorSpace.get()); + sk_sp<SkColorSpace> srcSpace = fColorSpace ? fColorSpace : SkColorSpace::MakeSRGB(); + auto xform = GrNonlinearColorSpaceXformEffect::Make(srcSpace.get(), colorSpace.get()); if (!xform) { return sk_ref_sp(const_cast<SkImage_Gpu*>(this)); } diff --git a/src/image/SkImage_Raster.cpp b/src/image/SkImage_Raster.cpp index 782093e27b..5327cfe36d 100644 --- a/src/image/SkImage_Raster.cpp +++ b/src/image/SkImage_Raster.cpp @@ -381,6 +381,11 @@ sk_sp<SkImage> SkImage_Raster::onMakeColorSpace(sk_sp<SkColorSpace> target) cons SkPixmap src; SkAssertResult(this->onPeekPixels(&src)); + // Treat nullptr srcs as sRGB. + if (!src.colorSpace()) { + src.setColorSpace(SkColorSpace::MakeSRGB()); + } + SkAssertResult(dst.writePixels(src)); dst.setImmutable(); return SkImage::MakeFromBitmap(dst); |