Bug 104670 - [GTK][AC] GraphicsLayers are not shown on the viewport
Summary: [GTK][AC] GraphicsLayers are not shown on the viewport
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joone Hur
URL:
Keywords:
Depends on:
Blocks: 73767 105699
  Show dependency tree / graph
 
Reported: 2012-12-11 06:44 PST by Joone Hur
Modified: 2013-01-14 10:46 PST (History)
4 users (show)

See Also:

Attachments
Patch (5.56 KB, patch)
2012-12-11 16:58 PST, Joone Hur 		no flags Details | Formatted Diff | Diff
Patch (5.34 KB, patch)
2012-12-12 03:13 PST, Joone Hur 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.