Correctly align natural layout in present windows
ClosedPublic

Authored by davidedmundson on Wed, May 13, 12:20 PM.

Details

Summary

Present windows works as follows:

  • It moves all windows about until nothing is overlapping with any

other window.

  • This doesn't resize anything so ultimately we end up with a new

co-ordinate space that's bigger than the screen depending on the amount
of overlap.

  • We then render this whole view transformed to the screen

The rectangle "bounds" is in overviewpixels, with "scale" being the
ratio to convert to screen pixels.

When adjusting the new bounds there's an attempt to centre align things.
As bounds is in "overviewpixels" we multiply references to the previous
bounds by scale, and divide everything through at the end. bounds.x/y
were missed.

This is mostly unoticable except on massive super-ultra-wide monitors
which will otherwise have a tendency to shift to the left.

Test Plan

Kai created a whole new test framework for this code that copy pasted
this algorithm then showed mock windows as rectangles

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.
Restricted Application added a project: KWin. · View Herald TranscriptWed, May 13, 12:20 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
davidedmundson requested review of this revision.Wed, May 13, 12:20 PM

Arguably this line could be refactored, but it's already confusing enough to explain, so I tried to keep the diff simple.

apol accepted this revision.Wed, May 13, 12:33 PM
apol added a subscriber: apol.

The change is sound but the code looks terrible.

This revision is now accepted and ready to land.Wed, May 13, 12:33 PM
broulik accepted this revision.Wed, May 13, 12:34 PM
zzag added a subscriber: zzag.Wed, May 13, 1:25 PM

Hmm, so we effectively deal with two coordinate spaces... rect / scale will map rect from the "area" coordinate space to the "bounds" coordinate space. rect * scale will map rect from the "bounds" coordinate space to "area" coordinate space.

  • bounds.x() is in the "bounds" coordinate space while (area.width() - 20 - bounds.width() * scale) / 2 is in the "area" coordinate space
  • bounds.y() is in the "bounds" coordinate space while (area.height() - 20 - bounds.height() * scale) / 2 is in the "area" coordinate space

It seems like instead of multiplying bounds.width() and bounds.height() by scale we could instead divide area.width() and area.height() by scale to simply math, e.g.

bounds.x() - (area.width() / scale - bounds.width()) / 2,
bounds.y() - (area.height() / scale - bounds.height()) / 2,
zzag accepted this revision.Wed, May 13, 1:33 PM
This revision was automatically updated to reflect the committed changes.