From ec59994831f8cba67bc4599435eb9dc6ee84685c Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Fri, 5 Sep 2025 15:38:33 +0100 Subject: [PATCH 1/4] Add `add_clip_layer`, to help removing `Mix::Clip` This is already covered by our existing tests, including `many_clips` --- examples/scenes/src/test_scenes.rs | 13 ++++------- vello/src/scene.rs | 37 ++++++++++++++++++++++++------ vello_encoding/src/draw.rs | 19 +++++++++++++-- 3 files changed, 51 insertions(+), 18 deletions(-) diff --git a/examples/scenes/src/test_scenes.rs b/examples/scenes/src/test_scenes.rs index 19f47d32e..ce23c3524 100644 --- a/examples/scenes/src/test_scenes.rs +++ b/examples/scenes/src/test_scenes.rs @@ -1152,7 +1152,7 @@ mod impls { const CLIPS_PER_FILL: usize = 3; for _ in 0..CLIPS_PER_FILL { let rot = Affine::rotate(rng.random_range(0.0..PI)); - scene.push_layer(Mix::Clip, 1.0, translate * rot, &base_tri); + scene.push_clip_layer(translate * rot, &base_tri); } let rot = Affine::rotate(rng.random_range(0.0..PI)); let color = Color::new([rng.random(), rng.random(), rng.random(), 1.]); @@ -1212,7 +1212,7 @@ mod impls { PathEl::LineTo((X0, Y1).into()), PathEl::ClosePath, ]; - scene.push_layer(Mix::Clip, 1.0, Affine::IDENTITY, &path); + scene.push_clip_layer(Affine::IDENTITY, &path); } let rect = Rect::new(X0, Y0, X1, Y1); scene.fill( @@ -1243,12 +1243,7 @@ mod impls { None, &make_diamond(1024.0, 125.0), ); - scene.push_layer( - Mix::Clip, - 1.0, - Affine::IDENTITY, - &make_diamond(1024.0, 150.0), - ); + scene.push_clip_layer(Affine::IDENTITY, &make_diamond(1024.0, 150.0)); scene.fill( Fill::NonZero, Affine::IDENTITY, @@ -1584,7 +1579,7 @@ mod impls { PathEl::ClosePath, ] }; - scene.push_layer(Mix::Clip, 1.0, Affine::IDENTITY, &clip); + scene.push_clip_layer(Affine::IDENTITY, &clip); { let text_size = 60.0 + 40.0 * (params.time as f32).sin(); let s = "Some clipped text!"; diff --git a/vello/src/scene.rs b/vello/src/scene.rs index 506ae004d..d4d45bdc4 100644 --- a/vello/src/scene.rs +++ b/vello/src/scene.rs @@ -83,12 +83,16 @@ impl Scene { /// previous layers using the specified blend mode. /// /// Every drawing command after this call will be clipped by the shape - /// until the layer is popped. + /// until the layer is [popped](Self::pop_layer). + /// For layers which are only added for clipping, you should + /// use [`push_clip_layer`](Self::push_clip_layer) instead. /// /// **However, the transforms are *not* saved or modified by the layer stack.** + /// That is, the `transform` argument to this function only applies a transform to the `clip` shape. /// /// Clip layers (`blend` = [`Mix::Clip`]) should have an alpha value of 1.0. /// For an opacity group with non-unity alpha, specify [`Mix::Normal`]. + #[track_caller] pub fn push_layer( &mut self, blend: impl Into, @@ -97,6 +101,9 @@ impl Scene { clip: &impl Shape, ) { let blend = blend.into(); + if blend.mix == Mix::Clip { + panic!(); + } if blend.mix == Mix::Clip && alpha != 1.0 { log::warn!("Clip mix mode used with semitransparent alpha"); } @@ -108,15 +115,16 @@ impl Scene { } /// Pushes a new layer clipped by the specified shape and treated like a luminance - /// mask for previous layers. + /// mask for the current layer. /// - /// That is, content drawn between this and the next `pop_layer` call will serve - /// as a luminance mask + /// That is, content drawn between this and the matching `pop_layer` call will serve + /// as a luminance mask for the prior content in this layer. /// /// Every drawing command after this call will be clipped by the shape - /// until the layer is popped. + /// until the layer is [popped](Self::pop_layer). /// /// **However, the transforms are *not* saved or modified by the layer stack.** + /// That is, the `transform` argument to this function only applies a transform to the `clip` shape. /// /// # Transparency and premultiplication /// @@ -137,6 +145,21 @@ impl Scene { ); } + /// Pushes a new layer clipped by the specified `clip` shape. + /// + /// The pushed layer is intended to not impact the "source" for blending; that is, any blends + /// within this layer will still include content from before this method was called in the "source" + /// of that blend operation. + /// + /// Every drawing command after this call will be clipped by the shape + /// until the layer is [popped](Self::pop_layer). + /// + /// **However, the transforms are *not* saved or modified by the layer stack.** + /// That is, the `transform` argument to this function only applies a transform to the `clip` shape. + pub fn push_clip_layer(&mut self, transform: Affine, clip: &impl Shape) { + self.push_layer_inner(DrawBeginClip::clip(), transform, clip); + } + /// Helper for logic shared between [`Self::push_layer`] and [`Self::push_luminance_mask_layer`] fn push_layer_inner( &mut self, @@ -872,7 +895,7 @@ impl ColorPainter for DrawColorGlyphs<'_> { }; self.clip_depth += 1; self.scene - .push_layer(Mix::Clip, 1.0, self.last_transform().to_kurbo(), &path.0); + .push_clip_layer(self.last_transform().to_kurbo(), &path.0); } fn push_clip_box(&mut self, clip_box: skrifa::raw::types::BoundingBox) { @@ -887,7 +910,7 @@ impl ColorPainter for DrawColorGlyphs<'_> { } self.clip_depth += 1; self.scene - .push_layer(Mix::Clip, 1.0, self.last_transform().to_kurbo(), &clip_box); + .push_clip_layer(self.last_transform().to_kurbo(), &clip_box); } fn pop_clip(&mut self) { diff --git a/vello_encoding/src/draw.rs b/vello_encoding/src/draw.rs index a80364364..7b2fdcc62 100644 --- a/vello_encoding/src/draw.rs +++ b/vello_encoding/src/draw.rs @@ -201,7 +201,14 @@ impl DrawBeginClip { /// /// The least significant 16 bits are reserved for Mix + Compose /// combinations. - pub const LUMINANCE_MASK_LAYER: u32 = 0x10000; + pub const LUMINANCE_MASK_BLEND_MODE: u32 = 0x10000; + /// The `blend_mode` used to indicate that a layer should be + /// treated as a clip. + /// + /// This is equivalent to `Compose::SrcOver` with a `Mix` of 128, + /// for legacy reasons. + /// We expect this to change in the future. + pub const CLIP_BLEND_MODE: u32 = 0x8003; /// Creates new clip draw data for a Porter-Duff blend mode. pub fn new(blend_mode: BlendMode, alpha: f32) -> Self { @@ -214,10 +221,18 @@ impl DrawBeginClip { /// Creates a new clip draw data for a luminance mask. pub fn luminance_mask(alpha: f32) -> Self { Self { - blend_mode: Self::LUMINANCE_MASK_LAYER, + blend_mode: Self::LUMINANCE_MASK_BLEND_MODE, alpha, } } + + /// Creates the clip draw data for a clip-only layer. + pub fn clip() -> Self { + Self { + blend_mode: Self::CLIP_BLEND_MODE, + alpha: 1.0, + } + } } /// Monoid for the draw tag stream. From feda2d3fd98f9d98d880e146f0a7fd8dcc4e674a Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Fri, 5 Sep 2025 15:59:56 +0100 Subject: [PATCH 2/4] Remove panic --- vello/src/scene.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/vello/src/scene.rs b/vello/src/scene.rs index d4d45bdc4..1ffbd529e 100644 --- a/vello/src/scene.rs +++ b/vello/src/scene.rs @@ -101,9 +101,6 @@ impl Scene { clip: &impl Shape, ) { let blend = blend.into(); - if blend.mix == Mix::Clip { - panic!(); - } if blend.mix == Mix::Clip && alpha != 1.0 { log::warn!("Clip mix mode used with semitransparent alpha"); } From b55c5e77a60c5e40d17896fc8ed6b43b0d709fcf Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Mon, 29 Sep 2025 17:34:36 +0100 Subject: [PATCH 3/4] Fixup docs --- vello/src/scene.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/vello/src/scene.rs b/vello/src/scene.rs index 1ffbd529e..2b14d2932 100644 --- a/vello/src/scene.rs +++ b/vello/src/scene.rs @@ -147,6 +147,9 @@ impl Scene { /// The pushed layer is intended to not impact the "source" for blending; that is, any blends /// within this layer will still include content from before this method was called in the "source" /// of that blend operation. + /// Note that this is not currently implemented correctly - + /// see [#1198](https://github.com/linebender/vello/issues/1198). + /// As such, you should currently not include any blend layers until this layer is popped. /// /// Every drawing command after this call will be clipped by the shape /// until the layer is [popped](Self::pop_layer). From dd477213dc1c16f55712efc590c03519879cd771 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Mon, 29 Sep 2025 17:38:56 +0100 Subject: [PATCH 4/4] Add a changelog entry --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4bc2980b1..4e3a0d7dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ This release has an [MSRV][] of 1.86. - `register_texture`, a helper for using `wgpu` textures in a Vello `Renderer`. ([#1161][] by [@DJMcNab][]) - `push_luminance_mask_layer`, content within which is used as a luminance mask. ([#1183][] by [@DJMcNab][]). This is a breaking change to Vello Encoding. +- `push_clip_layer`, which replaces the previous `push_layer` using `Mix::Clip`, and has fewer footguns. ([#1192][] by [@DJMcNab][]) + This is not a breaking change, as `Mix::Clip` is still supported (although it is deprecated). ### Changed @@ -346,6 +348,7 @@ This release has an [MSRV][] of 1.75. [#1182]: https://github.com/linebender/vello/pull/1182 [#1183]: https://github.com/linebender/vello/pull/1183 [#1187]: https://github.com/linebender/vello/pull/1187 +[#1192]: https://github.com/linebender/vello/pull/1192 [#1224]: https://github.com/linebender/vello/pull/1224 [#1229]: https://github.com/linebender/vello/pull/1229