Re: Request for feedback for my GSoC project to improve Present support in Xwayland

[Michel Dänzer]

On 24/07/17 11:07 AM, Roman Gilg wrote:>
> I'm writing this mail in order to get some feedback from the X.Org
> community regarding my current approach and the progress I've made so
> far. You can find my main working branch at
> https://github.com/subdiff/xserver/tree/presentInXwayland
> 
> I've tried to limit the changes to the Present code to a minimum. At the
> moment it should be able to flip Glamor/GBM Pixmaps of Present
> supporting clients to a Wayland surface for one full screen window
> without copies. The restriction of only one window, which is full
> screen, is in line with what is possible with the currently available
> functionality by the Present code in the Xserver.

This restriction doesn't make much sense in the rootless case though.


> I have also worked on a second solution as a long term strategy. But
> it's probably not in the scope of my GSoC project: The idea would be to
> remodel the Present code in order to support Pixmap flips per CRTC,
> which would enable the Xwayland DDX to flip every Wayland window
> individually. This would be important if a compositor wants to support
> the scanout of client buffers to overlay planes different to the primary
> planes of the CRTCs. In my opinion it would also greatly increase the
> functionality in other DDX, since being able to flip in a multi monitor
> setting a full screen window on one CRTC alone is to my knowledge
> currently not possible when using only one Screen struct in combination
> with RandR. But Daniel already warned me that this rewrite would be very
> difficult to get right or maybe not doable at all. The main reason for
> that is presumably that the Present code uses in central parts the
> Screen Pixmap for its flipping.

It doesn't really use the screen pixmap for flipping, but maybe you mean
that it generally assumes rootful operation. Apart from that, per-CRTC
flipping would require wrapping the screen and GC rendering hooks to
deal with different parts of the root window being stored in different
pixmaps.

> Also it would mean, that current Present supporting DDX drivers need to
> be rewritten in some parts.

I'd expect that the driver interface could be extended in a backwards
compatible way, so only drivers which want to support per-CRTC flips
would need any changes.


However, per-CRTC flips might still not be a very good match for the
rootless case.

Maybe it would be better to start by duplicating the whole Present code
in the Xwayland DDX, and modifying it in any way necessary to allow
per-window flipping in the rootless case. Then you could see what if
anything can be usefully shared between the rootless and rootful cases.


> This also leads to an explicit question in the end: Present changes the
> Screen

Present doesn't change the screen pixmap, it just sets the flip pixmap
instead of the screen pixmap as the window pixmap of the root and
flipping windows while flipping.

> and Window Pixmaps via a TraverseTree operation. Why is this necessary

Because every window has its own window pixmap pointer, so the above
also has to be done for any children of the flipping window.

> and why did using the same construction not work in my Xwayland code? 

What happened when you tried? I guess it might be related to rootless.

> I change the Window Pixmaps currently directly in the Xwayland DDX
> without traversing the tree:
> https://github.com/subdiff/xserver/blob/presentInXwayland/hw/xwayland/xwayland-present.c#L224

This might not work correctly when the flipping window has children.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: Request for feedback for my GSoC project to improve Present support in Xwayland

[Daniel Stone]

Hi Pekka,

On 24 July 2017 at 08:48, Pekka Paalanen  wrote:
> Another idea talked about a long time ago in relation to
> Presentation-time was a vblank feedback stream, where every vblank (at
> least the ones the compositor processed) would be signalled to
> subscribed clients regardless of wl_surface updates, and without
> explicit request for every event. It didn't raise enough interest at
> the time.

It does exist, in Chromium's Wayland compositor:
https://chromium.googlesource.com/experimental/chromium/src/+/master/third_party/wayland-protocols/unstable/vsync-feedback/vsync-feedback-unstable-v1.xml

Cheers,
Daniel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: Request for feedback for my GSoC project to improve Present support in Xwayland

[Pekka Paalanen]

On Mon, 24 Jul 2017 04:07:11 +0200
Roman Gilg  wrote:

> Hi,
> 
> I'm working for my GSoC project on a better Present integration in
> Xwayland. This should improve performance as well as minimize tearing. My
> mentor is Daniel Stone and the starting point was my work on KWin and this
> bug report: https://bugs.freedesktop.org/show_bug.cgi?id=99702.
> 
> I'm writing this mail in order to get some feedback from the X.Org
> community regarding my current approach and the progress I've made so far.
> You can find my main working branch at
> https://github.com/subdiff/xserver/tree/presentInXwayland
> 
> I've tried to limit the changes to the Present code to a minimum. At the
> moment it should be able to flip Glamor/GBM Pixmaps of Present supporting
> clients to a Wayland surface for one full screen window without copies. The
> restriction of only one window, which is full screen, is in line with what
> is possible with the currently available functionality by the Present code
> in the Xserver.
> 
> I have tried to queue events on msc timings, but it's not really
> straightforward, since I think we need to read out some sort of Vblank
> counter from the Wayland server instead of the kernel. The frame callback
> is a candidate for that, but it needs at least in my KWin testing
> environment always a previous buffer commit to signal back. I create
> therefore dummy buffer commits, which then trigger the callback after every
> frame if there are events in the queue. This seems to work most of the
> times, but I've problems on window resizing. Apparently in these situations
> it's difficult to pick the right Pixmap and the associated Wayland buffer
> for the dummy commit. So for now I don't allow queuing events, which means
> that Pixmap flips are applied always immediately. What's your opinion? Is
> the construction with the frame callback as a msc counter and the dummy
> commits a bad idea in general or do you maybe know the answer why the
> window resizing is problematic in this context?

Hi,

the frame callback was never intended for "counting vblanks".
Compositors are encouraged to throttle down or completely stop frame
callbacks when the window cannot be seen. Also the definition of the
frame callback is "a good time to start drawing the next frame", which
intends for clients to have the maximum possible time available for
hitting the next deadline for screen. This is an arbitrary point in the
monitor refresh cycle depending on how the Wayland compositor is
scheduling its compositing.

Btw. I would advise against using the frame callback's argument for
anything serious. It is a timestamp in an undefined base and poorly
defined what it might correspond to, and may suffer from wraparound
issues.

Wayland Presentation-time is the extension intended for timing
feedback, and it allows to predict the future presentation times for a
few frames in advance if you can assume a constant frame rate.

https://cgit.freedesktop.org/wayland/wayland-protocols/tree/stable/presentation-time

I believe there are still some gaps in how to reliably submit frames at
a rate lower than the output refresh rate.

Wayland Presentation-queue was an early companion to Presentation-time,
but did not make it in the protocol due to difficulties in defining it
and existing open issues. That extension would have allowed queueing
frames in the Wayland compositor based on target timestamp.

Another idea talked about a long time ago in relation to
Presentation-time was a vblank feedback stream, where every vblank (at
least the ones the compositor processed) would be signalled to
subscribed clients regardless of wl_surface updates, and without
explicit request for every event. It didn't raise enough interest at
the time.

In any case, Wayland aims for timestamp based presentation, not vblank
counting, because of outputs changing their frequencies (mode switch,
variable refresh rate monitors, ...). The feedback in Presentation-time
is ok for that, but using it for prediction is not guaranteed.
Presentation-queue would have improved the functionality a lot by
helping the Wayland compositor's frame scheduling.

Presentation-time does provide the MSC value though, if it exists.

I think we may need to clarify how Presentation-time should work with
wl_surface.commits that do not change the buffer nor apply damage. If
committed feedback request was guaranteed to deliver feedback
regardless of damage (I recall Weston might already work like this),
that would allow an easy way for a client to sync to the output refresh
cycle. The frame callback OTOH definitely should not be used for this.

Could you maybe start a discussion on wayland-devel@ about clarifying
Presentation-time on commits that do not cause a need to repaint in the
compositor?


Thanks,
pq


pgpsguolQSWGg.pgp
Description: OpenPGP digital signature
___
xorg-devel@lists.x.org: X.Org development
Archives: 

Request for feedback for my GSoC project to improve Present support in Xwayland

[Roman Gilg]

Hi,

I'm working for my GSoC project on a better Present integration in
Xwayland. This should improve performance as well as minimize tearing. My
mentor is Daniel Stone and the starting point was my work on KWin and this
bug report: https://bugs.freedesktop.org/show_bug.cgi?id=99702.

I'm writing this mail in order to get some feedback from the X.Org
community regarding my current approach and the progress I've made so far.
You can find my main working branch at
https://github.com/subdiff/xserver/tree/presentInXwayland

I've tried to limit the changes to the Present code to a minimum. At the
moment it should be able to flip Glamor/GBM Pixmaps of Present supporting
clients to a Wayland surface for one full screen window without copies. The
restriction of only one window, which is full screen, is in line with what
is possible with the currently available functionality by the Present code
in the Xserver.

I have tried to queue events on msc timings, but it's not really
straightforward, since I think we need to read out some sort of Vblank
counter from the Wayland server instead of the kernel. The frame callback
is a candidate for that, but it needs at least in my KWin testing
environment always a previous buffer commit to signal back. I create
therefore dummy buffer commits, which then trigger the callback after every
frame if there are events in the queue. This seems to work most of the
times, but I've problems on window resizing. Apparently in these situations
it's difficult to pick the right Pixmap and the associated Wayland buffer
for the dummy commit. So for now I don't allow queuing events, which means
that Pixmap flips are applied always immediately. What's your opinion? Is
the construction with the frame callback as a msc counter and the dummy
commits a bad idea in general or do you maybe know the answer why the
window resizing is problematic in this context?

One of the basic ideas I had in the beginning was to create a separate
dummy RRCrtc for every Xwayland window. This was done under the assumption
that we don't only want to flip one window at a time. But since the Present
code doesn't support flipping multiple windows at all at the moment, we
could also use the xwl_outputs structs instead. That said, the current code
works and if there is nothing wrong with creating these RRCrtc, I would
leave it for now this way.

The code is only tested in a KWin session with a rootless Xwayland server.
I don't know if it could work in a non rootless Xwayland server as well,
maybe with some small adjustments. For now in this case it just falls back
to the old behavior without flips.

I have also worked on a second solution as a long term strategy. But it's
probably not in the scope of my GSoC project: The idea would be to remodel
the Present code in order to support Pixmap flips per CRTC, which would
enable the Xwayland DDX to flip every Wayland window individually. This
would be important if a compositor wants to support the scanout of client
buffers to overlay planes different to the primary planes of the CRTCs. In
my opinion it would also greatly increase the functionality in other DDX,
since being able to flip in a multi monitor setting a full screen window on
one CRTC alone is to my knowledge currently not possible when using only
one Screen struct in combination with RandR. But Daniel already warned me
that this rewrite would be very difficult to get right or maybe not doable
at all. The main reason for that is presumably that the Present code uses
in central parts the Screen Pixmap for its flipping. Also it would mean,
that current Present supporting DDX drivers need to be rewritten in some
parts.

This also leads to an explicit question in the end: Present changes the
Screen and Window Pixmaps via a TraverseTree operation. Why is this
necessary and why did using the same construction not work in my Xwayland
code? I change the Window Pixmaps currently directly in the Xwayland DDX
without traversing the tree:
https://github.com/subdiff/xserver/blob/presentInXwayland/hw/xwayland/xwayland-present.c#L224

Thanks in advance for your feedback.

Cheers
Roman
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel