From f7edc30701e30765c897a18e02fe073ced5310bb Mon Sep 17 00:00:00 2001 From: sagudev <16504129+sagudev@users.noreply.github.com> Date: Mon, 6 Oct 2025 10:35:56 +0200 Subject: [PATCH 1/2] [classic] Support unpremultiplied gradient interpolation in Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com> --- Cargo.lock | 3 +- Cargo.toml | 3 + vello_encoding/src/encoding.rs | 122 +++++++++--------- vello_encoding/src/ramp_cache.rs | 57 ++++++-- vello_encoding/src/resolve.rs | 10 +- .../gradient_color_alpha_unpremultiplied.png | Bin 0 -> 247 bytes vello_tests/tests/known_issues.rs | 37 +++++- 7 files changed, 154 insertions(+), 78 deletions(-) create mode 100644 vello_tests/snapshots/smoke/gradient_color_alpha_unpremultiplied.png diff --git a/Cargo.lock b/Cargo.lock index 2986ae9ae..c30ed666e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2576,8 +2576,7 @@ dependencies = [ [[package]] name = "peniko" version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b3c76095c9a636173600478e0373218c7b955335048c2bcd12dc6a79657649d8" +source = "git+https://github.com/sagudev/peniko?branch=interp-eq-hash#58dd6acce6bf59347405c9c4e09e5bf2a889b791" dependencies = [ "bytemuck", "color", diff --git a/Cargo.toml b/Cargo.toml index 2df7dbb32..ecc9eb3c4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -151,3 +151,6 @@ console_log = "1.0" proc-macro2 = "1.0.95" syn = { version = "2.0.101", features = ["full", "extra-traits"] } quote = "1.0.40" + +[patch.crates-io] +peniko = { git = "https://github.com/sagudev/peniko", branch = "interp-eq-hash" } diff --git a/vello_encoding/src/encoding.rs b/vello_encoding/src/encoding.rs index 163282bc8..78d37cd5b 100644 --- a/vello_encoding/src/encoding.rs +++ b/vello_encoding/src/encoding.rs @@ -126,12 +126,14 @@ impl Encoding { draw_data_offset: offset, stops, extend, + interpolation_alpha_space, } => { let stops = stops.start + stops_base..stops.end + stops_base; Patch::Ramp { draw_data_offset: offset + offsets.draw_data, stops, extend: *extend, + interpolation_alpha_space: *interpolation_alpha_space, } } Patch::GlyphRun { index } => Patch::GlyphRun { @@ -286,65 +288,60 @@ impl Encoding { }; self.encode_color(color); } - BrushRef::Gradient(gradient) => { - if gradient.interpolation_alpha_space != InterpolationAlphaSpace::Premultiplied { - unimplemented!( - "We don't yet support gradient interpolation which isn't premultiplied, found {:?}.", - gradient.interpolation_alpha_space - ) + BrushRef::Gradient(gradient) => match gradient.kind { + GradientKind::Linear(LinearGradientPosition { start, end }) => { + self.encode_linear_gradient( + DrawLinearGradient { + index: 0, + p0: point_to_f32(start), + p1: point_to_f32(end), + }, + gradient.stops.iter().copied(), + alpha, + gradient.extend, + gradient.interpolation_alpha_space, + ); } - match gradient.kind { - GradientKind::Linear(LinearGradientPosition { start, end }) => { - self.encode_linear_gradient( - DrawLinearGradient { - index: 0, - p0: point_to_f32(start), - p1: point_to_f32(end), - }, - gradient.stops.iter().copied(), - alpha, - gradient.extend, - ); - } - GradientKind::Radial(RadialGradientPosition { - start_center, - start_radius, - end_center, - end_radius, - }) => { - self.encode_radial_gradient( - DrawRadialGradient { - index: 0, - p0: point_to_f32(start_center), - p1: point_to_f32(end_center), - r0: start_radius, - r1: end_radius, - }, - gradient.stops.iter().copied(), - alpha, - gradient.extend, - ); - } - GradientKind::Sweep(SweepGradientPosition { - center, - start_angle, - end_angle, - }) => { - use core::f32::consts::TAU; - self.encode_sweep_gradient( - DrawSweepGradient { - index: 0, - p0: point_to_f32(center), - t0: start_angle / TAU, - t1: end_angle / TAU, - }, - gradient.stops.iter().copied(), - alpha, - gradient.extend, - ); - } + GradientKind::Radial(RadialGradientPosition { + start_center, + start_radius, + end_center, + end_radius, + }) => { + self.encode_radial_gradient( + DrawRadialGradient { + index: 0, + p0: point_to_f32(start_center), + p1: point_to_f32(end_center), + r0: start_radius, + r1: end_radius, + }, + gradient.stops.iter().copied(), + alpha, + gradient.extend, + gradient.interpolation_alpha_space, + ); } - } + GradientKind::Sweep(SweepGradientPosition { + center, + start_angle, + end_angle, + }) => { + use core::f32::consts::TAU; + self.encode_sweep_gradient( + DrawSweepGradient { + index: 0, + p0: point_to_f32(center), + t0: start_angle / TAU, + t1: end_angle / TAU, + }, + gradient.stops.iter().copied(), + alpha, + gradient.extend, + gradient.interpolation_alpha_space, + ); + } + }, BrushRef::Image(image) => { self.encode_image(image, alpha); } @@ -366,8 +363,9 @@ impl Encoding { color_stops: impl Iterator, alpha: f32, extend: Extend, + interpolation_alpha_space: InterpolationAlphaSpace, ) { - match self.add_ramp(color_stops, alpha, extend) { + match self.add_ramp(color_stops, alpha, extend, interpolation_alpha_space) { RampStops::Empty => self.encode_color(palette::css::TRANSPARENT), RampStops::One(color) => { self.encode_color(color); @@ -387,6 +385,7 @@ impl Encoding { color_stops: impl Iterator, alpha: f32, extend: Extend, + interpolation_alpha_space: InterpolationAlphaSpace, ) { // Match Skia's epsilon for radii comparison const SKIA_EPSILON: f32 = 1.0 / (1 << 12) as f32; @@ -394,7 +393,7 @@ impl Encoding { self.encode_color(palette::css::TRANSPARENT); return; } - match self.add_ramp(color_stops, alpha, extend) { + match self.add_ramp(color_stops, alpha, extend, interpolation_alpha_space) { RampStops::Empty => self.encode_color(palette::css::TRANSPARENT), RampStops::One(color) => self.encode_color(color), RampStops::Many => { @@ -412,13 +411,14 @@ impl Encoding { color_stops: impl Iterator, alpha: f32, extend: Extend, + interpolation_alpha_space: InterpolationAlphaSpace, ) { const SKIA_DEGENERATE_THRESHOLD: f32 = 1.0 / (1 << 15) as f32; if (gradient.t0 - gradient.t1).abs() < SKIA_DEGENERATE_THRESHOLD { self.encode_color(palette::css::TRANSPARENT); return; } - match self.add_ramp(color_stops, alpha, extend) { + match self.add_ramp(color_stops, alpha, extend, interpolation_alpha_space) { RampStops::Empty => self.encode_color(palette::css::TRANSPARENT), RampStops::One(color) => self.encode_color(color), RampStops::Many => { @@ -521,6 +521,7 @@ impl Encoding { color_stops: impl Iterator, alpha: f32, extend: Extend, + interpolation_alpha_space: InterpolationAlphaSpace, ) -> RampStops { let offset = self.draw_data.len(); let stops_start = self.resources.color_stops.len(); @@ -540,6 +541,7 @@ impl Encoding { draw_data_offset: offset, stops: stops_start..stops_end, extend, + interpolation_alpha_space, }); RampStops::Many } diff --git a/vello_encoding/src/ramp_cache.rs b/vello_encoding/src/ramp_cache.rs index 67001f7ce..dfa570376 100644 --- a/vello_encoding/src/ramp_cache.rs +++ b/vello_encoding/src/ramp_cache.rs @@ -4,8 +4,8 @@ use std::collections::HashMap; use peniko::color::cache_key::CacheKey; -use peniko::color::{HueDirection, Srgb}; -use peniko::{ColorStop, ColorStops}; +use peniko::color::{AlphaColor, HueDirection, Srgb}; +use peniko::{ColorStop, ColorStops, InterpolationAlphaSpace}; const N_SAMPLES: usize = 512; const RETAINED_COUNT: usize = 64; @@ -21,7 +21,7 @@ pub struct Ramps<'a> { #[derive(Default)] pub(crate) struct RampCache { epoch: u64, - map: HashMap, (u32, u64)>, + map: HashMap<(InterpolationAlphaSpace, CacheKey), (u32, u64)>, data: Vec, } @@ -35,14 +35,25 @@ impl RampCache { } } - pub(crate) fn add(&mut self, stops: &[ColorStop]) -> u32 { - if let Some(entry) = self.map.get_mut(&CacheKey(stops.into())) { + pub(crate) fn add( + &mut self, + interpolation_alpha_space: InterpolationAlphaSpace, + stops: &[ColorStop], + ) -> u32 { + if let Some(entry) = self + .map + .get_mut(&(interpolation_alpha_space, CacheKey(stops.into()))) + { entry.1 = self.epoch; entry.0 } else if self.map.len() < RETAINED_COUNT { let id = (self.data.len() / N_SAMPLES) as u32; - self.data.extend(make_ramp(stops)); - self.map.insert(CacheKey(stops.into()), (id, self.epoch)); + self.data + .extend(make_ramp(stops, interpolation_alpha_space)); + self.map.insert( + (interpolation_alpha_space, CacheKey(stops.into())), + (id, self.epoch), + ); id } else { let mut reuse = None; @@ -57,16 +68,23 @@ impl RampCache { let start = id as usize * N_SAMPLES; for (dst, src) in self.data[start..start + N_SAMPLES] .iter_mut() - .zip(make_ramp(stops)) + .zip(make_ramp(stops, interpolation_alpha_space)) { *dst = src; } - self.map.insert(CacheKey(stops.into()), (id, self.epoch)); + self.map.insert( + (interpolation_alpha_space, CacheKey(stops.into())), + (id, self.epoch), + ); id } else { let id = (self.data.len() / N_SAMPLES) as u32; - self.data.extend(make_ramp(stops)); - self.map.insert(CacheKey(stops.into()), (id, self.epoch)); + self.data + .extend(make_ramp(stops, interpolation_alpha_space)); + self.map.insert( + (interpolation_alpha_space, CacheKey(stops.into())), + (id, self.epoch), + ); id } } @@ -81,7 +99,10 @@ impl RampCache { } } -fn make_ramp(stops: &[ColorStop]) -> impl Iterator + '_ { +fn make_ramp( + stops: &[ColorStop], + interpolation_alpha_space: InterpolationAlphaSpace, +) -> impl Iterator + '_ { let mut last_u = 0.0; let mut last_c = stops[0].color.to_alpha_color::(); let mut this_u = last_u; @@ -104,7 +125,17 @@ fn make_ramp(stops: &[ColorStop]) -> impl Iterator + '_ { let c = if du < 1e-9 { this_c } else { - last_c.lerp(this_c, (u - last_u) / du, HueDirection::default()) + match interpolation_alpha_space { + InterpolationAlphaSpace::Premultiplied => { + last_c.lerp(this_c, (u - last_u) / du, HueDirection::default()) + } + InterpolationAlphaSpace::Unpremultiplied => { + AlphaColor::::new(std::array::from_fn(|i| { + last_c.components[i] + + (this_c.components[i] - last_c.components[i]) * ((u - last_u) / du) + })) + } + } }; c.premultiply().to_rgba8().to_u32() }) diff --git a/vello_encoding/src/resolve.rs b/vello_encoding/src/resolve.rs index 4a8a41540..3cc44922c 100644 --- a/vello_encoding/src/resolve.rs +++ b/vello_encoding/src/resolve.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 OR MIT use bytemuck::{Pod, Zeroable}; -use peniko::{Extend, ImageData}; +use peniko::{Extend, ImageData, InterpolationAlphaSpace}; use std::ops::Range; use std::sync::Arc; @@ -401,8 +401,12 @@ impl Resolver { draw_data_offset, stops, extend, + interpolation_alpha_space, } => { - let ramp_id = self.ramp_cache.add(&resources.color_stops[stops.clone()]); + let ramp_id = self.ramp_cache.add( + *interpolation_alpha_space, + &resources.color_stops[stops.clone()], + ); self.patches.push(ResolvedPatch::Ramp { draw_data_offset: *draw_data_offset + sizes.draw_data, ramp_id, @@ -524,6 +528,8 @@ pub enum Patch { stops: Range, /// Extend mode for the gradient. extend: Extend, + /// Interpolation alpha space for the gradient. + interpolation_alpha_space: InterpolationAlphaSpace, }, /// Glyph run resource. GlyphRun { diff --git a/vello_tests/snapshots/smoke/gradient_color_alpha_unpremultiplied.png b/vello_tests/snapshots/smoke/gradient_color_alpha_unpremultiplied.png new file mode 100644 index 0000000000000000000000000000000000000000..0ac85d7d7cc44221696bb46ed890c5fd0db5c660 GIT binary patch literal 247 zcmeAS@N?(olHy`uVBq!ia0vp^DL`z*!2~2#!(YAxQu{nz978H@y}jVb*J8lqd{Jb_ z|He$={)MO7uF5=!o;r2O?sCtr6&-hUuS%#^UR}QQQ@~Tb^F_N>Z`pruseji#7x}Bj zDc^pTrG2q6dRof=ZtFM0dpBjXZL^=n-Cg`V*^mF9RoVCH_l34&nR4F{CKvI&i@AjLqj?gpS@ ckl0T~jeEDxp7!764fF|vr>mdKI;Vst0N}l7Pyhe` literal 0 HcmV?d00001 diff --git a/vello_tests/tests/known_issues.rs b/vello_tests/tests/known_issues.rs index 56ea71d63..16ed2bf5d 100644 --- a/vello_tests/tests/known_issues.rs +++ b/vello_tests/tests/known_issues.rs @@ -16,7 +16,10 @@ use scenes::ImageCache; use vello::{ AaConfig, Scene, kurbo::{Affine, Rect, Triangle}, - peniko::{Color, ColorStop, Extend, Gradient, ImageFormat, ImageQuality, Mix, color::palette}, + peniko::{ + Color, ColorStop, Extend, Gradient, ImageFormat, ImageQuality, InterpolationAlphaSpace, + Mix, color::palette, + }, }; use vello_tests::{TestParams, smoke_snapshot_test_sync, snapshot_test_sync}; @@ -191,6 +194,38 @@ fn test_gradient_color_alpha() { .assert_mean_less_than(0.001); } +/// +/// See . +#[test] +#[cfg_attr(skip_gpu_tests, ignore)] +fn test_gradient_color_alpha_unpremultiplied() { + let mut scene = Scene::new(); + let viewport = Rect::new(0., 0., 100., 50.); + scene.fill( + vello::peniko::Fill::NonZero, + Affine::IDENTITY, + &Gradient::new_linear((0., 0.), (100., 0.)) + .with_stops([ + ColorStop { + offset: 0., + color: Color::from_rgba8(255, 255, 0, 0).into(), + }, + ColorStop { + offset: 1., + color: Color::from_rgba8(0, 0, 255, 255).into(), + }, + ]) + .with_interpolation_alpha_space(InterpolationAlphaSpace::Unpremultiplied), + None, + &viewport, + ); + let mut params = TestParams::new("gradient_color_alpha_unpremultiplied", 100, 50); + params.base_color = Some(palette::css::WHITE); + smoke_snapshot_test_sync(scene, ¶ms) + .unwrap() + .assert_mean_less_than(0.001); +} + /// See #[test] #[cfg_attr(skip_gpu_tests, ignore)] From 2536e30e4752f4ac5f77d59c1f647d532fe6f8ab Mon Sep 17 00:00:00 2001 From: sagudev <16504129+sagudev@users.noreply.github.com> Date: Mon, 6 Oct 2025 10:41:48 +0200 Subject: [PATCH 2/2] huh Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com> --- vello_encoding/src/ramp_cache.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/vello_encoding/src/ramp_cache.rs b/vello_encoding/src/ramp_cache.rs index dfa570376..8195c6bf8 100644 --- a/vello_encoding/src/ramp_cache.rs +++ b/vello_encoding/src/ramp_cache.rs @@ -130,10 +130,7 @@ fn make_ramp( last_c.lerp(this_c, (u - last_u) / du, HueDirection::default()) } InterpolationAlphaSpace::Unpremultiplied => { - AlphaColor::::new(std::array::from_fn(|i| { - last_c.components[i] - + (this_c.components[i] - last_c.components[i]) * ((u - last_u) / du) - })) + last_c + (this_c - last_c) * ((u - last_u) / du) } } };