[RFC] Patch series: switching to internal events

Peter Hutterer peter.hutterer at who-t.net
Wed Feb 4 15:34:49 PST 2009


On Wed, Feb 04, 2009 at 12:06:45PM +0100, Simon Thum wrote:
> looks pretty good to me.
> 
> One tiny nit however: E.g. in eventconvert.c there are switch statements
> which go like
> 
> switch(EventType){
> ...big list of labels for one thing...: do it; break;
> }
> 
> So essentially, I guess, you're checking for some commonality, or
> property. Centralizing such checks is one of the benefits enabled by
> your approach (less if/else/misc). IMO it would be a good thing to show
> that "you're right" by reducing such examples. Maybe by introducing a
> 'event class' or similar, to be stored with the event.

Main reason: readability. It's very easy to see which events are affected by
this. 

I don't think event classes would help here. There's bits that only work on
button events, others on key events, others on all pointer events (including
proximity, but not necessarily enter/leave) and so on and so forth.
So we might end up having 5 classes for 7 event types which isn't quite the
point of the exercise either.

I'm happy to be wrong on that, but I'll leave that for later.
Thanks much for the review.
 
Cheers,
  Peter


> Peter Hutterer wrote:
> > git://people.freedesktop.org/~whot/xserver.git internal-events
> > 
> > The current X server implementation uses the protocol wire format for input
> > event processing. This goes from event generation in GetPointerEvents() and
> > friends through to the actual event delivery.
> > 
> > Thus, the server is bound internally to a 32-byte wire format + the two
> > GenericEvents we have atm, with random if/then/else/other sprinkled to deal
> > with core, XI and GenericEvents. Adding new events for XI2 is painful, as we
> > have to extrapolate new information from structs that don't have them. And
> > sprinkle more if/else/misc across the code.
> > 
> > This patch series introduces a new InternalEvent, visible only to the server.
> > Event generation and most of the event processing now only uses this
> > InternalEvent.
> > Towards the end of event delivery, we switch back into core/XI events.
> > Arguably, this could be pushed even further but requires a rework of the mess
> > that constitutes event masks.
> > 
> > I've been running versions of it during development, and put some effort in to
> > make all commits run-able (for future bisecting). They're rebased onto today's
> > master.
> > 
> > Casualties: custom event handlers, DGA, event callbacks are broken for now.
> > No reason they can't be fixed, I just didn't bother yet.
> > 
> > Comments and testing much appreciated.
> > 
> > I'm not sure how to continue from there. The options are
> > - merge into master, keep working on master
> > - keep rebasing as non-fast-forward on external branch on my people repo
> > - push into a public branch and merge into master when complete.
> > 
> > Cheers,
> >   Peter



More information about the xorg mailing list