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

[Jonas Ådahl]

On Tue, Jun 14, 2016 at 01:01:15PM -0400, Jonas Ådahl wrote:
> On Thu, Jun 09, 2016 at 12:05:28AM +0200, Benoit Gschwind wrote:
> > Hello Jonas,
> > 
> > The proposal of xdg_positioner is a good idea. I still need to review
> > your proposal deeper, but at the moment I have few question to help my
> > understanding.
> > 

... snip ...

> > > +
> > > +    <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 a 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.
> > > +
> > > +	  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"/>
> > 
> > Does setting all the bitmask mean do what you prefer to the compositor ?
> 
> No, as written, its "one bit per axis".
> 
> > 
> > Would it be better to give a priority list like: try flip_x first, then
> > try slice_x then try flip_y for example ?
> 
> That would complicate the semantics of the constraints we have. For
> example, you could not try flipping after having tried sliding, since
> sliding is defined in a moving mechanism, not an boolean "on-off".
> 
> I think it might be more sane to just leave that case (flip and fallback
> to slide) out right now, and if we actually end up needing it, adding
> more rules to the enum is easy, and we can clearly specify the semantics
> without having to deal with inter-rule semantics.
> 

I have changed my mind about this part, I think it makes sense to be
able to combine them, kind of in a way that is what you described. But
rather than having a array, which would allow to create invalid
combinations, we should just allow combining them in a bitmask,
documenting the meaning of the combination.

For example, starting with "sliding" then falling back to "flipping" is
tricky, because "sliding" is defined as a set of actions; while
"flipping" and then falling back to "sliding" does make sense, because
flipping is more of a "boolean" transition.

So in the next version I'll change it to make it posible to allow more
than one bit set per axis, while what can be combined and the meaning of
it.


Jonas