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.
BUG: 477820
BUG: 477886
XdgPopupWindow disregards it for the most part anyway and asks workspace
for the placement area directly. Also gives XdgPopupWindow more control
on the placement when it's all contained inside of it for the upcoming commit.
`Placements::placeSmart` searches for an optimal position for windows, attempting to minimize overlap. The core of this algorithm tracks the
client's height and width in `ch` and `cw`, which have been adjusted by -1. This simplifies logic determining the bottom and right points of a
window when you are starting at the top and left points.
However, this decision requires adjusting that number by +1 when doing the opposite: determining the top and left points when you start with
the bottom and right points.
placeSmart cycles through window locations, searching for acceptable nooks and crannies to fit a window in, nicely. It begins by checking
for places to put the top left corner of the window which abut another constraint. If that fails, it then tries to place the bottom right
abutting a constraining feature.
After finding a suitable bottom (or right) location, the top (or left) location must be determined, requiring the -1 adjustment to be undone.
This patch adds that +1 back in.
# The bug it solves
This error can be seen by opening a bunch of windows that are placed using the "Minimal Overlapping" rule. The open space on the screen will be tiled from left to right, and then top to bottom in the windows. Once no more windows can be placed like that, the next window will be placed at the extreme bottom-right corner. However, it will be one pixel too low and one pixel too far to the right---if you try to move the window, it will "snap" to the correct spot.
This single pixel may seem minor or even irrelevant, but when you use the "Present Windows" desktop effect on a multiple-monitor setup, this one pixel will cause the window to show up on both monitors.
Currently windows are scattered in a few separate lists. If you need to
go through the windows, you have to do it piece by piece. On the other
hand, with the overhaul of window types, we've started converging
towards one universal type: Window. Keeping windows in the separate
buckets goes against this design.
Workspace::stackingOrder() already contains all windows. This change
repurposes Workspace::allClientList() from a list of "normal" windows to
all windows, i.e. Workspace::windows(), to be consistent.
There's one API change though. Scripting API will expose other window
types too. This is an intentional change so scripted effects could
operate with all windows. It also matches the current behavior observed
in libkwineffects, which exposes all windows as well.
When checking for overlap with other windows when placing a new window and cascading to avoid complete overlap, ignore those windows that are already covered by other windows further on the top anyway.
The computation of the covered area is not entirely accurate as it uses the bounding rect rather than the combined rects of the windows, but okay enough for our use case imo.
BUG: 466135
Other policy enums are declared in options.h so let's do the same for
placement policy. Besides consistency, another advantage of moving the
enum in kwin namespace is that the enum could be forward declared.
The Window::moveResizeOutput() property is used to track the output
where the window is expected to land after the move or resize operation
completes.
This can be used to decouple the current output from the next output,
which allows us to send better xdg_toplevel.configure_bounds events or
make windows stick to outputs while keeping Window::output() in sync
with the current output layout.
With fractional scaling integer based logical geometry may not match
device pixels. Once we have a floating point base we can fix that. This
also is
important for our X11 scale override, with a scale of 2 we could
get logical sizes with halves.
We already have all input being floating point, this doubles down on it
for all remaining geometry.
- Outputs remain integer to ensure that any screen on the right remains
aligned.
- Placement also remains integer based for now.
- Repainting is untouched as we always expand outwards
(QRectF::toAdjustedRect().
- Decoration is untouched for now
- Rules are integer in the config, but floating in the adjusting/API
This should also be fine.
At some point we'll add a method to snap to the device pixel
grid. Effectively `round(value * dpr) / dpr` though right now things
mostly work.
This also gets rid of a lot of hacks for QRect right and bottom which
are very
confusing.
Parts to watch out in the port are:
QRectF::contains now includes edges
QRectF::right and bottom are now sane so previous hacks have to be
removed
QRectF(QPoint, QPoint) behaves differently for the same reason
QRectF::center too
In test results some adjusted values which are the result of
QRect.center because using QRectF's center should behave the same to the
user.
AbstractOutput is not so Abstract and it's common to avoid the word
"Abstract" in class names as it doesn't contribute any new information.
It also significantly reduces the line width in some places.
The .clang-format file is based on the one in ECM except the following
style options:
- AlwaysBreakBeforeMultilineStrings
- BinPackArguments
- BinPackParameters
- ColumnLimit
- BreakBeforeBraces
- KeepEmptyLinesAtTheStartOfBlocks
A good portion of geometry handling code was written during the X11
times. The main difference between X11 and Wayland is that kwin doesn't
know where a window will exactly be after resize() or moveResize().
In order to handle Wayland specifics, every window has a bounding
geometry that is being manipulated by move(), resize(), and moveResize().
The frameGeometry(), the clientGeometry(), and the bufferGeometry() are
not manipulated by move(), resize(), and moveResize() directly. Almost
everything that manipulates geometry should use moveResizeGeometry().
This creates a problem though, since the clientGeometry() will be
updated only after the client provides a new buffer, kwin has absolutely
no idea what the client geometry for a given move resize geometry will
be.
Another side of the coin is that decoration updates are performed
asynchronously on wayland, meaning that you cannot use border properties
for anything related to geometry handling and you should avoid using
borderLeft(), borderTop(), borderRight(), and borderBottom() in general.
clientGeometry(), bufferGeometry(), and border*() are good only if you
want to forward an event or render something. They can't be used for
manipulating the geometry.
Unfortunately, AbstractClient::checkWorkspacePosition() needs both,
which is a bit of a problem. To add more oil to the fire, contents
of a decorated window can be snapped to a screen edge. This goes against
the nature of geometry updates on wayland, where we try to indicate
the bounds of the frame geometry and avoid using client and buffer
geometries.
In order to make geometry handling more correct on wayland, this change
removes the ability to snap the contents of a decorated window to a
screen edge. This allows to avoid using the client geometry in
checkWorkspacePosition(), which is a very important function that ensures
the window is inside the workspace.
There is nothing wrong with snapping the frame rather than its contents
and that's what kwin used to do. It was changed with the removal of
"Display borders on maximized windows" option, the relevant commit
didn't provide any reasoning behind the change.
The English word "pack" is not really the correct word for these
actions, and does not succeed in communicating what they will do. Since
the actions simply move the active window as far as it will go in the
specified direction, the actions can be renamed to say that instead.
Also rename the action names in the code to match their new UI text for
clarity.
The new overloads take the client (as context) and the desired screen id
or a point and return the client area.
The main motivation behind this change is to make the transition to the
new virtual desktop model where a window can be on several desktops less
painful.
toplevel.h is included in many places. Changing virtualdesktops.h may
trigger rebuild of all kwin.
With this change, only cpp files that use virtualdesktops.h will need to
be recompiled.
Window management features were written with synchronous geometry
updates in mind. Currently, this poses a big problem on Wayland because
geometry updates are done in asynchronous fashion there.
At the moment, geometry is updated in a so called pseudo-asynchronous
fashion, meaning that the frame geometry will be reset to the old value
once geometry updates are unblocked. The main drawback of this approach
is that it is too error prone, the data flow is hard to comprehend, etc.
It is worth noting that there is already a machinery to perform async
geometry which is used during interactive move/resize operations.
This change extends the move/resize geometry usage beyond interactive
move/resize to make asynchronous geometry updates less error prone and
easier to comprehend.
With the proposed solution, all geometry updates must be done on the
move/resize geometry first. After that, the new geometry is passed on to
the Client-specific implementation of moveResizeInternal().
To be more specific, the frameGeometry() returns the current frame
geometry, it is primarily useful only to the scene. If you want to move
or resize a window, you need to use moveResizeGeometry() because it
corresponds to the last requested frame geometry.
It is worth noting that the moveResizeGeometry() returns the desired
bounding geometry. The client may commit the xdg_toplevel surface with a
slightly smaller window geometry, for example to enforce a specific
aspect ratio. The client is not allowed to resize beyond the size as
indicated in moveResizeGeometry().
The data flow is very simple: moveResize() updates the move/resize
geometry and calls the client-specific implementation of the
moveResizeInternal() method. Based on whether a configure event is
needed, moveResizeInternal() will update the frameGeometry() either
immediately or after the client commits a new buffer.
Unfortunately, both the compositor and xdg-shell clients try to update
the window geometry. It means that it's possible to have conflicts
between the two. With this change, the compositor's move resize geometry
will be synced only if there are no pending configure events, meaning
that the user doesn't try to resize the window.
Once in a while, we receive complaints from other fellow KDE developers
about the file organization of kwin. This change addresses some of those
complaints by moving all of source code in a separate directory, src/,
thus making the project structure more traditional. Things such as tests
are kept in their own toplevel directories.
This change may wreak havoc on merge requests that add new files to kwin,
but if a patch modifies an already existing file, git should be smart
enough to figure out that the file has been relocated.
We may potentially split the src/ directory further to make navigating
the source code easier, but hopefully this is good enough already.
