tych0 / xcffib

A drop-in replacement for xpyb based on cffi
Apache License 2.0
94 stars 26 forks source link

Patching xcb-proto for `GeGenericEvent` #125

Closed elParaguayo closed 1 year ago

elParaguayo commented 2 years ago

@tych0 @flacjacket I'm hoping one of you can provide some pointers here.

Following on from #99, I've been trying to implement a fix. I started off updating the GeGenericEvent definition in xbc-xprotoo/src/xproto.xml, adding definitions for type, extension, sequence_number, length and event_type. However, when I trigger an event in qtile, I get the new fields, but I also get some extra ones: {'response_type': 35, 'sequence': 3029, 'type': 131, 'extension': 9, 'sequence_number': 0, 'length': 33560832, 'event_type': 63744, 'bufsize': 13}. response_type, sequence and bufsize are not in the XML definition and so are being added by something else.

Looking at the Event class definition in xcb-xproto/xcbgen/xtypes I can see:

    def resolve(self, module):
        def add_event_header():
            self.fields.append(Field(tcard8, tcard8.name, 'response_type', False, True, True))
            if self.has_seq:
                self.fields.append(_placeholder_byte)
                self.fields.append(Field(tcard16, tcard16.name, 'sequence', False, True, True))

        def add_ge_event_header():
            self.fields.append(Field(tcard8,  tcard8.name,  'response_type', False, True, True))
            self.fields.append(Field(tcard8,  tcard8.name,  'extension', False, True, True))
            self.fields.append(Field(tcard16, tcard16.name, 'sequence', False, True, True))
            self.fields.append(Field(tcard32, tcard32.name, 'length', False, True, True))
            self.fields.append(Field(tcard16, tcard16.name, 'event_type', False, True, True))

        if self.resolved:
            return

        # Add the automatic protocol fields
        if self.is_ge_event:
            add_ge_event_header()
        else:
            add_event_header()

        ComplexType.resolve(self, module)

so it looks like the fields are being added by add_event_header. However, the xml file includes the xge attribute so it looks like add_ge_event_header should be being called instead.

This is where I get stuck.

I'd like to debug and see why the add_ge_event_header function is not being called but I've no idea when it's actually called.

Building xcffib with make XCBDIR=../xcb-proto/src xcffib doesn't seem to result in any of the xcb-proto/xcbgen methods being called and none of my changes are reflected (e.g. changing the field names added by add_event_header).

Can either of you help me piece together how xbb-proto and xcffib are related and how I can make my changes to xcbgen take effect when building xcffib.

Thanks.

tych0 commented 2 years ago

response_type, sequence and bufsize are not in the XML definition and so are being added by something else.

these are "internal" xcffib things, they are the physical memory response buffer, its size, and the xcb sequence number of the request that the response is for. I think any name collision with things in xcb-proto is purely a coincidence.

Building xcffib with make XCBDIR=../xcb-proto/src xcffib doesn't seem to result in any of the xcb-proto/xcbgen methods being called and none of my changes are reflected (e.g. changing the field names added by add_event_header).

yep, this is because the haskell code fulfills the role of the xcbgen code, so we don't necessarily invoke any of it.

(sorry for the delay, been a bit of a crazy week here. let me go read your other thread now)

elParaguayo commented 2 years ago

Thanks.

It feels like the xml definition in xcb-xproto is correct as that python code looks like it injects the fields that would otherwise be there. So, if we're ignoring that xcbgen stuff, we probably need to patch in xcffib as a change to the xml file wouldn't get merged.

elParaguayo commented 2 years ago

Ah, so maybe I could add the xcb_ge_generic_event_t definition type in xcffib and, if the event_type indicates a GeGenericEvent then we cast to that instead of xcb_generic_event_t.

tych0 commented 2 years ago

i think you could get a change to the xml merged, it would just take some work outside of xcffib. but probably necessary for things going forward.

i think it's entirely possible (likely even?) that the only changes you'll need to get this to work would be to the ge.xml file in xcb-proto. assuming it uses field types and stuff that the generator already understands, you should just be able to fill out the xml, re-run the haskell generator, and see your new stuff show up in ge.py

tych0 commented 2 years ago

Ah, so maybe I could add the xcb_ge_generic_event_t definition type in xcffib and, if the event_type indicates a GeGenericEvent then we cast to that instead of xcb_generic_event_t.

Yep, some python equivalent of that. IIRC most of that magic is autogenerated, though.

elParaguayo commented 2 years ago

i think you could get a change to the xml merged, it would just take some work outside of xcffib. but probably necessary for things going forward.

i think it's entirely possible (likely even?) that the only changes you'll need to get this to work would be to the ge.xml file in xcb-proto. assuming it uses field types and stuff that the generator already understands, you should just be able to fill out the xml, re-run the haskell generator, and see your new stuff show up in ge.py

But the GeGenericEvent is in xtypes.xml. Not sure how ge.xml helps. Sorry if I'm being dumb!

tych0 commented 2 years ago

But the GeGenericEvent is in xtypes.xml. Not sure how ge.xml helps. Sorry if I'm being dumb!

Oh, no, you're not. I just assumed that stuff with ge in the name would be defined in ge.xml :). xtypes.xml it is, then.

tych0 commented 2 years ago

btw, I am subscribed to the xcb list and generally people there are helpful, so I wouldn't shy away from sending patches there.

elParaguayo commented 2 years ago

But the GeGenericEvent is in xtypes.xml. Not sure how ge.xml helps. Sorry if I'm being dumb!

Oh, no, you're not. I just assumed that stuff with ge in the name would be defined in ge.xml :). xtypes.xml it is, then.

Good!

The trouble with patching that is that it looks like they add the fields via that xtypes.py file, rather than the XML, so patching the XML could cause files for anything that's relying on xcbgen but, at the same time, xcffib needs the XML to have all the fields defined than being added dynamically by xtypes.

This is hard.

tych0 commented 2 years ago

they add the fields via that xtypes.py file, rather than the XML, so patching the XML could cause files for anything that's relying on xcbgen

You could maybe drop those bits from xcbgen if they do interfere; the major user of xcbgen that I'm aware of would have been xpyb, and that project is dead: https://gitlab.freedesktop.org/xorg/lib/xpyb/-/blob/master/README#L6 but maybe there are others (I guess probably the C bindings use it?). Their CI should tell you if it breaks things too badly though... right? :D

elParaguayo commented 2 years ago

Yeah. Worth I try. Still need to stop those other fields being added to the event as that's happening separately to the XML definition.

elParaguayo commented 2 years ago

OK - some progress. Getting the right fields on the GeGenericEvent object. That gives me the extension's opcode and the event ID. Now I just need to work out how to search installed extensions for those values...

elParaguayo commented 2 years ago

How can I list events for a given extension in xcffib?

Connection has an _event_offsets attribute which is populated via the _add_ext function but this seems to be called from some haskell code. I've never used haskell and I can't understand where it's pulling the event list from.

I was also hoping to use the Connection._event_offsets to find the right event but I'm having some issues there too. That seems to be a list of tuples where the first field is first_event field from the extension and the second is a dict of id and the event. I don't think I can use this for GeGenericEvents as what I actually get is the extension's opcode and the event id. (Interestingly, in Qtile, this list does not contain every extension and, if I try to add some, it looks like there's some overlap with offsets - e.g. BarrierHit is in Xinput - the first_event is 66 and the event is 25 so that's an offset of 91 but RandR's first_event is 89)

For now, I can play around with creating a different dictionary using the extension's major_opcode as the key but I can't work out where the events come from.

EDIT: Never mine, found it!

elParaguayo commented 2 years ago

OK - finally got my head round this.

Everything seems to be working. I'm able to hoist GeGenericEvents to the relevant extension event.

I'll see if xcb-xproto tests still pass with the changes and, if so, I'll make some PRs.

elParaguayo commented 1 year ago

This is sorted now.