Bug 50451

Summary: [GTK] Drop GdkDrawable usage, it's been removed in GTK+3.x and we can use GdkWindow
Product: WebKit Reporter: Xan Lopez <xan.lopez>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: a9016009, buildbot, cskeogh, eric, mrobinson, uws+webkit, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
drawable.diff
none
drawable.diff
mrobinson: review+
drawable.diff none

Description Xan Lopez 2010-12-03 04:24:24 PST
SSIA
Comment 1 Xan Lopez 2010-12-03 04:33:05 PST
Created attachment 75488 [details]
drawable.diff
Comment 2 Martin Robinson 2010-12-03 04:39:34 PST
Comment on attachment 75488 [details]
drawable.diff

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

> WebCore/platform/graphics/GraphicsContext.h:95
>  #if PLATFORM(GTK)
> -typedef struct _GdkDrawable GdkDrawable;
> +typedef struct _GdkWindow GdkWindow;
>  typedef struct _GdkEventExpose GdkEventExpose;
>  #endif

This typedef will have to be typedef struct _GdkDrawable GdkWindow for GTK+ 2.x. I guess we'll need an #ifdef. You could move these to JavaScriptCore/gobject/GTypedefs.h and do it there.
Comment 3 Xan Lopez 2010-12-03 06:06:18 PST
Created attachment 75496 [details]
drawable.diff
Comment 4 Martin Robinson 2010-12-03 06:28:24 PST
Comment on attachment 75496 [details]
drawable.diff

Seems reasonable. Aren't there any callers for GraphicsContext::gdkWindow() ?
Comment 5 Build Bot 2010-12-03 06:50:11 PST
Attachment 75496 [details] did not build on win:
Build output: http://queues.webkit.org/results/6832031
Comment 6 Eric Seidel (no email) 2010-12-03 08:49:47 PST
Attachment 75496 [details] did not build on mac:
Build output: http://queues.webkit.org/results/6824028
Comment 7 Xan Lopez 2010-12-03 09:33:29 PST
Created attachment 75504 [details]
drawable.diff
Comment 8 Wouter Bolsterlee 2010-12-03 11:12:56 PST
GDK_DRAWABLE and some other deprecated macros and functions have been removed altogether from GTK+ 3 (git master), so without this patch Webkit doesn't compile with recent GTK+ 3.
Comment 9 Martin Robinson 2010-12-03 12:58:56 PST
Comment on attachment 75504 [details]
drawable.diff

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

I tested this patch locally with exceptional results. But unless I'm crazy, I think that the change to the comment is incorrect.

> WebCore/platform/gtk/WidgetRenderingContextGtk2.cpp:91
> +    // paint directly to the target window. This will not render CSS rotational transforms properly.

This comment is actually correct, because m_target may either be a GdkWindow or a GdkPixmap, both of which are drawables. I don' think a pixmap is a window though.
Comment 10 Xan Lopez 2010-12-03 16:20:50 PST
(In reply to comment #9)
> > WebCore/platform/gtk/WidgetRenderingContextGtk2.cpp:91
> > +    // paint directly to the target window. This will not render CSS rotational transforms properly.
> 
> This comment is actually correct, because m_target may either be a GdkWindow or a GdkPixmap, both of which are drawables. I don' think a pixmap is a window though.

You are right, I was too quick with search and replace.

(In reply to comment #8)
> GDK_DRAWABLE and some other deprecated macros and functions have been removed altogether from GTK+ 3 (git master), so without this patch Webkit doesn't compile with recent GTK+ 3.

Right again! I was just sloppy with my words, I know GdkDrawable has been removed and this patch is needed to build. I can tell because of the failed local build ;)
Comment 11 Xan Lopez 2010-12-04 06:54:21 PST
Committed r73330: <http://trac.webkit.org/changeset/73330>