SVGFEMergeNodeElement doesn't support dynamic invalidation
Created attachment 69792 [details] Patch for feMergeNode properties
Comment on attachment 69792 [details] Patch for feMergeNode properties Not sure, why you need feComposite on the tests. A test like this should be as simple as possible :-) Nevertheless, the patch itself looks great, but you should describe in the ChangeLog what you did to fix this issue. For example why did you ask the parent? why doesn't feMergeNode have a renderer, so that you have to call invalidation via the parent? And so on. As long as you don't have commit privileges, you have to upload a second patch. :-( r- because of the ChangeLog.
Created attachment 69913 [details] Patch for feMergeNode properties
Comment on attachment 69913 [details] Patch for feMergeNode properties View in context: https://bugs.webkit.org/attachment.cgi?id=69913&action=review Looks good other than that, though as you can't commit yet on your own, you probably need to upload a new patch, hence r-. > WebCore/ChangeLog:8 > + Since feMergeNode doesn't have own renderer, we have to call the invalidation via it's parent. s/it's/its/ > WebCore/svg/SVGFEMergeNodeElement.cpp:55 > + if (attrName == SVGNames::inAttr) { If there's only one attribute to consider, you may want to use early exit style here. if (attrName != SVGNames::inAttr) return;
Created attachment 69921 [details] Patch for feMergeNode properties
Comment on attachment 69921 [details] Patch for feMergeNode properties View in context: https://bugs.webkit.org/attachment.cgi?id=69921&action=review Other than that it looks good... > WebCore/svg/SVGFEMergeNodeElement.cpp:62 > + if (attrName != SVGNames::inAttr) > + return; > + if (Node* parentNode = parent()) { > + RenderObject* renderer = parentNode->renderer(); > + if (renderer && renderer->isSVGResourceFilterPrimitive()) > + RenderSVGResource::markForLayoutAndParentResourceInvalidation(renderer); > + } > +} Sorry, for not being clear. I meant: if (attrName != SVGNames::inAttr) return; Node* parentNode = parent(); if (!parentNode) return; RenderObject* renderer = parentNode->renderer(); if (!renderer || !renderer->isSVGResourceFilterPrimitive()) return; RenderSVGResource::mark...
Created attachment 69927 [details] Patch for feMergeNode properties
Comment on attachment 69927 [details] Patch for feMergeNode properties Looks good, r=me.
Comment on attachment 69927 [details] Patch for feMergeNode properties Clearing flags on attachment: 69927 Committed r69206: <http://trac.webkit.org/changeset/69206>
All reviewed patches have been landed. Closing bug.