Bug to get accelerated compositing working with webkit2.
<rdar://problem/7660600>
Created attachment 59538 [details] Patch
Comment on attachment 59538 [details] Patch Style review: > + Get accelerated compositing working with webkit2 Please include the Radar URL. s/webkit2/WebKit2/. > + First step: display the compositing layer tree from the webkit process s/webkit/web/ > + (WebKit::ChunkedUpdateDrawingArea::attachCompositingContext): Non-mac stub. Mac. > +#if PLATFORM(MAC) > + SetupAcceleratedCompositingPort > +#endif The verb is “set up” so the name should be “SetUpAcceleratedCompositingPort”. > +#if USE(ACCELERATED_COMPOSITING) > + NSView* _layerHostingView; > +#endif The star should go next to the name. > +#if USE(ACCELERATED_COMPOSITING) > +- (void)_startAcceleratedCompositing:(CALayer*)rootLayer There should be a space before the star. > + NSView* hostingView = [[NSView alloc] initWithFrame:[self bounds]]; Move the star over to the other side. > + [self addSubview:hostingView]; > + [hostingView release]; > + // hostingView is owned by being a subview of self I think the comment is redundant. Being a full sentence, it should be terminated by a period. Or you could just remove it. > + CALayer* viewLayer = [CALayer layer]; Star should be next to the name. > +- (void)_startAcceleratedCompositing:(CALayer*)rootLayer; Needs a space before the star. > +#if USE(ACCELERATED_COMPOSITING) > + void setupAcceleratedCompositing(); > +#endif s/setup/setUp/ > --- a/WebKit2/UIProcess/mac/ChunkedUpdateDrawingAreaProxyMac.mm > +++ b/WebKit2/UIProcess/mac/ChunkedUpdateDrawingAreaProxyMac.mm > @@ -25,11 +25,15 @@ > > #include "ChunkedUpdateDrawingAreaProxy.h" > > +#include <QuartzCore/QuartzCore.h> > + > #include "DrawingAreaMessageKinds.h" > #include "DrawingAreaProxyMessageKinds.h" > #include "UpdateChunk.h" > #include "WKAPICast.h" > #include "WKView.h" > +#include "WKViewInternal.h" > +#include "WebKitSystemInterface.h" > #include "WebPageProxy.h" <QuartzCore/QuartzCore.h> should not be in its own paragraph. It should be at the end of the main paragraph. You can change all of the #include directives to #import directives now that this is an Objective-C file. > + // Cleanup. This comment tells me absolutely nothing, but at least it’s period-terminated. > --- a/WebKit2/WebProcess/WebPage/mac/ChunkedUpdateDrawingAreaMac.cpp > +++ b/WebKit2/WebProcess/WebPage/mac/ChunkedUpdateDrawingAreaMac.mm > @@ -25,9 +25,15 @@ > > #include "ChunkedUpdateDrawingArea.h" > > +#include <QuartzCore/QuartzCore.h> > + Should not be in its own separate paragraph. > +#include "DrawingAreaProxyMessageKinds.h" > #include "UpdateChunk.h" > #include "WebPage.h" > +#include "WebProcess.h" > #include <WebCore/GraphicsContext.h> > +#include <WebCore/GraphicsLayer.h> > +#include "WebKitSystemInterface.h" > #include <wtf/RetainPtr.h> This should be sorted. > + CALayer* caLayer = layer->platformLayer(); Star is on the wrong side. > +#if PLATFORM(MAC) > + , m_compositingRenderServerPort(MACH_PORT_NULL) > +#endif Is this needed in non-accelerated-compositing builds? > +#if PLATFORM(MAC) > + mach_port_t compositingRenderServerPort() const { return m_compositingRenderServerPort; } > +#endif Ditto. > +#if PLATFORM(MAC) > + mach_port_t m_compositingRenderServerPort; > +#endif Ditto.
Comment on attachment 59538 [details] Patch Taking a different tack now.
Created attachment 60173 [details] Patch
Created attachment 60268 [details] GraphicsLayer changes
Comment on attachment 60268 [details] GraphicsLayer changes I’m not a huge fan of xxxRecursive as the name for the version that includes sublayers. The "recursive" suffix seems a little vague and I don’t like it in places we already have it. If the normal case would be to sync the sublayers, then I'd suggest calling the single-layer version syncCompositingStateForThisLayerOnly or something like that. Or you could call the new one syncCompositingStateIncludingSublayers.
Comment on attachment 60173 [details] Patch http://trac.webkit.org/changeset/62283
Comment on attachment 60268 [details] GraphicsLayer changes http://trac.webkit.org/changeset/62286
Created attachment 60284 [details] Add layer-backed drawing area
Attachment 60284 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebKit2/WebProcess/WebPage/LayerBackedDrawingArea.cpp:28: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebKit2/WebProcess/WebPage/LayerBackedDrawingArea.cpp:153: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] WebKit2/WebProcess/WebPage/LayerBackedDrawingArea.h:34: Alphabetical sorting problem. [build/include_order] [4] WebKit2/WebProcess/WebPage/LayerBackedDrawingArea.h:47: Code inside a namespace should not be indented. [whitespace/indent] [4] WebKit2/UIProcess/LayerBackedDrawingAreaProxy.cpp:28: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebKit2/UIProcess/LayerBackedDrawingAreaProxy.cpp:121: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] Total errors found: 6 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 60284 [details] Add layer-backed drawing area http://trac.webkit.org/changeset/62303
Comment on attachment 60173 [details] Patch Export some webcore symbols
Created attachment 60301 [details] Render server setup, and style fixes
Comment on attachment 60173 [details] Patch Cleared Sam Weinig's review+ from obsolete attachment 60173 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment on attachment 60268 [details] GraphicsLayer changes Cleared Darin Adler's review+ from obsolete attachment 60268 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment on attachment 60284 [details] Add layer-backed drawing area Cleared Anders Carlsson's review+ from obsolete attachment 60284 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Created attachment 62110 [details] Patch
Attachment 62110 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit2/WebProcess/WebPage/DrawingArea.cpp:40: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] Total errors found: 1 in 24 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 62110 [details] Patch > } // namespace WebKit > diff --git a/WebKit2/UIProcess/ChunkedUpdateDrawingAreaProxy.h b/WebKit2/UIProcess/ChunkedUpdateDrawingAreaProxy.h > index 2644abb4907b608351abaa5ccbe5c74ffa323e6d..05323b42f9dec1c1141a17d53cbb6fc5ef804c79 100644 > --- a/WebKit2/UIProcess/ChunkedUpdateDrawingAreaProxy.h > +++ b/WebKit2/UIProcess/ChunkedUpdateDrawingAreaProxy.h > @@ -71,6 +71,7 @@ private: > > // DrawingAreaProxy > virtual void didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder&); > + virtual void didReceiveSyncMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder&, CoreIPC::ArgumentEncoder&); > virtual void paint(const WebCore::IntRect&, PlatformDrawingContext); > virtual void setSize(const WebCore::IntSize&); > virtual void setPageIsVisible(bool isVisible); > diff --git a/WebKit2/UIProcess/DrawingAreaProxy.h b/WebKit2/UIProcess/DrawingAreaProxy.h > index 1bd7da3cf1692d013efc8d75ac0571e9ed65c3c6..3817eb1f55c70fadafbcb58045cb464edcc62d59 100644 > --- a/WebKit2/UIProcess/DrawingAreaProxy.h > +++ b/WebKit2/UIProcess/DrawingAreaProxy.h > @@ -55,7 +55,9 @@ typedef QPainter* PlatformDrawingContext; > > +void WebPageProxy::setDrawingArea(DrawingAreaProxy* drawingArea) Since m_drawingArea is an OwnPtr, this should take a PassOwnPtr. Also, the setDrawingArea call site should use OwnPtrs. > +#if USE(ACCELERATED_COMPOSITING) > + case WebPageProxyMessage::DidChangeAcceleratedCompositing: { > + bool compositing; > + if (!arguments.decode(CoreIPC::Out(compositing))) > + return; > + > + if (compositing) > + didEnterAcceleratedCompositing(); > + else > + didLeaveAcceleratedCompositing(); > + > + DrawingAreaProxy::Type drawingAreaType = drawingArea()->type(); > + reply.encode(CoreIPC::In(static_cast<uint32_t>(drawingAreaType))); > + break; We usually don't do any work other than decoding/encoding in the didReceiveMessage case handlers. This should just call a didChangeAcceleratedCompositing function.
http://trac.webkit.org/changeset/63843