Re: Cursors (again)




Hi,

Looks nice! Still, lots of comments to get the patch ready for gnome-libs:

 - use the linux coding style - in Emacs, M-x c-set-style linux
 - if GnomeCursorPrivate is private, don't put it in the header file ;-)
   Also, you probably want all the fields private, I don't see 
   a reason to access the ones in the public struct
 - actually now that I look at it, the GnomeCursorPrivate is the 
   stuff that's registered, and the GnomeCursor is the handle
   on this - so GnomeCursorPrivate isn't really named properly.
   I would suggest GnomeCursorStyle or something, then 
   change the register/unregister functions to 
   gnome_cursor_style_register(), etc.
 - no need for "extern" on the function prototypes
 - gnome_cursor_create() should be called gnome_cursor_new()
 - you need a gnome_cursor_ref() and gnome_cursor_unref()
 - what happens if you unregister a cursor style while a cursor 
   instance is outstanding? you need to refcount the
   cursor style to handle this probably
 - a cursor is attached to a GdkWindow, not a GtkWidget
 - I think a useful feature would be a cursor stack for 
   each GdkWindow, so instead of gnome_cursor_set() have 
   gnome_cursor_push() and gnome_cursor_pop() - you'd need 
   to use a hash from GdkWindow* to the stack I guess
 - I like the animation
 - foreground/background should be set when you do 
   gnome_cursor_new() I think, not when you set the cursor 
   on a window
 - default foreground/background should probably be part of the 
   GnomeCursorStyle, rather than hardcoded

OK that should keep you busy ;-) Maybe wait and see what other people
think.

Havoc





[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]