On Mon, 30 Jul 2018 13:09:06 +0200
Jonas Ådahl <[email protected]> wrote:

> On Mon, Jul 30, 2018 at 12:57:52PM +0200, Dorota Czaplejewicz wrote:
> > On Mon, 30 Jul 2018 12:36:45 +0200
> > Jonas Ådahl <[email protected]> wrote:
> >   
> > > On Sat, Jul 28, 2018 at 08:17:56PM +0200, Dorota Czaplejewicz wrote:  
> > > > From: Carlos Garnacho <[email protected]>
> > > > 
> > > > This new protocol description is an evolution of v2.
> > > > 
> > > > - All pre-edit text styling is gone.
> > > > - Pre-edit cursor can span characters.
> > > > - No events regarding input panel (OSK) state nor covered rectangle.
> > > >   Compositors are still free to handle situations where the keyboard
> > > >   focus rectangle is covered by the input panel.
> > > > - No set_preferred_language request for clients.
> > > > - There is no event to send keysyms. Compositors can use wl_keyboard
> > > >   interface instead.
> > > > - All state is double-buffered, with specified defaults.
> > > > - The compositor can be notified about external changes to the state.
> > > > - The client can detect outdated requests.
> > > > 
> > > > Signed-off-by: Dorota Czaplejewicz <[email protected]>
> > > > Signed-off-by: Carlos Garnacho <[email protected]>
> > > > ---
> > > > Hello all,
> > > > 
> > > > this new change stems from real experiences developing an 
> > > > implementation of text-input. Under Carlos' guidance, I've been 
> > > > developing support in GTK3, as well as wlroots [0]. There were lessons 
> > > > to be learned, and they are incorporated into this new patch revision.
> > > > 
> > > > The one significant change consists of adding a serial number to each 
> > > > state sent by the client. The input method (e.g. the compositor) will 
> > > > then update its beliefs about the text field state, and send new 
> > > > requests, along with the serial number. This allows the client to 
> > > > detect requests based on outdated states and handle them in a special 
> > > > way, i.e. ignore requests for permanent changes.
> > > > 
> > > > At the same time, the enable request becomes effective only after a 
> > > > commit, to allow the client to send a state bundle together with it.
> > > > 
> > > > The second important change regards passing set_surrounding_text 
> > > > metadata in the set_text_change_cause request. It's necessary for the 
> > > > input method to understand when the user stopped using it to back off. 
> > > > The change_cause enumeration could be extended in the future with 
> > > > causes such as "navigation", "typing", or "redaction" to achieve better 
> > > > granularity than a catchall "other".
> > > > 
> > > > Apart from these changes, some descriptions have been clarified, and 
> > > > circumstances were under which set_surrounding_text should be sent were 
> > > > spelled out.
> > > > 
> > > > I hope that you enjoy the read!    
> > > 
> > > I've had a read through, and looks pretty solid to me. A few minor
> > > comments are inlined below.  
> > 
> > Hi, thanks for the review!
> > 
> > Replies inline.  
> > >   
> 
> 
> ... snip ...
> 
> > > > +
> > > > +    <request name="commit">
> > > > +      <description summary="commit state">
> > > > +        Atomically applies state changes recently sent to the 
> > > > compositor.
> > > > +
> > > > +        The commit request establishes and updates the state of the 
> > > > client, and
> > > > +        must be issued immediately after before the compositor
> > > > +
> > > > +        Text input state (enabled status, content purpose, content 
> > > > hint,
> > > > +        surrounding text and change cause, cursor rectangle) is 
> > > > conceptually
> > > > +        double-buffered within the context of a text input, i.e. 
> > > > between an
> > > > +        enable request and the following enable or disable request.
> > > > +
> > > > +        Protocol requests modify the pending state, as opposed to the 
> > > > current
> > > > +        state in use by the input method. A commit request atomically 
> > > > applies
> > > > +        all pending state, replacing the current state. After commit, 
> > > > the new
> > > > +        pending state is as documented for each related request.
> > > > +
> > > > +        The enable request plays a special role by indicating that the 
> > > > state
> > > > +        should be reset and updated with new values on the nearest 
> > > > commit.
> > > > +
> > > > +        Neither current nor pending state are modified unless noted 
> > > > otherwise.
> > > > +
> > > > +        The serial number identifies the current state of the
> > > > +        zwp_text_input_v3 object. The serial value of the Nth request 
> > > > issued
> > > > +        on the object must be the Nth number of an infinite sequence of
> > > > +        integers, in which each pair of equal values is separated by 
> > > > at least
> > > > +        4096 positions.    
> > > 
> > > So this request will send an integer to the compositor the compositor
> > > already knows about, right? Any reason why we can't just make it
> > > implicit, and just have the client keep the count client side? A proper
> > > compositor implementation would have the same counter, and issue errors
> > > when the client sets the wrong serial anyway.
> > >   
> > I had the same idea as you - the compositor doesn't (and shouldn't) need to 
> > know the sequence. I would prefer to keep the idea explicit: someone 
> > grokking the idea of a serial may not need the explanation, but a strict 
> > description will hopefully be clear to anyone.  
> 
> It does need to keep the same counter if it wants to validate input
> though, since you state exactly what number should be sent (Nth request
> on the object).
> 
> I suggest two go one of two paths:
> 
> 1) Describe the exact expected value of a serial number, and let the
> compositor send it without first receiving it from the client.
> 
> 2) Make the serial undefined; and a opaque tool for clients to use for
> synchronization, but suggesting that a plain every increasing serial
> number is a potential tool.
> 
> What do you think about that?
> 

What I wanted to do is similar to 2), except with the condition that numbers do 
not repeat too often (the 4096 statement). I think it's close enough as it is?

I didn't go the way of 1) because of input-method. It's simpler to forward the 
sequence numbers to the input method client without modifying. I was operating 
under the assumption that the input method *shouldn't* know future sequence 
numbers. If that assumption is wrong, I would actually prefer to define the 
numbers and use 1).

I'd like to know your opinion: should sequence numbers coming from the 
compositor to the input method be unguessable or not?

Cheers,
Dorota
> 
> Jonas
> 
> > 
> > Will it be clearer if I turn the "an" into "any", as in "any ininite 
> > sequence"?
> > 
> > --Dorota
> >   
> > > 
> > > Jonas
> > >   
> > > > +      </description>
> > > > +      <arg name="serial" type="uint"/>
> > > > +    </request>
> > > > +
> > > > +    <event name="enter">
> > > > +      <description summary="enter event">
> > > > +        Notification that this seat's text-input focus is on a certain 
> > > > surface.
> > > > +
> > > > +        When the seat has the keyboard capability the text-input focus 
> > > > follows
> > > > +        the keyboard focus. This event sets the current surface for the
> > > > +        text-input object.
> > > > +      </description>
> > > > +      <arg name="surface" type="object" interface="wl_surface"/>
> > > > +    </event>
> > > > +
> > > > +    <event name="leave">
> > > > +      <description summary="leave event">
> > > > +        Notification that this seat's text-input focus is no longer on 
> > > > a
> > > > +        certain surface. The client should reset any preedit string 
> > > > previously
> > > > +        set.
> > > > +
> > > > +        The leave notification clears the current surface. It is sent 
> > > > before
> > > > +        the enter notification for the new focus.
> > > > +
> > > > +        When the seat has the keyboard capability the text-input focus 
> > > > follows
> > > > +        the keyboard focus.
> > > > +      </description>
> > > > +      <arg name="surface" type="object" interface="wl_surface"/>
> > > > +    </event>
> > > > +
> > > > +    <event name="preedit_string">
> > > > +      <description summary="pre-edit">
> > > > +        Notify when a new composing text (pre-edit) should be set at 
> > > > the
> > > > +        current cursor position. Any previously set composing text 
> > > > must be
> > > > +        removed. Any previously existing selected text must be removed.
> > > > +
> > > > +        The argument text contains the pre-edit string buffer.
> > > > +
> > > > +        The parameters cursor_begin and cursor_end are counted in bytes
> > > > +        relative to the beginning of the submitted text buffer. Cursor 
> > > > should
> > > > +        be hidden when both are equal to -1.
> > > > +
> > > > +        They could be represented by the client as a line if both 
> > > > values are
> > > > +        the same, or as a text highlight otherwise.
> > > > +
> > > > +        Values set with this event are double-buffered. They must be 
> > > > applied
> > > > +        and reset to initial on the next zwp_text_input_v3.done event.
> > > > +
> > > > +        The initial value of text is an empty string, and cursor_begin,
> > > > +        cursor_end and cursor_hidden are all 0.
> > > > +      </description>
> > > > +      <arg name="text" type="string" allow-null="true"/>
> > > > +      <arg name="cursor_begin" type="int"/>
> > > > +      <arg name="cursor_end" type="int"/>
> > > > +    </event>
> > > > +
> > > > +    <event name="commit_string">
> > > > +      <description summary="text commit">
> > > > +        Notify when text should be inserted into the editor widget. 
> > > > The text to
> > > > +        commit could be either just a single character after a key 
> > > > press or the
> > > > +        result of some composing (pre-edit).
> > > > +
> > > > +        Values set with this event are double-buffered. They must be 
> > > > applied
> > > > +        and reset to initial on the next zwp_text_input_v3.done event.
> > > > +
> > > > +        The initial value of text is an empty string.
> > > > +      </description>
> > > > +      <arg name="text" type="string" allow-null="true"/>
> > > > +    </event>
> > > > +
> > > > +    <event name="delete_surrounding_text">
> > > > +      <description summary="delete surrounding text">
> > > > +        Notify when the text around the current cursor position should 
> > > > be
> > > > +        deleted.
> > > > +
> > > > +        Before_length and after_length are the number of bytes before 
> > > > and after
> > > > +        the current cursor index (excluding the selection) to delete.
> > > > +
> > > > +        If a preedit text is present, in effect before_length is 
> > > > counted from
> > > > +        the beginning of it, and after_length from its end (see done 
> > > > event
> > > > +        sequence).
> > > > +
> > > > +        Values set with this event are double-buffered. They must be 
> > > > applied
> > > > +        and reset to initial on the next zwp_text_input_v3.done event.
> > > > +
> > > > +        The initial values of both before_length and after_length are 
> > > > 0.
> > > > +      </description>
> > > > +      <arg name="before_length" type="uint" summary="length of text 
> > > > before current cursor position"/>
> > > > +      <arg name="after_length" type="uint" summary="length of text 
> > > > after current cursor position"/>
> > > > +    </event>
> > > > +
> > > > +    <event name="done">
> > > > +      <description summary="apply changes">
> > > > +        Instruct the application to apply changes to state requested 
> > > > by the
> > > > +        preedit_string, commit_string and delete_surrounding_text 
> > > > events. The
> > > > +        state relating to these events is double-buffered, and each one
> > > > +        modifies the pending state. This event replaces the current 
> > > > state with
> > > > +        the pending state.
> > > > +
> > > > +        The application must proceed by evaluating the changes in the 
> > > > following
> > > > +        order:
> > > > +
> > > > +        1. Replace existing preedit string with the cursor.
> > > > +        2. Delete requested surrounding text.
> > > > +        3. Insert commit string with the cursor at its end.
> > > > +        4. Calculate surrounding text to send.
> > > > +        5. Insert new preedit text in cursor position.
> > > > +        6. Place cursor inside preedit text.
> > > > +
> > > > +        The serial number reflects the last state of the 
> > > > zwp_text_input_v3
> > > > +        object known to the compositor. The value of the serial 
> > > > argument must
> > > > +        come from the most recently received commit request. When the 
> > > > client
> > > > +        receives a done event with a serial different from the last 
> > > > serial
> > > > +        in the commit request, it must proceed as normal, except it 
> > > > must not
> > > > +        change the current state.
> > > > +        <arg name="serial" type="uint"/>
> > > > +      </description>
> > > > +    </event>
> > > > +  </interface>
> > > > +
> > > > +  <interface name="zwp_text_input_manager_v3" version="1">
> > > > +    <description summary="text input manager">
> > > > +      A factory for text-input objects. This object is a global 
> > > > singleton.
> > > > +    </description>
> > > > +
> > > > +    <request name="destroy" type="destructor">
> > > > +      <description summary="Destroy the wp_text_input_manager">
> > > > +        Destroy the wp_text_input_manager object.
> > > > +      </description>
> > > > +    </request>
> > > > +
> > > > +    <request name="get_text_input">
> > > > +      <description summary="create a new text input object">
> > > > +        Creates a new text-input object for a given seat.
> > > > +      </description>
> > > > +      <arg name="id" type="new_id" interface="zwp_text_input_v3"/>
> > > > +      <arg name="seat" type="object" interface="wl_seat"/>
> > > > +    </request>
> > > > +  </interface>
> > > > +</protocol>
> > > > -- 
> > > > 2.14.4
> > > > 
> > > > _______________________________________________
> > > > wayland-devel mailing list
> > > > [email protected]
> > > > https://lists.freedesktop.org/mailman/listinfo/wayland-devel    
> >   
> 
> 

Attachment: pgpoEl5xxC0Q4.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to