[librsvg: 9/12] Use integer math for Pixel::premultiply()

[Federico Mena Quintero <federico src gnome org>]

commit 4f9db850abb1fce6125184700c6428256a402ac1
Author: Sven Neumann <sven svenfoo org>
Date:   Tue Oct 13 00:41:37 2020 +0200

    Use integer math for Pixel::premultiply()
    
    Add tests using proptest to verify the implementation against the
    existing floating point implmentation as the reference.
    
    Also add tests for Pixel::unpremultiply() that check that the
    roundtrip pixel.premultiply().unpremultiply() doesn't differ more
    than expected from the original pixel.

 Cargo.lock                              | 69 +++++++++++++++++++++++++++++++++
 rsvg_internals/Cargo.toml               |  1 +
 rsvg_internals/src/surface_utils/mod.rs | 46 +++++++++++++++++-----
 3 files changed, 106 insertions(+), 10 deletions(-)
---
diff --git a/Cargo.lock b/Cargo.lock
index b1e22c00..806e9e55 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -60,6 +60,21 @@ version = "1.0.1"
 source = "registry+https://github.com/rust-lang/crates.io-index";
 checksum = "cdb031dd78e28731d87d56cc8ffef4a8f36ca26c38fe2de700543e627f8a464a"
 
+[[package]]
+name = "bit-set"
+version = "0.5.2"
+source = "registry+https://github.com/rust-lang/crates.io-index";
+checksum = "6e11e16035ea35e4e5997b393eacbf6f63983188f7a2ad25bfb13465f5ad59de"
+dependencies = [
+ "bit-vec",
+]
+
+[[package]]
+name = "bit-vec"
+version = "0.6.2"
+source = "registry+https://github.com/rust-lang/crates.io-index";
+checksum = "5f0dc55f2d8a1a85650ac47858bb001b4c0dd73d79e3c455a842925e68d29cd3"
+
 [[package]]
 name = "bitflags"
 version = "1.2.1"
@@ -455,6 +470,12 @@ dependencies = [
  "num-traits",
 ]
 
+[[package]]
+name = "fnv"
+version = "1.0.7"
+source = "registry+https://github.com/rust-lang/crates.io-index";
+checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1"
+
 [[package]]
 name = "futf"
 version = "0.1.4"
@@ -1312,6 +1333,32 @@ dependencies = [
  "unicode-xid",
 ]
 
+[[package]]
+name = "proptest"
+version = "0.10.1"
+source = "registry+https://github.com/rust-lang/crates.io-index";
+checksum = "12e6c80c1139113c28ee4670dc50cc42915228b51f56a9e407f0ec60f966646f"
+dependencies = [
+ "bit-set",
+ "bitflags",
+ "byteorder",
+ "lazy_static",
+ "num-traits",
+ "quick-error",
+ "rand",
+ "rand_chacha",
+ "rand_xorshift",
+ "regex-syntax",
+ "rusty-fork",
+ "tempfile",
+]
+
+[[package]]
+name = "quick-error"
+version = "1.2.3"
+source = "registry+https://github.com/rust-lang/crates.io-index";
+checksum = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0"
+
 [[package]]
 name = "quote"
 version = "1.0.7"
@@ -1381,6 +1428,15 @@ dependencies = [
  "rand_core",
 ]
 
+[[package]]
+name = "rand_xorshift"
+version = "0.2.0"
+source = "registry+https://github.com/rust-lang/crates.io-index";
+checksum = "77d416b86801d23dde1aa643023b775c3a462efc0ed96443add11546cdf1dca8"
+dependencies = [
+ "rand_core",
+]
+
 [[package]]
 name = "rawpointer"
 version = "0.2.1"
@@ -1500,6 +1556,7 @@ dependencies = [
  "pango-sys",
  "pangocairo",
  "pkg-config",
+ "proptest",
  "rayon",
  "rctree",
  "regex",
@@ -1520,6 +1577,18 @@ dependencies = [
  "semver",
 ]
 
+[[package]]
+name = "rusty-fork"
+version = "0.3.0"
+source = "registry+https://github.com/rust-lang/crates.io-index";
+checksum = "cb3dcc6e454c328bb824492db107ab7c0ae8fcffe4ad210136ef014458c1bc4f"
+dependencies = [
+ "fnv",
+ "quick-error",
+ "tempfile",
+ "wait-timeout",
+]
+
 [[package]]
 name = "ryu"
 version = "1.0.5"
diff --git a/rsvg_internals/Cargo.toml b/rsvg_internals/Cargo.toml
index e66a9d1b..6843fef8 100644
--- a/rsvg_internals/Cargo.toml
+++ b/rsvg_internals/Cargo.toml
@@ -47,6 +47,7 @@ xml5ever = "0.16.1"
 
 [dev-dependencies]
 criterion = "0.3"
+proptest = "0.10.1"
 
 [build-dependencies]
 pkg-config = "0.3.14"
diff --git a/rsvg_internals/src/surface_utils/mod.rs b/rsvg_internals/src/surface_utils/mod.rs
index 6d27df73..c4e2182b 100644
--- a/rsvg_internals/src/surface_utils/mod.rs
+++ b/rsvg_internals/src/surface_utils/mod.rs
@@ -95,8 +95,8 @@ impl PixelOps for Pixel {
     /// Returns a premultiplied value of this pixel.
     #[inline]
     fn premultiply(self) -> Self {
-        let alpha = f64::from(self.a) / 255.0;
-        self.map_rgb(|x| ((f64::from(x) * alpha) + 0.5) as u8)
+        let a = self.a as u32;
+        self.map_rgb(|x| (((x as u32) * a + 127) / 255) as u8)
     }
 
     /// Returns a 'mask' pixel with only the alpha channel
@@ -117,6 +117,7 @@ impl PixelOps for Pixel {
     ///    (we only care about the most significant byte)
     /// if pixel = 0x00FFFFFF, pixel' = 0xFF......
     /// if pixel = 0x00020202, pixel' = 0x02......
+
     /// if pixel = 0x00000000, pixel' = 0x00......
     #[inline]
     fn to_mask(self, opacity: u8) -> Self {
@@ -168,6 +169,7 @@ impl<'a> ImageSurfaceDataExt for &'a mut [u8] {}
 #[cfg(test)]
 mod tests {
     use super::*;
+    use proptest::prelude::*;
 
     #[test]
     fn pixel_diff() {
@@ -177,15 +179,39 @@ mod tests {
         assert_eq!(a.diff(&b), Pixel::new(0x40, 0xdf, 0xd0, 0x30));
     }
 
-    #[test]
-    fn pixel_premultiply() {
-        let pixel = Pixel::new(0x22, 0x44, 0xff, 0x80);
-        assert_eq!(pixel.premultiply(), Pixel::new(0x11, 0x22, 0x80, 0x80));
+    // Floating-point reference implementation
+    fn premultiply_float(pixel: Pixel) -> Pixel {
+        let alpha = f64::from(pixel.a) / 255.0;
+        pixel.map_rgb(|x| ((f64::from(x) * alpha) + 0.5) as u8)
     }
 
-    #[test]
-    fn pixel_unpremultiply() {
-        let pixel = Pixel::new(0x11, 0x22, 0x80, 0x80);
-        assert_eq!(pixel.unpremultiply(), Pixel::new(0x22, 0x44, 0xff, 0x80));
+    prop_compose! {
+        fn arbitrary_pixel()(a: u8, r: u8, g: u8, b: u8) -> Pixel {
+            Pixel { r, g, b, a }
+        }
+    }
+
+    proptest! {
+        #[test]
+        fn pixel_premultiply(pixel in arbitrary_pixel()) {
+            prop_assert_eq!(pixel.premultiply(), premultiply_float(pixel));
+        }
+
+        #[test]
+        fn pixel_unpremultiply(pixel in arbitrary_pixel()) {
+            let roundtrip = pixel.premultiply().unpremultiply();
+            if pixel.a == 0 {
+                prop_assert_eq!(roundtrip, Pixel::default());
+            } else {
+                // roundtrip can't be perfect, the accepted error depends on alpha
+                let tolerance = 0xff / pixel.a;
+                let diff = roundtrip.diff(&pixel);
+                prop_assert!(diff.r <= tolerance, "red component value differs by more than {}: {:?}", 
tolerance, roundtrip);
+                prop_assert!(diff.g <= tolerance, "green component value differs by more than {}: {:?}", 
tolerance, roundtrip);
+                prop_assert!(diff.b <= tolerance, "blue component value differs by more than {}: {:?}", 
tolerance, roundtrip);
+
+                prop_assert_eq!(pixel.a, roundtrip.a);
+            }
+       }
     }
 }