[webkit-reviews] review denied: [Bug 68457] Style changes applied to filtered elements do not invalidate the element : [Attachment 108717] patch!

[bugzilla-daemon at webkit.org]

Dirk Schulze <krit at webkit.org> has denied  review:
Bug 68457: Style changes applied to filtered elements do not invalidate the
element
https://bugs.webkit.org/show_bug.cgi?id=68457

Attachment 108717: patch!
https://bugs.webkit.org/attachment.cgi?id=108717&action=review

------- Additional Comments from Dirk Schulze <krit at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=108717&action=review


r- because I'd like an even more reduced test case. Also have a question to
your patch itself.

Btw. Don't you have commit rights? Why do you set cq?

> LayoutTests/svg/filters/animate-fill.svg:6
> +    <filter id="filt">
> +	   <feGaussianBlur stdDeviation="3" />
> +    </filter>

Shouldn't we still use the filter code path if we don't have an effect at all?
If not, even an offset effect (feOffset) without attributes would be enough.

> LayoutTests/svg/filters/animate-fill.svg:9
> +    <clipPath id="clip">
> +	   <rect width="50px" height="50px" x="25" y="25" />
> +    </clipPath>

Is it just reproducible with a clipPath? If you just used it to have a solid
filled rect, the suggestion above should help.

> Source/WebCore/rendering/svg/SVGResourcesCache.cpp:152
> +    SVGResources* resources =
SVGResourcesCache::cachedResourcesForRenderObject(renderer);
> +    if (!resources)
> +	   return;
> +
> +    resources->removeClientFromCache(renderer);

Do we still need to invalidate the client for clipPath? Is there a CSS property
that could influence any visual affect on clipPath? IIRC SVGResourceCache is
used for PaintServers as well (pattern and gradient), can you reproduce the
problem for these resources as well?