[webkit-reviews] review granted: [Bug 232840] [GPU Process] [Filters 18/23] Remove the result FilterImage from FilterEffect : [Attachment 448302] 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 232840: [GPU Process] [Filters 18/23] Remove the result FilterImage from
FilterEffect
https://bugs.webkit.org/show_bug.cgi?id=232840

Attachment 448302: Patch

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




--- Comment #4 from Darin Adler <darin at apple.com> ---
Comment on attachment 448302
  --> https://bugs.webkit.org/attachment.cgi?id=448302
Patch

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

> Source/WebCore/platform/graphics/filters/FilterResults.cpp:40
> +    for (auto input : inputs) {

Using "auto" here means we copy each Ref<FilterImage>. I think that instead we
should use "auto&" and save a bit of reference count churn.

> Source/WebCore/platform/graphics/filters/FilterResults.cpp:46
> +	   auto addResult = m_resultReferences.ensure(input, [] {
> +	       return FilterEffectSet();
> +	   });
> +
> +	   auto& references = addResult.iterator->value;
> +	   references.add(effect);

This can just use add, because FilterEffectSet() is a null pointer, very cheap
to construct and destruct. Can write this as a one-liner.

    m_resultReferences.add(input, { }).iterator->value.add(effect);

> Source/WebCore/svg/graphics/filters/SVGFilterBuilder.h:52
> +    void appendEffectToEffectRenderer(FilterEffect*, RenderObject*);

This should take a FilterEffect& instead of a FilterEffect*.