Resizing a SVG document containing an absolute sized object where a resources is applied leads to pixelation (for masks, atm.) The resource is not being invalidated. When using a target object with relative lengths everything works fine.
Created attachment 64229 [details] Patch
Comment on attachment 64229 [details] Patch > void SVGRenderSupport::layoutChildren(RenderObject* start, bool selfNeedsLayout) > { > bool layoutSizeChanged = svgRootTreeObject(start)->isLayoutSizeChanged(); > + HashSet<RenderObject*> notlayoutedObjects; > > for (RenderObject* child = start->firstChild(); child; child = child->nextSibling()) { > bool needsLayout = selfNeedsLayout; > @@ -252,11 +263,25 @@ void SVGRenderSupport::layoutChildren(Re > if (needsLayout) { > child->setNeedsLayout(true, false); > child->layout(); > - } else > - child->layoutIfNeeded(); > + } else { > + if (child->needsLayout()) > + child->layout(); > + else if (layoutSizeChanged) > + notlayoutedObjects.add(child); > + } > > ASSERT(!child->needsLayout()); > } > + > + if (!layoutSizeChanged) { > + ASSERT(notlayoutedObjects.isEmpty()); > + return; > + } > + > + // If the layout size changed, invalidate all resources of all children that didn't go through the layout() code path. > + HashSet<RenderObject*>::iterator end = notlayoutedObjects.end(); > + for (HashSet<RenderObject*>::iterator it = notlayoutedObjects.begin(); it != end; ++it) > + invalidateResourcesOfChildren(*it); > } > I'm not sure about the last loop here. You store the objects, because they don't need a relayout. But when you call invalidateResourcesOfChildren, the resources want the objects to relayout at least the boundaries. Seems unneccessary to me, because we already know, that they don't need a relayout.
(In reply to comment #2) > > + // If the layout size changed, invalidate all resources of all children that didn't go through the layout() code path. > > + HashSet<RenderObject*>::iterator end = notlayoutedObjects.end(); > > + for (HashSet<RenderObject*>::iterator it = notlayoutedObjects.begin(); it != end; ++it) > > + invalidateResourcesOfChildren(*it); > > } > > > > I'm not sure about the last loop here. You store the objects, because they don't need a relayout. But when you call invalidateResourcesOfChildren, the resources want the objects to relayout at least the boundaries. Seems unneccessary to me, because we already know, that they don't need a relayout. You mixed up something. They call setNeedsBoundariesUpdate(), but not do any relayouting. It just marks for a boundaries update, next time it relayouts. But this relayout doesn't happen, so there's no problem.
(In reply to comment #3) > (In reply to comment #2) > > > + // If the layout size changed, invalidate all resources of all children that didn't go through the layout() code path. > > > + HashSet<RenderObject*>::iterator end = notlayoutedObjects.end(); > > > + for (HashSet<RenderObject*>::iterator it = notlayoutedObjects.begin(); it != end; ++it) > > > + invalidateResourcesOfChildren(*it); > > > } > > > > > > > I'm not sure about the last loop here. You store the objects, because they don't need a relayout. But when you call invalidateResourcesOfChildren, the resources want the objects to relayout at least the boundaries. Seems unneccessary to me, because we already know, that they don't need a relayout. > > You mixed up something. They call setNeedsBoundariesUpdate(), but not do any relayouting. > It just marks for a boundaries update, next time it relayouts. But this relayout doesn't happen, so there's no problem. Yes, I know it's just a flag. This makes still no sense. Why should we mark something for realyouting, but never relayout it? And even if we call layout sometime later, we have to update the boundaries, independent why we call layout. An expensive operation for RenderPath.
Created attachment 64313 [details] Patch v2 New approach after discussing with Dirk.
Comment on attachment 64313 [details] Patch v2 LGTM. r=me
Landed in r65310.