<div dir="ltr"><div><div>I just added an implementation of RFCv4 which can be found here:<br><br><a href="https://github.com/jekstrand/weston/tree/fullscreen-shell-RFCv4">https://github.com/jekstrand/weston/tree/fullscreen-shell-RFCv4</a><br>
<br></div>Thanks,<br></div>--Jason Ekstrand<br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Sat, Feb 15, 2014 at 1:12 AM, Pekka Paalanen <span dir="ltr"><<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Fri, 14 Feb 2014 11:11:33 -0600<br>
<div><div class="h5">Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
<br>
> Hi Pekka!  Thanks for the review.  Comments follow.<br>
><br>
><br>
> On Fri, Feb 14, 2014 at 1:14 AM, Pekka Paalanen<br>
> <<a href="mailto:ppaalanen@gmail.com">ppaalanen@gmail.com</a>> wrote:<br>
><br>
> > Hi Jason<br>
> ><br>
> > On Thu, 13 Feb 2014 22:37:53 -0600<br>
> > Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
> ><br>
> > > The following is yet another take on the fullscreen shell<br>
> > > protocol. Previous versions more-or-less followed the<br>
> > > approach taken in wl_shell. This version completely reworks<br>
> > > the concept.  In particular, the protocol is split into two<br>
> > > use-cases.  The first is that of a simple client that wants<br>
> > > to present a surface or set of surfaces possibly with some<br>
> > > scaling. This happens through the present_surface request<br>
> > > which looks similar to that of wl_shell only without the<br>
> > > modesetting.<br>
> > ><br>
> > > The second use-case is of a client that wants more control<br>
> > > over the outputs.  In this case, the client uses the<br>
> > > present_surface_for_mode request to present the surface at a<br>
> > > particular output mode.  This request provides a more-or-less<br>
> > > atomic modeset operation.  If the compositor can satisfy the<br>
> > > requested mode, then the mode is changed and the new surface<br>
> > is<br>
> > > presented.  Otherwise, the compositor harmlessly falls back<br>
> > > to the previously presented surface and the client is<br>
> > > informed that the switch failed.  This way, the surface is<br>
> > > either displayed correctly or not at<br>
> > all.<br>
> > > Of course, a client is free to call present_surface_for_mode<br>
> > > with the currently presented surface and hope for the best.<br>
> > > However, this may result in strange behavior and there is no<br>
> > > reliable fallback if the mode switch fails.<br>
> > ><br>
> > > In particular, I would like feedback on the modesetting<br>
> > > portion of this protocol.  This is particularly targetted at<br>
> > > compositors that want to run inside weston or some other<br>
> > > fullscreen compositor.  In the next week or<br>
> > so,<br>
> > > I will attempt to implement all this in weston and see how<br>
> > > well it works. However, I would also like to know how well<br>
> > > this will work for other compositors such as KWin or Hawaii.<br>
> > ><br>
> > > Thanks for your feedback,<br>
> > > --Jason Ekstrand<br>
> > ><br>
> > > ===== Protocol follows: =====<br>
> > ><br>
> > > <protocol name="fullscreen_shell"><br>
> > >   <interface name="wl_fullscreen_shell" version="1"><br>
> ><br>
> > This interface should have a destructor request IMO. It's not<br>
> > stricly required, but I think it would be consistent (I think<br>
> > all global interfaces need an explicit destructor request) and<br>
> > more future-proof.<br>
> ><br>
><br>
> Thanks for reminding me.  I'll get one added.<br>
><br>
><br>
> ><br>
> > >     <description summary="Displays a single surface per<br>
> > > output"> Displays a single surface per output.<br>
> > ><br>
> > >       This interface provides a mechanism for a single client<br>
> > > to display simple full-screen surfaces.  While there<br>
> > > technically may be<br>
> > multiple<br>
> > >       clients bound to this interface, only one of those<br>
> > > clients should<br>
> > be<br>
> > >       shown at a time.<br>
> > ><br>
> > >       To present a surface, the client uses either the<br>
> > > present_surface or present_surface_for_mode requests.<br>
> > > Presenting a surface takes<br>
> > effect<br>
> > >       on the next wl_surface.commit.  See the individual<br>
> > > requests for details about scaling and mode switches.<br>
> > ><br>
> > >       The client can have at most one surface per output at<br>
> > > any time. Requesting a surface be presented on an output that<br>
> > > already has a surface replaces the previously presented<br>
> > > surface.  Presenting a<br>
> > null<br>
> > >       surface removes its content and effectively disables<br>
> > > the output. Exactly what happens when an output is "disabled"<br>
> > > is compositor-specific.  The same surface may be presented<br>
> > > multiple outputs simultaneously.<br>
> ><br>
> > If the same surface is presented on multiple outputs, should<br>
> > the client have a way to say which output is to be considered<br>
> > the surface's main output, where e.g. presentation feedback is<br>
> > synced to?<br>
> ><br>
><br>
> That's a good question.  Simple clients probably don't care.<br>
> More complex clients such as compositors probably will.  However,<br>
> I'd expect them to have one surface per output most of the time<br>
> anyway.  I'll give that some thought.<br>
><br>
><br>
> > Maybe also note explicitly, that once a surface has been<br>
> > presented on an output, it stays on that output until<br>
> > explicitly removed, or output is unplugged? So that simple<br>
> > attach+damage+commit can be used to update the content, if that<br>
> > is the intention.<br>
> ><br>
><br>
> Yes, that's a good point.  And I do intend to provide that<br>
> guarantee.<br>
><br>
><br>
> ><br>
> > >     </description><br>
> > ><br>
> > >     <enum name="present_method"><br>
> > >       <description summary="different method to set the<br>
> > > surface<br>
> > fullscreen"><br>
> > >       Hints to indicate to the compositor how to deal with a<br>
> > > conflict between the dimensions of the surface and the<br>
> > > dimensions of the output. The compositor is free to ignore<br>
> > > this parameter. </description><br>
> > >       <entry name="default" value="0" summary="no preference,<br>
> > > apply<br>
> > default policy"/><br>
> > >       <entry name="center" value="1" summary="center the<br>
> > > surface on the<br>
> > output"/><br>
> > >       <entry name="zoom" value="2" summary="scale the surface,<br>
> > preserving aspect ratio, to the largest size that will fit on<br>
> > the output" /><br>
> > >       <entry name="zoom_crop" value="3" summary="scale the<br>
> > > surface,<br>
> > preserving aspect ratio, to fully fill the output cropping if<br>
> > needed" /><br>
> > >       <entry name="stretch" value="4" summary="scale the<br>
> > > surface to the<br>
> > size of the output ignoring aspect ratio" /><br>
> > >     </enum><br>
> > ><br>
> > >     <request name="present_surface"><br>
> > >       <description summary="present surface for display"><br>
> > >       Present a surface on the given output.<br>
> > ><br>
> > >       If the output is null, the compositor will present the<br>
> > > surface on whatever display (or displays) it thinks best.  In<br>
> > > particular, this may replace any or all surfaces currently<br>
> > > presented so it should not be used in combination with<br>
> > > placing surfaces on specific outputs.<br>
> > ><br>
> > >       The method parameter is a hit to the compositor for how<br>
> > > the surface<br>
> ><br>
> > *hint<br>
> ><br>
><br>
> Thanks<br>
><br>
><br>
> ><br>
> > >       is to be presented.  In particular, it tells the<br>
> > > compostior how to handle a size mismatch between the<br>
> > > presented surface and the output.  The compositor is free to<br>
> > > ignore this parameter.<br>
> > ><br>
> > >       The "zoom", "zoom_crop", and "stretch" methods imply a<br>
> > > scaling operation on the surface.  This will override any<br>
> > > kind of output scaling, so the buffer_scale property of the<br>
> > > surface is effectively ignored.<br>
> > >       </description><br>
> > >       <arg name="surface" type="object"<br>
> > > interface="wl_surface"/> <arg name="method" type="uint"/><br>
> > >       <arg name="output" type="object" interface="wl_output"<br>
> > allow-null="true"/><br>
> > >     </request><br>
> > ><br>
> > >     <request name="present_surface_for_mode"><br>
> > >       <description summary="present surface for display at a<br>
> > > particular<br>
> > mode"><br>
> > >       Presents a surface on the given output for a particular<br>
> > > mode.<br>
> > ><br>
> > >       If the current size of the output differs from that of<br>
> > > the surface, the compositor will attempt to change the size<br>
> > > of the output to match the surface.  The result of the<br>
> > > mode-swith operation will be returned via the provided<br>
> > > wl_fullscreen_shell_mode_feedback object.<br>
> ><br>
> > Is it sufficient to infer the mode from the buffer size, or<br>
> > could there be use cases for forcing a particular mode and<br>
> > scaling a buffer from a different size?<br>
> ><br>
> > If you had a separate "set_mode" request, you could do with<br>
> > only the "present_surface" request for setting the surface.<br>
> ><br>
><br>
> Hehe.  That was my original thought on the matter.  Later, I<br>
> thought better of it.  I'll have more comments on the matter at<br>
> the end.<br>
><br>
><br>
> > Taking this idea further, like atomic mode setting over all<br>
> > outputs, we're getting pretty close to the DRM KMS APIs. Would<br>
> > it make sense to model these interfaces according to the KMS<br>
> > APIs but sanitized to offer all the modern features like atomic<br>
> > modeset and planes in a clean uniform way, or do you<br>
> > intentionally want to keep this protocol interface simple and<br>
> > not e.g. consider planes explicitly?<br>
> ><br>
><br>
> I'll take a look at KMS.  However, I think planes are better left<br>
> to subsurfaces.  More on that below.<br>
><br>
><br>
> > >       If the current output mode matches the one requested or<br>
> > > if the compositor successfully switches the mode to match the<br>
> > > surface, then the mode_successfull event will be sent and the<br>
> > > output will contain the contents of the given surface.  If<br>
> > > the compositor cannot match the output size to the surface<br>
> > > size, the mode_failed will be sent and the output will<br>
> > > contain the contents of the previously presented surface (if<br>
> > > any).  If another surface is presented on the given output<br>
> > > before either of these has a chance to happen, the<br>
> > > present_canceled event will be sent.<br>
> > ><br>
> > >       If the size of the presented surface changes, the<br>
> > > resulting output is undefined.  The compositor may attempt to<br>
> > > change the output mode to compensate.  However, there is no<br>
> > > guarantee that a suitable mode will be found and the client<br>
> > > has no way to be notified of success or failure.<br>
> ><br>
> > The above sounds to me like you want the client be in explicit<br>
> > control of the video mode when it asks one, without the server<br>
> > fuzzing in between with scaling.<br>
> ><br>
> > With an explicit "set_mode" request, I think you could make the<br>
> > above cases defined.<br>
> ><br>
> > How much control do you want to give to the client? Apparently<br>
> > you want the two different cases: simple client, smart server;<br>
> > and smart client with explicit mode setting, simple server just<br>
> > obeying or refusing without any fuzz.<br>
> ><br>
><br>
> Pretty much.   Again, read below.<br>
><br>
><br>
> ><br>
> > >       The framerate parameter specifies the target framerate<br>
> > > for the output.  The compositor is free to ignore this<br>
> > > parameter.  A value of 0 indicates that the client has no<br>
> > > preference.<br>
> > ><br>
> > >       If the surface has a buffer_scale greater than 1, the<br>
> > > compositor may choose a mode that matches either the buffer<br>
> > > size or the surface size.  In either case, the surface will<br>
> > > fill the output. </description><br>
> > >       <arg name="surface" type="object"<br>
> > > interface="wl_surface"/> <arg name="output" type="object"<br>
> > > interface="wl_output"/> <arg name="framerate" type="int"/><br>
> > >       <arg name="feedback" type="new_id"<br>
> > interface="wl_fullscreen_shell_mode_feedback"/><br>
> > >     </request><br>
> > ><br>
> > >     <enum name="error"><br>
> > >       <description summary="wl_fullscreen_shell error values"><br>
> > >       These errors can be emitted in response to<br>
> > > wl_fullscreen_shell<br>
> > requests<br>
> > >       </description><br>
> > >       <entry name="invalid_method" value="0"<br>
> > > summary="present_method is<br>
> > not known"/><br>
> > >     </enum><br>
> > >   </interface><br>
> > ><br>
> > >   <interface name="wl_fullscreen_shell_mode_feedback"<br>
> > > version="1"> <event name="mode_successful"><br>
> > >       <description summary="mode switch succeeded"><br>
> > >       This event indicates that the attempted mode switch<br>
> > > operation was successful.  A surface of the size requested in<br>
> > > the mode switch will fill the output without scaling.<br>
> > ><br>
> > >       Upon recieving this event, the client should destroy the<br>
> > >       wl_fullscreen_shell_mode_feedback object.<br>
> > >       </description><br>
> > >     </event><br>
> > >     <event name="mode_failed"><br>
> > >       <description summary="mode switch succeeded"><br>
> > >       This event indicates that the attempted mode switch<br>
> > > operation failed. This may be because the requested output<br>
> > > mode is not possible or it may mean that the compositor does<br>
> > > not want to allow mode switches at this time.<br>
> > ><br>
> > >       Upon recieving this event, the client should destroy the<br>
> > >       wl_fullscreen_shell_mode_feedback object.<br>
> > >       </description><br>
> > >     </event><br>
> > >     <event name="present_canceled"><br>
> > >       <description summary="mode switch succeeded"><br>
> > >       This event indicates that the attempted mode switch<br>
> > > operation was canceled.  Most likely this is because the<br>
> > > client requested a second mode switch before the first one<br>
> > > completed.<br>
> > ><br>
> > >       Upon recieving this event, the client should destroy the<br>
> > >       wl_fullscreen_shell_mode_feedback object.<br>
> > >       </description><br>
> > >     </event><br>
> ><br>
> > This interface has no destructor protocol, so the server cannot<br>
> > know when it gets destroyed by the client. I'm always a bit<br>
> > wary of that, but here it seems ok, since this is a one-shot<br>
> > feedback object without any requests. It is short-lived,<br>
> > doesn't take resources in the server much nor bandwidth when it<br>
> > triggers, so there's no need to ever be able to cancel it.<br>
> ><br>
><br>
> Yeah, I thought about having a destructor but it's intentionally<br>
> a one-shot and that just adds more protocol noise for little<br>
> benefit. Maybe it's a good idea, but I don't think it's needed.<br>
><br>
> Ok, now for the promised "more detail"...<br>
><br>
> When designing this interface, I have three primary use-cases in<br>
> mind:<br>
><br>
> 1) Simple clients such as splash screens and terminal emulators.<br>
> These would rather not mess with KMS and, in a world without<br>
> VT's, need some way to get to the screen.  I want to provide a<br>
> simple but powerful enough interface to serve these clients.<br>
><br>
> 2) Full compositors such as KWin that would rather not deal with<br>
> KMS themselves but would prefer to let Weston or some other<br>
> implementation handle it.  In this case, we want more<br>
> fine-grained control and we want things like planes, cursors, etc.<br>
><br>
> 3) Other non-KMS "backends" such as VNC/RDP servers, screen<br>
> recorders, or anything else where it may not be practical to<br>
> implement full DRM/KMS.<br>
><br>
> As you surmised, the first request is to handle the first of these<br>
> use-cases and the second request is for the other two.  Breaking<br>
> it into two completely separate cases was a deliberate decision.<br>
> Maybe not the right one, but deliberate none the less.  Allow me<br>
> to explain.<br>
><br>
> First, I think that planes and the like are already pretty-well<br>
> covered by wl_subsurface and wl_viewport.  There's no reason for<br>
> me to duplicate that protocol here if it's already covered by<br>
> those.  This may not map perfectly, but I think it will cover<br>
> well enough for most cases.  Axel, this also covers cursors and<br>
> the like.<br>
<br>
</div></div>Hi,<br>
<br>
the sub-surface approach for planes has one complication. If your<br>
fullscreen-compositor is the one making all decisions about hw<br>
plane usage and never exposing any details of them to the<br>
child-compositor, then the child-compositor must always forward all<br>
surfaces as sub-surfaces to give the fullscreen-compositor a chance<br>
to use all planes. If the child-compositor composites itself, it<br>
excludes some opportunities to use hw planes.<br>
<br>
Is the cost of forwarding all surfaces into sub-surfaces<br>
significant? I don't really know. Is it your intended way of<br>
working?<br>
<br>
It certainly does make the protocol simple and easy to use.<br>
<div class=""><br>
> Second, I wanted to ensure that things were atomic.  By having<br>
> presentation explicitly tied to mode set, we can ensure this.<br>
> There is the issue of the fallback not being atomic.  One<br>
> solution to this would be to have a fallback mode option so if it<br>
> can't find a mode it will scale or something. However, I would<br>
> rather clients pick one interface and stick with it than<br>
> switching between the two.  I thought about splitting it into a<br>
> set_mode request and a present_surface request.  However, a) it's<br>
> harder (not impossible) to make this atomic and b) it makes the<br>
> compositor's life more difficult (see next paragraph).  If they<br>
> want scaling+modeset, they can just use a wl_viewport.<br>
<br>
</div>Sorry, perhaps I wasn't clear. I meant to keep your semantics the<br>
same, mode set being triggered on wl_surface.commit. The set_mode<br>
request would be just the mode set part of your<br>
present_surface_for_mode. Atomicity would be exactly the same. That<br>
way you could replace present_surface_for_mode with a pair of<br>
set_mode and present_surface, and have the very same result.<br>
<div class=""><br>
> Third, I want to allow VNC/RDP-type compositors to be as simple as<br>
> possible.  If they don't provide wl_subsurface and wl_viewport<br>
> then they don't have to do any real composting.  They still have<br>
> to handle the first request but that's easily enough done with<br>
> pixman or similar.  Also, it explicitly states that they can<br>
> ignore the method parameter and just always center the surface or<br>
> something.  For the present_for_mode request, it means that they<br>
> can always do a direct pixel copy from the source to the<br>
> destination (or directly flip in the case of DRM/KMS).  By<br>
> requiring the surface and output size to match, it takes a lot of<br>
> the guesswork out of the mode switches.<br>
<br>
</div>You could still keep those semantics too, I guess, but now I<br>
realize that you indeed do not have the "method" argument for<br>
present_surface_for_mode. Yes, your design makes sense.<br>
<div class=""><br>
> I hope that makes my thinking on this whole protocol more clear.<br>
> Thanks for reviewing;  I look forward to any future comments you<br>
> may have! --Jason Ekstrand<br>
<br>
</div>Sure!<br>
<br>
Thanks,<br>
pq<br>
</blockquote></div><br></div>