Bug 235338 - filterRegion and outsets of referenced SVG filter are calculated incorrectly
Summary: filterRegion and outsets of referenced SVG filter are calculated incorrectly
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
Keywords: InRadar
Depends on:
Reported: 2022-01-18 15:37 PST by Said Abou-Hallawa
Modified: 2022-01-19 00:39 PST (History)
17 users (show)

See Also:

test case (2.84 KB, text/html)
2022-01-18 15:37 PST, Said Abou-Hallawa
no flags Details
Patch (21.07 KB, patch)
2022-01-18 15:49 PST, Said Abou-Hallawa
darin: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (21.38 KB, patch)
2022-01-18 20:43 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Said Abou-Hallawa 2022-01-18 15:37:31 PST
Created attachment 449435 [details]
test case

Open the attached test case.

Expected: There should be a drop-shadow effect around the first four boxes. There should be kind of a black border around the fifth box.
Result: None of the five boxes has any effect.

The filterRegion of the referenced is currently set to the targetBoundingBox. Given the geometry of the SVGFilterElement and given the targetBoundingBox, the filterRegion should be calculated using SVGLengthContext::resolveRectangle().
The outsets of the SVGFilter is calculated by getting the outsets of the lastEffect. The outsets of the SVGFilter should be the outsets of the all the effects which are involved in producing the lastEffect.
Comment 1 Said Abou-Hallawa 2022-01-18 15:49:39 PST
Created attachment 449437 [details]
Comment 2 Darin Adler 2022-01-18 16:00:27 PST
Comment on attachment 449437 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=449437&action=review

> Source/WebCore/ChangeLog:3
> +        filterRegion and outsets of referenced SVG filter are calculated correctly

Should this title say "incorrectly"? I don’t understand

> Source/WebCore/platform/graphics/filters/FEOffset.cpp:78
> +IntOutsets FEOffset::outsets(const Filter& filter) const
> +{
> +    auto offset = expandedIntSize(filter.resolvedSize({ m_dx, m_dy }));
> +    
> +    IntOutsets outsets;
> +    if (offset.height() < 0)
> +        outsets.setTop(-offset.height());
> +    else
> +        outsets.setBottom(offset.height());
> +
> +    if (offset.width() < 0)
> +        outsets.setLeft(-offset.width());
> +    else
> +        outsets.setRight(offset.width());
> +    return outsets;
> +}

Paragraphing here is a little strange. Not sure why we have the second blank line where we do. Maybe I’d add more lines or remove that one.

> Source/WebCore/svg/graphics/filters/SVGFilter.cpp:168
> +    for (auto& term : m_expression) {
> +        auto& effect = term.effect;
> +        outsets += effect->outsets(*this);
> +    }

Local variable isn’t really helping here. I would write it tighter like this:

    IntOutsets outsets;
    for (auto& term : m_expression)
        outsets += term.effect->outsets(*this);
    return outsets;

I also don’t know why we need to assert !isEmpty. This algorithm looks like it works fine for an empty vector.
Comment 3 Said Abou-Hallawa 2022-01-18 20:43:19 PST
Created attachment 449462 [details]
Comment 4 EWS 2022-01-19 00:38:19 PST
Committed r288183 (246164@main): <https://commits.webkit.org/246164@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 449462 [details].
Comment 5 Radar WebKit Bug Importer 2022-01-19 00:39:20 PST