Bug 224169 – Wasted vector capacity in FEColorMatrix and filters

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Simon Fraser (smfr)]

Wasted vector capacit in FEColorMatrix and filters

[Simon Fraser (smfr)]

Created attachment 425123 [details]
Patch

[Simon Fraser (smfr)]

*** Bug 224168 has been marked as a duplicate of this bug. ***

[Said Abou-Hallawa]

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());

[Simon Fraser (smfr)]

Created attachment 425176 [details]
Patch

[EWS]

Committed r275471: <https://commits.webkit.org/r275471>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 425176 [details].

[Radar WebKit Bug Importer]

<rdar://problem/76247318>