Bug 74392 – [SVG] Use element disappears after scripted change

• Component: SVG
• Status: RESOLVED FIXED

[Branimir Lambov]

Created attachment 118982 [details]
Testcase

The attached SVG file contains two references, one plain and one having a filter, to a rect element which is updated by script after the initial paint.

The update causes the rectangle under the filter to disappear. Forcing a refresh by zooming in/out causes the rectangle to appear again.

Just ran a bisect:


WEBKIT CHANGELOG URL:
http://trac.webkit.org/log/trunk/?rev=87341&stop_rev=87266&verbose=on
CHANGELOG URL:
http://build.chromium.org/f/chromium/perf/dashboard/ui/changelog.html?url=/trunk/src&range=86624:86779
Built at revision:
http://src.chromium.org/viewvc/chrome?view=rev&revision=86779

http://trac.webkit.org/changeset/87302/trunk looks suspicious.

[Levi Weintraub]

(In reply to comment #1)
> http://trac.webkit.org/changeset/87302/trunk looks suspicious.

87302 is literally all refactoring and should have nothing to do with this issue.

More suspect to me appears to be http://trac.webkit.org/changeset/87277/trunk or even more likely http://trac.webkit.org/changeset/87310/trunk (which deals specifically with SVG filters)

[Nikolas Zimmermann]

It's <use> related, if you change the testcase to:
<rect filter="url(#simple)" x="50" id="rect" width="30" y="10" height="30" fill="red" />
it will work as expected. Tracing it now.

[Nikolas Zimmermann]

Some more findings:
...
<rect filter="url(#simple)" x="50" id="rect" width="30" y="10" height="30" fill="red" />
</defs>
<g>
<use xlink:href="#rect"/>
works fine.

....
<rect x="50" id="rect" width="30" y="10" height="30" fill="red" />
</defs>
<g>
<use filter="url(#simple)" xlink:href="#rect"/>
doesn't work.

Debugging session:
I agree with Levi, that  87310 may be the root of the problem.
Breaking on SVGRenderSupport.cpp:117 (what Rob added) yields:

Breakpoint 4, WebCore::SVGRenderSupport::prepareToRenderSVGContent (object=0x108162b68, paintInfo=@0x7fff5fbf7828) at SVGRenderSupport.cpp:117
117	            return false;
(gdb) bt
#0  WebCore::SVGRenderSupport::prepareToRenderSVGContent (object=0x108162b68, paintInfo=@0x7fff5fbf7828) at SVGRenderSupport.cpp:117
#1  0x000000010263cd51 in WebCore::RenderSVGContainer::paint (this=0x108162b68, paintInfo=@0x7fff5fbf7948) at RenderSVGContainer.cpp:116
#2  0x000000010263cdf8 in WebCore::RenderSVGContainer::paint (this=0x106b928b8, paintInfo=@0x7fff5fbf7a68) at RenderSVGContainer.cpp:121
#3  0x000000010263cdf8 in WebCore::RenderSVGContainer::paint (this=0x106b9ef88, paintInfo=@0x7fff5fbf7b18) at RenderSVGContainer.cpp:121
#4  0x00000001025348b4 in WebCore::RenderBox::paint (this=0x106b99cf8, paintInfo=@0x7fff5fbf7c90, paintOffset=@0x7fff5fbf7c10) at /Users/nzimmermann/Coding/WebKit/Source/WebCore/rendering/RenderBox.cpp:862
#5  0x000000010265305e in WebCore::RenderSVGRoot::paintReplaced (this=0x106b99cf8, paintInfo=@0x7fff5fbf8148, adjustedPaintOffset=@0x7fff5fbf7e70) at RenderSVGRoot.cpp:264
#6  0x000000010261f1e7 in WebCore::RenderReplaced::paint (this=0x106b99cf8, paintInfo=@0x7fff5fbf8148, paintOffset=@0x7fff5fbf8238) at /Users/nzimmermann/Coding/WebKit/Source/WebCore/rendering/RenderReplaced.cpp:152
#7  0x00000001025bd658 in WebCore::RenderLayer::paintLayerContents (this=0x106b8e738, rootLayer=0x106b3c8f8, context=0x7fff5fbf9390, paintDirtyRect=@0x7fff5fbf9348, paintBehavior=0, paintingRoot=0x0, region=0x0, overlapTestRequests=0x7fff5fbf90b0, paintFlags=224) at /Users/nzimmermann/Coding/WebKit/Source/WebCore/rendering/RenderLayer.cpp:2917
#8  0x00000001025bccd6 in WebCore::RenderLayer::paintLayerContentsAndReflection (this=0x106b8e738, rootLayer=0x106b3c8f8, context=0x7fff5fbf9390, paintDirtyRect=@0x7fff5fbf9348, paintBehavior=0, paintingRoot=0x0, region=0x0, overlapTestRequests=0x7fff5fbf90b0, paintFlags=224) at /Users/nzimmermann/Coding/WebKit/Source/WebCore/rendering/RenderLayer.cpp:2818
#9  0x00000001025bc3f5 in WebCore::RenderLayer::paintLayer (this=0x106b8e738, rootLayer=0x106b3c8f8, context=0x7fff5fbf9390, paintDirtyRect=@0x7fff5fbf9348, paintBehavior=0, paintingRoot=0x0, region=0x0, overlapTestRequests=0x7fff5fbf90b0, paintFlags=224) at /Users/nzimmermann/Coding/WebKit/Source/WebCore/rendering/RenderLayer.cpp:2799
#10 0x00000001025be65f in WebCore::RenderLayer::paintList (this=0x106b3c8f8, list=0x106b5b7a0, rootLayer=0x106b3c8f8, context=0x7fff5fbf9390, paintDirtyRect=@0x7fff5fbf9348, paintBehavior=0, paintingRoot=0x0, region=0x0, overlapTestRequests=0x7fff5fbf90b0, paintFlags=224) at /Users/nzimmermann/Coding/WebKit/Source/WebCore/rendering/RenderLayer.cpp:2982
#11 0x00000001025bd8a1 in WebCore::RenderLayer::paintLayerContents (this=0x106b3c8f8, rootLayer=0x106b3c8f8, context=0x7fff5fbf9390, paintDirtyRect=@0x7fff5fbf9348, paintBehavior=0, paintingRoot=0x0, region=0x0, overlapTestRequests=0x7fff5fbf90b0, paintFlags=224) at /Users/nzimmermann/Coding/WebKit/Source/WebCore/rendering/RenderLayer.cpp:2938
#12 0x00000001025bccd6 in WebCore::RenderLayer::paintLayerContentsAndReflection (this=0x106b3c8f8, rootLayer=0x106b3c8f8, context=0x7fff5fbf9390, paintDirtyRect=@0x7fff5fbf9348, paintBehavior=0, paintingRoot=0x0, region=0x0, overlapTestRequests=0x7fff5fbf90b0, paintFlags=0) at /Users/nzimmermann/Coding/WebKit/Source/WebCore/rendering/RenderLayer.cpp:2818
#13 0x00000001025bc3f5 in WebCore::RenderLayer::paintLayer (this=0x106b3c8f8, rootLayer=0x106b3c8f8, context=0x7fff5fbf9390, paintDirtyRect=@0x7fff5fbf9348, paintBehavior=0, paintingRoot=0x0, region=0x0, overlapTestRequests=0x7fff5fbf90b0, paintFlags=0) at /Users/nzimmermann/Coding/WebKit/Source/WebCore/rendering/RenderLayer.cpp:2799
#14 0x00000001025bbd4c in WebCore::RenderLayer::paint (this=0x106b3c8f8, context=0x7fff5fbf9390, damageRect=@0x7fff5fbf9348, paintBehavior=0, paintingRoot=0x0, region=0x0, paintFlags=0) at /Users/nzimmermann/Coding/WebKit/Source/WebCore/rendering/RenderLayer.cpp:2616
#15 0x0000000101a65224 in WebCore::FrameView::paintContents (this=0x108129400, p=0x7fff5fbf9390, rect=@0x7fff5fbf9348) at /Users/nzimmermann/Coding/WebKit/Source/WebCore/page/FrameView.cpp:2884
#16 0x0000000100d8aa7d in -[WebFrame(WebInternal) _drawRect:contentsOnly:] (self=0x10810ce60, _cmd=0x7fff933c46d6, rect={origin = {x = 50, y = 10}, size = {width = 36, height = 36}}, contentsOnly=1 '\001') at /Users/nzimmermann/Coding/WebKit/Source/WebKit/mac/WebView/WebFrame.mm:573
#17 0x0000000100ddfe89 in -[WebHTMLView drawSingleRect:] (self=0x108128680, _cmd=0x7fff933e88ba, rect={origin = {x = 50, y = 10}, size = {width = 36, height = 36}}) at /Users/nzimmermann/Coding/WebKit/Source/WebKit/mac/WebView/WebHTMLView.mm:3236
#18 0x0000000100de0571 in -[WebHTMLView drawRect:] (self=0x108128680, _cmd=0x7fff8e770370, rect={origin = {x = 50, y = 10}, size = {width = 36, height = 36}}) at /Users/nzimmermann/Coding/WebKit/Source/WebKit/mac/WebView/WebHTMLView.mm:3299
#19 0x00007fff8dead0cc in -[NSView _drawRect:clip:] ()
#20 0x00007fff8deaad7e in -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] ()
#21 0x0000000100dd3bea in -[WebHTMLView(WebPrivate) _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] (self=0x108128680, _cmd=0x7fff8e7a3996, rect={origin = {x = 0, y = 0}, size = {width = 800, height = 600}}, isVisibleRect=1 '\001', visibleView=0x106d15730, topView=0 '\0') at /Users/nzimmermann/Coding/WebKit/Source/WebKit/mac/WebView/WebHTMLView.mm:1313
#22 0x00007fff8deab86f in -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] ()
#23 0x00007fff8deab86f in -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] ()
#24 0x00007fff8deab86f in -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] ()
#25 0x00007fff8deab86f in -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] ()
#26 0x00007fff8deab86f in -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] ()
#27 0x00007fff8deab86f in -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] ()
#28 0x00007fff8dff9454 in -[NSNextStepFrame _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] ()
#29 0x00007fff8dea4ec9 in -[NSView _displayRectIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:] ()
#30 0x00007fff8de9d93e in -[NSView displayIfNeeded] ()
#31 0x000000010004aa7b in createBitmapContextFromWebView (onscreen=false, incrementalRepaint=false, sweepHorizontally=false, drawSelectionRect=false) at /Users/nzimmermann/Coding/WebKit/Tools/DumpRenderTree/mac/PixelDumpSupportMac.mm:152

(gdb) p object->showRenderTreeForThis()
RenderView 0x106b3c698                 	#document	0x10701a000
  RenderSVGRoot 0x106b99cf8            	svg	0x106b6f820 STYLE=width:300px;height:400px
    RenderSVGHiddenContainer 0x106b8b128	defs	0x106b8aa50
      RenderSVGResourceFilter 0x106b887f8	filter	0x106b8b260
        RenderSVGResourceFilterPrimitive 0x106b9d958	feOffset	0x106b88950
      RenderSVGRect 0x106b9e788        	rect	0x106b9db50
    RenderSVGContainer 0x106b9ef88     	g	0x106b9e8f0
      RenderSVGContainer 0x106b928b8   	use	0x106b9ce70
*       RenderSVGContainer 0x108162b68 	g	0x106b92a30
          RenderSVGRect 0x108158298    	rect	0x108165ef0

The SVG Shadow tree Root container (marked with a star *) is the first element in the <use> shadow tree.
As the break point got hit, it means the <g> has a filter resource associated:

(gdb) p *svgStyle->resources->m_data->m_ptr
$15 = {
....
  filter = {
    m_impl = {
      m_ptr = 0x106b284e0
    }
  }, 

Indeed the SVGRenderStyle of the shadow tree root container <g>, claims it had a <filter> - but its first child is the one carrying it - something weird is going on. I'll dig in deeper :-)

[Nikolas Zimmermann]

(In reply to comment #4)

> The SVG Shadow tree Root container (marked with a star *) is the first element in the <use> shadow tree.
> Indeed the SVGRenderStyle of the shadow tree root container <g>, claims it had a <filter> - but its first child is the one carrying it - something weird is going on. I'll dig in deeper :-)
Wrong conclusion, I was still testing with <use filter="...">. So its completely correct that the <use> SVGRenderStyle claims it has a filter.

[Nikolas Zimmermann]

Jeez, took me two days of debugging to find the culprit:
SVGUseElement::didRecalcStyle:

     shadowRoot->updateFromElement();
-
-    if (!needsStyleUpdate)
-        return;
-
     shadowRoot->updateStyle(change);

This will fix the underlying problem, will turn this into a patch with this test case tomorrow.

[Levi Weintraub]

Created attachment 125891 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 125891 [details]
Patch

LGTM.

[WebKit Review Bot]

Comment on attachment 125891 [details]
Patch

Clearing flags on attachment: 125891

Committed r107005: <http://trac.webkit.org/changeset/107005>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Nikolas Zimmermann]

(In reply to comment #8)
> (From update of attachment 125891 [details])
> LGTM.

The test is unfortunately not correct - that's why I held back uploading a patch.
We have to use the repaint.js harness to make it reliable, I'll make sure to fix it soon.

[Levi Weintraub]

(In reply to comment #11)
> The test is unfortunately not correct - that's why I held back uploading a patch.
> We have to use the repaint.js harness to make it reliable, I'll make sure to fix it soon.

While there may be reason the test could benefit from repaint.js, I verified that this test fails without the patch, and passes with it.