Summary: | Wasted vector capacity in FEColorMatrix and filters | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||
Component: | SVG | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | changseok, dino, esprehn+autocc, ews-watchlist, fmalita, glenn, gyuyoung.kim, kondapallykalyan, pdr, sabouhallawa, schenney, sergio, simon.fraser, webkit-bug-importer, zimmermann | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2021-04-03 20:55:57 PDT
Created attachment 425123 [details]
Patch
*** Bug 224168 has been marked as a duplicate of this bug. *** Comment on attachment 425123 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425123&action=review > Source/WebCore/ChangeLog:8 > + Shrink the vector of input effects in FilterEffect. I think we can reserveInitialCapacity the inputEffects for all FilterEffects instead of shrinking them later. Also do not we care about shrinking CSSFilter::m_effects? > Source/WebCore/rendering/CSSFilter.cpp:299 > effect->inputEffects().append(WTFMove(previousEffect)); I think this can be written like this: effect->inputEffects().reserveInitialCapacity(1); effect->inputEffects().uncheckedAppend(WTFMove(previousEffect)); Or effect->inputEffects() = { WTFMove(previousEffect) }; And we do not need to call didFinishBuilding(). > Source/WebCore/rendering/CSSFilter.cpp:303 > + effect->didFinishBuilding(); Why do we need to call didFinishBuilding() for the case filterOperation.type() == FilterOperation::REFERENCE? CSSFilter::buildReferenceFilter() already calls didFinishBuilding() for all the effects including the last one which it returns and which is stored in "effect". > Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp:108 > + effect->didFinishBuilding(); Why don't we let the SVG FilterEffect elements reserveInitialCapacity the inputEffects before appending to it instead of shrinking it here? I think all of them, except SVGFEMergeElement, have a fixed number of inputEffects. For example in SVGFEGaussianBlurElement::build() we can do this: auto effect = FEGaussianBlur::create(filter, stdDeviationX(), stdDeviationY(), edgeMode()); effect->inputEffects().reserveInitialCapacity(1); effect->inputEffects().uncheckedAppend(WTFMove(input1)); Or auto effect = FEGaussianBlur::create(filter, stdDeviationX(), stdDeviationY(), edgeMode()); effect->inputEffects() = { WTFMove(input1) }; > Source/WebCore/svg/SVGFEColorMatrixElement.cpp:141 > filterValues = values(); > + filterValues.shrinkToFit(); Can this be written like this? filterValues.reserveInitialCapacity(values().size()); filterValues.appendVector(values()); Created attachment 425176 [details]
Patch
Committed r275471: <https://commits.webkit.org/r275471> All reviewed patches have been landed. Closing bug and clearing flags on attachment 425176 [details]. |