Bug 47087 – [GTK] Use pixbufs instead of pixmaps when creating platform cursors

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

[Carlos Garcia Campos]

Created attachment 69634 [details]
Use GdkPixbuf to create platform cursors

gdk_cursor_new_from_pixmap() has been removed in gtk3. We can use a pixbuf instead of a pixman and use gdk_cursor_new_from_pixbuf() instead.

[Claudio Saavedra]

Should actually use

+    GdkPixbuf* pixbuf = gdk_pixbuf_get_from_surface(surface, 0, 0, 32, 32);

since this method's interface changed in gtk master to this (see gtk rev. 872ef1).

[Carlos Garcia Campos]

Created attachment 69897 [details]
Updated patch to use GdkPixbuf when creating cursors

Updated patch including a ChangeLog entry and fixing the API to current master gtk+ as Claudio pointed out.

[WebKit Review Bot]

Attachment 69897 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
Last 3072 characters of output:
ent?  [whitespace/indent] [3]
WebCore/platform/gtk/GtkVersioning.c:249:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/GtkVersioning.c:254:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/GtkVersioning.c:254:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/GtkVersioning.c:255:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/GtkVersioning.c:255:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/GtkVersioning.c:256:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/GtkVersioning.c:256:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/gtk/GtkVersioning.c:257:  This { should be at the end of the previous line  [whitespace/braces] [4]
WebCore/platform/gtk/GtkVersioning.c:258:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/GtkVersioning.c:262:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/GtkVersioning.c:263:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/GtkVersioning.c:264:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/GtkVersioning.c:265:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/GtkVersioning.c:266:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/GtkVersioning.c:269:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/GtkVersioning.c:270:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/GtkVersioning.c:271:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/GtkVersioning.c:272:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/GtkVersioning.c:273:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/GtkVersioning.c:277:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/GtkVersioning.c:277:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/GtkVersioning.c:278:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/GtkVersioning.c:280:  Should have a space between // and comment  [whitespace/comments] [4]
WebCore/platform/gtk/GtkVersioning.h:95:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/gtk/GtkVersioning.h:95:  dest_x is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 107 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Attachment 69897 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/4195106

[Carlos Garcia Campos]

Created attachment 69922 [details]
Updated patch

Fixes coding style and build issues.

[WebKit Review Bot]

Attachment 69922 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/platform/gtk/GtkVersioning.h:94:  gdk_pixbuf_get_from_surface is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Xan Lopez]

Comment on attachment 69922 [details]
Updated patch

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

> WebCore/ChangeLog:6
> +

Usually the 'syntax' for this is, title in one line, URL in the next line. We also usually use [GTK] in the title when the bug is GTK specific.

> WebCore/ChangeLog:13
> +

So... like this, but it goes at the beginning.

> WebCore/ChangeLog:15
> +

If you don't add tests you have to remove this.

> WebCore/platform/gtk/CursorGtk.cpp:51
> +    cairo_surface_t* surface = cairo_image_surface_create(CAIRO_FORMAT_A1, 32, 32);

Should use PlatformRefPtrCairo here, see WebCore/platform/graphics/cairo/PlatformRefPtrCairo.{cpp,h}

> WebCore/platform/gtk/CursorGtk.cpp:52
> +    cairo_t* cr = cairo_create(surface);

Same thing.

> WebCore/platform/gtk/CursorGtk.cpp:60
> +    GdkPixbuf* pixbuf = gdk_pixbuf_get_from_surface(surface, 0, 0, 32, 32);

Use a GRefPtr here.

> WebCore/platform/gtk/GtkVersioning.h:97
> +

Group this with the already existing #ifdef GTK_API_VERSION_2 thing?

[Carlos Garcia Campos]

Created attachment 69928 [details]
New patch according to review

New patch, it fixes the changlog syntax and uses PlatformRefPtr

[WebKit Review Bot]

Attachment 69928 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/platform/gtk/GtkVersioning.h:35:  gdk_pixbuf_get_from_surface is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Xan Lopez]

Comment on attachment 69928 [details]
New patch according to review

Looks good, let's see what happens!

[WebKit Review Bot]

Attachment 69928 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/4204096

[Carlos Garcia Campos]

Created attachment 69937 [details]
New version of the patch

Sorry, I uploaded a wrong version of the patch by mistake.

[WebKit Review Bot]

Attachment 69937 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/platform/gtk/GtkVersioning.h:36:  gdk_pixbuf_get_from_surface is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Xan Lopez]

Comment on attachment 69937 [details]
New version of the patch

r=me

[WebKit Commit Bot]

Comment on attachment 69937 [details]
New version of the patch

Clearing flags on attachment: 69937

Committed r69204: <http://trac.webkit.org/changeset/69204>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.