Include filters in SVG occlusion tests [chromium/src : main]
1 view
Skip to first unread message
Philip Rogers (Gerrit)
May 24, 2026, 5:36:48 PM (2 days ago) May 24
to Fredrik Söderquist, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, kouhe...@chromium.org, pdr+svgw...@chromium.org, zol...@webkit.org
Attention needed from Fredrik Söderquist
Philip Rogers voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Fredrik Söderquist
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Fredrik Söderquist (Gerrit)
May 25, 2026, 8:12:44 AM (19 hours ago) May 25
to Philip Rogers, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, kouhe...@chromium.org, pdr+svgw...@chromium.org, zol...@webkit.org
Attention needed from Philip Rogers
Fredrik Söderquist added 5 comments![]( "Open in Gerrit")
File third_party/blink/renderer/core/layout/svg/layout_svg_container.h
Line 86, Patchset 22 (Latest):
gfx::RectF ComputeVisualOverflowRectIncludingFilters() const {Fredrik Söderquist . unresolved
```suggestion
gfx::RectF ComputeContentVisualOverflowRectIncludingFilters() const {
```
?
File third_party/blink/renderer/core/layout/svg/layout_svg_shape.cc
Line 547, Patchset 22 (Latest):
if (is_visual_overflow || hit_rules.can_hit_bounding_box) {Fredrik Söderquist . unresolved
So this is now always "imprecise" - could we then return early if it doesn't intersect (rather than running the "actual" intersection test as well)?
Line 548, Patchset 22 (Latest):
gfx::RectF bounds = ObjectBoundingBox();Fredrik Söderquist . unresolved
This would need to be the decorated bbox for the `is_visual_overflow` case, no?
File third_party/blink/renderer/core/layout/svg/svg_layout_support.h
Line 73, Patchset 22 (Latest):
static gfx::RectF ComputeVisualOverflowRectIncludingFilters(Fredrik Söderquist . unresolved
Not sure why we'd want this here. Couldn't it go in `SVGContentContainer`?
File third_party/blink/renderer/core/layout/svg/svg_layout_support.cc
Line 115, Patchset 22 (Latest):
if (!filter_reference_box.size().IsZero()) {Fredrik Söderquist . unresolved
This check is not very obvious to me, could you document why it is needed here?
Related details
Attention is currently required from:
- Philip Rogers
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Rogers (Gerrit)
May 25, 2026, 10:14:04 PM (5 hours ago) May 25
to Fredrik Söderquist, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, fmalit...@chromium.org, kouhe...@chromium.org, pdr+svgw...@chromium.org, zol...@webkit.org
Attention needed from Fredrik Söderquist
Philip Rogers voted and added 6 comments![]( "Open in Gerrit")
Votes added by Philip Rogers
| Commit-Queue | +1 |
6 comments
Patchset-level comments
File third_party/blink/renderer/core/layout/svg/layout_svg_container.h
Line 86, Patchset 22:
gfx::RectF ComputeVisualOverflowRectIncludingFilters() const {Fredrik Söderquist . resolved
```suggestion
gfx::RectF ComputeContentVisualOverflowRectIncludingFilters() const {
```
?
Philip Rogers
Done
File third_party/blink/renderer/core/layout/svg/layout_svg_shape.cc
Line 547, Patchset 22:
if (is_visual_overflow || hit_rules.can_hit_bounding_box) {Fredrik Söderquist . resolved
So this is now always "imprecise" - could we then return early if it doesn't intersect (rather than running the "actual" intersection test as well)?
Philip Rogers
Done
This would need to be the decorated bbox for the `is_visual_overflow` case, no?
Philip Rogers
Done, which made some other tidying possible.
File third_party/blink/renderer/core/layout/svg/svg_layout_support.h
Line 73, Patchset 22:
static gfx::RectF ComputeVisualOverflowRectIncludingFilters(Fredrik Söderquist . resolved
Not sure why we'd want this here. Couldn't it go in `SVGContentContainer`?
Philip Rogers
Done
File third_party/blink/renderer/core/layout/svg/svg_layout_support.cc
This check is not very obvious to me, could you document why it is needed here?
Philip Rogers
This was introduced by https://crrev.com/c/1364152 and the instance here was copied from LayoutBoxModelObject::ApplyFiltersToRect. I don't think either of these codepaths need this check. I'll go ahead and remove it here.
Related details
Attention is currently required from:
- Fredrik Söderquist
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages