Bug 477886

Summary: Minimal Overlapping Window Placement Sometimes Places windows between two monitors under fractional scaling
Product: [Plasma] kwin Reporter: fanzhuyifan
Component: generalAssignee: fanzhuyifan
Status: RESOLVED FIXED    
Severity: minor CC: nate
Priority: NOR Keywords: qt6
Version: git master   
Target Milestone: ---   
Platform: Other   
OS: Linux   
See Also: https://bugs.kde.org/show_bug.cgi?id=477820
https://bugs.kde.org/show_bug.cgi?id=464842
Latest Commit: Version Fixed In:
Attachments: screenshot showing windows off by one pixel

Description fanzhuyifan 2023-12-02 03:27:35 UTC
Created attachment 163740 [details]
screenshot showing windows off by one pixel

SUMMARY
***
NOTE: If you are reporting a crash, please try to attach a backtrace with debug symbols.
See https://community.kde.org/Guidelines_and_HOWTOs/Debugging/How_to_create_useful_crash_reports
***


STEPS TO REPRODUCE
1. Create a dual monitor setup with fractional scaling (e.g., see output of kscreen-doctor -o below). The two monitors should be placed horizontally
2. Set Window Placement to Minimal Overlapping
3. Open a bunch of windows on the monitor on the left (e.g. in my setup, I ran seq 1 5 | xargs -I{} bash -c "xclock -geometry 800x1000 &")

kscreen-doctor -o:

Output: 1 eDP-2
        enabled
        connected
        priority 1
        Panel
        Modes:  0:2560x1600@240*!  1:2560x1600@60  2:1600x1200@60  3:1280x1024@60  4:1024x768@60  5:2560x1600@60  6:1920x1200@60  7:1280x800@60  8:2560x1440@60  9:1920x1080@60  10:1600x900@60  11:1368x768@60  12:1280x720@60
        Geometry: 1920,0 1707x1067
        Scale: 1.5
        Rotation: 1
        Overscan: 0
        Vrr: Automatic
        RgbRange: Automatic
        HDR: incapable
        Wide Color Gamut: incapable
        ICC profile: none
Output: 2 HDMI-A-1
        enabled
        connected
        priority 2
        HDMI
        Modes:  0:1920x1080@60*!  1:1920x1080@72  2:1920x1080@60  3:1920x1080@50  4:1680x1050@60  5:1600x900@60  6:1280x1024@60  7:1440x900@60  8:1280x800@60  9:1280x720@60  10:1280x720@60  11:1280x720@50  12:1024x768@70  13:1024x768@60  14:800x600@72  15:800x600@60  16:800x600@56  17:720x576@50  18:720x480@60  19:640x480@73  20:640x480@60
        Geometry: 0,0 1920x1080
        Scale: 1
        Rotation: 1
        Overscan: 0
        Vrr: incapable
        RgbRange: unknown
        HDR: incapable
        Wide Color Gamut: incapable
        ICC profile: none


OBSERVED RESULT
1. Notice that some windows have one pixel on the screen on the right
2. Toggle "Present Windows", and notice that the windows with one pixel on the other monitor show up on both desktops.

EXPECTED RESULT
1. All windows should fully reside in the desktop opening them.

SOFTWARE/OS VERSIONS
Operating System: Arch Linux 
KDE Plasma Version: 5.90.0
KDE Frameworks Version: 5.246.0
Qt Version: 6.6.1
Kernel Version: 6.6.3-arch1-1 (64-bit)
Graphics Platform: Wayland
Processors: 20 × 13th Gen Intel® Core™ i9-13900H
Memory: 15.2 GiB of RAM
Graphics Processor: Mesa Intel® Graphics
Manufacturer: ASUSTeK COMPUTER INC.
Product Name: ROG Zephyrus G16 GU603VV_GU603VV
System Version: 1.0

ADDITIONAL INFORMATION

This is exactly the same issue that commit 5502ce9 [1] was supposed to solve.
But it still happens in setups with fractional scaling.
The cause is probably mixing ints and qreals in the calculations.

[1] https://invent.kde.org/plasma/kwin/-/commit/5502ce9a9c0420bc4e12c4e224b926020a2e1170
Comment 1 fanzhuyifan 2023-12-02 03:31:45 UTC
I am planning to work on this. Solution probably very similar to the solution in 477820
Comment 2 fanzhuyifan 2023-12-02 04:16:51 UTC
(In reply to fanzhuyifan from comment #1)
> I am planning to work on this. Solution probably very similar to the
> solution in 477820

https://invent.kde.org/plasma/kwin/-/merge_requests/4740
Comment 3 fanzhuyifan 2023-12-04 19:42:53 UTC
Git commit c441ce5cd1a691c245d0d65ae5d9df10c91d328a by Yifan Zhu.
Committed on 04/12/2023 at 17:59.
Pushed by davidedmundson into branch 'master'.

Avoid accidental mixing of qreal and int

The code for placeSmart uses ints to store coordinates and dimensions,
but the various methods of QRectF return qreal types, which can be
non-integers under fractional scaling, causing multiple issues.

This commit explicitly converts the needed quantities to ints,
avoiding the issues.

Previously, the code relied on the assumption that
y = area.y() + area.height(); implies !(y < area.y() + area.height()).
This doesn't always hold when mixing qreals and ints, causing infinite loops
under fractional scaling when attempting to open large windows, as
reported in BUG 477820.

This commit also extends the fix in 5502ce9 to fractional scaling scenarios.
The ceiling of client widths and height is used, instead of the implicit
floor, which caused BUG 477886.
Related: bug 477820

M  +12   -7    src/placement.cpp

https://invent.kde.org/plasma/kwin/-/commit/c441ce5cd1a691c245d0d65ae5d9df10c91d328a