[PATCH wayland] protocol: Add set_xwayland request for new shell surface type

[Pekka Paalanen]

On Thu, 13 Dec 2012 14:13:04 -0200
Tiago Vignatti <tiago.vignatti at linux.intel.com> wrote:

> 
> On 12/13/2012 05:38 AM, Pekka Paalanen wrote:
> >
> > On Wed, 12 Dec 2012 13:26:33 -0200
> > Tiago Vignatti <tiago.vignatti at intel.com> wrote:
> >
> >> X11 apps use global coordinates most of the time for window placement and the
> >> current approach we have, where transient windows is placed parent relative,
> >> is not sufficient. IOW we can't use the relation of parent <-> child
> >> coordinates because we can't go out there and fix all the clients to place
> >> subwindows relative.
> >>
> >> Besides, it has to be a separate wl_shell_surface request specifically for
> >> XWayland because we don't want stock Wayland applications using global
> >> coordinates either.
> >>
> >> At the moment only one method for XWayland windows was implemented, which is
> >> the 'inactive' to not set focus when the window is configured. Later we will
> >> want to add a few more methods for dealing with different types of windows.
> >>
> >> Signed-off-by: Tiago Vignatti <tiago.vignatti at intel.com>
> >> ---
> >>   protocol/wayland.xml |   20 ++++++++++++++++++++
> >>   1 file changed, 20 insertions(+)
> >>
> >> diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> >> index 0ce68ef..e6df6f3 100644
> >> --- a/protocol/wayland.xml
> >> +++ b/protocol/wayland.xml
> >> @@ -683,6 +683,26 @@
> >>         <arg name="output" type="object" interface="wl_output" allow-null="true"/>
> >>       </request>
> >>
> >> +    <enum name="xwayland">
> >> +      <description summary="XWayland surface types">
> >> +      </description>
> >> +
> >> +      <entry name="inactive" value="0x1" summary="do not set keyboard focus"/>
> >
> > So the only supported type of xwayland windows is the "inactive" one?
> > I.e. no "normal" windows at all? Or are normal X windows realised by not
> > using set_xwayland at all?
> 
> No. The approach I'm using is a bit confusing indeed: X top-level 
> windows use set_toplevel, while X override-redirected windows use 
> set_xwayland.
> 
> So at the moment with this patch, I'm assuming that *all* 
> override-redirected windows are transient and inactive. That being said, 
> by no means this is enough for mapping all types of X windows on 
> Wayland, but is definitely better than the previous approach where we 
> relied on the parent <-> child for placement. Eventually we'll need to 
> extend the xwayland methods accordingly to support popup and other types.
> 

Oh ok, I didn't realize xwayland windows are *that* tied to wl_shell,
but of course they are.

> >> +    </enum>
> >> +
> >> +    <request name="set_xwayland">
> >> +      <description summary="make the surface a X11 type of surface">
> >> +	X11 apps use global coordinates most of the time for window placement
> >> +	and this request has to be used only for X Windows. Hence, the
> >> +	coordinates 'x' and 'y' that are global, ie relative to window output
> >> +	dimensions.
> >> +      </description>
> >> +
> >> +      <arg name="x" type="int"/>
> >> +      <arg name="y" type="int"/>
> >> +      <arg name="method" type="uint"/>
> >> +     </request>
> >> +
> >
> > I wonder, why is this request part of wl_shell_surface? Here it is
> > available to all normal Wayland clients which in my opinion is
> > undesireable. If we later add more wl_shell_surface features, those
> > features cannot be implemented without also implementing and offering
> > this one, because we have monotonic interface versioning (no pick and
> > choose).
> 
> set_xwayland is part of wl_shell_surface because we're re-using the 
> existent *desktop* methods of the wl_shell; namely the move, resize and 
> toplevel ones. And the map & configure of desktop shell makes sense on 
> most of the cases also, so for convenience I opted for reusing it.
> 
> About the availability of it to regular Wayland clients, I agree, it's a 
> problem. Bill Spitzak pointed this out on the previous set. It sets a 
> bad example because it exposes global position to all regular clients 
> and we don't want this.
...
> > Besides the obvious of exposing a global coordinate system, this will
> > also tie xwayland support to the wl_shell protocol, i.e. desktops.
> > Other public shell protocols would then have to copy this request, if
> > they were to support xwayland. Therefore I would prefer to keep all
> > xwayland specific protocol in the xwayland protocol extension.
> 
> I don't see a problem on having xwayland typical applications linked 
> with "desktop" style of interfaces. X11 and its WM standards are 
> flexible enough for building different graphics interfaces and the 
> wl_shell is the best at the moment to fitting them.

Is calling the wl_shell interfaces limited to the X WM, or does the X
server call them? If they are limited to X WM, then it is very nice,
because for a different Wayland shell, you would have to have a
different X WM anyway.

If the X server calls them, it could be awkward to extend to non-desktop
systems.

Could that work? Does this make sense to tie Wayland shell with X WM,
and leave the X server things as shell and WM agnostic? Could everything
shell related be in the private X WM interface, including the
wl_shell_surface:set_xwayland?

Or maybe that is how you already designed it, but I'm too lazy to look
it up in the patches. ;-)

Maybe it would make sense to split the "wm" interface into its own xml
file, too, so that xserver interface is shared between the Wayland
compositor and the X server, and "wm" is shared between the Wayland
compositor and the X WM? On a quick look I didn't see any
interdependencies preventing that.

> But yeah, I think we'll eventually have to go for a specific xwayland 
> extension only for the sake of having a private interface for XWayland 
> only, ie not let regular clients see it.

Ok, cool.


Thanks,
pq