[PATCH 1/3] fixesproto: Add a query to get the screen layout.
Aaron Plattner
aplattner at nvidia.com
Tue Jul 28 22:50:29 PDT 2009
On Tue, Jul 28, 2009 at 10:34:52PM -0700, Keith Packard wrote:
> * PGP Signed by an unknown key
>
> On Tue, 2009-07-28 at 21:53 -0700, Aaron Plattner wrote:
>
> > @@ -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
>
> Document revision should go back to 0 for the new major version.
Ah, okay, thanks. Will do.
> > 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
>
> So, the first question is why this is in XFixes and not in RandR --
> should it go there instead?
I didn't think RandR really dealt with multiple screens, other than
allowing you to address them. It really deals with the positions of
outputs within a single X screen, and not with the relative positions of
the X screens themselves.
Perhaps when we have our grand unified RandR/Xinerama/Shatter
implementation, it will solve all of these problems. In the meantime, it
doesn't really matter where this request goes, it just doesn't really seem
like it belongs in RandR.
> > +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.
>
> Is there some reason this doesn't also allow this data to be changed at
> run-time? From what I can see, that should be trivial and would
> eliminate yet another 'must restart' case in the X server.
Just poking the data into dixScreenOrigins wasn't enough to change the
cursor behavior. I'd have to make more invasive updates into the rest of
the server and I didn't have the time to look at what would be required.
Do you think it would be worth adding a request to change it, and then just
return BadImplementation for now?
> > +/*************** 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;
> > +
> > +#define sz_xXFixesGetScreenLayoutReply sizeof(xXFixesGetScreenLayoutReply)
>
> You have to use an explicit size (no sizeof(foo)) for every request I'm
> afraid
Okay, will do. I'll fix the rest of the requests in a separate change.
> -- and you can't use xXFixesScreenLayoutRec in the structure,
> that's not portable C. Looks like there are other errors of this type in
Lame!
> the file. The reason for explicit sizes is that some requests are *not*
> a multiple of 64-bits long, and so sizeof will give the wrong answer. Of
> course, I encourage you to make all requests a multiple of 64-bits so
> that Xlib doesn't have to insert Noops to align the output buffer.
>
> > +
> > #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>
>
> What is Xmd.h required for here? And how did it work without this
> before?
It was needed for CARD32/CARD16 in the xXFixesScreenLayoutRec structure.
It looks like I'm going to have to create a separate redundant
XFixesScreenLayout structure and add dumb translation code to the library,
so I'll move this into xfixesproto.h and get rid of the Xmd.h include.
> > +typedef struct {
> > + CARD32 x B32;
> > + CARD32 y B32;
> > + CARD16 positionType B16;
> > + CARD16 refScreen B16;
> > +} xXFixesScreenLayoutRec;
>
> You'll probably want to make this a multiple of 64 bits long, and then
> also provide a sz_xXFixesScreenLayoutRec for the Xlib code to use.
>
> Also, the tradition has been to use minimal datatypes for each object,
> so positionType should probably be a CARD8. You'll then need to place
> explicit padding entries to ensure that everything sits on a natural
> boundary and that the whole structure is a multiple of 64 bits long.
Okay. Thanks for your feedback.
More information about the xorg-devel
mailing list