Re: multi head porting

[Hans Breuer <hans breuer org>]

At 11:01 10.05.02 -0400, Owen Taylor wrote:
>
>Hans Breuer <hans breuer org> writes:
>
>> The attached patch allows to compile gtk head on win32
>> again (not supporting multiple screens yet). 
>> It was done with the major goal to minimize the required 
>> changes on the backend code.
>
>Thanks a lot for looking at this. I've been meaning to try
>and get some time to get the other backends straightened up 
>but haven't had any time.
>
>Did you see:
>
> http://mail.gnome.org/archives/gtk-devel-list/2002-April/msg00103.html
>
Yupp. After proting the first backend I should probably 
comment on it.

<Re: Ports and multihead>
At 18:02 18.04.2002 -0400, Owen Taylor wrote:
>The basic steps I see needing to be done here are:
>
> * For functions that have gotten per-display or per-screen variants,
>   move the compatibility function that works on the default display
>   out of the port-specific code into the port independent code.
>
Gone the other way to simplify porting other backends which already 
have all these single head functons.
 
>   For instance, gdk_pointer_ungrab() would be in the port-independent
>   part and call gdk_display_pointer_ungrab (gdk_get_default_display (), ...)
>
>   An important goal is to avoid increasing the number of port-specific
>   functions that need to be implemented, and the amount of cut-and-paste
>   between the different ports.
>
That's my goal too.

> * Add dummy GdkScreen and GdkDisplay implementations to the other
>   ports. Hopefully this will be small enough that it can be cut
>   and pasted without introducing maintainence problems.
>
>   (If it isn't one possibility would be to implement some of the
>   dummy implementation in the GdkScreen/GdkDisplay base classes.)
>
Taking kind of the latter approach to avoid copy and pasting the 
GdkScreen/GdkDisplay to every backend, but implement a single
single head version. The single head classes do use already
existing backend functions.

> * Rename the old functions in the ports to the new name, add 
>   GdkDisplay/GdkScreen arguments, but ignore them.
>
Not always possible as mentioned elsewhere.

> * (later) Implement the "xinerama" functionality for Windows.
>
Not me. I currently feel perfectly comfortable with one head :-)

> * (later) Consider moving global variables into the GdkDisplay and GdkScreen
>   structures even for ports that only support one display and one screen,
>   for cleanliness.
>
This (later) is already done/required, e.g. display->queued_events
which appears to be an indirection too much for single head 
versions ...
</Re: Ports and multihead>

>I'm not sure that minimizing the required changes to the 
>backend code is really the right goal. I think, instead,
>the right goal is minimizing the total amount of backend
>code.
>
I'm not sure if it is the right way to go to continously
break other backends for 'no good reason'. At some point
in time there even was an almost working Nano X port.
Currently AFAIK linux-fb and direct-fb are significant
falling behind ...

With almost the same amount of work I could have added
the GdkDisplay* / GdkScreen* parameters [I even have done
it somewhere - where I don't think it's right - to not 
break the common gdk api (and the X11 backend, which 
I can't compile)]
 
>If we take your approach of:
>
>==
>void
>gdk_display_beep (GdkDisplay * display)
>{
>  g_return_if_fail (GDK_IS_DISPLAY (display));
>
>  gdk_beep ();
>}
>===
>
>Then we need to implement:
>
> gdk_display_beep:  in gdk/x11/ and gdk/
> gdk_beep:          in gdk/x11/ and gdk/win32
>
Not really. All the working ports allready _have_
gdk_beep (). The could be compiled with minimized
changes after applying my patch. For future addition
your suggestions is obviously the right way to go.

[...]
Skipping the rest of the gdk_beep() example, which
IMHO doesn't give good reasons for going either way.

>For code that really does need to be duplicated between all
>single-head ports, it certainly makes sense to figure out a way 
>to put that in gdk/, though it might be easier to do one port first
>(say Win32) and then try to figure out from there what makes
>sense as common code. 
>
That's what I tried :) Not always in the patch but also 
in my mail ...

>> With this goal in mind there 
>> are some places where it could get huge improvements if the
>> multi-head api isn't inscribed in stone yet :
>> 
>> gdk_colormap_get_screen()
>> Only the multi-head version needs to store a screen with
>> every colormap. Single Head version would return the 
>> default screen. [The GdkScreen* field from the GdkColormap 
>> struct could be removed and the multi-head version would
>> maintain it's own hashlist to remember the Colormap/Screen
>> relations.]
>> 
>> gdk_visual_get_screen()
>> Same logic as for gdk_colormap_get_screen(), removing
>> GdkScreen* from _GdkVisual struct, whiches content needs to be 
>> maintained by the backend.
>
>We could move the GdkScreen structure in to the X11 private
>part ... I don't really see much of a difference though.
>
The point: Currently every backend needs to maintain information 
for the common case, which isn't at all relevant at single-head.
See: gdk/win32/gdkcolor-win32.c gdk/win32/gdkvisual-win32.c

>> gdk_event_* ()
>> Removing the display parameter from the gdk_event_* () function
>> again and determining it from the window given in _GdkEventAny
>> would allow for multple event queues but did not enforce
>> them. [They appear unnecessary on win32 even if it would support
>> multi-head. And they are obviously unnecessary for any gdk port
>> which is bound to single-head through real backend 'limitations'
>> (linux-fb, directfb ?).]
>
>The hierarchy we have here is:
>
> * GdkDisplay ... a set of monitors on one system with an attached 
>   keyboard and mouse.
>
> * GdkScreen - a separate virtual area of one or more monitors,
>   with it's own set of colormaps and visuals.
>
> * gdk_screen_get_n_monitors() [etc]... find out how many monitors
>   a screen is made
>
>To my knowledge, win32 can "only" do multiple monitors, so I
>don't think there is ever going to be multiple screens or
>multiple displays for win32.
>
Thanks for the clarification. Could I have read it anywhere in the
docs or on the mailing-list ?
Anyway: If different Displays allow different depths, but
different Monitors on a single Screen do not, the current
model does not fit the windoze multiple-head definition 
at all [as long as I've understood the MSDN docs].
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/gdi/monitor
_82yb.asp

>But I do think that it  makes sense that if you have a separate
>display ... a separate keyboard and mouse ... then you have a
>separate event queue.
>
Yes. But we appear to agree that this is only possible with
X Windows at the moment ?
If so: removing the Display* parameter from the common 
_gdk_event_*() API would at least save all the single headed
backends an additional indirection.

>In the case of a port supporting only a single GdkDisplay, they
>can just ignore the GdkDisplay * parameters to 
>gdk_display_put_event() and so forth, and use an event queue.
> 
Again, not really. Look at gdk/win32/gdkevents-win32.c in my patch.
And the implementation in gdk/gdkvents.c
 
>> If we could agree on single-head being the common case (for which
>> to optimize) and multi-head being advanced where some extra
>> code doesn't matter that much, further improvements to the
>> backend independent code are possible, e.g. by allowing
>> display and screen to be NULL in the api and maybe totally 
>> eliminate the need to have GdkScreenSingle and 
>> GdkDisplaySingle classes ...
>
>I'm not sure what you mean by 'optimize' here. 
> 
> - Optimize work for backend authors. As I said above, I think
>   the most important thing is to optimize on-going maintainence,
>   rather than to optimize the amount of work that we have to
>   do right now.
>
As noted above: every change required for every backend may break 
it forever (or at least for a long time). I currently can't 
imagine the next change which breaks all backends but having 
some statement that we actually do care about least possible 
impact on backend authors would be nice.

> - Optimize efficiency? I don't see any significant efficiency
>   gains that are possible by prioritizing single head.
>
Not having to call gdk_get_default_display () *_screen ()
doesn't matter that much, but why not try to avoid it if
it isn't necessary. That's what I meant with gdk_event_*()-
API-Display removing and nuking them from GdkColormap and
GdkVisual. [It would also have the nice side effect of
reducing required changes to current backend code.]

> - Optimize for app authors? None of the non-multihead API has
>   been removed, so it's possible to write non-multihead-aware
>   apps and widget just as before.
>
I noticed it and it probably should remain for binary compatibility
for a long long time (?). That's what I had in mind with
the <GdkScreen/GdkDisplay>Single classes.

>   I've tried the allow NULL to mean the default before in various
>   places and never been very happy with the result ... it 
>   gives you an odd looking API and obscures what is going
>   on for the user. Plus, it doesn't work in languages where
>   methods are really methods.
>
Yeah. Allowing NULL was just a thought. The simpler way would
be a factory created all gdk single screen/display singleton
or having the single head code implemented in the base class.

>Single case is a special case of multihead, if we try to organize the
>code the other way around, it will be more complicated and 
>messier ... a similar problem to having GtkCTree derive from
>GtkCList.
>
There is no computer problem which can't be solved with another 
indirection; beside the problem of too much indirections :-)

>> What do you think ?
>> And if you think all the above doenn't make much sense: Is
>> the patch in its current form ok to commit ?
>
>I'd really like to see something more like what I outlined
>in my mail... move the non-multihead variants into the common 
>code and only duplicate the multihead variants. 
>(With non-multihead backends ignoring the screen/display 
>arguments.)
>
Ignoring the screen/display arguments isn't at all possible
at the moment, i.e. see gdk/gdkevents.c :

GList*
_gdk_event_queue_find_first (GdkDisplay *display)
{
  GList *tmp_list = display->queued_events;
...

GList *
_gdk_event_queue_append (GdkDisplay *display,
			 GdkEvent   *event)
{
  display->queued_tail = g_list_append (display->queued_tail, event);
...


So there should be no commonly reusable <GdkScreen/GdkDisplay>Single
implementation ?
And there needs to be an event queue indirection for all the
backends though only one does sometimes need it ?

If the above arguments still don't make my point clear do you
think a patch implementing the goals (but possibly breaking 
the X11 backend) could help ?

Regards,
	Hans
-------- Hans "at" Breuer "dot" Org -----------
Tell me what you need, and I'll tell you how to 
get along without it.                -- Dilbert