Bug 28346 – [GTK] Support for cursors that are images

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

[Martin Robinson]

Created attachment 34911 [details]
Support for images as cursors

GTK does not yet seem to support cursors that are images. I've attached a patch which adds this, but it does not seem to work correctly because of #28345.

[Martin Robinson]

Just to clarify: currently this patch creates cursors which have a lots of artifacts. This can be observed by applying this patch and visiting: http://www.worldtimzone.com/mozilla/testcase/css3cursors.html

[Jan Alonzo]

(In reply to comment #1)
> Just to clarify: currently this patch creates cursors which have a lots of
> artifacts. This can be observed by applying this patch and visiting:
> http://www.worldtimzone.com/mozilla/testcase/css3cursors.html

Hi Martin. Is it possible to add a test for this similar to the one above?

[Martin Robinson]

Created attachment 34912 [details]
Patch with manual test added

I've added a manual test for this issue in WebCore/manual-tests/gtk/cursor-image.html

[Dirk Schulze]

You need te set the review flag to get this patch reviewed.

[Martin Robinson]

Created attachment 35022 [details]
Updated patch ready for review

I had planned to wait for the blocking issue to be resolved, but I've taken your cue and prepared the patch for review.

[Eric Seidel (no email)]

Comment on attachment 35022 [details]
Updated patch ready for review

Why is the memory management correct here?  Do we need to release/destroy this cursor ever?

I take it "getGdkPixbuf()" causes the pixbuf to be reffed?

[Martin Robinson]

(In reply to comment #6)
> (From update of attachment 35022 [details])
> Why is the memory management correct here?  Do we need to release/destroy this
> cursor ever?
> 
> I take it "getGdkPixbuf()" causes the pixbuf to be reffed?

getGdkPixbuf() constructs a new GdkPixbuf* from a cairo_surface_t*. Its reference count will be initialized to 1. gdk_cursor_new_from_pixbuf will copy bytes from a GdkPixbuf* as can be seen here: http://google.com/codesearch/p?hl=en&sa=N&cd=2&ct=rc#83-cTy5ZCbw/gtk+-2.4.12/gdk/x11/gdkcursor-x11.c&q=gdk_cursor_new_from_pixbuf&l=387

Since the GdkPixbuf was merely an intermediary between the cairo_surface_t and the GdkPointer, it is no longer needed. gdk_pixbuf_unref will decrease the reference count of the GdkPixbuf from 1 to 0, freeing the memory and preventing a leak.

[Martin Robinson]

I should clarify the removal of the original gdk_cursor_ref(...) as well. Previously the stub method increased the reference count of a GdkCursor created via another Cursor constructor. This patch constructs an entirely new GdkCursor instead, which should already have a reference count of 1.

[Gustavo Noronha (kov)]

Comment on attachment 35022 [details]
Updated patch ready for review

Good to see this moving forward =).

[Eric Seidel (no email)]

Comment on attachment 35022 [details]
Updated patch ready for review

Rejecting patch 35022 from commit-queue.  This patch will require manual commit.

Patch https://bugs.webkit.org/attachment.cgi?id=35022 from bug 28346 failed to download and apply.

[Gustavo Noronha (kov)]

Comment on attachment 35022 [details]
Updated patch ready for review

Landed as r47712. I had overlooked the fact that I could not determine the origin of the .cur file. I replaced the one you provided with a cell.cur from Mozilla sources, that I know is licensed in a good way.

[Eric Seidel (no email)]

Sadly svn-apply can't handle git binary patches:
patch: **** Only garbage was found in the patch input.
patch -p0 "WebCore/manual-tests/gtk/resources/redcursor.cur" returned 2.  Pass --force to ignore patch failures.
Logging in as [e-mail]...
Rejecting patch 35022 from commit-queue.  This patch will require manual commit.

bug 26830.