[webkit-reviews] review granted: [Bug 248181] [Filters] FilterEffect::calculatePrimitiveSubregion() and calculateImageRect() should take a span of the input image rects : [Attachment 463641] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 248181: [Filters] FilterEffect::calculatePrimitiveSubregion() and
calculateImageRect() should take a span of the input image rects
https://bugs.webkit.org/show_bug.cgi?id=248181

Attachment 463641: Patch

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




--- Comment #7 from Darin Adler <darin at apple.com> ---
Comment on attachment 463641
  --> https://bugs.webkit.org/attachment.cgi?id=463641
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.