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