[webkit-reviews] review granted: [Bug 86037] [GTK] Add an accelerated compositing implementation for WebKit2 : [Attachment 146664] Patch

[bugzilla-daemon at webkit.org]

Alejandro G. Castro <alex at igalia.com> has granted Martin Robinson
<mrobinson at webkit.org>'s request for review:
Bug 86037: [GTK] Add an accelerated compositing implementation for WebKit2
https://bugs.webkit.org/show_bug.cgi?id=86037

Attachment 146664: Patch
https://bugs.webkit.org/attachment.cgi?id=146664&action=review

------- Additional Comments from Alejandro G. Castro <alex at igalia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=146664&action=review


Looks good to me, I've tested and it works great :), let's try it! I've added
some nits.

> Source/WebKit2/WebProcess/WebPage/WebPage.h:547
> +#if PLATFORM(GTK)
> +    uint64_t nativeWindowHandle() { return m_nativeWindowHandle; }
> +#endif

Why aren't we protecting here with the USE(TEXTURE_MAPPER_GL) define?

> Source/WebKit2/WebProcess/WebPage/WebPage.h:765
> +    // Our view's window in the UI process.
> +    uint64_t m_nativeWindowHandle;

Same here.

> Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.h:74
> +    // GraphicsLayerClient

Dot at the end.

> Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.h:105
> +    // The root layer.
> +    OwnPtr<WebCore::GraphicsLayer> m_rootLayer;

Probably we can remove this comment, I think the variable name is correctly
saying what it is. I think there was some discussion at some point in the list
about these comments, do not recall the conclusion though :). In case you
decide to remove it there are others like this in the file.