[PATCH 1/3] fixesproto: Add a query to get the screen layout.

[Keith Packard]

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.

>  			    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?

> +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.

> +/*************** 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 -- and you can't use xXFixesScreenLayoutRec in the structure,
that's not portable C. Looks like there are other errors of this type in
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?

> +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.

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.x.org/archives/xorg-devel/attachments/20090728/385d0dad/attachment-0001.pgp