Bug 104670

Summary: [GTK][AC] GraphicsLayers are not shown on the viewport
Product: WebKit Reporter: Joone Hur <joone>
Component: WebKitGTKAssignee: Joone Hur <joone>
Status: RESOLVED FIXED    
Severity: Normal CC: alex, gustavo, mrobinson, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 73767, 105699    
Attachments:
Description Flags
Patch
none
Patch none

Description Joone Hur 2012-12-11 06:44:42 PST 
GraphicsLayers are not shown on the viewport because we don't resize and show the GtkClutterEmbed yet.
Comment 1 Joone Hur 2012-12-11 16:58:50 PST 
Created attachment 178922 [details]
Patch
Comment 2 Gustavo Noronha (kov) 2012-12-12 01:32:17 PST 
Comment on attachment 178922 [details]
Patch

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

LGTM, except for the duplicate timer scheduling.

> Source/WebKit/gtk/WebCoreSupport/AcceleratedCompositingContextClutter.cpp:59
> -    return false;
> +    return true;

This is what we need to implement to render the page contents, right?

> Source/WebKit/gtk/WebCoreSupport/AcceleratedCompositingContextClutter.cpp:93
> +    scheduleLayerFlush();
> +    m_layerFlushTimerCallbackId = g_timeout_add_full(GDK_PRIORITY_EVENTS, 0, reinterpret_cast<GSourceFunc>(layerFlushTimerFiredCallback), this, 0);

You're scheduling the flush twice here, gotta remove the g_timeout_add_full call.

> Source/WebKit/gtk/WebCoreSupport/AcceleratedCompositingContextClutter.cpp:132
> -    return FALSE;
> +    return false;

this returns a gboolean, so I'd keep FALSE here
Comment 3 Joone Hur 2012-12-12 03:03:45 PST 
Comment on attachment 178922 [details]
Patch

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

>> Source/WebKit/gtk/WebCoreSupport/AcceleratedCompositingContextClutter.cpp:59
>> +    return true;
> 
> This is what we need to implement to render the page contents, right?

Yes, it seems to paint the main content on the root layer.

>> Source/WebKit/gtk/WebCoreSupport/AcceleratedCompositingContextClutter.cpp:93
>> +    m_layerFlushTimerCallbackId = g_timeout_add_full(GDK_PRIORITY_EVENTS, 0, reinterpret_cast<GSourceFunc>(layerFlushTimerFiredCallback), this, 0);
> 
> You're scheduling the flush twice here, gotta remove the g_timeout_add_full call.

Yes, I will remove it.

>> Source/WebKit/gtk/WebCoreSupport/AcceleratedCompositingContextClutter.cpp:132
>> +    return false;
> 
> this returns a gboolean, so I'd keep FALSE here

Ok.
Comment 4 Joone Hur 2012-12-12 03:13:45 PST 
Created attachment 179012 [details]
Patch
Comment 5 WebKit Review Bot 2012-12-12 03:35:46 PST 
Comment on attachment 179012 [details]
Patch

Clearing flags on attachment: 179012

Committed r137447: <http://trac.webkit.org/changeset/137447>
Comment 6 Gustavo Noronha (kov) 2013-01-14 10:46:09 PST 
Comment on attachment 178922 [details]
Patch

Clearing flag for committed patch.