[PATCH wayland-protocols v2 7/7] xdg-shell: Introduce xdg_positioner

Jonas Ådahl jadahl at gmail.com
Wed May 18 03:19:48 UTC 2016


On Tue, May 17, 2016 at 03:41:37PM -0500, Yong Bakos wrote:
> On May 11, 2016, at 12:50 AM, Jonas Ådahl <jadahl at gmail.com> wrote:
> > 
> > xdg_positioner is a method for declarative positioning of child surfaces
> > (currently only xdg_popup surfaces). A client creates a description of a
> > positioning logic using the xdg_positioner interface. The xdg_positioner
> > object is then used when creating a xdg_popup for describing how the
> > child surface should be positioned in relation to the parent surface.
> > 
> > Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> > Signed-off-by: Mike Blumenkrantz <zmike at samsung.com>
> 
> Hi,
> A few errors corrected inline below.
> 
> yong
> 
> 
> > ---
> > 
> > Changes since v1:
> > 
> > - Clarify that the rules of xdg_positioner is copied when used, and itself
> >   doesn't get "attached" to any surface.
> > - Make it clear errors are raised on invalid input and when used incorrectly
> > - Added xdg_positioner.set_size, in order to not relying on a possibly
> >   incorrect xdg_surface.set_window_geometry.
> > - Added the concept of a "complete" positioner, i.e. one with a set size and
> >   anchor rect.
> > - Fixed bitfield enum field.
> > - Made the slide_x/y semantics less undefined.
> > 
> > 
> > Jonas
> > 
> > unstable/xdg-shell/xdg-shell-unstable-v6.xml | 248 ++++++++++++++++++++++++++-
> > 1 file changed, 246 insertions(+), 2 deletions(-)
> > 
> > diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> > index 2edc341..dfd7e84 100644
> > --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> > +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> > @@ -45,6 +45,8 @@
> > 	     summary="the client specified an invalid popup parent surface"/>
> >       <entry name="invalid_surface_state" value="4"
> > 	     summary="the client provided an invalid surface state"/>
> > +      <entry name="invalid_positioner" value="5"
> > +	     summary="the client provided an invalid positioner"/>
> >     </enum>
> > 
> >     <request name="destroy" type="destructor">
> > @@ -57,6 +59,15 @@
> >       </description>
> >     </request>
> > 
> > +    <request name="create_positioner">
> > +      <description summary="create a positioner object">
> > +	Create a positioner object. A positioner object is used to position
> > +	surfaces relative to some parent surface. See the interface description
> > +	and xdg_surface.get_popup for details.
> > +      </description>
> > +      <arg name="id" type="new_id" interface="zxdg_positioner_v6"/>
> > +    </request>
> > +
> >     <request name="get_xdg_surface">
> >       <description summary="create a shell surface from a surface">
> > 	This creates an xdg_surface for the given surface. While xdg_surface
> > @@ -96,6 +107,223 @@
> >     </event>
> >   </interface>
> > 
> > +  <interface name="zxdg_positioner_v6" version="1">
> > +    <description summary="child surface positioner">
> > +      The xdg_positioner provides an interface for constructing positioning
> > +      rules used for positioning a child surface relative to another surface
> 
> Perhaps "its parent surface" rather than "another surface."

Yea, maybe thats better. Or "a parent surface", since at this point that
relationship is not set up yet.

> 
> 
> > +      in a certain way. It allows methods for defining a rule that will make
> > +      the child surface stay within the border of the visible area of the
> > +      screen, with different ways in which the child surface should change
> > +      its position, including sliding along an axis, or flipping around a
> > +      rectangle.
> > +
> > +      See the various requests for details about possible rules.
> > +
> > +      Semantically, an xdg_positioner is a collection of positioning rules. When
> > +      used for positioning a surface, for example when passed as an argument to
> > +      xdg_surface.get_popup, the compositor copies the rules that were set up at
> > +      the time of the request. Making any changes or destroying the object after
> > +      it was used has no effect on previous usages.
> > +
> > +      For a xdg_positioner object to be considered complete, it must have a
> 
> an xdg_positioner
> 
> 
> > +      non-zero size set by set_size, and a non-zero anchor rectangle set by
> > +      set_anchor_rect. Passing an incomplete xdg_positioner object when
> > +      positioning a surface raises an error.
> > +    </description>
> > +
> > +    <enum name="error">
> > +      <entry name="invalid_input" value="0" summary="invalid input provided"/>
> > +    </enum>
> > +
> > +    <request name="destroy" type="destructor">
> > +      <description summary="destroy the xdg_positioner object">
> > +	Notify the compositor that the xdg_positioner will no longer be used.
> > +      </description>
> > +    </request>
> > +
> > +    <request name="set_size">
> > +      <description summary="set the size of the to be positioned rectangle">
> 
> to-be-positioned
> or
> of the rectangle to be positioned
> 
> And should that be surface instead of rectangle? See the first line of the
> description, here:
> 
> > +	Set the size of the surface that is to be positioned with the positioner
> > +	object. The size is in surface-local coordinates and corresponds to the
> > +	window geometry width. See xdg_surface.set_window_geometry.

This should be rectangle. We are not setting future surface sizes, but
the "window geometry" i.e. part of the popup without shadow etc.

> 
> window geometry. (omit width?)

or change it to size rather, since it is equivalent to the size
(width/height) part of the window geometry.

> 
> 
> > +
> > +	If a zero or negative size is set the invalid_input error is raised.
> > +      </description>
> > +      <arg name="width" type="int" summary="set width of positioned rectangle"/>
> > +      <arg name="height" type="int" summary="set height of positioned rectangle"/>
> 
> Maybe just "width of" instead of "set width of". Same for height.

Sure.

> 
> 
> > +    </request>
> > +
> > +    <request name="set_anchor_rect">
> > +      <description summary="set the anchor rectangle within the parent surface">
> > +	Specify the anchor rectangle within the parent surface that the child
> > +	surface will be placed relative to. The rectangle is relative to the
> > +	window geometry as defined by xdg_surface.set_window_geometry of the
> > +	parent surface. The rectangle must be at least 1x1 large.
> > +
> > +	When the xdg_positioner object is used to position a child surface, the
> > +	anchor rectangle may not extend outside the window geometry of the
> > +	positioned child's parent surface.
> 
> Will exceeding those bounds result in an error, or just clipping?

We won't be able to rais an error now, since the parent is not known,
but I suppose we can add a note that when the positioner is used, if the
anchor rect extends outside of the parent surface, an error is raised.

> 
> 
> > +
> > +	If a zero or negative size is set the invalid_input error is raised.
> > +      </description>
> > +      <arg name="x" type="int" summary="x position of anchor rectangle"/>
> > +      <arg name="y" type="int" summary="y position of anchor rectangle"/>
> 
> Perhaps add "relative to parent surface window geometry," similar to the
> 'configure' event at the end of this patch.

Good point.

> 
> 
> > +      <arg name="width" type="int" summary="width of anchor rectangle"/>
> > +      <arg name="height" type="int" summary="height of anchor rectangle"/>
> > +    </request>
> > +
> > +    <enum name="anchor" bitfield="true">
> > +      <entry name="none" value="0"
> > +	     summary="the center of the anchor rectangle"/>
> > +      <entry name="left" value="1"
> > +	     summary="the left edge of the anchor rectangle"/>
> > +      <entry name="right" value="2"
> > +	     summary="the right edge of the anchor rectangle"/>
> > +      <entry name="top" value="4"
> > +	     summary="the top edge of the anchor rectangle"/>
> > +      <entry name="bottom" value="8"
> > +	     summary="the bottom edge of the anchor rectangle"/>
> > +    </enum>
> 
> Note that the protocol's 'resize' enum follows a different order.
> Might be a plus to follow suit, just for consistency, if it makes sense.

Hmm. Yes, I suppose that makes sense.

> 
> 
> > +
> > +    <request name="set_anchor">
> > +      <description summary="set anchor rect anchor edges">
> 
> Spell out "rectangle".
> 
> 
> > +	Set the anchor edges of the anchor rectangle. The anchor edges define
> > +	where on the anchor rectangle the child surface should be positioned
> > +	relative to. An anchor is a bit mask of up to two values of the anchor
> > +	enum.
> > +
> > +	If two values on the same axis (for example left and right) is set the
> 
> are set,
> 
> 
> > +	invalid_input error is raised.
> > +
> > +	If no anchor is set on any axis, the anchor position will be positioned
> > +	at the center of the anchor rectangle on the unset axis. The default
> > +	value is "none".
> > +      </description>
> > +      <arg name="anchor" type="uint" summary="bit mask of anchor edges"/>
> 
> Shouldn't this arg have the enum attribute?

I suppose it should indeed.

> 
> 
> > +    </request>
> > +
> > +    <enum name="gravity" bitfield="true">
> > +      <entry name="none" value="0"
> > +	     summary="center over the anchor edge"/>
> > +      <entry name="left" value="1"
> > +	     summary="position to the left of the anchor edge"/>
> > +      <entry name="right" value="2"
> > +	     summary="position to the right of the anchor edge"/>
> > +      <entry name="top" value="4"
> > +	     summary="position above the anchor edge"/>
> > +      <entry name="bottom" value="8"
> > +	     summary="position below the anchor edge"/>
> > +    </enum>
> 
> Ditto here re: my comment about the anchor enum above.
> 
> 
> > +
> > +    <request name="set_gravity">
> > +      <description summary="set child surface gravity">
> > +	The gravity defines in what direction a surface would be positioned,
> > +	relative to the anchor edges on the parent surface. A gravity is a
> > +	bit mask of up to two values of the gravity enum.
> > +
> > +	If two values on the same axis (for example left and right) is set the
> 
> are set,
> 
> 
> > +	invalid_input eror is raised.
> 
> error
> 
> 
> > +
> > +	If no gravity is set on an axis, the gravity for that axis will be
> > +	equivalent to setting "none" for that axis, resulting in the child being
> > +	centered over the anchor edges on that axis.
> > +      </description>
> > +      <arg name="gravity" type="uint" summary="bit mask of gravity directions"/>
> 
> Shouldn't this arg have the enum attribute?
> 
> 
> > +    </request>
> > +
> > +    <enum name="constrain_adjustment" bitfield="true">
> > +      <entry name="none" value="0">
> > +	<description summary="don't move the child surface when constrained">
> > +	  Don't alter the surface position even if it is constrained on some
> > +	  axis, for example partially outside the edge of monitor.
> 
> a monitor.
> 
> 
> > +	</description>
> > +      </entry>
> > +      <entry name="slide_x" value="1">
> > +	<description summary="move along the X-axis until unconstrained">
> 
> x axis and y axis, in all cases found within this enum. I first looked to libinput
> for precedent, but it was inconsistent there too, so I had to turn to the grammar
> gods to verify. (Lower case and no hyphen when it's a noun.)

If this is what the grammar gods have said, I will comply.

Thanks for the thorough review. The things I didn't reply to I'll fix up
as well.


Jonas

> 
> 
> > +	  Slide the surface along the X-axis until it is no longer constrained.
> > +
> > +	  First try to slide towards the direction of the gravity on the X-axis
> > +	  until either the edge in the opposite direction of the gravity is
> > +	  unconstrained or the edge in the direction of the gravity is
> > +	  constrained.
> > +
> > +	  Then try to slide towards the opposite direction of the gravity on the
> > +	  X-axis until either the edge in the direction of the gravity is
> > +	  unconstrained or the edge in the opposite direction of the gravity is
> > +	  constrained.
> > +	</description>
> > +      </entry>
> > +      <entry name="slide_y" value="2">
> > +	<description summary="move along the Y-axis until unconstrained">
> 
> > +	  Slide the surface along the Y-axis until it is no longer constrained.
> > +
> > +	  First try to slide towards the direction of the gravity on the Y-axis
> > +	  until either the edge in the opposite direction of the gravity is
> > +	  unconstrained or the edge in the direction of the gravity is
> > +	  constrained.
> > +
> > +	  Then try to slide towards the opposite direction of the gravity on the
> > +	  Y-axis until either the edge in the direction of the gravity is
> > +	  unconstrained or the edge in the opposite direction of the gravity is
> > +	  constrained.
> > +	</description>
> > +      </entry>
> > +      <entry name="flip_x" value="4">
> > +	<description summary="invert the anchor and gravity on the X-axis">
> 
> > +	  Invert the anchor and gravity on the X-axis if the surface is
> > +	  constrained on the X-axis. For example, if the left edge of the
> > +	  surface is constrained, the gravity is 'left' and the anchor is
> > +	  'left', change the gravity to 'right' and the anchor to 'right'. If
> > +	  the resulting position after inverting ends up also being constrained,
> > +	  the behaviour is undefined.
> > +	</description>
> > +      </entry>
> > +      <entry name="flip_y" value="8">
> > +	<description summary="invert the anchor and gravity on the Y-axis">
> > +	  Invert the anchor and gravity on the Y-axis if the surface is
> > +	  constrained on the Y-axis. For example, if the bottom edge of the
> > +	  surface is constrained, the gravity is 'bottom' and the anchor is
> > +	  'bottom', change the gravity to 'top' and the anchor to 'top'. If
> > +	  the resulting position after inverting ends up also being constrained,
> > +	  the behaviour is undefined.
> > +	</description>
> > +      </entry>
> > +    </enum>
> > +
> > +    <request name="set_constrain_adjustment">
> > +      <description summary="set the adjustment to be done when constrained">
> > +	Specify how the window should be positioned if the originally intended
> > +	position caused the surface to be constrained, meaning at least
> > +	partially outside positioning boundaries set by the compositor. The
> > +	adjustment is set by constructing a bitmask with one bit per axis set
> > +	describing the adjustment to be made when the surface is constrained on
> > +	that axis. If no bit for one axis is set, the compositor will assume
> > +	that the child surface should not change its position on that axis when
> > +	constrained. The default adjustment is none.
> > +      </description>
> > +      <arg name="constrain_adjustment" type="uint"
> > +	   summary="bit mask of constrain adjustments"/>
> > +    </request>
> > +
> > +    <request name="set_offset">
> > +      <description summary="set surface position offset">
> > +	Specify the surface position offset relative to the position of the
> > +	anchor on the anchor rectangle and the anchor on the surface. For
> > +	example if the anchor of the anchor rectangle is at (x, y), the surface
> > +	has the gravity bottom|right, and the offset is (ox, oy), the calculated
> > +	surface position will be (x + ox, y + oy). The offset position of the
> > +	surface is the one used for constraint testing. See
> > +	set_constraint_adjustment.
> 
> set_constrain_adjustment.
> 
> 
> > +
> > +	An example use case is placing a popup menu on top of a user interface
> > +	element, while aligning the user interface element of the parent surface
> > +	with some user interface element placed somewhere in the popup surface.
> > +      </description>
> > +      <arg name="x" type="int" summary="surface position x offset"/>
> > +      <arg name="y" type="int" summary="surface position y offset"/>
> > +    </request>
> > +  </interface>
> > +
> >   <interface name="zxdg_surface_v6" version="1">
> >     <description summary="desktop user interface surface base interface">
> >       An interface that may be implemented by a wl_surface, for
> > @@ -163,8 +391,7 @@
> >       </description>
> >       <arg name="id" type="new_id" interface="zxdg_popup_v6"/>
> >       <arg name="parent" type="object" interface="zxdg_surface_v6"/>
> > -      <arg name="x" type="int"/>
> > -      <arg name="y" type="int"/>
> > +      <arg name="positioner" type="object" interface="zxdg_positioner_v6"/>
> >     </request>
> > 
> >     <request name="set_window_geometry">
> > @@ -755,6 +982,23 @@
> >       <arg name="serial" type="uint" summary="the serial of the user event"/>
> >     </request>
> > 
> > +    <event name="configure">
> > +      <description summary="configure the popup surface">
> > +	This event asks the popup surface to configure itself given the
> > +	configuration. It is not sent by itself but is as a latched state prior
> > +	to the xdg_surface.configure event. See xdg_surface.configure for
> > +	details.
> > +
> > +	The x and y arguments represent the position the popup was placed at
> > +	given the xdg_positioner rule, relative to the upper left corner of the
> > +	window geometry of the parent surface.
> > +      </description>
> > +      <arg name="x" type="int"
> > +	   summary="x position relative to parent surface window geometry"/>
> > +      <arg name="y" type="int"
> > +	   summary="y position relative to parent surface window geometry"/>
> > +    </event>
> > +
> >     <event name="popup_done">
> >       <description summary="popup interaction is done">
> > 	The popup_done event is sent out when a popup is dismissed by the
> > -- 
> > 2.5.5
> > 
> > _______________________________________________
> > 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