Bug 117230 – [WK2][GTK] Let DrawingArea manage setAcceleratedCompositingWindowId

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Gwang Yoon Hwang]

This is a preparation patch for Threaded Coordinated Graphics.
    
    LayerTreeHost uses a native window handle to make glContext for
    accelerated compositing. Therefore it is natural for DrawingArea to take
    responsibility for the native window handle. And, in Coordinated
    Graphics case, WebPage creates LayerTreeHost before receiving a native
    window handle from UIProcess. Therefore we need a method to set the
    native window handle to already created LayerTreeHost.
    
    This patch uses DrawingAreaProxy::setNativeCompositingSurfaceHandle
    instead of WebCoreProxy::setAcceleratedCompositingWindowId to set window
    ID for accelerated compositing.
    
    Also, this patch renames the setAcceleratedCompositingWindowId with a
    more generic name, setNativeCompositingSurfaceHandle.
    
    No new tests. No change in functionality.

[Gwang Yoon Hwang]

Created attachment 203997 [details]
Patch

[WebKit Commit Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Martin Robinson]

Comment on attachment 203997 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=203997&action=review

This change seems okay to me. I have suggestions for a few changes and after that it needs the approval of a WebKit owner.

> Source/WebKit2/UIProcess/DrawingAreaProxyImpl.cpp:375
> +void DrawingAreaProxyImpl::setNativeCompositingSurfaceHandle(uint64_t handle)

May I suggest a new name: setNativeSurfaceHandleForCompositing or setCompositingNativeSurfaceHandle. This eliminates any confusion between native "compositing surface handle" or a "surface handle" for native compositing.

> Source/WebKit2/WebProcess/WebPage/LayerTreeHost.h:99
> +#if USE(TEXTURE_MAPPER_GL) && PLATFORM(GTK)
> +    virtual void setNativeCompositingSurfaceHandle(uint64_t) { }
> +#endif
> +

This is unused for this patch, so maybe it can be removed.

[Gwang Yoon Hwang]

Created attachment 205474 [details]
Patch

[Gwang Yoon Hwang]

(In reply to comment #3)
> (From update of attachment 203997 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=203997&action=review
> 
> This change seems okay to me. I have suggestions for a few changes and after that it needs the approval of a WebKit owner.
> 
Sorry for late. :(

> > Source/WebKit2/UIProcess/DrawingAreaProxyImpl.cpp:375
> > +void DrawingAreaProxyImpl::setNativeCompositingSurfaceHandle(uint64_t handle)
> 
> May I suggest a new name: setNativeSurfaceHandleForCompositing or setCompositingNativeSurfaceHandle. This eliminates any confusion between native "compositing surface handle" or a "surface handle" for native compositing.
>

I prefer setNativeSurfaceHandleForCompositing. I've renamed with it.
 
> > Source/WebKit2/WebProcess/WebPage/LayerTreeHost.h:99
> > +#if USE(TEXTURE_MAPPER_GL) && PLATFORM(GTK)
> > +    virtual void setNativeCompositingSurfaceHandle(uint64_t) { }
> > +#endif
> > +
> 
> This is unused for this patch, so maybe it can be removed.
Yes, exactly. removed.

[Gwang Yoon Hwang]

Created attachment 206037 [details]
Rebase after r152375

[Martin Robinson]

(In reply to comment #6)
> Created an attachment (id=206037) [details]
> Rebase after r152375

The previous patch looked good to me, so if this is just a rebase, you simply need an owner to approve it.

[Gwang Yoon Hwang]

Created attachment 242761 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 242761 [details]
Patch

Clearing flags on attachment: 242761

Committed r176954: <http://trac.webkit.org/changeset/176954>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.