Bug 207451
| Summary: | RenderSVGShape invalidates all its resources when it needs layout, but is that necessary? | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> |
| Component: | SVG | Assignee: | Nobody <webkit-unassigned> |
| Status: | NEW | ||
| Severity: | Normal | CC: | ahmad.saleem792, sabouhallawa, simon.fraser, zimmermann |
| Priority: | P2 | ||
| Version: | Safari Technology Preview | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
Simon Fraser (smfr)
The MotionMark suits test runs by setting transforms on shapes, which does a layout. This layout invalidates all the RenderSVGShape's resources:
RenderSVGShape::layout():
...
// Invalidate all resources of this client if our layout changed.
if (everHadLayout() && selfNeedsLayout())
SVGResourcesCache::clientLayoutChanged(*this);
but that doesn't seem necessary if the layout doesn't make that resource invalid (e.g. the size doesn't change).
Invalidating resources causes us to rebuild the gradients on every frame, which means making a new platform gradient.
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Ahmad Saleem
We still have this code:
In LBSE:
https://searchfox.org/wubkat/source/Source/WebCore/rendering/svg/RenderSVGShape.cpp#160
// Invalidate all resources of this client if our layout changed.
if (everHadLayout() && selfNeedsLayout())
SVGResourcesCache::clientLayoutChanged(*this);
https://searchfox.org/wubkat/source/Source/WebCore/rendering/svg/LegacyRenderSVGShape.cpp#146
// Invalidate all resources of this client if our layout changed.
if (everHadLayout() && selfNeedsLayout())
SVGResourcesCache::clientLayoutChanged(*this);
________
Just wanted to share updated bit as of today.