[PATCH wayland-protocols 7/7] xdg-shell: Introduce xdg_positioner
Mike Blumenkrantz
michael.blumenkrantz at gmail.com
Tue Apr 19 16:50:31 UTC 2016
I've emerged from my bike shed in order to join this expert-level
shedcraftsmanship discussion.
On Thu, Apr 14, 2016 at 4:28 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>
> ---
> unstable/xdg-shell/xdg-shell-unstable-v6.xml | 206
> ++++++++++++++++++++++++++-
> 1 file changed, 204 insertions(+), 2 deletions(-)
>
> diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> index a2a6f12..2b51802 100644
> --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> @@ -57,6 +57,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
>
If this is going to operate based on a surface's parent (ie. set_parent)
then it should read "A position object is used to position its surface(s)
relative to their parent surface (see set_parent)" in order to avoid any
confusion.
Moreover, I'm thinking this object needs to somehow be tethered to a single
toplevel hierarchy; the current text has no restrictions about the
positioner's use, meaning that it could be used for eg.
Surface A
[Positioner P]
Popup A
Surface B
[Positioner P]
Popup B
[Positioner P]
Popup B2
which would be confusing, and could also lead to annoying calculation
issues if Surface A was huge and Surface B was tiny.
> + 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 +105,186 @@
> </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
>
Same as above.
> + 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
>
I'd rather have this specifically reference "the positioner's surface(s)"
or similar instead of "the child surface" in order to keep context within
the positioner interface.
> + 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.
> +
> + An xdg_positioner object may be re-used when positioning different
>
Saying "re-used" here makes it sound like the positioner can only have one
surface attached to it at a time. I think something like "...may continue
to be applied to any xdg_popup surfaces until destroyed..." would be more
clear.
> + surfaces until destroyed using xdg_positioner.destroy.
> + </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.
>
Should this include an explicit note about what to do with surfaces which
were using the positioner, eg. "No changes should be made to users of the
positioner upon its destruction" ?
> + </description>
> + </request>
> +
> + <request name="set_anchor_rect">
> + <description summary="set the anchor rectangle of the parent
> surface">
>
"set the anchor rectangle within the parent surface"
> + Specify the anchor rectangle of 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 be 1x1 large.
> +
> + When used to position a child surface, the attach rectangle may not
> + extend outside of the window geometry of the parent surface.
>
I think this also needs to be clarified to something like "The positioner's
attach rectangle may not extend outside the window geometry of the user
surface's parent" in order to handle the case where the positioner is used
for multiple surfaces with different parents in the same hierarchy chain
(assuming this is going to be allowed).
> + </description>
> + <arg name="x" type="int" summary="x position of anchor rectangle"/>
> + <arg name="y" type="int" summary="y position of anchor rectangle"/>
> + <arg name="width" type="int" summary="width of anchor rectangle"/>
> + <arg name="height" type="int" summary="height of anchor rectangle"/>
> + </request>
> +
> + <enum name="anchor">
> + <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>
> +
> + <request name="set_anchor">
> + <description summary="set anchor rect anchor edges">
> + Set the anchor edges of the anchor rectangle. The anchor edges
> + defines where on the anchor rectangle the child surface should
> + positioned relative to. An anchor is a bit mask of zero to two
> values of
>
I understand what you mean by "zero to two", but saying "zero values of the
anchor enum" is a bit weird since 0/none is an enum value.
> + the anchor enum. Two values on the same axis (for example left and
> + right) may not be combined.
>
Are we defining an error for this case or ?
> +
> + 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" bitfield="true"
> + summary="bit mask of anchor edges"/>
> + </request>
> +
> + <enum name="gravity">
> + <entry name="none" value="0"
> + summary="center above the anchor position"/>
>
I think maybe you meant "center over" ?
> + <entry name="left" value="1"
> + summary="position to the left of the anchor position"/>
> + <entry name="right" value="2"
> + summary="position to the right of the anchor position"/>
> + <entry name="top" value="4"
> + summary="position above the anchor position"/>
> + <entry name="bottom" value="8"
> + summary="position below the anchor position"/>
> + </enum>
> +
> + <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 position on the parent surface. A gravity
> + is a bit mask of zero to two values of the gravity enum. Two values
> + on the same axis (for example left and right) may not be combined.
> +
> + 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 on that axis.
>
I think for all cases of "anchor" and "anchor position" in this request and
the preceding enum we should probably be strict in using the term "anchor
rectangle" in order to maintain consistency.
> + </description>
> + <arg name="gravity" type="uint" bitfield="true"
> + summary="bit mask of gravity directions"/>
> + </request>
> +
> + <enum name="constrain_action">
>
I don't like the use of "action" here as, to me, it seems to imply user
action rather than compositor calculations. Maybe something more like
"fallback_position" or "position_adjustment"?
> + <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.
> + </description>
> + </entry>
> + <entry name="slide_x" value="1">
> + <description summary="move along the X-axis until unconstrained">
> + Slide the surface along the X-axis until it is no longer
> constrained.
> + If the left side of the surface is constrained, slide towards the
> + right; if the right side of the surface is constrained, slide
> towards
> + the left. If both sides are ends up being constrained, the
> behaviour is
>
I think the directionality here should be based on the gravity, eg. if LEFT
gravity is used then it tries to move left if possible, moving right
afterward only if a leftward movement is impossible or does not solve the
positioning. This handles the case of both sides being constrained since
the compositor is then unable to perform any positioning corrections.
> + undefined.
> + </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.
> + If the top side of the surface is constrained, slide towards the
> + bottom; if the bottom side of the surface is constrained, slide
> towards
> + the top. If both sides are ends up being constrained, the
> behaviour is
> + undefined.
> + </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_action">
> + <description summary="set the action to be done when constrained">
>
Same as my above comment re:action.
> + Specify how the window should be positioned if the originally
> intended
> + position caused the surface to be constrained, meaning partially
> outside
>
Probably "meaning at least partially" to cover the case where the surface
is completely outside the screen.
> + positioning boundaries set by the compositor. The action is set by
> + constructing a bitmask with one bit per axis set describing the
> action
> + to be taken 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 action is none.
> + </description>
> + <arg name="constrain_action" type="uint" bitfield="true"
> + summary="bit mask of constrain actions"/>
> + </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_actions.
> +
> + 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"/>
>
These should probably be "anchor rectangle x/y 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
> @@ -167,8 +356,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">
> @@ -675,6 +863,20 @@
> <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 a latched state
> finalized
> + by the xdg_surface.configure event.
> +
> + The x and y arguments represents 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"/>
> + <arg name="y" type="int" summary="Y position relative to parent
> surface"/>
> + </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.4.3
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160419/26a3bcc1/attachment-0001.html>
More information about the wayland-devel
mailing list