Fix buggy blend modes.

The problem turns out to be not over- or underflow like I thought, but that I used different math for x*y/255 for alphas and colors, sometimes resulting in colors where alpha was one less than the maximum color component, which is not a valid SkPMColor.

To be safe, I've switched over all four of these similar complex modes to use exact math everywhere.  They now match the byte-by-byte code in SkXfermode.cpp exactly.

This will slow down Darken and Lighten by about 2x.  I plan to follow up with a CL to see if I can eek out some speed there, and another CL to add asserts that Sk4px code always writes valid SkPMColors.

BUG=skia:4052

Review URL: https://codereview.chromium.org/1241683003

diff --git a/src/core/Sk4pxXfermode.h b/src/core/Sk4pxXfermode.h
index 97321b7..c671b67 100644 (file)
--- a/src/core/Sk4pxXfermode.h
+++ b/src/core/Sk4pxXfermode.h
@@ -62,37 +62,53 @@ XFERMODE(Exclusion) {
     return (s - p) + (d - p.zeroAlphas());
 }
 
-XFERMODE(HardLight) {
-    auto alphas = SrcOver::Xfer(s,d);
+// We take care to use exact math for these next few modes where alphas
+// and colors are calculated using significantly different math.  We need
+// to preserve premul invariants, and exact math makes this easier.
+//
+// TODO: Some of these implementations might be able to be sped up a bit
+// while maintaining exact math, but let's follow up with that.
 
+XFERMODE(HardLight) {
     auto sa = s.alphas(),
          da = d.alphas();
 
+    auto srcover = s + (d * sa.inv()).div255();
+
     auto isLite = ((sa-s) < s).widenLoHi();
 
-    auto dark = s*d << 1,
-         lite = sa*da - ((da-d)*(sa-s) << 1),
+    auto lite = sa*da - ((da-d)*(sa-s) << 1),
+         dark = s*d << 1,
          both = s*da.inv() + d*sa.inv();
 
+    auto alphas = srcover;
     auto colors = (both + isLite.thenElse(lite, dark)).div255();
     return alphas.zeroColors() + colors.zeroAlphas();
 }
 XFERMODE(Overlay) { return HardLight::Xfer(d,s); }
 
 XFERMODE(Darken) {
-    auto sda = s.approxMulDiv255(d.alphas()),
-         dsa = d.approxMulDiv255(s.alphas());
-    auto srcover = s + (d - dsa),
-         dstover = d + (s - sda);
+    auto sa = s.alphas(),
+         da = d.alphas();
+
+    auto sda = (s*da).div255(),
+         dsa = (d*sa).div255();
+
+    auto srcover = s + (d * sa.inv()).div255(),
+         dstover = d + (s * da.inv()).div255();
     auto alphas = srcover,
          colors = (sda < dsa).thenElse(srcover, dstover);
     return alphas.zeroColors() + colors.zeroAlphas();
 }
 XFERMODE(Lighten) {
-    auto sda = s.approxMulDiv255(d.alphas()),
-         dsa = d.approxMulDiv255(s.alphas());
-    auto srcover = s + (d - dsa),
-         dstover = d + (s - sda);
+    auto sa = s.alphas(),
+         da = d.alphas();
+
+    auto sda = (s*da).div255(),
+         dsa = (d*sa).div255();
+
+    auto srcover = s + (d * sa.inv()).div255(),
+         dstover = d + (s * da.inv()).div255();
     auto alphas = srcover,
          colors = (dsa < sda).thenElse(srcover, dstover);
     return alphas.zeroColors() + colors.zeroAlphas();