[PATCH weston] xdg-shell: Make stable

[Giulio Camuffo]

2014-08-27 11:38 GMT+03:00 Pekka Paalanen <ppaalanen at gmail.com>:
> On Wed, 27 Aug 2014 11:03:04 +0300
> Giulio Camuffo <giuliocamuffo at gmail.com> wrote:
>
>> 2014-08-27 10:32 GMT+03:00 Pekka Paalanen <ppaalanen at gmail.com>:
>> > On Tue, 26 Aug 2014 17:50:47 +0300
>> > Giulio Camuffo <giuliocamuffo at gmail.com> wrote:
>> >
>> >> 2014-08-26 17:39 GMT+03:00 Jason Ekstrand <jason at jlekstrand.net>:
>> >> >
>> >> > On Aug 26, 2014 1:01 AM, "Giulio Camuffo" <giuliocamuffo at gmail.com> wrote:
>> >> >>
>> >> >> 2014-08-26 10:24 GMT+03:00 Pekka Paalanen <ppaalanen at gmail.com>:
>> >> >> > On Mon, 25 Aug 2014 21:51:57 -0700
>> >> >> > Jason Ekstrand <jason at jlekstrand.net> wrote:
>> >> >> >
>> >
>> >> >> >> On Aug 22, 2014 1:48 PM, "Jasper St. Pierre" <jstpierre at mecheye.net>
>> >> >> >> wrote:
>> >> >> >> >
>> >> >> >> > On Wed, Aug 6, 2014 at 9:39 AM, Pekka Paalanen <ppaalanen at gmail.com>
>> >> >> >> wrote:
>> >> >> >> >>
>> >> >> >> >> On Thu, 17 Jul 2014 17:57:45 -0400
>> >> >> >> >> "Jasper St. Pierre" <jstpierre at mecheye.net> wrote:
>> >> >> >> >>
>> >
>> >> >> >> >> >       <entry name="fullscreen" value="2" summary="the surface is
>> >> >> >> fullscreen">
>> >> >> >> >> >         The surface is fullscreen. The window geometry specified
>> >> >> >> >> > in
>> >> >> >> the configure
>> >> >> >> >> >         event must be obeyed by the client.
>> >> >> >> >>
>> >> >> >> >> Really? So, will we rely on wl_viewport for scaling low-res apps to
>> >> >> >> >> fullscreen? No provision for automatic black borders in aspect ratio
>> >> >> >> >> or
>> >> >> >> >> size mismatch, even if the display hardware would be able to
>> >> >> >> >> generate
>> >> >> >> >> those for free while scanning out the client buffer, bypassing
>> >> >> >> >> compositing?
>> >> >> >> >
>> >> >> >> >
>> >> >> >> >> Since we have a big space for these states, I suppose we could do
>> >> >> >> >> those
>> >> >> >> >> mismatch cases in separate and explicit state variants of
>> >> >> >> >> fullscreen,
>> >> >> >> >> could we not?
>> >> >> >> >
>> >> >> >> >
>> >> >> >> > I explicitly removed this feature from the first draft of the patch
>> >> >> >> simply to make my life easier as a compositor writer. We could add
>> >> >> >> additional states for this, or break fullscreen into multiple states:
>> >> >> >> "fullscreen + size_strict" or "fullscreen + size_loose" or something.
>> >> >> >> >
>> >> >> >> > I am not familiar enough with the sixteen different configurations of
>> >> >> >> > the
>> >> >> >> > old fullscreen system to make an informed decision about what most
>> >> >> >> > clients
>> >> >> >> > want. Your help and experience is very appreciated. I'm not keen to
>> >> >> >> > add
>> >> >> >> > back the matrix of configuration options.
>> >> >> >>
>> >> >> >> Yeah, not sure what to do here.  I like the idea of the compositor
>> >> >> >> doing it
>> >> >> >> for the client.  Sure, you could use wl_viewport, subsurfaces, and a
>> >> >> >> couple
>> >> >> >> black surfaces for letterboxing.  However that is going to be far more
>> >> >> >> difficult for the compositor to translate into overlays/planes than
>> >> >> >> just
>> >> >> >> the one surface and some scaling instructions.
>> >> >> >
>> >> >> > I don't think it would be that hard for a compositor to use overlays
>> >> >> > even then. Have one surface with a 1x1 wl_buffer, scaled with
>> >> >> > wl_viewport to fill the screen, and then have another surface on top
>> >> >> > with the video wl_buffers being fed in, scaled with wl_viewport to keep
>> >> >> > aspect ratio. A compositor can easily put the video on an overlay, and
>> >> >> > if the CRTC hardware supports, it might even eliminate the black surface
>> >> >> > and replace it with a background color.
>> >> >>
>> >> >> What is not clear to me is what advantages does the
>> >> >> wl_subsurface+wl_viewport approach has compared to the compositor just
>> >> >> scaling the surface and putting a black surface behind it.
>> >> >> Is it just to remove the hint in the wl_fullscreen request? This seems
>> >> >> a lazy reason to me, implementing that hint in the compositor is not
>> >> >> hard, and in turn it means increasing the complexity of every client
>> >> >> that would ever want to go fullscreen.
>> >> >> There is also one use case for which the wl_subsurface+wl_viewport
>> >> >> cannot work. Having the same surface fullscreen on two differently
>> >> >> sized outputs (think of presentations), like this:
>> >> >> http://im9.eu/picture/phx647
>> >> >> Sure, the compositor can send the configure event with one output's
>> >> >> size and scale the surface to fit the other one, but then what is the
>> >> >> purpose of wl_viewport again here, if the compositor must scale the
>> >> >> surface anyway?
>> >> >
>> >> > Yeah, I think I still have to go with Guilio here.  It really it's probably
>> >> > less work for the compositor to implement fullscreen modes than to implement
>> >> > viewport+subsurface correctly and it's certainly less work for clients.
>> >> >
>> >> >> >
>> >> >> > In my previous reply, I concluded that wl_viewport+wl_subsurface would
>> >> >> > be enough (I suprised myself), and we would not really need yet another
>> >> >> > way from xdg_surface. Obviously, I forgot something, that the IRC
>> >> >> > discussion of last night brought back to my mind.
>> >> >> >
>> >> >> > The wl_shell fullscreening has three different cases for dealing with
>> >> >> > size mismatch between the output and the window:
>> >> >> > - aspect-correct scaling
>> >> >> > - centered, no scaling
>> >> >> > - please, I would really like a mode switch
>> >> >> >
>> >> >> > There is also the fourth, which means the client does not care at all
>> >> >> > what the compositor does. This protocol was designed before
>> >> >> > sub-surfaces or wl_viewport were a thing.
>> >> >> >
>> >> >> > If we do not have that in xdg_shell, but instead rely on
>> >> >> > wl_viewport+wl_subsurface, there are two consequences:
>> >> >> > - scaling vs. centered is no longer a hint, but it is dictated by the
>> >> >> >   client
>> >> >> > - the mode switch case is lost
>> >> >> > (- all desktop compositors are required to implement both wl_scaler
>> >> >> >   and wl_subcompositor)
>> >> >> >
>> >> >> > The first point is likely not an issue, but the second may very well
>> >> >> > be. If xdg_surface requires fullscreen windows to be the exact output
>> >> >> > size, and clients obey, there is no way to trigger the mode switch.
>> >> >> >
>> >> >> > I suspect there are at least gamers out there, who would not like this.
>> >> >> > In fact, they would be pushed back to the way X11 works right now: if
>> >> >> > you want a mode switch, use a helper app to permanently change the
>> >> >> > output resolution (this screws up your whole desktop layout), then
>> >> >> > launch the game, afterwards do a manual switch back.
>> >> >> >
>> >> >> > No, sorry, that would actually be *worse* than the situation on X11
>> >> >> > right now.
>> >> >> >
>> >> >> > Recalling that, I do think we need something to support the mode switch
>> >> >> > case. A crucial part of a video mode for a gamer is the monitor refresh
>> >> >> > rate, which is why wl_shell_surface.set_fullscreen includes the
>> >> >> > framerate parameter.
>> >> >>
>> >> >> I don't think games need the screen to be at a NxM pixels mode,
>> >> >> scaling up the surface would be good and possibly better, since we can
>> >> >> scale better than what LCD screens usually do.
>> >> >> On the other hand, there is the framerate parameter, and games may
>> >> >> care about that... I'm not sure what is the best course of action
>> >> >> here.
>> >> >
>> >> > I'd agree.  Even on X, when you do a "mode switch" it frequently isn't a
>> >> > mode switch at all.  I know that on the nVidia driver, X will do scaling on
>> >> > the graphics card if it's plugged in via a digital connection (DVI, HDMI,
>> >> > DP, etc.)  There really is no "mode switch" involved.  This is precicely
>> >> > because of what Guilio mentioned:  LCD monitors do crappy scaling.
>> >
>> > Yes, I know that, and the driver scaling has bitten me when that leads
>> > to *down*scaling, but that is completely irrelevant here. :-)
>> >
>> >> > That said, there's a counter example!  Look at LCD projectors.  Unless you
>> >> > have the projector oriented perfectly so that no keystoning is required
>> >> > (BTW: this never happens in practice), the projector will transform the
>> >> > image you give it to make it more rectangular on the projected screen.  In
>> >> > this case, the graphics card stretching it and the projector transforming it
>> >> > will probably look worse than the projector transforming the original image.
>> >> >
>> >> > I think the point here is that the app can't really know whether it wants
>> >> > scaling or a mode switch.  Apps shouldn't need to be dealing with all the
>> >> > details of what outputs are connected and how beyond maybe knowing that the
>> >> > presentation goes on the projector.  Compositors are much better situated to
>> >> > decide when do a mode-switch vs. putting the fullscreen surface in a plane.
>> >
>> > Well, as I've tried to explain, "mode switch" is not a real mode
>> > switch, it is just a wish. The compositor chooses the best course of
>> > action, also based on the user's preferences.
>> >
>> > Fullscreening is still about going to one wl_output, so you have to
>> > care about outputs. Outputs have a mode list, and apps may or may not
>> > use that list as options to show to the user for fullscreen window
>> > size. The mode list would probably be just a supplement to the app's
>> > own hardcoded list, plus maybe completely freely chosen resolution. The
>> > only thing I see is that it should help the user to pick a good
>> > resolution, if the user really has to go changing it.
>> >
>> >> > I think the bigger issue here is framerate.  The reason why games want to
>> >> > run at these odd modes is because your card may not be able to render the
>> >> > game at 1920x1080 at 60fps (to use an extreme example).  So what does the game
>> >> > do?  It asks for a lower resolution and maybe a lower framerate.  If it can
>> >> > only hit 50fps consistently, then using a 50hz vsync looks better than the
>> >> > jitter you would get on a 60hz vsync.  Can we use a heuristic for this?
>> >> > Maybe, maybe not.
>> >
>> > Video playback is another thing. Some people really want the refresh
>> > rate to match the video framerate.
>> >
>> >> >> > Also remember, that the mode switch is not meant to be mandatory if
>> >> >> > requested by a client. It is only a preference, that when this app is
>> >> >> > active (and top-most?), it would really like the video mode to be
>> >> >> > changed to the nearest compatible with the window. The compositor is
>> >> >> > free to switch between the game's mode and the native desktop mode at
>> >> >> > will. Minimize the game? Sure, switch back to the native desktop video
>> >> >> > mode. Bring the game back - switch back to the game mode. Alt+tab?
>> >> >> > Switch only when something else than the game gets activated, maybe.
>> >> >
>> >> > As I mentioned above, I don't think clients should care whether they get a
>> >> > new mode or not beyond a possible FPS change.  However, I don't know that I
>> >> > like the word "hint" in the fullscreen spec in wl_shell_surface.  The way he
>> >> > spec was written before, the client could ask for centered and get a
>> >> > mode-switch or vice-versa.  What was the intent here?
>> >
>> > Clients would not know if they got a new mode in any case.
>> >
>> >> > Perhaps we can do everything we want by saying that fullscreen means "fill
>> >> > the screen without changing aspect-ratio" and add a couple of hints requests
>> >> > that the client calls before calling set_fullscreen:
>> >> >
>> >> > - Preferred framerate:  "I don't think I can render any faster than this, so
>> >> > it will look better if you refresh at this rate."  This could, in theory,
>> >> > apply to non-fullscreen windows as well.  Also, it could be a bogus rate
>> >> > such as 24fps (movies) and the compositor could try to refresh at a multiple
>> >> > of 24 or something.
>> >>
>> >> We should also have a special value for "i don't care", maybe 0.
>> >
>> > I think the preferred framerate hint is a very good idea. It is one
>> > part of the essence of this whole thing.
>> >
>> >> > - Preferred fullscreen scaling:  "I would like to be as large as possible",
>> >> > "I would like to be pixel-perfect, even if that means smaller and surrounded
>> >> > in black", etc.  TBH, I don't know how many clients would actually like the
>> >> > later one.  Maybe if they have text they don't want getting blurry?
>> >>
>> >> I also don't see much use for the second (centered) mode. Scaling
>> >> should definitely be the default, imho.
>> >>
>> >> This is going in the right direction, i think.
>> >
>> > This is the question about whether xdg_surface should require the
>> > window to match exactly the configured size. I interpret your replies
>> > as "at most as big as the configured size", allowing to be smaller in
>> > either or both dimensions. Right?
>> >
>> > Let's do a summary:
>> > - a request to go fullscreen
>> > - preferred framerate hint, including dont-care option
>> > - allow the fullscreen window to be smaller than configured, defaulting
>> >   to scaling up as big as possible without losing any image content or
>> >   changing aspect ratio
>>
>> What about bigger than configured? The compositor could scale them
>> down, i don't see any difference here.
>
> I would say that the app is being stupid, no-one wants to have
> downscaling when the window is upposed be shown in its normal way.
>
> IIRC we already forbid larger than configured anyway.

Forbid as in protocol error or undefined behavior? I agree that's a
stupid thing to do, but the compositor should be able to deal with
stupideness, and scaling down seems the most sensible thing to do. I'm
fine with leaving it as undefined behavior in the spec, anyway.

>
> I'll leave that to Jasper.
>
>> > If a client wants any specific kind of scaling / black bars, it can
>> > still do that with wl_viewport and sub-surfaces. This is kind of an
>> > override for special apps with special needs.
>>
>> So you think we should drop the method hint and make the apps use
>> wl_viewport+subsurfaces if they want to not be scaled up, right?
>
> Yeah.
>
>> > A video player might want to use the preferred framerate hint even
>> > while not fullscreen. (Make it an independent xdg_surface request?)
>>
>> That could work. The compositor can then switch to a mode with that
>> framerate if it works fine with other surfaces, and otherwise ignore
>> it until the surface goes fullscreen.
>
> Exactly. Up to compositor's policy.
>
>> > A game may be using window geometry smaller than configured, and set
>> > the framerate hint. The compositor can choose to change video mode
>> > and/or scale up, or not.
>> >
>> > The compositor's user preferences might favour integer scaling factors
>> > if the user thinks they are better than fractional. In the game
>> > example, the compositor is free to use an integer factor and fill the
>> > rest with black.
>> >
>> > A hardcore gamer using a CRT would set his compositor preferences so,
>> > that the compositor will always switch video mode (perhaps per app) to
>> > the app's fullscreen resolution.
>> >
>> > Would that sound good?
>> > Any other use cases to consider?
>> >
>> > The two key features would be the framerate hint, and allowing smaller
>> > than configured fullscreen windows.
>
> Thanks,
> pq