Re: Multiple Display support proposed Layout
- From: Havoc Pennington <hp redhat com>
- To: Erwann Chenede <Erwann Chenede ireland sun com>
- Cc: gtk-devel-list gnome org
- Subject: Re: Multiple Display support proposed Layout
- Date: 15 Feb 2001 20:31:00 -0500
Hi,
I'll give you some initial comments, I think you'll also want a round
of comments from Owen at some point. Looks pretty good in general.
Erwann Chenede <Erwann Chenede ireland sun com> writes:
> e.g. for gdk_visual_init I've added a gdk_visual_init_screen
> variant, gdk_visual_init called only for the default version.
> See gdkmultihead.h for the "variant" signature I've already done.
>
> So my question is : Is the naming convention I've used OK ?
Our usual convention would be _with_screen() probably. So
gdk_visual_init_with_screen(), etc.
Specific comments on the headers, mostly nitpicks, I'll leave any
large design comments to others.
> struct _GdkDisplay
> {
> GObject parent_instance;
>
> };
Need to use 2-space indentation.
>
> struct _GdkDisplayClass
> {
> GObjectClass parent_class;
> GdkDisplay * (*new_display) (gchar *
> display_name);
Instead of doing this, make display_name a G_PARAM_CONSTRUCT_ONLY
object property. Look at gtk/gtktexttag.c for an example of using
G_PARAM_CONSTRUCT_ONLY. The issue here is that a call to g_object_new
(GDK_TYPE_DISPLAY, "name", ":0", NULL) should work, which means the
display object should be valid after the instance init function.
> gchar * (*get_display_string) (GdkDisplay *dpy);
get_display_name would be a better name I think.
> gint (*screen_count) (GdkDisplay *dpy);
>
get_n_screens is our usual convention.
> GdkDisplay * gdk_display_new (void);
I'm guessing this function is never used by app writers, one just asks
the display manager to open the display. So probably just
call g_object_new (GDK_TYPE_DISPLAY, NULL) in the display manager
implementation, and don't have this function.
> GdkDisplay * gdk_display_ref (GdkDisplay* display);
> void gdk_display_unref (GdkDisplay* display);
>
These aren't needed, since you can use g_object_ref(). The other
ref/unref calls in GDK just exist because they predate those types
being GObject subclasses.
> struct _GdkDisplayMgr
This name shouldn't be abbreviated - GdkDisplayManager. Long function
names are just inevitable in C, have to bite the bullet. ;-)
> {
> GObject parent_instance;
> GdkDisplay * default_display;
> GSList * display_list;
> };
display_list is presumably just a list of open displays, right? Maybe
call it open_displays
> gint (*get_num_display) (GdkDisplayMgr* dpy_mgr);
I assume this returns the number of opened displays, so maybe call it
get_n_open_displays or something like that. Though I'm not sure it's
useful to find out how many displays are open anyhow.
> GType gdk_display_mgr_type (void);
> GdkDisplayMgr * gdk_display_mgr_new (void);
> /*GdkDisplayMgr * gdk_display_mgr_unref (GdkDisplayMgr * dpy_mgr);
> void gdk_display_mgr_ref (GdkDisplayMgr * dpy_mgr);*/
>
Presumably this is a singleton object, so maybe have
gdk_display_manager_get() instead of gdk_display_manager_new()?
Also, it looks like there's no gdk_display_manager_open(). That
function will need to be able to fail gracefully by returning NULL or
the like.
> GdkAtom
> gdk_atom_intern_display(const gchar * atom_name,
> gboolean only_if_exists,
> GdkDisplay * dpy);
Maybe we can avoid this; it could mean a lot of pain in end-user
apps. You could instead keep a mapping from a client-side GdkAtom
value that's the same for all displays to the real X value. This
would break a few apps using Xlib directly, but not too big a deal.
> void
> gdk_key_repeat_restore_display(GdkDisplay * dpy);
Don't bother with this, the key repeat stuff is deprecated and bad.
> void gdk_visual_add_screen(GdkVisual * visual, GdkScreen * scr);
Do screens contain visuals or vice versa?
> void gdk_query_depths_screen(gint ** depths,
> gint * count,
> GdkScreen * scr);
Here and in similar cases maybe a for_screen or of_screen would be
better than with_screen as the naming convention.
Havoc
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]