On Sat, Jan 17, 2009 at 8:19 PM, Dr. David Alan Gilbert <gilbe...@treblig.org> wrote: > * Matthias Clasen (matthias.cla...@gmail.com) wrote: >> Thanks for the patch. It looks good in general, just a couple of notes: >> >> * There are some stylistic issues (mostly missing spaces here and there) > > No problem; please point out a few of the ones that need fixing and > I'll try and make they all get done.
+ cursor_cache = g_slist_prepend(cursor_cache, cursor); We put a space between the function and the opening ( + GdkCursorPrivate* curs=(GdkCursorPrivate*)listelem; and around = in assignments + if ((curs->cursor.type!=key->type) || and around binary operators like !=, >, == + if (display->closed) { + } else { Parens go on a new line + if (private) + { and are indented + { + XFixesChangeCursor (xdisplay, new_cursor, private->xcursor); + private->xcursor=new_cursor; + } basic indent is 2, not 8 * We need to purge cursors from the cache when their display is closed > > OK - is there a hook for that? Also is there an easy way to test > multidisplay code like that - what can I easily get to open something > on two displays? gtk-demo has a 'change display' demo. gdk/x11/gdkdisplay-x11.c:gdk_display_x11_finalize is the right place, I think. >> * Do you have any data how big the cache grows in cursor-heavy apps ? > > No, but I've not found anything that uses more than 3 or 4 cursors; > they just open the same damn ones over and over again. The overhead > for each cache entry is small, cursors seem to vary on disc, but seem > to be between 15 and 35k except for some big ones but you are only paying > a penalty for those that are used rarely. Ah, I was really interested in the length of the list. That sounds fine then. >> Wrt. to blank cursors, adding a new cursor type or special-casing a >> cursor name like "none" would work, although we probably cannot change >> GDK_LAST_CURSOR, judging from google code search results. > > It's a pity if GDK_LAST_CURSOR has to stay put, so it probably doesn't > make sense to add a new type. Special caseing the name sounds a bit > odd; since in either case it's effectively changing the API would it > be cleaner just to have a new function call for it? > The other way I'd considered was special casing the pixmap code for 1x1 - > but again that feels dirty. GDK_BLANK_CURSOR = -2 should work fine. _______________________________________________ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list