Software rotation: orient the screen regardless of whether drm accepted it
Needs ReviewPublic

Authored by apol on Mar 18 2019, 5:00 PM.

Details

Reviewers
None
Group Reviewers
KWin
Summary

In case it fails, tell the OpenGL scene how to rotate the scene.

Test Plan

Rotated my system and the system rotated with me.

Diff Detail

Repository
R108 KWin
Branch
master
Lint
Lint SkippedExcuse: aaa
Unit
No Unit Test Coverage
Build Status
Buildable 9852
Build 9870: arc lint + arc unit
There are a very large number of changes, so older changes are hidden. Show Older Changes
apol updated this revision to Diff 54330.Mar 19 2019, 3:30 PM

cleanup

apol updated this revision to Diff 54343.Mar 19 2019, 5:29 PM

Update text

apol retitled this revision from NOT READY Software rotation: orient the screen regardless of whether drm accepted it to Software rotation: orient the screen regardless of whether drm accepted it.Mar 19 2019, 5:31 PM
apol edited the summary of this revision. (Show Details)
apol edited the test plan for this revision. (Show Details)
apol added a reviewer: KWin.
zzag added a subscriber: zzag.Mar 19 2019, 6:00 PM

It seems like there are still some issues with rotatino :(

zzag added a comment.Mar 19 2019, 6:03 PM

Does this revision depend on D19731?

apol added a comment.Mar 20 2019, 12:48 PM
In D19860#434603, @zzag wrote:

Does this revision depend on D19731?

Not really, no. Maybe it even deprecates it.

apol updated this revision to Diff 54420.Mar 20 2019, 3:51 PM

Rebase to master

apol updated this revision to Diff 54881.Mar 26 2019, 6:44 PM

Fix viewport for external displays

apol updated this revision to Diff 54885.Mar 26 2019, 7:24 PM

polish

zzag added a comment.Mar 26 2019, 8:47 PM

Software rotation still doesn't work correctly on my machine :'(

zzag added inline comments.Mar 28 2019, 7:36 AM
abstract_output.h
121
/**
 * Returns ...
 **/
virtual int softwareRotationAngle() const {
    return 0;
}
outputscreens.cpp
117 ↗(On Diff #54885)

return 0

platformsupport/scenes/opengl/abstract_egl_backend.cpp
31 ↗(On Diff #54885)

This is a kwin core include, thus it should go above.

platformsupport/scenes/opengl/abstract_egl_backend.h
81 ↗(On Diff #54885)

Pointers have to be aligned to the right, i.e.

AbstractOutput *output,
plugins/platforms/drm/drm_output.h
90
int softwareRotationAngle() const {
    return m_softwareRotationAngle;
}
plugins/scenes/opengl/scene_opengl.cpp
659

const int rotation

981–982

Missing braces as well spaces after commas.

apol marked 6 inline comments as done.Mar 28 2019, 2:31 PM
apol updated this revision to Diff 54987.Mar 28 2019, 2:33 PM
apol marked an inline comment as done.

address zzag's comments

apol updated this revision to Diff 55009.Mar 29 2019, 12:09 AM

Address coding style issues pointed out by zzag

apol added a comment.Apr 2 2019, 1:06 PM

Ping, I'm not sure what we're holding off on here.

Software rotation still doesn't work correctly on my machine :'(

What's the status of this?

Also, I would really expect that we have work to do on:

  • blur, coverswitch, screenshot - anything that blits the framebuffer

it'll be taking untransformed contents.

zzag added a comment.Apr 2 2019, 1:54 PM

What's the status of this?

IIRC, the problem is that KWin can't correctly detect whether hardware rotation is supported, which seems still the case.

apol added a comment.EditedApr 2 2019, 4:03 PM
In D19860#442282, @zzag wrote:

What's the status of this?

IIRC, the problem is that KWin can't correctly detect whether hardware rotation is supported, which seems still the case.

So you want me to include [1] from D19731 here? I'm happy to do that, I didn't do it because I think they're separate issues but if that's what's keeping you from reviewing my patch I can include it.
[1]

zzag added inline comments.Apr 3 2019, 9:08 PM
plugins/platforms/virtual/virtual_output.h
53 ↗(On Diff #55009)

Style.

zzag added inline comments.Apr 3 2019, 9:18 PM
plugins/platforms/virtual/virtual_output.h
53 ↗(On Diff #55009)

Also, why 90?

apol updated this revision to Diff 55374.Apr 3 2019, 9:18 PM

Address vlad's comment

apol updated this revision to Diff 55375.Apr 3 2019, 9:19 PM

Actually no 90

apol marked 2 inline comments as done.Apr 3 2019, 9:20 PM
apol added inline comments.
plugins/platforms/virtual/virtual_output.h
53 ↗(On Diff #55009)

I just added that to test locally rotation. Reverted to 0.

zzag added a comment.Apr 3 2019, 9:53 PM

Tweaking projection matrix seems to be very sensible solution, though it would lead to a bunch of other issues (probably not the full list):

  • Background Contrast, Blur, Screenshot, Glide and maybe some others effects would not work correctly on rotated screens;
  • Buffer age is most likely broken;
  • The cursor is corrupted.

Tackling those problems in this patch is not a good idea. Maybe create a new task?

outputscreens.cpp
115 ↗(On Diff #55375)

I know that you follow the rest of the code, but could you please rename this variable?

According to the Frameworks coding style, one shall not use short variable names.

platformsupport/scenes/opengl/abstract_egl_backend.cpp
27 ↗(On Diff #55375)

Please move it before "composite.h"

plugins/platforms/drm/drm_output.cpp
882–900

I know that you copied this code, but could we maybe re-format it accordingly to the Frameworks coding style?

Also, it would be great to use more const.

plugins/platforms/drm/egl_gbm_backend.cpp
242

Abuse of auto.

plugins/platforms/virtual/virtual_output.h
52 ↗(On Diff #55375)

Please remove extra new line.

screens.cpp
292

Please remove extra new line.

apol marked 5 inline comments as done.Apr 3 2019, 11:30 PM
apol added inline comments.
plugins/platforms/drm/drm_output.cpp
882–900

I would rather not.

apol updated this revision to Diff 55386.Apr 3 2019, 11:30 PM

Coding style

apol updated this revision to Diff 55433.Apr 4 2019, 6:13 PM

style

zzag added inline comments.Apr 4 2019, 6:39 PM
plugins/platforms/drm/egl_gbm_backend.cpp
242

Please align pointers to the right.

Side note: It would be great to use clang-format so we don't have to deal with coding style issues.

apol updated this revision to Diff 55447.Apr 4 2019, 11:55 PM

coding style

apol marked an inline comment as done.Apr 4 2019, 11:55 PM
apol added a comment.Apr 9 2019, 11:42 PM

Software rotation still doesn't work correctly on my machine :'(

What's the status of this?

Also, I would really expect that we have work to do on:

  • blur, coverswitch, screenshot - anything that blits the framebuffer it'll be taking untransformed contents.

No it's not. As you can see in SceneOpenGL2::paintSimpleScreen, m_screenProjectionMatrix which is the matrix used by blur (at least) is already taking the rotation into account.

Proof: https://www.youtube.com/watch?v=KWscNeq_AGA

apol updated this revision to Diff 55986.Apr 11 2019, 1:18 PM

Pass screen rotation information to effects and fix taking screenshots of rotated views

apol updated this revision to Diff 56028.Apr 12 2019, 2:50 AM

Clean patch

apol updated this revision to Diff 56343.Apr 16 2019, 2:07 AM

Detect when the display reports portrait by default

apol updated this revision to Diff 56906.Apr 24 2019, 4:37 PM

Disable blur and bg contrast when rotated

It doesn't transform entirely right yet, this way we can fix it down the line without weird corrupt renders

apol updated this revision to Diff 57766.May 8 2019, 3:25 PM

Fix background contrast when rotated

apol updated this revision to Diff 60361.Sat, Jun 22, 3:29 PM

rebase on top of master