[PATCH xserver] dix: Move {Change, Copy, Destroy}Clip from GCFuncs to Screen

[Keith Packard]

Adam Jackson <ajax at redhat.com> writes:

> On Wed, 2017-03-01 at 00:07 -0800, Keith Packard wrote:
>> > Eric Anholt <eric at anholt.net> writes:
>> 
>> > I tried to come up with a reason I would want a layer to be able to
>> > change the funcs per-gc at runtime, and failed.  Yay for killing
>> > boilerplate.
>> 
>> So, if we're changing wrappers, anyone want to consider a more
>> outlandish plan, like replacing the current ad-hoc plan of stashing the
>> current value away in a screen private and writing your own version?
>
> Mmm. Maybe? I think I'd want to approach it with a clear idea of what
> we're trying to improve though, and I suspect if we did that we'd
> discover that the problem is not wrapping as a technique per se.

Hrm. Possibly? Being able to interpose code along particular paths has
proven a useful technique for development, and I think we should plan on
preserving that semantic, but trying to make it a bit more flexible and
less error prone.

> The discussion about dix-level damage, for example: the problem being
> solved there is that you don't want to encapsulate that state in a
> layer, you want it across layers.

Good point; damage isn't best served by interposing code as the
semantics below damage should be different than the semantics above.

> CloseScreen is an awful interface, yes. It's also one of the few screen
> interfaces that every driver wraps, so it'll be a little painful to
> clean up.

We could add a callback interface and migrate code over a bit at a time,
of course.
>
>> For the real wrappers, I think we can start with some requirements:
>> 
>>  1) Remove wrappers at any time.
>> 
>>  2) Insert at the 'right place' in the chain, independent of where the
>>     wrapping happens.
>> 
>>  3) "impossible" to get wrong, so no cult-n-paste code
>>     needed for every step of wrapping
>
> I'm not really sold on the need for 1. Where would that be useful?

Right now, the only place you can unwrap is while the wrapper is
running. That means you can't decide to stop catching a pile of screen
funcs all at once, you have to unwrap each one as invoked. Ick.

> I don't understand what you mean by 2.

I'd like to be able to insert a set of wrappers after the server has
started, but currently there's no way to make sure they get into the
chain at the right place. What we end up doing is adding wrappers during
server init and leaving them in place.

> 3 is... probably good? We could at least have a standard set of macros
> for the wrap/unwrap convention that would get things right like "be
> sure to re-stash the fptr from the layer below every time".

Yeah, I was thinking more along the lines of making the wrapping chains
explicit so you aren't mashing function pointers around on the fly. Not
sure how this should work though.

> It's close. Xinerama wraps closest to dispatch, if it's enabled. vga
> arb wraps in between damage and accel. rootless lives at the accel
> layer, but all it does is coerce window planemasks to preserve alpha.
> And 'fb' is fb or wfb or nest/dmx where the latter two don't hit mi at
> all.

Yeah, maybe we just need an array of function pointers per entry point,
and then calling down is a simple matter of pulling the function pointer
out of the next layer down.

#define SCREEN_LAYER_DIX   	0
#define SCREEN_LAYER_DAMAGE     1
#define SCREEN_LAYER_ACCEL      2
#define SCREEN_LAYER_FB         3
#define SCREEN_LAYER_MI         4
#define NUM_SCREEN_LAYERS       5

typedef struct _Screen {
        ...
        CreateGCProcPtr CreateGC[NUM_SCREEN_LAYERS];
        ...
} ScreenRec;

void
set_create_gc_func(ScreenPtr screen, int layer, CreateGCProcPtr proc) {
        assert (layer < NUM_SCREEN_LAYERS);
        assert (screen->CreateGC[layer] != proc);

        CreateGCProcPtr prev = screen->CreateGC[layer];

        while (layer >= 0 && screen->CreateGC[layer] == prev) {
                screen->CreateGC[layer] = proc;
                --layer;
        }
}

void
clear_create_gc_func(ScreenPtr screen, int layer) {

        assert (layer < NUM_SCREEN_LAYERS - 1);

        CreageGCProcPtr this = screen->CreateGC[layer];
        CreateGCProcPtr next = screen->CreateGC[layer + 1];

        assert (next != NULL); /* Make sure there's something left to call */
        assert (next != this); /* Make sure the function is currently wrapped */

        while (layer >= 0 && screen->CreateGC[layer] == this) {
                screen->CreateGC[layer] = next;
                --layer;
        }
}

Then, DIX would use screen->CreateGC[0], glamor would use
screen->CreateGC[2], and when glamor wanted to call down to a software
layer, it would use screen->CreateGC[3].

I'm not sure the ordering is correct or complete, and of course we could
use AOS instead of SOA for the function pointers.

I just cooked this up; I was thinking before about having linked lists
or something, but arrays have a nice simplicity about them...

-- 
-keith
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <https://lists.x.org/archives/xorg-devel/attachments/20170301/31a27c1b/attachment.sig>