Oh, I apologize. I must have accidentally made a mistake while rebasing. Let me try this again. Please disregard this patch. <div class="gmail_quote"><div dir="ltr">On Thu, Apr 14, 2016, 8:04 PM Yong Bakos <<a href="mailto:junk@humanoriented.com">junk@humanoriented.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Apr 14, 2016, at 3:23 PM, Dennis Kempin <<a href="mailto:denniskempin@google.com" target="_blank">denniskempin@google.com</a>> wrote: > > This CL updates the wl_touch interface with a shape and > orientation event. > The shape/orientation of a touch point is not relevant for most UI > applications, but allows a better experience in some cases > such as drawing apps. > > The events are used by the compositor to inform the client > about changes in the shape and orientation of a touchpoint, which is > approximated by an ellipse and it's angle to the y-axis. > > The event is optional and only sent when compositor and the > touch device support this type of information. The client is > responsible for making a reasonable assumption about the > touch shape if no shape is reported. > > Signed-off-by: Dennis Kempin <<a href="mailto:denniskempin@google.com" target="_blank">denniskempin@google.com</a>> Hi Dennis, I've noted some concerns inline below. I'm a bit inexperienced here, so please do correct/educate me where I'm wrong. > --- > protocol/wayland.xml | 75 ++++++++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 64 insertions(+), 11 deletions(-) > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml > index 12a6efd..d19d898 100644 > --- a/protocol/wayland.xml > +++ b/protocol/wayland.xml > @@ -910,11 +910,6 @@ > copy-and-paste and drag-and-drop. These mechanisms are tied to > a wl_seat and this interface lets a client get a wl_data_device > corresponding to a wl_seat. > - > - Depending on the version bound, the objects created from the bound > - wl_data_device_manager object will have different requirements for > - functioning properly. See wl_data_source.set_actions, > - wl_data_offer.accept and wl_data_offer.finish for details. What does the deletion of this paragraph have to do with adding the two new touchpoint shape events? > </description> > > <request name="create_data_source"> > @@ -1656,7 +1651,7 @@ > </request> > </interface> > > - <interface name="wl_seat" version="5"> > + <interface name="wl_seat" version="6"> Why are you bumping this instead of just bumping the version of the wl_touch interface? > <description summary="group of input devices"> > A seat is a group of keyboards, pointer and touch devices. This > object is published as a global during start up, or when such a > @@ -1765,7 +1760,7 @@ > > </interface> > > - <interface name="wl_pointer" version="5"> > + <interface name="wl_pointer" version="6"> Why are you bumping this instead of just bumping the version of the wl_touch interface? > <description summary="pointer input device"> > The wl_pointer interface represents one or more input devices, > such as mice, which control the pointer location and pointer_focus > @@ -2038,7 +2033,7 @@ > > The timestamp is to be interpreted identical to the timestamp in the > wl_pointer.axis event. The timestamp value may be the same as a > - preceding wl_pointer.axis event. > + preceeding wl_pointer.axis event. Why are you introducing a misspelling? > </description> > <arg name="time" type="uint" summary="timestamp with > millisecond granularity"/> > <arg name="axis" type="uint" enum="axis" summary="the axis > stopped with this event"/> > @@ -2078,7 +2073,7 @@ > </event> > </interface> > > - <interface name="wl_keyboard" version="5"> > + <interface name="wl_keyboard" version="6"> Why are you bumping this instead of just bumping the version of the wl_touch interface? > <description summary="keyboard input device"> > The wl_keyboard interface represents one or more keyboards > associated with a seat. > @@ -2192,7 +2187,7 @@ > </event> > </interface> > > - <interface name="wl_touch" version="5"> > + <interface name="wl_touch" version="6"> > <description summary="touchscreen input device"> > The wl_touch interface represents a touchscreen > associated with a seat. > @@ -2242,7 +2237,12 @@ > > <event name="frame"> > <description summary="end of touch frame event"> > - Indicates the end of a contact point list. > + Indicates the end of a contract point list. The wayland protocol requires contact point list. > + touch point updates to be sent sequentially, however all events within a > + frame should be considered one hardware event. A wl_touch.frames terminates wl_touch.frame > + at least one event but otherwise no guarantee is provided about the set of > + events within a frame. A client must assume that any state not updated in a > + frame is unchanged from the previously known state. Am I misreading this or has indentation been obliterated? > </description> > </event> > > @@ -2262,6 +2262,59 @@ > <request name="release" type="destructor" since="3"> > <description summary="release the touch object"/> > </request> > + > +  > + > + <event name="shape" since="6"> > + <description summary="update shape of touch point"> > + Sent when a touchpoint has changed its shape. If the touch position > + or orientation changed at the same time, the wl_touch.motion, > + wl_touch.orientation and wl_touch.shape are sent within the same > + wl_touch.frame. > + Otherwise, only a wl_touch.shape is sent within this wl_touch.frame. Same paragraph? And indentation? > + The protocol does not guarantee specific ordering of wl_touch.orientation, > + wl_touch.shape and wl_touch.motion events. > + > + A touchpoint shape is approximated by an ellipse through the major and minor > + axis length. The major axis length describes the longest diameter of the > + ellipse, while the minor axis length describes the shortest diameter. > + Both are specified in surface coordinates. > + The center of the ellipse is always at the touchpoint location as reported Same paragraph? And indentation? > + by wl_touch.down or wl_touch.move. > + > + This event is only sent by the compositor if the touch device supports shape > + reports. The client has to make reasonable assumptions about the shape if > + it did not receive this event. This last sentence seems unnecessary. > + </description> > + <arg name="id" type="int" summary="the unique ID of this touch point"/> > + <arg name="major" type="fixed" summary="length of the major > axis in surface coordinates"/> > + <arg name="minor" type="fixed" summary="length of the minor > axis in surface coordinates"/> I politely encourage "surface local coordinates." <a href="https://lists.freedesktop.org/archives/wayland-devel/2016-April/027804.html" rel="noreferrer" target="_blank">https://lists.freedesktop.org/archives/wayland-devel/2016-April/027804.html</a> > + </event> > + > + <event name="orientation" since="6"> > + <description summary="update orientation of touch point"> > + Sent when a touchpoint has changed its orientation. If the touch position > + or shape changed at the same time, the wl_touch.motion, wl_touch.orientation > + and wl_touch.shape are sent within the same wl_touch.frame. > + Otherwise, only a wl_touch.orientation is sent within this wl_touch.frame. > + The protocol does not guarantee specific ordering of wl_touch.orientation, > + wl_touch.shape and wl_touch.motion events. > + > + The orientation describes the clockwise angle of touchpoints major axis to > + the surface y-axis and is normalized to the -180 to +180 degrees range. > + The granuality of orientation depends on the touch device, some devices only > + support binary rotation values between 0 and 90 degrees. > + > + This event is only sent by the compositor if the touch device supports > + orientation reports. > + The client has to make reasonable assumptions about the orientation if > + it did not receive this event. As mentioned in the shape event, I feel this last sentence is unnecessary. And, am I misreading or is the indentation missing? > + </description> > + <arg name="id" type="int" summary="the unique ID of this touch point"/> > + <arg name="orientation" type="fixed" > + summary="angle between major axis and surface y-axis in degrees"/> > + </event> > + Omitting this line break would match the current convention. > </interface> > > <interface name="wl_output" version="2"> > -- > 2.8.0.rc3.226.g39d4020 > _______________________________________________ > wayland-devel mailing list > <a href="mailto:wayland-devel@lists.freedesktop.org" target="_blank">wayland-devel@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/wayland-devel" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/wayland-devel</a> </blockquote></div>