commit 9fb5a53c8e6982db235b8053169f1d6f09f50e09
author Mike Klein <mtklein@google.com> Thu Sep 13 15:23:38 2018 -0400
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Thu Sep 13 21:14:56 2018 +0000
tree e58e8c97bcd6b4d6d9fd622a6d346c266bfdc4a3
parent d29dcd1ae6a3819f7a938c38b341c8fb7dbae877

add two more cases to P3 gm

These are a sprite and non-sprite A8 image draw,
where we pick up the color from the paint.

I'm slightly disturbed to see these working.
It turns out I had no idea how A8 draws happened.

There is probably some dead code to remove in
SkBlitter_Sprite (I think, never called for A8)
and in SkImageShader (only ever called with a black
paint color for A8), in this CL as TODOs.

Change-Id: I1d276f8d9b145b57e3ab793735fb184bdae670fb
Reviewed-on: https://skia-review.googlesource.com/154461
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>

gm/p3.cpp

diff --git a/gm/p3.cpp b/gm/p3.cpp
index b3b21d6..5801a04 100644
--- a/gm/p3.cpp
+++ b/gm/p3.cpp
@@ -141,7 +141,7 @@
     }
 }
 
-DEF_SIMPLE_GM(p3, canvas, 450, 200) {
+DEF_SIMPLE_GM(p3, canvas, 450, 400) {
     auto p3 = SkColorSpace::MakeRGB(SkColorSpace::kSRGB_RenderTargetGamma,
                                     SkColorSpace::kDCIP3_D65_Gamut);
 
@@ -185,5 +185,35 @@
         canvas->restore();
     }
 
+    canvas->translate(0,80);
+
+    // Draw an A8 image with the paint color set to P3 red, hitting a sprite case.
+    //    and
+    // Draw a scaled A8 image with the paint color set to P3 red, hitting a non-sprite case.
+    {
+        uint8_t mask[256];
+        for (int i = 0; i < 256; i++) {
+            mask[i] = 255-i;
+        }
+        SkBitmap bm;
+        bm.installPixels(SkImageInfo::MakeA8(16,16), mask, 16);
+
+        SkPaint paint;
+        paint.setFilterQuality(kLow_SkFilterQuality);
+        paint.setColor4f({1,0,0,1}, p3.get());
+
+        canvas->drawBitmap(bm, 10,10, &paint);
+        compare_pixel("A8 sprite P3 red",
+                      canvas, 10,10,
+                      {1,0,0,1}, p3.get());
+
+        canvas->translate(0,80);
+
+        canvas->drawBitmapRect(bm, {10,10,70,70}, &paint);
+        compare_pixel("A8 image P3 red",
+                      canvas, 10,10,
+                      {1,0,0,1}, p3.get());
+    }
+
     // TODO: draw P3 colors more ways
 }

src/core/SkBlitter_Sprite.cpp

diff --git a/src/core/SkBlitter_Sprite.cpp b/src/core/SkBlitter_Sprite.cpp
index cb88ac4..c31716a 100644
--- a/src/core/SkBlitter_Sprite.cpp
+++ b/src/core/SkBlitter_Sprite.cpp
@@ -112,6 +112,9 @@
         fLeft = left;
         fTop  = top;
 
+        // TODO: confirm that fSource.colorType() is never called with kAlpha_8_SkColorType,
+        // cut that support and the need to handle fPaintColor at all.
+
         // Just like in SkImageShader, we'll keep the paint color as floats in sRGB.
         swizzle_rb(Sk4f_fromL32(paint.getColor())).store(fPaintColor.vec());
 
@@ -134,6 +137,7 @@
                                             p.append(SkRasterPipeline::force_opaque     ); break;
             default: SkASSERT(false);
         }
+
         if (fSource.colorType() == kAlpha_8_SkColorType) {
             // The color for A8 images comes from the (sRGB) paint color.
             p.append(SkRasterPipeline::set_rgb, &fPaintColor);

src/shaders/SkImageShader.cpp

diff --git a/src/shaders/SkImageShader.cpp b/src/shaders/SkImageShader.cpp
index 28c569f..df0b043 100644
--- a/src/shaders/SkImageShader.cpp
+++ b/src/shaders/SkImageShader.cpp
@@ -384,6 +384,8 @@
         // to do the color space transformation.  Might be possible to streamline.
         if (info.colorType() == kAlpha_8_SkColorType) {
             // The color for A8 images comes from the (sRGB) paint color.
+            // TODO: confirm that this is only ever called with the paint color set to black,
+            // then drop this step and the color correction below.
             p->append(SkRasterPipeline::set_rgb, &misc->paint_color);
             p->append(SkRasterPipeline::premul);
         } else if (info.alphaType() == kUnpremul_SkAlphaType) {