Merge pull request #1626

render: keep even-odd holes open in shadow, glow and reflection silhouettes
This commit is contained in:
Brandon Thomas
2026-11-02 04:51:25 -05:00
4 changed files with 284 additions and 7 deletions
+3
View File
@@ -94,6 +94,9 @@ mod tests;
#[cfg(test)]
mod tests_wild;
#[cfg(test)]
mod tests_effect_holes;
#[cfg(test)]
mod mt_tests {
use super::*;
+35 -7
View File
@@ -1125,12 +1125,15 @@ impl Renderer {
// see-through as its fill (none for an outline-only shape) …
None => {
if let Some(a) = fill_alpha {
let mut filled = BezPath::new();
for sp in geo.paths.iter().filter(|sp| sp.fill != FillMode::None) {
filled.extend(sp.path.iter());
}
ctx.set_paint(color(c.with_alpha(255), a));
ctx.fill_path(&filled);
// Each subpath under its own fill rule, as the shape itself is
// drawn: one merged path filled NonZero fills the hole of an
// even-odd shape (a donut, a frame) (#1596).
for sp in geo.paths.iter().filter(|sp| sp.fill != FillMode::None) {
ctx.set_fill_rule(if sp.even_odd { peniko::Fill::EvenOdd } else { peniko::Fill::NonZero });
ctx.fill_path(&sp.path);
}
ctx.set_fill_rule(peniko::Fill::NonZero);
}
}
}
@@ -1161,7 +1164,19 @@ impl Renderer {
ctx.stroke_path(&silhouette);
// A shape without a fill (an outline, a frame) glows along its line only (#225).
if fill_alpha.is_some() || content.is_some() {
ctx.fill_path(&silhouette);
// Per subpath under its own fill rule, as in the shadow above: the
// merged silhouette filled NonZero fills an even-odd hole (#1596).
// When no subpath takes part the silhouette is the outline fallback.
let mut filled_any = false;
for sp in geo.paths.iter().filter(|sp| sp.fill != FillMode::None || fill.is_none()) {
ctx.set_fill_rule(if sp.even_odd { peniko::Fill::EvenOdd } else { peniko::Fill::NonZero });
ctx.fill_path(&sp.path);
filled_any = true;
}
if !filled_any {
ctx.fill_path(&silhouette);
}
ctx.set_fill_rule(peniko::Fill::NonZero);
}
ctx.pop_layer();
}
@@ -1180,7 +1195,20 @@ impl Renderer {
}
None => {
if let Some(fl) = fill {
paint::fill_path(ctx, rctx, fl, ph, &silhouette, Rect::new(0.0, 0.0, w, h), mm, FillMode::Norm, 1.0, rctx.pres);
// Per subpath under its own fill rule, as in the shadow above:
// the merged silhouette filled NonZero fills an even-odd hole
// (#1596). The outline fallback covers shapes with no
// participating subpath.
let mut filled_any = false;
for sp in geo.paths.iter().filter(|sp| sp.fill != FillMode::None || fill.is_none()) {
ctx.set_fill_rule(if sp.even_odd { peniko::Fill::EvenOdd } else { peniko::Fill::NonZero });
paint::fill_path(ctx, rctx, fl, ph, &sp.path, Rect::new(0.0, 0.0, w, h), mm, FillMode::Norm, 1.0, rctx.pres);
filled_any = true;
}
if !filled_any {
paint::fill_path(ctx, rctx, fl, ph, &silhouette, Rect::new(0.0, 0.0, w, h), mm, FillMode::Norm, 1.0, rctx.pres);
}
ctx.set_fill_rule(peniko::Fill::NonZero);
}
}
}
+50
View File
@@ -515,6 +515,56 @@ fn glow_of_a_shape_with_no_fill_and_no_line_is_not_drawn() {
}
}
#[test]
fn shadow_and_glow_keep_an_even_odd_shapes_hole() {
// #1596: a donut is one even-odd subpath — its hole exists only under the
// even-odd rule. The effects were cast from a merged silhouette filled
// NonZero, so the shadow and the glow filled the hole solid. The hole's
// centre must stay the slide's white in both, while the effects still draw.
let donut = |effects: deckcraft_model::Effects| {
let mut sh = Shape {
id: ShapeId(612),
xfrm: Some(Xfrm::new(50.0, 50.0, 200.0, 200.0)),
geom: Geom::preset("donut"),
fill: Some(Fill::solid(ColorRef::rgb(deckcraft_color::Rgba::rgb(255, 160, 0)))),
..Default::default()
};
sh.effects = Some(effects);
render_slide(&deck_with(vec![sh]), 0, &RenderOpts { scale: 1.0, ..Default::default() })
};
let img = donut(deckcraft_model::Effects {
outer_shadow: Some(deckcraft_model::style::Shadow {
color: ColorRef::rgb(deckcraft_color::Rgba::BLACK),
blur: 8.0,
dist: 0.0,
dir: 45.0,
inner: false,
sx: 1.0,
sy: 1.0,
kx: 0.0,
ky: 0.0,
align: String::new(),
rotate_with_shape: false,
}),
..Default::default()
});
let hole = img.pixel(150, 150);
assert!(hole[0] > 245 && hole[1] > 245 && hole[2] > 245, "shadow in the hole: {hole:?}");
// The shadow still falls around the shape (just below its bottom edge).
let below = img.pixel(150, 251);
assert!(below[0] < 200 && below[0] == below[2], "no shadow below: {below:?}");
let img = donut(deckcraft_model::Effects {
glow: Some(deckcraft_model::style::Glow { color: ColorRef::rgb(deckcraft_color::Rgba::rgb(255, 0, 0)), radius: 8.0 }),
..Default::default()
});
let hole = img.pixel(150, 150);
assert!(hole[0] > 245 && hole[1] > 245 && hole[2] > 245, "glow in the hole: {hole:?}");
// The ring itself is the shape's orange.
let ring = img.pixel(150, 75);
assert!(ring[0] > 200 && ring[1] > 100 && ring[2] < 100, "ring: {ring:?}");
}
#[test]
fn placeholder_prompts_only_in_edit_view() {
let p = Presentation::default();
+196
View File
@@ -0,0 +1,196 @@
//! Contributor-original pixel goldens for #1596 and the review of #1626.
//! The fixed samples cover background, hole/centre, ring, visible halos and reflected coverage.
use super::{Fill, Image, RenderOpts, Rgba, render_slide};
use deckcraft_model::style::{ColorRef, Effects, Glow, Line, Reflection, Shadow};
use deckcraft_model::{Geom, Presentation, Shape, ShapeId, Xfrm};
const PROBES: [(u32, u32); 9] = [
(20, 20), // Clear background.
(130, 130), // Clear donut hole; filled ellipse centre.
(130, 70), // Original ring/ellipse fill.
(130, 47), // Visible shadow/glow above the outer edge.
(130, 213), // Visible shadow/glow below the outer edge.
(130, 93), // Shadow/glow along the inner rim, inside the hole.
(130, 230), // Reflected upper ring.
(130, 300), // Reflected hole/ellipse centre.
(70, 300), // Reflected ring at the same fade as the hole.
];
#[derive(Clone, Copy, Debug)]
enum Effect {
Shadow,
Glow,
Reflection,
}
fn scene(shape: &str, effect: Effect) -> Presentation {
let mut p = Presentation::default();
let mut effects = Effects::default();
match effect {
Effect::Shadow => {
effects.outer_shadow = Some(Shadow {
color: ColorRef::rgb(Rgba::BLACK),
blur: 8.0,
dist: 0.0,
dir: 45.0,
inner: false,
sx: 1.0,
sy: 1.0,
kx: 0.0,
ky: 0.0,
align: String::new(),
rotate_with_shape: false,
})
}
Effect::Glow => effects.glow = Some(Glow { color: ColorRef::rgb(Rgba::rgb(255, 0, 0)), radius: 8.0 }),
Effect::Reflection => effects.reflection = Some(Reflection { blur: 0.0, start_alpha: 1.0, end_alpha: 0.0, end_pos: 1.0, dist: 10.0 }),
}
let slide = std::sync::Arc::make_mut(&mut p.slides[0]);
slide.background = Some(deckcraft_model::Background::Fill { fill: Fill::solid(ColorRef::rgb(Rgba::WHITE)) });
slide.shapes = vec![Shape {
id: ShapeId(1596),
xfrm: Some(Xfrm::new(50.0, 50.0, 160.0, 160.0)),
geom: Geom::preset(shape),
fill: Some(Fill::solid(ColorRef::rgb(Rgba::rgb(32, 96, 224)))),
line: Some(Line::none()),
effects: Some(effects),
..Default::default()
}];
p
}
fn draw(shape: &str, effect: Effect) -> Image {
let p = scene(shape, effect);
let image = render_slide(&p, 0, &RenderOpts { scale: 1.0, ..Default::default() });
if let Ok(dir) = std::env::var("DECKCRAFT_EFFECT_HOLES_QA") {
// Overwrite, so a QA directory can be reused across runs.
std::fs::create_dir_all(&dir).unwrap();
std::fs::write(std::path::Path::new(&dir).join(format!("{shape}-{effect:?}.png")), image.to_png()).unwrap();
}
image
}
// Fixed contributor-original RGBA goldens; tolerate one final quantization step.
// The nonwhite halo/reflection samples also fail if an effect is disabled outright.
fn assert_golden(shape: &str, effect: Effect, expected: [[u8; 4]; 9]) {
let image = draw(shape, effect);
for ((x, y), want) in PROBES.into_iter().zip(expected) {
let got = image.pixel(x, y);
assert!(got.iter().zip(want).all(|(a, b)| (*a as i32 - b as i32).abs() <= 1), "{shape} {effect:?} at {x},{y}: {got:?} vs golden {want:?}");
}
}
#[test]
fn donut_shadow_matches_golden() {
assert_golden(
"donut",
Effect::Shadow,
[
[255, 255, 255, 255],
[255, 255, 255, 255],
[32, 96, 224, 255],
[214, 214, 214, 255],
[233, 233, 233, 255],
[227, 227, 227, 255],
[255, 255, 255, 255],
[255, 255, 255, 255],
[255, 255, 255, 255],
],
);
}
#[test]
fn donut_glow_matches_golden() {
assert_golden(
"donut",
Effect::Glow,
[
[255, 255, 255, 255],
[255, 255, 255, 255],
[32, 96, 224, 255],
[255, 49, 49, 255],
[255, 111, 111, 255],
[255, 96, 96, 255],
[255, 255, 255, 255],
[255, 255, 255, 255],
[255, 255, 255, 255],
],
);
}
#[test]
fn donut_reflection_matches_golden() {
assert_golden(
"donut",
Effect::Reflection,
[
[255, 255, 255, 255],
[255, 255, 255, 255],
[32, 96, 224, 255],
[255, 255, 255, 255],
[255, 255, 255, 255],
[255, 255, 255, 255],
[130, 166, 238, 255],
[255, 255, 255, 255],
[189, 208, 246, 255],
],
);
}
#[test]
fn ellipse_shadow_matches_golden() {
assert_golden(
"ellipse",
Effect::Shadow,
[
[255, 255, 255, 255],
[32, 96, 224, 255],
[32, 96, 224, 255],
[214, 214, 214, 255],
[233, 233, 233, 255],
[32, 96, 224, 255],
[255, 255, 255, 255],
[255, 255, 255, 255],
[255, 255, 255, 255],
],
);
}
#[test]
fn ellipse_glow_matches_golden() {
assert_golden(
"ellipse",
Effect::Glow,
[
[255, 255, 255, 255],
[32, 96, 224, 255],
[32, 96, 224, 255],
[255, 49, 49, 255],
[255, 111, 111, 255],
[32, 96, 224, 255],
[255, 255, 255, 255],
[255, 255, 255, 255],
[255, 255, 255, 255],
],
);
}
#[test]
fn ellipse_reflection_matches_golden() {
assert_golden(
"ellipse",
Effect::Reflection,
[
[255, 255, 255, 255],
[32, 96, 224, 255],
[32, 96, 224, 255],
[255, 255, 255, 255],
[255, 255, 255, 255],
[32, 96, 224, 255],
[130, 166, 238, 255],
[189, 208, 246, 255],
[189, 208, 246, 255],
],
);
}