Bug 116469 - Invalidate SVG filter results in SVGResourcesCache::clientLayoutChanged()
Summary: Invalidate SVG filter results in SVGResourcesCache::clientLayoutChanged()
Status: RESOLVED CONFIGURATION CHANGED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: BlinkMergeCandidate
Depends on:
Blocks:
 
Reported: 2013-05-20 15:42 PDT by Ryosuke Niwa
Modified: 2022-09-14 21:28 PDT (History)
10 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2013-05-20 15:42:07 PDT
We should probably merge
https://chromium.googlesource.com/chromium/blink/+/25d72ebd50bec9a4caa426e735e5ee7292c4da69

Invalidate SVG filter results in SVGResourcesCache::clientLayoutChanged().

This is a regression introduced by http://trac.webkit.org/changeset/103539.

For the SVG tree, filter result invalidation is handled by
RenderSVGResource::markForLayoutAndParentResourceInvalidation. But non-SVG content (such as
a <foreignObject> HTML subtree) does not call markForLayoutAndParentResourceInvalidation and
instead uses the regular setNeedsLayout() invalidation path.

Hence, changes in the HTML subtree do not invalidate cached results for filters applied on
ancestor SVG elements. In order to cover this case, clientLayoutChanged() needs to invalidate
filter results even if the SVG element itself does not need a re-layout. Note that prior to
http://trac.webkit.org/changeset/103539 the method used to do exactly that, but the branch was
removed on the assumption that markForLayoutAndParentResourceInvalidation() is already taking care
of it. Obviously, the assumption doesn’t hold for non-SVG content.
Comment 1 Ahmad Saleem 2022-08-21 05:03:47 PDT
The line of code is following in Webkit GitHub source as of today:

https://github.com/WebKit/WebKit/blob/cdb0c4a68794035df705609ca0ec8c7fb373091b/Source/WebCore/rendering/svg/SVGResourcesCache.cpp#L100

________

if (renderer.selfNeedsLayout() && hasResourcesRequiringRemovalOnClientLayoutChange(*resources))

________

Chromium / Blink patch changed it to following:

if (object->selfNeedsLayout() || resources->filter())

_____

Is this needed anymore? Thanks!
Comment 2 Ahmad Saleem 2022-09-14 12:23:02 PDT
This commit introduced:

https://github.com/WebKit/WebKit/commit/ce878379210ced398d994217e26519b433761149

hasResourcesRequiringRemovalOnClientLayoutChange

and it has "resources.filter()", which was added by Chromium / Blink patch.

I think this is not needed anymore. Please appreciate if someone can confirm. Thanks!