[RFC PATCH] CreateRootWindow: Set root windows' drawable.x/y to -pScreen->x/y.

[Jamey Sharp]

On Thu, Jun 10, 2010 at 6:59 PM, Keith Packard <keithp at keithp.com> wrote:
> As you might imagine, there are a million corner cases with tile origins
> that could be broken here. Certainly the basic idea of making
> drawable.x/drawable.y correct for tiling has a lot of merit.
>
> Fortunately, the test suite should be able to catch errors of that
> nature, so making sure that works would be a good first step.

I guess I'd need to run the test suite on a Xinerama server, and
probably need to ensure that windows that XTS creates span two
screens. Or something? I'm not sure how to even hit these cases,
especially in XTS.

> On Thu, 10 Jun 2010 15:37:40 -0700, Jamey Sharp <jamey at minilop.net> wrote:
>> If this approach is good, I have or can write patches to move ValidateGC
>> and DestroyGC from GCFuncs to ScreenRec, and eliminate the other five
>> GCFuncs. The patches I have along those lines so far kill 730 lines of
>> code, on top of the 800 removed here, and promise more to come.
>
> One of the benefits of having the GC funcs in a shared structure is that
> you can unwrap all of the ops and funcs with two pointer exchanges,
> which makes calling down to the next layer fairly inexpensive.

That remains true: AFAICT, ValidateGC is the only function that needs
to be unwrapped together with the GC ops, so it should be a single
pointer exchange no matter where it lives. Certainly a GC op had
better not destroy the GC it is operating on, and I believe all the
other GCFuncs can be deleted outright (with a small quantity of
cleverness).

In my opinion it would be better to arrange that GC ops never need to
unwrap ValidateGC than to continue with the current double-unwrapping
mess. I think it's easy enough to ensure that every ValidateGC call
inside a GC op is on a scratch GC, although setting up the scratch GC
efficiently might be harder. I have part of a patch written that does
this, just copying all elements of the real GC into the scratch GC.

The current scheme is crazy. We call ValidateGC because we want the
ops updated for whatever is suitable for our hardware--but in the
cases where double-unwrapping matters, AFAICT we're guaranteed to get
an mi version of all ops. That's surely not desirable, and I'm not
even convinced it's correct!

One reason I'd like to move ValidateGC in particular to the ScreenRec
is that several CreateGC wrappers exist solely to set up a ValidateGC
wrapper, and those always use the same implementation of ValidateGC
regardless of any parameters in the GC. If you can set the ValidateGC
wrapper on the screen in the first place, the currently piles of code
just look silly.

The other reason is I'm hoping to kill the PanoramiXRes structure and
have a single GC/Drawable/Colormap/etc. object allocated for each
protocol-visible XID. I don't know exactly what that would look like
(I can sketch pieces if anyone's curious), but I think it's easier to
get there with as little screen-dependent data in each object as
possible. I guess GCOps are worth keeping inside the GC, assuming
somebody cares about hardware acceleration for those ops, but I can't
see any reason for GCFuncs to live there.

> Then there are a few functions, like CloseScreen, which need to be
> replaced with a callback instead of wrapping. It would be cool if we
> identified two classes of functions, wrapped drawing functions and
> callback state change functions.

That makes sense to me (for DestroyGC and such as well) but I think
it's completely orthogonal to what I'm proposing. For example, I claim
the hypothetical  DestroyGC callback chain should be in the ScreenRec,
not the GC, and especially not in some GCFuncs-like structure pointed
to by the GC.

I can send along my patch to move DestroyGC to ScreenRec if you'd like
to base a callback-chain patch on that.

Jamey