extend_mode_normalized and extend_mode (#974)

Current `extend_mode` is renamed to `extend_mode_normalized` (because it
require normalized coordinates). To avoid rounding errors we introduce
`extend_mode` that relaxes normalization requirement. Currently only
EXTEND_PAD is implemented differently, others fallback to
`extend_mode_normalized` for now.

partial fix #972

---------

Signed-off-by: sagudev <[email protected]>

Author: sagudev

vello_shaders/shader/fine.wgsl

@@ -831,7 +831,7 @@ fn read_end_clip(cmd_ix: u32) -> CmdEndClip {
 const EXTEND_PAD: u32 = 0u;
 const EXTEND_REPEAT: u32 = 1u;
 const EXTEND_REFLECT: u32 = 2u;
-fn extend_mode(t: f32, mode: u32) -> f32 {
+fn extend_mode_normalized(t: f32, mode: u32) -> f32 {
     switch mode {
         case EXTEND_PAD: {
             return clamp(t, 0.0, 1.0);
@@ -845,6 +845,20 @@ fn extend_mode(t: f32, mode: u32) -> f32 {
     }
 }
 
+fn extend_mode(t: f32, mode: u32, max: f32) -> f32 {
+    switch mode {
+        case EXTEND_PAD: {
+            return clamp(t, 0.0, max);
+        }
+        case EXTEND_REPEAT: {
+            return extend_mode_normalized(t / max, mode) * max;
+        }
+        case EXTEND_REFLECT, default: {
+            return extend_mode_normalized(t / max, mode) * max;
+        }
+    }
+}
+
 const PIXELS_PER_THREAD = 4u;
 
 #ifndef msaa
@@ -1067,7 +1081,7 @@ fn main(
                 let d = lin.line_x * xy.x + lin.line_y * xy.y + lin.line_c;
                 for (var i = 0u; i < PIXELS_PER_THREAD; i += 1u) {
                     let my_d = d + lin.line_x * f32(i);
-                    let x = i32(round(extend_mode(my_d, lin.extend_mode) * f32(GRADIENT_WIDTH - 1)));
+                    let x = i32(round(extend_mode_normalized(my_d, lin.extend_mode) * f32(GRADIENT_WIDTH - 1)));
                     let fg_rgba = textureLoad(gradients, vec2(x, i32(lin.index)), 0);
                     let fg_i = fg_rgba * area[i];
                     rgba[i] = rgba[i] * (1.0 - fg_i.a) + fg_i;
@@ -1109,7 +1123,7 @@ fn main(
                         is_valid = a >= 0.0 && t >= 0.0;
                     }
                     if is_valid {
-                        t = extend_mode(focal_x + t_sign * t, rad.extend_mode);
+                        t = extend_mode_normalized(focal_x + t_sign * t, rad.extend_mode);
                         t = select(t, 1.0 - t, is_swapped);
                         let x = i32(round(t * f32(GRADIENT_WIDTH - 1)));
                         let fg_rgba = textureLoad(gradients, vec2(x, i32(rad.index)), 0);
@@ -1144,7 +1158,7 @@ fn main(
                     phi = select(phi, 1.0 - phi, y < 0.0);
                     phi = select(phi, 0.0, phi != phi); // check for NaN
                     phi = (phi - sweep.t0) * scale;
-                    let t = extend_mode(phi, sweep.extend_mode);
+                    let t = extend_mode_normalized(phi, sweep.extend_mode);
                     let ramp_x = i32(round(t * f32(GRADIENT_WIDTH - 1)));
                     let fg_rgba = textureLoad(gradients, vec2(ramp_x, i32(sweep.index)), 0);
                     let fg_i = fg_rgba * area[i];
@@ -1155,16 +1169,15 @@ fn main(
             case CMD_IMAGE: {
                 let image = read_image(cmd_ix);
                 let atlas_max = image.atlas_offset + image.extents - vec2(1.0);
-                let extents_inv = vec2(1.0) / image.extents;
                 switch image.quality {
                     case IMAGE_QUALITY_LOW: {
                         for (var i = 0u; i < PIXELS_PER_THREAD; i += 1u) {
                             // We only need to load from the textures if the value will be used.
                             if area[i] != 0.0 {
                                 let my_xy = vec2(xy.x + f32(i), xy.y);
                                 var atlas_uv = image.matrx.xy * my_xy.x + image.matrx.zw * my_xy.y + image.xlat;
-                                atlas_uv.x = extend_mode(atlas_uv.x * extents_inv.x, image.x_extend_mode) * image.extents.x;
-                                atlas_uv.y = extend_mode(atlas_uv.y * extents_inv.y, image.y_extend_mode) * image.extents.y;
+                                atlas_uv.x = extend_mode(atlas_uv.x, image.x_extend_mode, image.extents.x);
+                                atlas_uv.y = extend_mode(atlas_uv.y, image.y_extend_mode, image.extents.y);
                                 atlas_uv = atlas_uv + image.atlas_offset;
                                 // TODO: If the image couldn't be added to the atlas (i.e. was too big), this isn't robust
                                 let atlas_uv_clamped = clamp(atlas_uv, image.atlas_offset, atlas_max);
@@ -1182,8 +1195,8 @@ fn main(
                             if area[i] != 0.0 {
                                 let my_xy = vec2(xy.x + f32(i), xy.y);
                                 var atlas_uv = image.matrx.xy * my_xy.x + image.matrx.zw * my_xy.y + image.xlat;
-                                atlas_uv.x = extend_mode(atlas_uv.x * extents_inv.x, image.x_extend_mode) * image.extents.x;
-                                atlas_uv.y = extend_mode(atlas_uv.y * extents_inv.y, image.y_extend_mode) * image.extents.y;
+                                atlas_uv.x = extend_mode(atlas_uv.x, image.x_extend_mode, image.extents.x);
+                                atlas_uv.y = extend_mode(atlas_uv.y, image.y_extend_mode, image.extents.y);
                                 atlas_uv = atlas_uv + image.atlas_offset - vec2(0.5);
                                 // TODO: If the image couldn't be added to the atlas (i.e. was too big), this isn't robust
                                 let atlas_uv_clamped = clamp(atlas_uv, image.atlas_offset, atlas_max);

vello_tests/tests/known_issues.rs

@@ -16,7 +16,7 @@ use scenes::ImageCache;
 use vello::{
     AaConfig, Scene,
     kurbo::{Affine, Rect},
-    peniko::{ImageFormat, ImageQuality, color::palette},
+    peniko::{Extend, ImageFormat, ImageQuality, color::palette},
 };
 use vello_tests::{TestParams, smoke_snapshot_test_sync};
 
@@ -82,15 +82,36 @@ const DATA_IMAGE_PNG: &[u8] = include_bytes!("../snapshots/smoke/data_image_roun
 
 /// Test for <https://github.com/linebender/vello/issues/972>
 #[test]
-// Note that this isn't exactly 0.3, it's a number starting with "0.3"
-#[should_panic(expected = "Expected mean to be less than 0.001, got 0.3")]
-fn test_data_image_roundtrip() {
+#[ignore = "CI runs these tests on a CPU, leading to them having unrealistic precision"]
+#[should_panic]
+fn test_data_image_roundtrip_extend_reflect() {
+    let mut scene = Scene::new();
+    let mut images = ImageCache::new();
+    let image = images
+        .from_bytes(0, DATA_IMAGE_PNG)
+        .unwrap()
+        .with_quality(ImageQuality::Low)
+        .with_extend(Extend::Reflect);
+    scene.draw_image(&image, Affine::IDENTITY);
+    let mut params = TestParams::new("data_image_roundtrip", image.width, image.height);
+    params.anti_aliasing = AaConfig::Area;
+    smoke_snapshot_test_sync(scene, &params)
+        .unwrap()
+        .assert_mean_less_than(0.001);
+}
+
+/// Test for <https://github.com/linebender/vello/issues/972>
+#[test]
+#[ignore = "CI runs these tests on a CPU, leading to them having unrealistic precision"]
+#[should_panic]
+fn test_data_image_roundtrip_extend_repeat() {
     let mut scene = Scene::new();
     let mut images = ImageCache::new();
     let image = images
         .from_bytes(0, DATA_IMAGE_PNG)
         .unwrap()
-        .with_quality(ImageQuality::Low);
+        .with_quality(ImageQuality::Low)
+        .with_extend(Extend::Repeat);
     scene.draw_image(&image, Affine::IDENTITY);
     let mut params = TestParams::new("data_image_roundtrip", image.width, image.height);
     params.anti_aliasing = AaConfig::Area;

vello_tests/tests/regression.rs

@@ -3,12 +3,13 @@
 
 //! Tests to ensure that certain issues which don't deserve a test scene don't regress
 
+use scenes::ImageCache;
 use vello::{
     AaConfig, Scene,
     kurbo::{Affine, RoundedRect, Stroke},
-    peniko::color::palette,
+    peniko::{Extend, ImageQuality, color::palette},
 };
-use vello_tests::{TestParams, snapshot_test_sync};
+use vello_tests::{TestParams, smoke_snapshot_test_sync, snapshot_test_sync};
 
 /// Test created from <https://github.com/linebender/vello/issues/616>
 #[test]
@@ -24,3 +25,23 @@ fn rounded_rectangle_watertight() {
         .unwrap()
         .assert_mean_less_than(0.001);
 }
+
+const DATA_IMAGE_PNG: &[u8] = include_bytes!("../snapshots/smoke/data_image_roundtrip.png");
+
+/// Test for <https://github.com/linebender/vello/issues/972>
+#[test]
+fn test_data_image_roundtrip_extend_pad() {
+    let mut scene = Scene::new();
+    let mut images = ImageCache::new();
+    let image = images
+        .from_bytes(0, DATA_IMAGE_PNG)
+        .unwrap()
+        .with_quality(ImageQuality::Low)
+        .with_extend(Extend::Pad);
+    scene.draw_image(&image, Affine::IDENTITY);
+    let mut params = TestParams::new("data_image_roundtrip", image.width, image.height);
+    params.anti_aliasing = AaConfig::Area;
+    smoke_snapshot_test_sync(scene, &params)
+        .unwrap()
+        .assert_mean_less_than(0.001);
+}