Revert "Revert "clamp to premul when reading premul sRGB""

This reverts commit 2e018f548d.

Reason for revert: doesn't appear to have been the roll problem.

Original change's description:
> Revert "clamp to premul when reading premul sRGB"
> 
> This reverts commit 04e10da836.
> 
> Reason for revert: roll?
> 
> Change-Id: Id0a8dcd62763bd6eddde120c513ca97e098a4268
> Reviewed-on: https://skia-review.googlesource.com/6022
> Commit-Queue: Mike Klein <mtklein@chromium.org>
> Reviewed-by: Mike Klein <mtklein@chromium.org>
> 

TBR=mtklein@chromium.org,reviews@skia.org,brianosman@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Change-Id: I399ca5e728ce6766c6707682c4c6b685681ffdeb
Reviewed-on: https://skia-review.googlesource.com/6025
Commit-Queue: Mike Klein <mtklein@chromium.org>
Reviewed-by: Mike Klein <mtklein@chromium.org>

bench/SkRasterPipelineBench.cpp

@@ -38,13 +38,13 @@ public:
 
         SkRasterPipeline p;
         p.append(SkRasterPipeline::load_8888, &src_ctx);
-        p.append(SkRasterPipeline::from_srgb);
+        p.append_from_srgb(kUnpremul_SkAlphaType);
         p.append(SkRasterPipeline::scale_u8, &mask_ctx);
         if (kF16) {
             p.append(SkRasterPipeline::load_f16_d, &dst_ctx);
         } else {
             p.append(SkRasterPipeline::load_8888_d, &dst_ctx);
-            p.append(SkRasterPipeline::from_srgb_d);
+            p.append_from_srgb_d(kPremul_SkAlphaType);
         }
         p.append(SkRasterPipeline::srcover);
         if (kF16) {

src/core/SkConfig8888.cpp

@@ -47,7 +47,7 @@ static bool copy_pipeline_pixels(const SkImageInfo& dstInfo, void* dstRow, size_
         case kBGRA_8888_SkColorType:
             pipeline.append(SkRasterPipeline::load_8888, &srcRow);
             if (src_srgb) {
-                pipeline.append(SkRasterPipeline::from_srgb);
+                pipeline.append_from_srgb(srcInfo.alphaType());
             }
             if (swap_rb) {
                 pipeline.append(SkRasterPipeline::swap_rb);

src/core/SkRasterPipeline.cpp

@@ -11,6 +11,14 @@
 SkRasterPipeline::SkRasterPipeline() {}
 
 void SkRasterPipeline::append(StockStage stage, void* ctx) {
+#ifdef SK_DEBUG
+    switch (stage) {
+        case from_srgb:
+        case from_srgb_d:
+            SkDEBUGFAIL("Please use append_srgb[_d]() instead.");
+        default: break;
+    }
+#endif
     fStages.push_back({stage, ctx});
 }
 
@@ -42,3 +50,29 @@ void SkRasterPipeline::dump() const {
     }
     SkDebugf("\n");
 }
+
+// It's pretty easy to start with sound premultiplied linear floats, pack those
+// to sRGB encoded bytes, then read them back to linear floats and find them not
+// quite premultiplied, with a color channel just a smidge greater than the alpha
+// channel.  This can happen basically any time we have different transfer
+// functions for alpha and colors... sRGB being the only one we draw into.
+
+// This is an annoying problem with no known good solution.  So apply the clamp hammer.
+
+void SkRasterPipeline::append_from_srgb(SkAlphaType at) {
+    //this->append(from_srgb);
+    fStages.push_back({from_srgb, nullptr});
+
+    if (at == kPremul_SkAlphaType) {
+        this->append(SkRasterPipeline::clamp_a);
+    }
+}
+
+void SkRasterPipeline::append_from_srgb_d(SkAlphaType at) {
+    //this->append(from_srgb_d);
+    fStages.push_back({from_srgb_d, nullptr});
+
+    if (at == kPremul_SkAlphaType) {
+        this->append(SkRasterPipeline::clamp_a_d);
+    }
+}

src/core/SkRasterPipeline.h

@@ -8,6 +8,7 @@
 #ifndef SkRasterPipeline_DEFINED
 #define SkRasterPipeline_DEFINED
 
+#include "SkImageInfo.h"
 #include "SkNx.h"
 #include "SkTArray.h"
 #include "SkTypes.h"
@@ -58,7 +59,7 @@
 #define SK_RASTER_PIPELINE_STAGES(M)                             \
     M(trace) M(registers)                                        \
     M(move_src_dst) M(move_dst_src) M(swap_rb) M(swap_rb_d)      \
-    M(clamp_0) M(clamp_a) M(clamp_1)                             \
+    M(clamp_0) M(clamp_1) M(clamp_a) M(clamp_a_d)                \
     M(unpremul) M(premul)                                        \
     M(set_rgb)                                                   \
     M(from_srgb) M(from_srgb_d) M(to_srgb)                       \
@@ -119,6 +120,11 @@ public:
         void*        ctx;
     };
 
+    // Conversion from sRGB can be subtly tricky when premultiplication is involved.
+    // Use these helpers to keep things sane.
+    void append_from_srgb  (SkAlphaType);
+    void append_from_srgb_d(SkAlphaType);
+
 private:
     std::vector<Stage> fStages;
 };

src/core/SkRasterPipelineBlitter.cpp

@@ -197,7 +197,7 @@ void SkRasterPipelineBlitter::append_load_d(SkRasterPipeline* p) const {
         p->append(SkRasterPipeline::swap_rb_d);
     }
     if (fDst.info().gammaCloseToSRGB()) {
-        p->append(SkRasterPipeline::from_srgb_d);
+        p->append_from_srgb_d(fDst.info().alphaType());
     }
 }
 

src/image/SkImageShader.cpp

@@ -350,7 +350,7 @@ bool SkImageShader::onAppendStages(SkRasterPipeline* p, SkColorSpace* dst, SkFal
             default: SkASSERT(false);
         }
         if (info.gammaCloseToSRGB() && dst != nullptr) {
-            p->append(SkRasterPipeline::from_srgb);
+            p->append_from_srgb(info.alphaType());
         }
     };
 

src/opts/SkRasterPipeline_opts.h

@@ -301,17 +301,23 @@ STAGE(clamp_0) {
     g = SkNf::Max(g, 0.0f);
     b = SkNf::Max(b, 0.0f);
 }
+STAGE(clamp_1) {
+    a = SkNf::Min(a, 1.0f);
+    r = SkNf::Min(r, 1.0f);
+    g = SkNf::Min(g, 1.0f);
+    b = SkNf::Min(b, 1.0f);
+}
 STAGE(clamp_a) {
     a = SkNf::Min(a, 1.0f);
     r = SkNf::Min(r, a);
     g = SkNf::Min(g, a);
     b = SkNf::Min(b, a);
 }
-STAGE(clamp_1) {
-    a = SkNf::Min(a, 1.0f);
-    r = SkNf::Min(r, 1.0f);
-    g = SkNf::Min(g, 1.0f);
-    b = SkNf::Min(b, 1.0f);
+STAGE(clamp_a_d) {
+    da = SkNf::Min(da, 1.0f);
+    dr = SkNf::Min(dr, da);
+    dg = SkNf::Min(dg, da);
+    db = SkNf::Min(db, da);
 }
 
 STAGE(unpremul) {