make append_gamut_tranform() never fail

The only way it could ostensibly fail is if we get non-XYZ color
spaces, which should just not happen.  Assert that doesn't happen
and safely do nothing instead of failing.

This is one of the leaf nodes to getting SkCreateRasterPipelineBlitter
to never fail.  Next come SkColorFilter:: and SkShader::appendStages().

Change-Id: I5c7a8c63d0a9837e2e55208e1674796d86f45307
Reviewed-on: https://skia-review.googlesource.com/16002
Commit-Queue: Mike Klein <mtklein@chromium.org>
Reviewed-by: Matt Sarett <msarett@google.com>

4 files changed, 19 insertions, 14 deletions

diff --git a/src/core/SkConvertPixels.cpp b/src/core/SkConvertPixels.cpp
index 67e8a28cbf..2d0cf68830 100644
--- a/src/core/SkConvertPixels.cpp
+++ b/src/core/SkConvertPixels.cpp
@@ -320,8 +320,7 @@ static void convert_with_pipeline(const SkImageInfo& dstInfo, void* dstRow, size
 
     float matrix[12];
     if (isColorAware) {
-        SkAssertResult(append_gamut_transform(&pipeline, matrix, srcInfo.colorSpace(),
-                                              dstInfo.colorSpace()));
+        append_gamut_transform(&pipeline, matrix, srcInfo.colorSpace(), dstInfo.colorSpace());
     }
 
     SkAlphaType dat = dstInfo.alphaType();
diff --git a/src/core/SkPM4fPriv.h b/src/core/SkPM4fPriv.h
index 821c8822cb..f4142737b5 100644
--- a/src/core/SkPM4fPriv.h
+++ b/src/core/SkPM4fPriv.h
@@ -100,17 +100,23 @@ static inline void analyze_3x4_matrix(const float matrix[12],
 
 
 // N.B. scratch_matrix_3x4 must live at least as long as p.
-static inline bool append_gamut_transform(SkRasterPipeline* p, float scratch_matrix_3x4[12],
+static inline void append_gamut_transform(SkRasterPipeline* p, float scratch_matrix_3x4[12],
                                           SkColorSpace* src, SkColorSpace* dst) {
-    if (src == dst) { return true; }
-    if (!dst)       { return true; }   // Legacy modes intentionally ignore color gamut.
-    if (!src)       { return true; }   // A null src color space means linear gamma, dst gamut.
+    if (src == dst) { return; }   // That was easy.
+    if (!dst)       { return; }   // Legacy modes intentionally ignore color gamut.
+    if (!src)       { return; }   // A null src color space means linear gamma, dst gamut.
 
     auto toXYZ = as_CSB(src)->  toXYZD50(),
        fromXYZ = as_CSB(dst)->fromXYZD50();
-    if (!toXYZ || !fromXYZ) { return false; }  // Unsupported color space type.
+    if (!toXYZ || !fromXYZ) {
+        SkASSERT(false);  // We really don't want to get here with a weird colorspace.
+        return;
+    }
 
-    if (as_CSB(src)->toXYZD50Hash() == as_CSB(dst)->toXYZD50Hash()) { return true; }
+    // Slightly more sophisticated version of if (src == dst)
+    if (as_CSB(src)->toXYZD50Hash() == as_CSB(dst)->toXYZD50Hash()) {
+        return;
+    }
 
     SkMatrix44 m44(*fromXYZ, *toXYZ);
 
@@ -127,12 +133,11 @@ static inline bool append_gamut_transform(SkRasterPipeline* p, float scratch_mat
     p->append(SkRasterPipeline::matrix_3x4, scratch_matrix_3x4);
     if (needs_clamp_0) { p->append(SkRasterPipeline::clamp_0); }
     if (needs_clamp_a) { p->append(SkRasterPipeline::clamp_a); }
-    return true;
 }
 
-static inline bool append_gamut_transform(SkRasterPipeline* p, SkArenaAlloc* scratch,
+static inline void append_gamut_transform(SkRasterPipeline* p, SkArenaAlloc* scratch,
                                           SkColorSpace* src, SkColorSpace* dst) {
-    return append_gamut_transform(p, scratch->makeArrayDefault<float>(12), src, dst);
+    append_gamut_transform(p, scratch->makeArrayDefault<float>(12), src, dst);
 }
 
 static inline SkColor4f to_colorspace(const SkColor4f& c, SkColorSpace* src, SkColorSpace* dst) {
diff --git a/src/core/SkShader.cpp b/src/core/SkShader.cpp
index 203209799f..1c22dd975c 100644
--- a/src/core/SkShader.cpp
+++ b/src/core/SkShader.cpp
@@ -294,8 +294,8 @@ bool SkShader::onAppendStages(SkRasterPipeline* p,
 
         // Legacy shaders aren't aware of color spaces. We can pretty
         // safely assume they're in sRGB gamut.
-        return append_gamut_transform(p, alloc,
-                                      SkColorSpace::MakeSRGB().get(), cs);
+        append_gamut_transform(p, alloc, SkColorSpace::MakeSRGB().get(), cs);
+        return true;
     }
     return false;
 }
diff --git a/src/image/SkImageShader.cpp b/src/image/SkImageShader.cpp
index ce0d6f504b..682ccea953 100644
--- a/src/image/SkImageShader.cpp
+++ b/src/image/SkImageShader.cpp
@@ -377,5 +377,6 @@ bool SkImageShader::onAppendStages(SkRasterPipeline* p, SkColorSpace* dst, SkAre
         p->append(SkRasterPipeline::clamp_0);
         p->append(SkRasterPipeline::clamp_a);
     }
-    return append_gamut_transform(p, scratch, info.colorSpace(), dst);
+    append_gamut_transform(p, scratch, info.colorSpace(), dst);
+    return true;
 }