Bug 149165 – [Win] Provide means for viewing layer tree

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Brent Fulgham]

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.

[Brent Fulgham]

Created attachment 261271 [details]
Patch

[Simon Fraser (smfr)]

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?

[Brent Fulgham]

(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.

[Brent Fulgham]

Committed r189833: <http://trac.webkit.org/changeset/189833>

[Ryosuke Niwa]

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

[Brent Fulgham]

(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.

[Brent Fulgham]

+alex so he can check the IWebViewPrivate.idl generation stage.

[Alex Christensen]

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?

[Brent Fulgham]

(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.