Differential D10465

Fix coverswitch broken reflection on multi-monitor setup
ClosedPublic
Actions

Authored by poboiko on Feb 12 2018, 4:06 PM.

Details

Reviewers
davidedmundson
Group Reviewers
KWin
Commits
R108:2eded918977f: [effects/coverswitch] Fix broken reflection on multi-monitor setup
R108:e3f0b537b6b4: [effects/coverswitch] Fix broken reflection on multi-monitor setup
Summary

This patch fixes bug 378688.

The code which was used to draw reflections before overrode ModelViewMatrix (which was aware of multi-screen setup) by reflection matrix.
Now it just multiplies old value by reflection matrix instead.

Since setProjectionMatrix is not needed when painting the window itself, I assumed it shouldn't be there to paint the reflection as well. Apparently, it was redundant.

Test Plan
  • Have single monitor, pressed alt+tab several times. Reflections are OK.
  • Attached another monitor, switched focus to it. Pressed Alt+Tab several times again. Still OK.
  • Switched focus back to prime monitor, alt+tab still works fine
  • Disconnected monitor, tested again - still works

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.
poboiko created this revision.Feb 12 2018, 4:06 PM
Restricted Application added a subscriber: kwin. · View Herald TranscriptFeb 12 2018, 4:06 PM
poboiko requested review of this revision.Feb 12 2018, 4:06 PM
davidedmundson accepted this revision.Mar 23 2018, 12:57 PM
davidedmundson added a subscriber: davidedmundson.

Works nicely, good stuff

This revision is now accepted and ready to land.Mar 23 2018, 12:57 PM
Closed by commit R108:e3f0b537b6b4: [effects/coverswitch] Fix broken reflection on multi-monitor setup (authored by poboiko). · Explain WhyMar 25 2018, 10:30 AM
This revision was automatically updated to reflect the committed changes.
fvogt added a subscriber: fvogt.Mar 25 2018, 10:43 AM

What about the Plasma/5.8 and Plasma/5.12 branches?

poboiko added a comment.Mar 25 2018, 1:16 PM
In D10465#233517, @fvogt wrote:

What about the Plasma/5.8 and Plasma/5.12 branches?

I don't have experience in backporting these kind of patches (and don't know how to decide whether some fix needs backporting or not).
Can someone give me a hand with this? Should I just duplicate this commit to corresponding (5.8, 5.12) git branches?

fvogt added a comment.Mar 25 2018, 1:22 PM

In general, all fixes which apply to maintained branches and have a low potential of causing regressions should be backported.

For this, just cherry-pick the commit into the Plasma/5.8 branch, merge Plasma/5.8 into Plasma/5.12 and then Plasma/5.12 into master and push all of them.

graesslin added a subscriber: graesslin.Mar 26 2018, 4:30 AM

I'm against pushing to 5.8 - this is not a security issue.

fvogt added a comment.Mar 26 2018, 5:00 PM

Tomorrow is release of 5.12.4, so I cherry-picked the commit into the Plasma/5.12 branch and merged.

Revision Contents
Changeset List

Diff 30483

View Options

effects/coverswitch/coverswitch.cpp

Loading...