[PATCH v8 wayland-protocols] text-input: Add v3 of the text-input protocol

Jonas Ådahl jadahl at gmail.com
Mon Jul 30 11:09:06 UTC 2018


On Mon, Jul 30, 2018 at 12:57:52PM +0200, Dorota Czaplejewicz wrote:
> On Mon, 30 Jul 2018 12:36:45 +0200
> Jonas Ådahl <jadahl at gmail.com> wrote:
> 
> > On Sat, Jul 28, 2018 at 08:17:56PM +0200, Dorota Czaplejewicz wrote:
> > > From: Carlos Garnacho <carlosg at gnome.org>
> > > 
> > > 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 <dorota.czaplejewicz at puri.sm>
> > > Signed-off-by: Carlos Garnacho <carlosg at gnome.org>
> > > ---
> > > 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?


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
> > > wayland-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/wayland-devel  
> 




More information about the wayland-devel mailing list