internal screen concept

Dave Airlie airlied at gmail.com
Wed Mar 23 03:30:21 PDT 2011


On Wed, Mar 23, 2011 at 6:39 PM, Keith Packard <keithp at keithp.com> wrote:
> On Wed, 23 Mar 2011 16:03:59 +1000, Dave Airlie <airlied at gmail.com> wrote:
>
>> All it does is split the protocol screen struct out from the screen struct
>> and reworks the code to get the screen struct via the protocol screen
>> in all the right places. I may have gone a step too far with allowing the
>> 1:n mapping between protocol screens and screens, but for xinerama
>> this does allow a few cleanups.
>
> Would be nice to have the protocolstr.h file

Oh oops I must have git reset once too often.

>
> However, looking at the code, I'm not sure I get the big plan. Things
> like the new connection setup code assume that for each protocol screen,
> the client visible information will be precisely mirrored in the 0th DDX
> screen in that object. Are we sure we want to do that? Would it be
> better to have the client-visible screen information directly present in
> the protocol screen?

Yeah I was trying to get there in easier proveable steps.without doing
a big change.

Moving the information out into the protocolscreen was a step or two
ahead, since
I was going to have the audit the drivers for any abuse or information
that we think
is protocol but they think is theirs.

> Similarly, for input, we want to hide DDX screens from all of the input
> code; they aren't relevant at all.

> Here's what I expect to see the ProtocolScreen and Screen look like:
>
> typedef struct _ProtocolScreen {
>    int                 myNum;  /* index of this instance in ProtocolScreens[] */
>    short               x, y, width, height;
>    short               mmWidth, mmHeight;
>    short               numDepths;
>    unsigned char       rootDepth;
>    DepthPtr            allowedDepths;
>    unsigned long       rootVisual;
>    unsigned long       defColormap;
>    short               minInstalledCmaps, maxInstalledCmaps;
>    char                backingStoreSupport, saveUnderSupport;
>    unsigned long       whitePixel, blackPixel;
>    short               numVisuals;
>    VisualPtr           visuals;
>    WindowPtr           root;
>    int                 num_screens;
>    ScreenPtr           *screens;
> } ProtocolScreenRec;
>
> typedef struct _Screen {
>    int                 myNum;  /* index of this instance in Screens[] */
>    short               x, y, width, height;
>
>    ProtocolScreenPtr   *protocol_screen;
>
>    GCPtr               GCperDepth[MAXFORMATS+1];
>                        /* next field is a stipple to use as default in
>                           a GC.  we don't build default tiles of all depths
>                           because they are likely to be of a color
>                           different from the default fg pixel, so
>                           we don't win anything by building
>                           a standard one.
>                        */
>    PixmapPtr           PixmapPerDepth[1];
>    pointer             devPrivate;
>    ScreenSaverStuffRec screensaver;
>
>    ... A million function pointers ...
>
>    /* set it in driver side if X server can copy the framebuffer content.
>     * Meant to be used together with '-background none' option, avoiding
>     * malicious users to steal framebuffer's content if that would be the
>     * default */
>    Bool                canDoBGNoneRoot;
> } ScreenRec;

Yes that looks like where I'll end up once I get the bisectable/reviewable bits,
the new process makes doing big changes like that nearly impossible as
nobody will be able to review it.

Dave.


More information about the xorg-devel mailing list