Skip to content

sparse_strips: Fix clipping for inverse blurred_rounded_rects#1721

Open
nicoburns wants to merge 1 commit into
linebender:mainfrom
nicoburns:fix-inverse-shadows
Open

sparse_strips: Fix clipping for inverse blurred_rounded_rects#1721
nicoburns wants to merge 1 commit into
linebender:mainfrom
nicoburns:fix-inverse-shadows

Conversation

@nicoburns

@nicoburns nicoburns commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Follow-up fix for #1715 which has a bug where "hard" shadows (with blur std_dev of 0) don't render correctly. This is because the inverse shadows were still using the clip-region logic used for "regular" shadows (inflate rect by 2.5 * std_dev) which happens to work if the std_dev is large enough, but in general clips far too aggressively if the std_dev is small and not aggressively enough when it is large.

For inverse shadows, what we want to do is clip to the exact size of the user-specified rect, but possibly at a location that is offset (translated) relative to where the shadow itself should render (if the shadow has a offset specified). This PR uses a bit of hack to encode this in the origin of the Rect:

  • The paint is clipped to position of the rect ignoring the origin
  • The shadow is draw offset by the origin of the rect

This results in correct rendering, but is arguably a bit of a hack. I could add a separate offset (Vec2) parameter if that's preferred.

This PR was entirely hand written.

Signed-off-by: Nico Burns <nico@nicoburns.com>
@nicoburns nicoburns requested a review from LaurenzV July 1, 2026 23:06
@LaurenzV

LaurenzV commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

This doesn't make a lot of sense to me, why are we expanding by the rectangle origin? Doesn't that mean the expansion depends on the exact position of the rectangle?

I think it would be best to figure out what Skia does here and then try to emulate that.

@nicoburns

nicoburns commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

This doesn't make a lot of sense to me, why are we expanding by the rectangle origin? Doesn't that mean the expansion depends on the exact position of the rectangle?

I think it would be best to figure out what Skia does here and then try to emulate that.

So, the API in this PR only works if you don't use the origin to encode the position. You have to use the transform to encode the position, and the use the origin the encode the "offset" of the shadow.

(probably I should change that to have a less confusing API with a dedicated offset parameter?)

@LaurenzV

LaurenzV commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

I’m not gonna block on it, but it seems to me like the semantics of an inset box shadow are slightly different than a normal box shadow? So maybe it should be a separate method altogether instead of just a boolean flag. But again, I think it would be worth finding out how Skia exposes both of those functionalities so we can draw some inspiration.

@nicoburns

Copy link
Copy Markdown
Contributor Author

I think there are exactly two differences between an "inset" and a "outset" (regular) box-shadow:

  1. The paint is inverted (literally 1 - alpha in the shader)
  2. The clip is inverted ("inset" shadows draw only inside the box, "outset" shadow draw outside the box)

In principle this is an exact inversion of the clip. In practice:

  • We optimise the "outset" clip so that it only draws a box 2.5 * std_dev around the specified box, rather than filling the full viewport outside of the box.
  • We don't actually implement clipping out the interior of the specified box for "outset" clips (IMO we should - currently in Blitz/AnyRender, I implement this manually as a clip layer).

What I would ideally like to do is implement a method that correctly handles all of this clipping for the user. At which point, I think it would probably make sense to rename the method box_shadow.

(I'm not sure if there are any use cases for filling a more general shape with the blurred_rounded_rect fill. We could expose that too if it's useful (but I suspect it's not))

@nicoburns

Copy link
Copy Markdown
Contributor Author

For Skia, anyrender currently has the following:

fn draw_box_shadow(
    &mut self,
    transform: kurbo::Affine,
    rect: kurbo::Rect,
    brush: peniko::Color,
    radius: f64,
    std_dev: f64,
    inset: bool,
) {
    self.set_matrix(transform);

    self.reset_paint();
    self.set_paint_brush(brush, None);
    self.cache.paint.set_style(PaintStyle::Fill);

    if std_dev > 0.0 {
        self.cache.paint.set_mask_filter(
            MaskFilter::blur(BlurStyle::Normal, std_dev as f32, false).unwrap(),
        );
    }

    let rrect = RRect::new_nine_patch(
        Rect::new(
            rect.x0 as f32,
            rect.y0 as f32,
            rect.x1 as f32,
            rect.y1 as f32,
        ),
        radius as f32,
        radius as f32,
        radius as f32,
        radius as f32,
    );

    if inset {
        let mut path = Path::rrect(rrect, None);
        path.set_fill_type(PathFillType::InverseWinding);
        self.inner.draw_path(&path, &self.cache.paint);
    } else {
        self.inner.draw_rrect(rrect, &self.cache.paint);
    }
}

I'm not sure if that is a "fast shadow" or a filter-based one. But it does produce the same output as the Vello shadows.

@LaurenzV

LaurenzV commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

I think there are exactly two differences between an "inset" and a "outset" (regular) box-shadow:

I'm no expert in this, but If I understood correctly, there is a third one. For outset, you literally just define the rectangle you want to draw and then blur that rectangle outward. For the inset thing, based on the screenshot you sent me in the DM, you additionally need to have the ability to define how much to offset the shadow from the top left:

image

For box shadow, adding an offset to the shadow literally just means "shift the whole blurred rectangle by x/y and draw it", so no special handling is needed here. But for the inset shadow, you additionally need to make sure that all of the padding area is filled in the given color, so it's not as simple as just shifting the inset shadow by x/y and that's it. If it weren't for this side padding, it seems to me like the two could be treated basically as the same.

Maybe I'm just confusing myself here. 😅 But to me, it seems really unintuitive that when you set invert to true, the semantics suddenly change to "the rectangle is always anchored at (0, 0) and any area in-between is treated as padding".

@nicoburns

nicoburns commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

My perspective on this is that if we were implementing the outset shadows "properly", then that would also have the semantics of:

  1. A clip area
  2. A shadow paint (blurred rect) that can be offset from that clip

See for example these outset shadows:

Screenshot 2026-07-03 at 21 42 45

CSS specifies that the shadow is not painted in the area of the box itself. And this is observable if the box has a transparent or partially transparent background. In that case the content from below the box (and the shadow) should show through.

The vellos do not currently implement this for outset shadows (Blitz applies the clip itself), but IMO they should.


My proposal for moving this forwards would be (in all vellos):

  • Add an offset: Vec2 parameter to the fill_blurred_rounded_rect method
  • Use that to implement the offset between the clip and paint for inset shadows (instead of the Rect origin which would now adjust the position of the "entire draw operation" like it usually does for other kinds of paint)
  • Implement "clipping" for outset shadows by drawing a "rounded rect donut" shape instead of a rounded rect (the offset parameter would then also be used to offset the outset shadow's paint relative to it's clip box)

And then optionally we could:

  • Rename the method to draw_box_shadow (as it will now implement CSS box-shadow semantics)
  • Remove the fill_blurred_rounded_rect_in methods. I believe they were added at my request. However, what I wanted was the clipping semantics above, but that is not what those methods actually do.

I believe all of this could be implemented "shallowly" in the top-level method without requiring further changes to the rendering pipelines.

@LaurenzV

LaurenzV commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

It does make me a bit uneasy implementing a primitive with such CSS specific semantics as a Vello primitive, but I think it makes sense and I could live with it. 😄

@nicoburns

Copy link
Copy Markdown
Contributor Author

It does make me a bit uneasy implementing a primitive with such CSS specific semantics as a Vello primitive, but I think it makes sense and I could live with it. 😄

I would (personally) be more worried if it was a "deep" feature. But given that the CSS-specific bit will be contained to the top-level method it seems less problematic to me. It's sugar for sure, but should be easy to maintain.

I'll try to code it up early next week (shouldn't be too hard I don't think), and then perhaps I can bring to Rendering Office hours when that next happens.

@nicoburns

Copy link
Copy Markdown
Contributor Author

I looked into building the full clipping into Vello, and am running into the problem that the shape I want for Blitz is rounded rect with eliptical radii which isn't available in Kurbo. And at some point I may want a superellipse. I'm now wondering if we ought to go the other way and allow an arbitrary impl Shape to be specified. This seems to be what the fill_blurred_rounded_rect_in function in Vello Classic allows.

Which also brings up the question of why blurred_round_rect isn't just a Paint type. Are there technical limitations there, or was that an API decision?

@LaurenzV

LaurenzV commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

I think that’s just what made the most sense to me back then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants