[Webkit-unassigned] [Bug 87572] New: [Master] Redesign SVG resources

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=87572

           Summary: [Master] Redesign SVG resources
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: Unspecified
        OS/Version: Unspecified
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: SVG
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: zimmermann at kde.org
                CC: rwlbuis at gmail.com, zimmermann at kde.org,
                    krit at webkit.org, zherczeg at webkit.org, reni at webkit.org,
                    pdr at google.com, schenney at chromium.org,
                    fmalita at chromium.org
        Depends on: 86941,87373


This bug tracks the redesign of SVG resources (linear/radialGradient, mask, clipPath, filter, marker).
In worst cases we need a 2-pass layout now from RenderSVGRoot, to avoid leaving the render tree with some objects still needing layout after RenderSVGRoot::layout() ran.

The question came up why renderers are needed at all for SVG resources. Here's a list of reasons:
- CSS Transitions/Animations support currently only work with RenderObjects (bug 87373 contains a fix)
- Tracking of style changes (styleDidChange/styleWillChange) - this can be avoided nowadays using nonRendererRenderStyle (bug 86941 eliminates RenderSVGradientStop)

These two can easily be fixed. That would already remove the need for <linearGradient> / <radialGradient> / <stop> renderers.
But what about <pattern>? <pattern> has children which need to be rendered at some point (indirectly).

<svg>
  <defs>
    <pattern id="somePattern">
      <rect/>
    </pattern>
   </defs>
  <circle fill="url(#somePattern)"/>
</svg>

Once the <circle> paints, it'll look up the <pattern> fill resource, and apply it to the current rendering context. For that to work we need a renderer for the <rect> that we can paint into an ImageBuffer.
In order to create renderers for the <rect> child of the <pattern>, the <pattern> needs a renderer as well: a RenderSVGHiddenContainer, which is never painted directly, nor participates in layout.
In ToT the design is similar: RenderSVGResourcePattern inherits from both RenderSVGResource (abstract base class) and RenderSVGResourceContainer, which itself inherits from RenderSVGHiddenContainer.
(RenderSVGHiddenContainer::layout() is not a no-op right now, it actually lays out the subtree, otherwise certain assumptions wouldn't hold anymore...)
--

In an ideal world RenderSVGHiddenContainer::layout() would be a no-op. All children of the RenderSVGHiddenContainer would always remain in needsLayout=true state, until they're used.
As they're used on paint() time, once the (see example above) <circle> paints, we basically would need to layout() the RenderSVGHiddenContainer subtree, while painting.
This is dangerous in general, as the <rect> inside the pattern could for instance be used by another <use> element, like this:

<svg>
  <defs>
    <pattern id="somePattern">
      <rect id="rect"/>
    </pattern>
   </defs>
  <use xlink:href="#rect"/>
  <circle fill="url(#somePattern)"/>
</svg>

Why is that dangerous? RenderSVGRoot::layout() is invoked, which does nothing for the <defs> element and its subtree, then proceeds laying out the <use> element + cloned shadow tree.
After that the <circle> is laid out, and the whole layout() is done, while the <rect> still needs layout.
Upon painting the subtree following happens: <defs> doesn't paint anything, the <use> is painted, then the <circle>. Once the <circle> paints the <pattern> children would need to be laid out.
As a result of laying out the <rect> the <use> element would be marked as needsLayout=true again.
So after RenderSVGRoot::layout() + RenderSVGRoot::paint() we still have an element that needs to be laid out...

This example is really basic, we have lots more complex test cases in LayoutTests/svg/ that would break when designing <pattern> like this.
It's yet undecided how to fix it for real. We should collect ideas here and prototype them.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.