[PATCH 1/3] fixesproto: Add a query to get the screen layout.
Aaron Plattner
aplattner at nvidia.com
Tue Jul 28 22:44:45 PDT 2009
On Tue, Jul 28, 2009 at 10:19:55PM -0700, Peter Hutterer wrote:
> On Tue, Jul 28, 2009 at 09:53:23PM -0700, Aaron Plattner wrote:
> > ---
> > configure.ac | 2 +-
> > fixesproto.txt | 41 +++++++++++++++++++++++++++++++++++++----
> > xfixesproto.h | 27 +++++++++++++++++++++++++++
> > xfixeswire.h | 27 +++++++++++++++++++++++++--
> > 4 files changed, 90 insertions(+), 7 deletions(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 0d422d4..1c9136e 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -24,7 +24,7 @@ dnl
> > dnl Process this file with autoconf to create configure.
> >
> > AC_PREREQ([2.57])
> > -AC_INIT([FixesProto], [4.1], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
> > +AC_INIT([FixesProto], [5.0], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
> > AM_INIT_AUTOMAKE([foreign dist-bzip2])
> > AM_MAINTAINER_MODE
> >
> > diff --git a/fixesproto.txt b/fixesproto.txt
> > index ededc8a..57b4dbe 100644
> > --- a/fixesproto.txt
> > +++ b/fixesproto.txt
> > @@ -1,7 +1,7 @@
> > The XFIXES Extension
> > - Version 4.0
> > - Document Revision 1
> > - 2006-12-14
> > + Version 5.0
> > + Document Revision 2
> > + 2009-07-27
> > Keith Packard
> > keithp at keithp.com
> >
> > @@ -29,6 +29,8 @@ developers, in particular,
> >
> > + Deron Johnson for cursor visibility
> >
> > + + Valery Moya for X screen origin queries
> > +
> > 3. Basic Premise
> >
> > Requests in this extension may seem to wander all over the map of X server
> > @@ -553,7 +555,38 @@ ShowCursor
> > If the client has made no outstanding HideCursor requests
> > a BadMatch error is generated.
> >
> > -
> > +************* XFIXES VERSION 5 OR BETTER ***********
> > +
> > +12. Screen Layout
> > +
> > +X configurator programs may want to know the relative positions of X screens in
> > +a multiple screen layout. This configuration is described in the ServerLayout
> > +section of xorg.conf and affects how the cursor moves from screen to screen when
> > +it hits screen edges. Configuration programs need to know this information so
> > +they can display it to users and build configuration files from the current
> > +screen layout.
> > +
> > +12.1 Types
> > +
> > + POSTYPE { PosAbsolute, PosLeftOf, PosAbove, PosBelow, PosRelative }
> > + LAYOUT [
> > + x, y: CARD32 absolute screen origin position
> > + positionType: POSTYPE relative position type
> > + relScreen: CARD16 relative screen
> > + ]
>
> I think the protocol spec needs to describe which values are valid in which
> combination. Right now, I'm guessing that if PosAbsolute is the type, then
> x/y is an absolute position relative to the virtual screen size (?) and and
> position type is relative to the screen but ignores the coordinates? Point
> is though, that I'm just guessing here.
>
> I know that at least some of these combinations have been used in the
> xorg.conf for a while now and may be considered common knowledge but
> nonetheless it should be explicitly stated.
Good point, I'll update it with that information.
> > +12.2 Requests
> > +
> > +GetScreenLayout
> > +
> > + ->
> > + layout: LISTofLAYOUT
> > +
> > + The GetScreenLayout reply contains one layout description structure for each
> > + X screen.
>
> Where do you get the number of screens from? IMO the request/reply should be
> consistent within itself, i.e. the reply should tell you how many layouts
> trail the reply (num_layouts or something). You can deduct that from the
> length field, but having it explicitly listed doesn't hurt.
It's hard-coded in the display connection after the library handshakes with
the server. You can get it with ScreenCount (which is just a macro for
(((_XPrivDisplay)dpy)->nscreens). This count will not change and including
it in the protocol is just redundant and adds another potential point of
failure.
> > +
> > + Errors: none
> > +
> > 99. Future compatibility
> >
> > This extension is not expected to remain fixed. Future changes will
> > diff --git a/xfixesproto.h b/xfixesproto.h
> > index 545e325..074ee13 100644
> > --- a/xfixesproto.h
> > +++ b/xfixesproto.h
> > @@ -2,6 +2,7 @@
> > * $XFree86: xc/include/extensions/xfixesproto.h,v 1.1 2002/11/30 06:21:43 keithp Exp $
> > *
> > * Copyright ? 2006 Sun Microsystems
> > + * Copyright ? 2009 NVIDIA Corporation
> > *
> > * Permission to use, copy, modify, distribute, and sell this software and its
> > * documentation for any purpose is hereby granted without fee, provided that
> > @@ -500,6 +501,32 @@ typedef struct {
> >
> > #define sz_xXFixesShowCursorReq sizeof(xXFixesShowCursorReq)
> >
> > +/*************** Version 5.0 ******************/
> > +
> > +typedef struct {
> > + CARD8 reqType;
> > + CARD8 xfixesReqType;
> > + CARD16 length B16;
> > +} xXFixesGetScreenLayoutReq;
> > +
> > +#define sz_xXFixesGetScreenLayoutReq sizeof(xXFixesGetScreenLayoutReq)
> > +
> > +typedef struct {
> > + BYTE type; /* X_Reply */
> > + BYTE pad1;
> > + CARD16 sequenceNumber B16;
> > + CARD32 length B32;
> > + CARD32 pad2 B32;
> > + CARD32 pad3 B32;
> > + CARD32 pad4 B32;
> > + CARD32 pad5 B32;
> > + CARD32 pad6 B32;
> > + CARD32 pad7 B32;
> > + xXFixesScreenLayoutRec screens[];
> > +} xXFixesGetScreenLayoutReply;
>
> What does screens resolve to? A pointer? or is it optimised away? This looks
> a bit awkward to me, the traditional approach is to leave the trailing data
> structures out of the structure. Removing the line should suffice, the
> protocol spec specifies what's trailing.
It allows you to do this:
xXFixesGetScreenLayoutReply *rep = pointer-to-the-reply-buffer;
whatever = rep->screens[scrn].something;
instead of this:
xXFixesGetScreenLayoutReply *rep = pointer-to-the-reply-buffer;
xXFixesScreenLayoutRec *layouts = (xXFixesScreenLayoutRec*)(rep + 1);
whatever = layouts[scrn].something;
I.e. it simplifies the code by telling the compiler that you expect an
unspecified number of xXFixesScreenLayoutRec structures immediately after
the entires in the reply structure itself. However, Keith claims it isn't
portable C, so I'm going to have to rewrite it the ugly way anyway.
> > +#define sz_xXFixesGetScreenLayoutReply sizeof(xXFixesGetScreenLayoutReply)
> > +
> > #undef Region
> > #undef Picture
> > #undef Window
> > diff --git a/xfixeswire.h b/xfixeswire.h
> > index 6f20270..c8c0ca1 100644
> > --- a/xfixeswire.h
> > +++ b/xfixeswire.h
> > @@ -2,6 +2,7 @@
> > * $XFree86: xc/include/extensions/xfixeswire.h,v 1.1 2002/11/30 06:21:43 keithp Exp $
> > *
> > * Copyright ? 2006 Sun Microsystems
> > + * Copyright ? 2009 NVIDIA Corporation
> > *
> > * Permission to use, copy, modify, distribute, and sell this software and its
> > * documentation for any purpose is hereby granted without fee, provided that
> > @@ -47,9 +48,11 @@
> > #define _XFIXESWIRE_H_
> >
> > #define XFIXES_NAME "XFIXES"
> > -#define XFIXES_MAJOR 4
> > +#define XFIXES_MAJOR 5
> > #define XFIXES_MINOR 0
> >
> > +#include <X11/Xmd.h>
> > +
> > /*************** Version 1 ******************/
> > #define X_XFixesQueryVersion 0
> > #define X_XFixesChangeSaveSet 1
> > @@ -85,8 +88,10 @@
> > /*************** Version 4 ******************/
> > #define X_XFixesHideCursor 29
> > #define X_XFixesShowCursor 30
> > +/*************** Version 5 ******************/
> > +#define X_XFixesGetScreenLayout 31
> >
> > -#define XFixesNumberRequests (X_XFixesShowCursor+1)
> > +#define XFixesNumberRequests (X_XFixesGetScreenLayout+1)
> >
> > /* Selection events share one event number */
> > #define XFixesSelectionNotify 0
> > @@ -124,4 +129,22 @@
> > #define WindowRegionBounding 0
> > #define WindowRegionClip 1
> >
> > +/*************** Version 5 ******************/
> > +
> > +/* Screen position information options */
> > +
> > +#define XFixesPosAbsolute 0
> > +#define XFixesPosRightOf 1
> > +#define XFixesPosLeftOf 2
> > +#define XFixesPosAbove 3
> > +#define XFixesPosBelow 4
> > +#define XFixesPosRelative 5
> > +
> > +typedef struct {
> > + CARD32 x B32;
> > + CARD32 y B32;
> > + CARD16 positionType B16;
> > + CARD16 refScreen B16;
> > +} xXFixesScreenLayoutRec;
> > +
>
> This struct is part of the protocol, not the client data. Should be moved to
> xfixesproto.h (You can remove the Xmd.h include then too)
> This goes in hand with the libXfixes commit where this struct is simply
> typedef'd, IMO wrapping it properly is better as it separates protocol and
> library better.
I was hoping to avoid having to define two separate structures, one for the
protocol and one for the libXfixes client, and then having to write dumb
code that does this:
layout[i].x = replyLayout[i].x;
layout[i].y = replyLayout[i].y;
layout[i].positionType = replyLayout[i].positionType;
layout[i].refScreen = replyLayout[i].refScreen;
just because there are two separate structure definitions that just happen
to be the same.
> > #endif /* _XFIXESWIRE_H_ */
> > --
> > 1.6.0.4
>
> Cheers,
> Peter
More information about the xorg-devel
mailing list