Bug 248181 – [Filters] FilterEffect::calculatePrimitiveSubregion() and calculateImageRect() should take a span of the input image rects

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Said Abou-Hallawa]

Instead of passing a FilterImageVector to these functions we should pass a Vector<FloatRect> since they care about the FilterImage::imageRect() only.
    
This work is towards supporting CoreGraphics filters since no FilterImage will be used while applying the FilterEffects.

[Radar WebKit Bug Importer]

<rdar://problem/102586592>

[Said Abou-Hallawa]

Pull request: https://github.com/WebKit/WebKit/pull/6706

[Said Abou-Hallawa]

Created attachment 463641 [details]
Patch

[Darin Adler]

Comment on attachment 463641 [details]
Patch

Functions like these should take Span<const X> rather than const Vector<X>& for greater flexibility if possible

[Said Abou-Hallawa]

A new PR was uploaded to https://github.com/WebKit/WebKit/pull/6706

[EWS]

Committed 256983@main (d4236a4ea1de): <https://commits.webkit.org/256983@main>

Reviewed commits have been landed. Closing PR #6706 and removing active labels.

[Darin Adler]

Comment on attachment 463641 [details]
Patch

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

> COMMIT_MESSAGE:8
> +Instead of passing a FilterImageVector to these functions we should pass a
> +Vector<FloatRect> since they care about the FilterImage::imageRect() only.

I suggest we pass Span<const FloatRect> instead. Still would have a Vector<FloatRect> locally at the call site. Just a good habit to pass Span any time so we don’t have to construct a vector. Lets us use FixedVector or an array or even a single element.

> Source/WebCore/platform/graphics/filters/FilterEffect.cpp:61
> +    Vector<FloatRect> inputPrimitiveSubregions;
> +    inputPrimitiveSubregions.reserveInitialCapacity(inputs.size());
> +
> +    for (auto& input : inputs)
> +        inputPrimitiveSubregions.uncheckedAppend(input->primitiveSubregion());
> +
> +    return inputPrimitiveSubregions;

This should use map.

> Source/WebCore/platform/graphics/filters/FilterEffect.cpp:99
> -    FloatRect imageRect;
> +    Vector<FloatRect> inputImageRects;
> +    inputImageRects.reserveInitialCapacity(inputs.size());
> +
>      for (auto& input : inputs)
> -        imageRect.unite(input->imageRect());
> +        inputImageRects.uncheckedAppend(input->imageRect());
> +
> +    return inputImageRects;

This should use map.

[Darin Adler]

Oh, looks like this already landed a while ago. Well consider my comments ideas of how to refine.

[Said Abou-Hallawa]

Comment on attachment 463641 [details]
Patch

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

>> Source/WebCore/platform/graphics/filters/FilterEffect.cpp:61
>> +    return inputPrimitiveSubregions;
> 
> This should use map.

This is already done in 256983@main.

>> Source/WebCore/platform/graphics/filters/FilterEffect.cpp:99
>> +    return inputImageRects;
> 
> This should use map.

This is already done in 256983@main.