Bug 149165 - [Win] Provide means for viewing layer tree
Summary: [Win] Provide means for viewing layer tree
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: PC All
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-09-15 09:45 PDT by Brent Fulgham
Modified: 2015-09-16 12:35 PDT (History)
4 users (show)

See Also:


Attachments
Patch (26.07 KB, patch)
2015-09-15 17:43 PDT, Brent Fulgham
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.