Implment DRM EGL scaling
ClosedPublic

Authored by davidedmundson on Nov 25 2016, 10:27 AM.

Details

Summary

We need to set the viewport so that we scale from device pixels to global compositor space.

Test Plan

Ran kwin_wayland properly on my laptop without setting KWIN_COMPOSE.
Most things worked.

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
davidedmundson retitled this revision from to Implment DRM EGL scaling.
davidedmundson updated this object.
davidedmundson edited the test plan for this revision. (Show Details)
davidedmundson added a reviewer: Plasma.
Restricted Application added a project: KWin. · View Herald TranscriptNov 25 2016, 10:27 AM
Restricted Application added subscribers: kwin, plasma-devel. · View Herald Transcript
graesslin requested changes to this revision.Nov 25 2016, 2:23 PM
graesslin added a reviewer: graesslin.
graesslin added a subscriber: graesslin.

Same as for the X11 backend, you cannot just change the viewport

This revision now requires changes to proceed.Nov 25 2016, 2:23 PM

As discussed when I met at the Plasma sprint - changing the viewport is semantically exactly what we want.

Yes it does break many things, but I have also (hopefullly!) fixed those many things.
Can you reconsider this and D3500 in light of that.

Then I think then this branch is all done \o/

Just a heads up: Atomic mode setting allows to scale buffers / content of a plane directly instead of using GL. But I assume this would be possible to add later.

Something else: How does this work, when Gl / compositing is skipped (i.e. in my case for the direct scanout of a wayland buffer)? Is it then not scaling at all? We need to make sure that in this case the dimensions are still fine. Otherwise we need to restrict direct scanout to AMS only.

plugins/platforms/drm/egl_gbm_backend.cpp
159–161

You could add to struct Output a value for that, which you then use in makeContextCurrent.

In D3504#97513, @subdiff wrote:

Just a heads up: Atomic mode setting allows to scale buffers / content of a plane directly instead of using GL. But I assume this would be possible to add later.

Although it sort of looks like it, we're not scaling the buffer, in fact this line does the exact opposite setting our viewport to match the size used in the gbm_surface_create.
It's the projection that's scaled, not the buffer.

Something else: How does this work, when Gl / compositing is skipped (i.e. in my case for the direct scanout of a wayland buffer)? Is it then not scaling at all? We need to make sure that in this case the dimensions are still fine. Otherwise we need to restrict direct scanout to AMS only.

From the wayland docs:

A method of "scale" or "driver" implies a scaling operation of the surface, either via a direct scaling operation or a change of the output mode. This will override any kind of output scaling, so that mapping a surface with a buffer size equal to the mode can fill the screen independent of buffer_scale.
A method of "fill" means we don't scale up the buffer, however any output scale is applied.

So given we currently don't do fullscreen_method==fill, we don't need to do any changes for scaling and everything will just work \o/

romangg added inline comments.Mar 25 2017, 7:26 PM
plugins/platforms/drm/egl_gbm_backend.cpp
159–161

Better: Add an additional variable of the "real" unscaled output size in DrmOutput with appropriate getter to also solve the rounding issue from the other Diff.

davidedmundson added inline comments.Mar 25 2017, 9:47 PM
plugins/platforms/drm/egl_gbm_backend.cpp
159–161

I like that. Taking that one step further. I'll just make size() the native size and adjust the rest of the code to that.

romangg added inline comments.Mar 25 2017, 10:24 PM
plugins/platforms/drm/egl_gbm_backend.cpp
159–161

Assuming that you mean with native size the unscaled output size: Just make sure that size() is not used anywhere in the compositor to determine the right rendering size for the compositor internal output representations (which should be native size divided by scaling factor if I've understood it correctly).

davidedmundson edited edge metadata.

use pixelSize

Restricted Application edited projects, added Plasma; removed KWin. · View Herald TranscriptMar 25 2017, 11:09 PM
Restricted Application added a subscriber: KWin. · View Herald Transcript
romangg added inline comments.Mar 31 2017, 1:47 AM
plugins/platforms/drm/egl_gbm_backend.cpp
198

I still don't think this is the right way to use scale here. But it's currently not possible to test it with scale > 1, is it? I tried to use the scale option from D3159, but initialOutputScale is currently only available for X11WindowedBackend. Can you make the test option usable in the DRM backend as well? This won't test different scale factors on multiple monitors though.

Also scale the offset, this fixes a bug if a scaled display is not in global position 0,0

Restricted Application edited projects, added KWin; removed Plasma. · View Herald TranscriptApr 28 2017, 9:23 AM
graesslin accepted this revision.Apr 28 2017, 12:51 PM

@davidedmundson feel free to push the complete series once you think it's ready.

This revision is now accepted and ready to land.Apr 28 2017, 12:51 PM
This revision was automatically updated to reflect the committed changes.