Bug 50451 - [GTK] Drop GdkDrawable usage, it's been removed in GTK+3.x and we can use GdkWindow
Summary: [GTK] Drop GdkDrawable usage, it's been removed in GTK+3.x and we can use Gdk...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Gtk (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-12-03 04:24 PST by Xan Lopez
Modified: 2010-12-04 06:55 PST (History)
7 users (show)

See Also:


Attachments
drawable.diff (6.35 KB, patch)
2010-12-03 04:33 PST, Xan Lopez
no flags Details | Formatted Diff | Diff
drawable.diff (8.92 KB, patch)
2010-12-03 06:06 PST, Xan Lopez
mrobinson: review+
Details | Formatted Diff | Diff
drawable.diff (10.36 KB, patch)
2010-12-03 09:33 PST, Xan Lopez
no flags Details | Formatted Diff | Diff

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