Differential D4309

fix isOutputRedundant logic
ClosedPublic
Actions

Authored by mart on Jan 27 2017, 1:57 PM.

Details

Reviewers
davidedmundson
sebas
Group Reviewers
Plasma
Commits
R871:f4faa6f1547c: fix isOutputRedundant logic
R871:3792ef9e51dc: fix isOutputRedundant logic
R120:996f5b199f40: fix isOutputRedundant logic
R120:f4faa6f1547c: fix isOutputRedundant logic
R120:3792ef9e51dc: fix isOutputRedundant logic
Summary

isOutputRedundant badly failed when two screens had
the exact same geometry (that should be, the base
case for "cloning" screens.
now the logic is:
a screen is redundant if:

  • its geometry is contained in another one
  • if their resolutions are different, the "biggest" one wins
  • if they have the same geometry, the one with the lowest id wins (arbitrary, but gives reproducible behavior and makes the primary

BUG:375507

Test Plan

tested with two screens:

  • overlapping, same resolution: the lowest id (0, the primary) wins, the other one doesn't get a view
  • overlapping, different resolutions: the biggest one wins
  • not overlapping: both get a view

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mart updated this revision to Diff 10616.Jan 27 2017, 1:57 PM
mart retitled this revision from to fix isOutputRedundant logic.
mart updated this object.
mart edited the test plan for this revision. (Show Details)
mart added reviewers: Plasma, sebas.
Restricted Application added a project: Plasma. · View Herald TranscriptJan 27 2017, 1:57 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
mart updated this revision to Diff 10617.Jan 27 2017, 1:59 PM
  • fix comment
davidedmundson added a subscriber: davidedmundson.Jan 27 2017, 2:02 PM
davidedmundson added inline comments.
shell/shellcorona.cpp
1132

there's a bug here, but I'm not sure what the right fix is.

If you have two screens A, B

both are new (so have ID == -1)

A is wider than B but B is taller than A

you return true for both.

mart added inline comments.Jan 27 2017, 2:06 PM
shell/shellcorona.cpp
1132

I don't think this can happenas at this point
otherGeometry.contains(thisGeometry, false) is true, which means either:

  • otherGeometry and thisGeometry have the same resolution,
  • otherGeometry has both width nad height bigger then thisGeometry
  • otherGeometry has the same width, but bigger height
  • othergeometry has the same height but bigger width

other cases, contains would be false

mart updated this revision to Diff 10629.Jan 27 2017, 3:15 PM
  • reconsider outputs when the primary changes
davidedmundson accepted this revision.Jan 27 2017, 3:22 PM
davidedmundson added a reviewer: davidedmundson.

Maybe 5.8 material too.

This revision is now accepted and ready to land.Jan 27 2017, 3:22 PM
mart added a comment.Jan 27 2017, 3:41 PM
In D4309#80804, @davidedmundson wrote:

Maybe 5.8 material too.

definitely, as is a serious multiscreen bug

Closed by commit R120:996f5b199f40: fix isOutputRedundant logic (authored by mart). · Explain WhyJan 27 2017, 3:43 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mshell/shellcorona.cpp (35 lines)

Diff 10630

View Options

shell/shellcorona.cpp

Loading...