Document how serial should work
Submitted by Pekka Paalanen
Assigned to Daniel Stone @daniels
Description
There is confusion on what the basic principles on using serials, at least in Weston, should be. Sure, Weston needs lots of fixes, but we should also document how a compositor is expected to use serials, and especially the wl_display_next_serial() API.
< pq> does anyone really understand how the serial (wl_display_{get,next}_serial()) system is *supposed* to be used?
< pq> when to use next_serial() vs. get_serial()
< pq> how to compare serials
< pq> when to record a valid serial for later comparison, etc.
< pq> and where to get the serial value to deliver to clients with input and other events: current value from wl_display or some custom saved value?
< pq> how do multiple wl_seats affect these?
< pq> does a pointer button-down on one wl_seat "invalidate" the button-down on another wl_seat?
< daniels> no
< daniels> ... is the short answer :P
< pq> ok, so I think all the serial tracking is at least broken to multiple wl_seats, which does not surprise me, but I think it might be broken even more
< daniels> i mean, if you have two seats (A & B) acting independently delivering to independent windows, window Y gets a button down from seat A and window Z gets a button down from seat B later, wl_shell_surface::resize with serial A on window Y should still work
< daniels> i'm absolutely astounded that multiple seats in weston work as well as they do
< daniels> it was only ever half-finished at best
< daniels> significant input events (button, key, touch) should always use next_serial; iirc, the only chaining (same-serial) event is key followed by modifier
< daniels> the intention being that it's sometimes nice to be able to relate modifier changes back to the provoking key event
< pq> and motion
< pq> okay
< daniels> right, motion isn't a significant event
< daniels> so it always sticks with current serial
< pq> but if I understood the code at all, it is sending the wl_display's current seial value to clients in events
< pq> the current value can be bumped by pretty much anything
< daniels> modifier will use the serial of a key event if it was directly provoked by that, or the next serial if it happened independently of a key event
< daniels> hmm, what bumps it other than button/key/touch?
< pq> if the serial checks compare for equal, a client might be using a serial that is too *new* and get rejected by that
< daniels> in which scenario?
< pq> I didn't identify an exact scenario yet, I was just looking at the grep results for wl_display_next_serial() and grab_serial
< daniels> do we ever bump the serial outside of button/key/touch? (don't have the source to hand, sorry)
< daniels> but yes, storing one _global_ value is going to break multi-seat completely
< daniels> or at least, multiple seats independently interacting with different surfaces
< pq> yes, a lot of places
< pq> http://pastie.org/pastes/9526487/text
< pq> one of the presumably latest additions is xdg_send_configure(), which wants to use a totally unrelated serial to match configure to ack_configure
< daniels> ugh, yes
< daniels> that's the main one i can see which is objectively broken
< pq> we only have one serial source counter in wl_display, so I think we should be wl_display_get_serial()'ing a lot less than we do now
< daniels> that really needs to be in a separate namespace
< pq> well, wouldn't we need one serial generator per wl_seat?
< pq> I mean namespace
< daniels> bumping serial on focus change is definitely broken too, though i guess probably inadvertently gets us the behaviour we want :P
< daniels> i.e. if you click on the titlebar of an unfocused window, you don't want to start a move
< daniels> so it might be that clients are trying to start moves with the button-down serial, which then gets rejected because focus is newer :P
< daniels> namespace/generator no, comparator yes
< daniels> i.e. store last_serial in wl_seat rather than wl_display, and use that to compare against
< pq> http://pastie.org/pastes/9526493/text for get_serial usage
< daniels> keymap bumping serial is a little questionable too, but trying to relate events across keymap changes is ... ugh
< pq> daniels, serials are already stored separately to be compared against, but the serials sent to clients are retrieved from wl_display, which may have advanced for unrelated reasons
< daniels> hmmm, yes
< daniels> right, so get_serial should also be retrieving from wl_seat where it relates to that
< pq> one question I had was, should the comparison be for equal, or greater or equal?
< daniels> ==
< daniels> if you're sending a serial greater than the last significant one, you've screwed something up
< pq> okay, then definitely one should be sending saved serials, not from wl_display
< daniels> hmm, i think we can actually keep using next_serial - we just need to deprecate get_serial on wl_display, and instead require that everyone who needs a serial store it somewhere appropriate, e.g. in the shell info structure