[Wayland-bugs] [Bug 83488] New: Document how serial should work

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=83488

          Priority: medium
            Bug ID: 83488
          Assignee: wayland-bugs at lists.freedesktop.org
            Blocks: 83429
           Summary: Document how serial should work
          Severity: enhancement
    Classification: Unclassified
                OS: All
          Reporter: ppaalanen at gmail.com
          Hardware: All
            Status: NEW
           Version: unspecified
         Component: wayland
           Product: Wayland

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

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-bugs/attachments/20140904/c0edddca/attachment-0001.html>