Summary: | Need CA_PRINT_TREE functionality on Windows | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Marrin <cmarrin> | ||||||
Component: | Layout and Rendering | Assignee: | Chris Marrin <cmarrin> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | ||||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Chris Marrin
2010-02-09 17:30:58 PST
Created attachment 48453 [details]
Patch
Comment on attachment 48453 [details] Patch > + CFArrayRef sublayers = CACFLayerGetSublayers(layer()); > + if (CFArrayGetCount(sublayers) <= index) > + return 0; If we're checking the range of index and returning 0 rather than calling CFArrayGetValueAtIndex, then suggest we check for negative numbers too since the index is signed. > + return layer(reinterpret_cast<CACFLayerRef>(const_cast<void*>(CFArrayGetValueAtIndex(sublayers, index)))); This should be static_cast instead of reinterpret_cast since it's a cast from void*. > - bool isTransformLayer() const { return CACFLayerGetClass(layer()) == kCACFTransformLayer; } > + bool isTransformLayer() const; Why this change? Is there a problem with it being inlined? r=me (In reply to comment #2) > (From update of attachment 48453 [details]) > > + CFArrayRef sublayers = CACFLayerGetSublayers(layer()); > > + if (CFArrayGetCount(sublayers) <= index) > > + return 0; > > If we're checking the range of index and returning 0 rather than calling > CFArrayGetValueAtIndex, then suggest we check for negative numbers too since > the index is signed. I'll fix it. > > > + return layer(reinterpret_cast<CACFLayerRef>(const_cast<void*>(CFArrayGetValueAtIndex(sublayers, index)))); > > This should be static_cast instead of reinterpret_cast since it's a cast from > void*. Will do. > > > - bool isTransformLayer() const { return CACFLayerGetClass(layer()) == kCACFTransformLayer; } > > + bool isTransformLayer() const; > > Why this change? Is there a problem with it being inlined? I had to change it to use the function call version of the enum since the DLL is delay loaded. Using that function can only be done in the cpp file. I will make a note of that in the changelog > > r=me Created attachment 48522 [details]
Replacement patch
I realized that it's a bad idea to be calling getenv() on each render to get CA_PRINT_TREE. So I now cache it.
Comment on attachment 48522 [details] Replacement patch > void WKCACFLayerRenderer::renderSoon() > Index: WebCore/platform/graphics/win/WKCACFLayerRenderer.h > =================================================================== > --- WebCore/platform/graphics/win/WKCACFLayerRenderer.h (revision 54620) > +++ WebCore/platform/graphics/win/WKCACFLayerRenderer.h (working copy) > @@ -97,6 +97,10 @@ private: > HWND m_hostWindow; > Timer<WKCACFLayerRenderer> m_renderTimer; > IntRect m_scrollFrame; > + > +#ifndef NDEBUG > + bool m_cachedPrintTreeFlag; > +#endif I don't think the name needs to include 'cached' or 'flag'; both are obvious. m_printTree is fine. This stuff is OK, but I'd still like to see a platform-independent layer dump we can use in DRT. Landed in http://trac.webkit.org/changeset/54630 |