Bug 149165

Summary: [Win] Provide means for viewing layer tree
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: Layout and RenderingAssignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, rniwa, simon.fraser
Priority: P2    
Version: WebKit Nightly Build   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Patch simon.fraser: review+

Description Brent Fulgham 2015-09-15 09:45:02 PDT
Debugging rendering problems on Windows is challenging because there is no easy way to view the layer tree of the rendered view. This makes it hard to identify duplicate layers, elements that are larger than they should be, or layers containing backing store that should not be present.

Provide a means by which MiniBrowser can dump the layer tree for user review.
Comment 1 Brent Fulgham 2015-09-15 17:43:06 PDT
Created attachment 261271 [details]
Patch
Comment 2 Simon Fraser (smfr) 2015-09-15 17:46:47 PDT
Comment on attachment 261271 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=261271&action=review

> Source/WebCore/platform/graphics/ca/win/CACFLayerTreeHost.h:80
> +    String printLayerTree() const;

layerTreeAsText()? This isn't printing anything.

> Source/WebCore/platform/graphics/ca/win/PlatformCALayerWinInternal.cpp:130
> +    // We always update the repaint count, but we do not paint it for layers that
> +    // only contain tiled backing layers. Those report their repaint counts themselves.
> +    if (!owner() || owner()->usesTiledBackingLayer())
> +        return;

Did you mean this to be here?
Comment 3 Brent Fulgham 2015-09-15 17:48:47 PDT
(In reply to comment #2)
> Comment on attachment 261271 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=261271&action=review
> 
> > Source/WebCore/platform/graphics/ca/win/CACFLayerTreeHost.h:80
> > +    String printLayerTree() const;
> 
> layerTreeAsText()? This isn't printing anything.
> 
> > Source/WebCore/platform/graphics/ca/win/PlatformCALayerWinInternal.cpp:130
> > +    // We always update the repaint count, but we do not paint it for layers that
> > +    // only contain tiled backing layers. Those report their repaint counts themselves.
> > +    if (!owner() || owner()->usesTiledBackingLayer())
> > +        return;
> 
> Did you mean this to be here?

Yes. I'll add it to the changelog.
Comment 4 Brent Fulgham 2015-09-15 18:00:43 PDT
Committed r189833: <http://trac.webkit.org/changeset/189833>
Comment 5 Ryosuke Niwa 2015-09-15 20:02:57 PDT
Looks like this patch may have broken WinCairo builds?

https://build.webkit.org/builders/WinCairo%2064-Bit%20Release/builds/49243/steps/compile-webkit/logs/stdio

Source\WebKit\win\WebView.h(86): error C2504: 'IWebViewPrivate3': base class undefined
Comment 6 Brent Fulgham 2015-09-15 20:29:26 PDT
(In reply to comment #5)
> Looks like this patch may have broken WinCairo builds?
> 
> https://build.webkit.org/builders/WinCairo%2064-Bit%20Release/builds/49243/
> steps/compile-webkit/logs/stdio
> 
> Source\WebKit\win\WebView.h(86): error C2504: 'IWebViewPrivate3': base class
> undefined

Maybe it needs a clean rebuild. IWebViewPrivate3 is generated from the same file that produces the IWebViewPrivate2 it was using previously, so it seems like it didn't pick up the need to regenerate from IDL.
Comment 7 Brent Fulgham 2015-09-15 20:30:57 PDT
+alex so he can check the IWebViewPrivate.idl generation stage.
Comment 8 Alex Christensen 2015-09-16 11:53:09 PDT
Comment on attachment 261271 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=261271&action=review

looks good

> Source/WebKit/win/Interfaces/IWebViewPrivate.idl:329
> +[ uuid(08C88359-76AA-48F2-BA27-820540E781E9) ]
> +interface IWebViewPrivate3 : IWebViewPrivate2
> +{
> +    HRESULT printLayerTree([out] BSTR*);
> +}

Are we just going to add more of these each time we add a function to IWebViewPrivate?  Maybe we should add a bunch of unused functions here so we can use them later without adding more interfaces.

> Tools/MiniBrowser/win/Common.cpp:93
> -SIZE s_windowSize = { 800, 400 };
> +SIZE s_windowSize = { 500, 200 };

Why smaller?
Comment 9 Brent Fulgham 2015-09-16 12:35:34 PDT
(In reply to comment #8)
> Comment on attachment 261271 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=261271&action=review
> 
> looks good
> 
> > Source/WebKit/win/Interfaces/IWebViewPrivate.idl:329
> > +[ uuid(08C88359-76AA-48F2-BA27-820540E781E9) ]
> > +interface IWebViewPrivate3 : IWebViewPrivate2
> > +{
> > +    HRESULT printLayerTree([out] BSTR*);
> > +}
> 
> Are we just going to add more of these each time we add a function to
> IWebViewPrivate?  Maybe we should add a bunch of unused functions here so we
> can use them later without adding more interfaces.

We only need to make a new interface version about once a year. For now we can keep adding to this one.

> > Tools/MiniBrowser/win/Common.cpp:93
> > -SIZE s_windowSize = { 800, 400 };
> > +SIZE s_windowSize = { 500, 200 };
> 
> Why smaller?

Whoops! I'll revert that next time I make a change.