| Summary: | [Filters] FilterEffect::calculatePrimitiveSubregion() and calculateImageRect() should take a span of the input image rects | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Said Abou-Hallawa <sabouhallawa> | ||||
| Component: | Layout and Rendering | Assignee: | Said Abou-Hallawa <sabouhallawa> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | bfulgham, darin, dino, ews-watchlist, kondapallykalyan, simon.fraser, webkit-bug-importer, zalan | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 243816 | ||||||
| Attachments: |
|
||||||
Pull request: https://github.com/WebKit/WebKit/pull/6706 Created attachment 463641 [details]
Patch
Comment on attachment 463641 [details]
Patch
Functions like these should take Span<const X> rather than const Vector<X>& for greater flexibility if possible
A new PR was uploaded to https://github.com/WebKit/WebKit/pull/6706 Committed 256983@main (d4236a4ea1de): <https://commits.webkit.org/256983@main> Reviewed commits have been landed. Closing PR #6706 and removing active labels. 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. Oh, looks like this already landed a while ago. Well consider my comments ideas of how to refine. 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. |
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.