Amends 7ab825cba1.
This shadow "directScanout" variable breaks
if (!directScanout) {
if (!m_backend->present(output, frame)) {
m_backend->repairPresentation(output);
}
}
which can result in present() function getting called twice with the
same `OutputFrame` object: first, at line 359, and the second time at
line 386. That, in its turn, would queue two drm commit objects with
the same OutputFrame and then the behavior is undefined.
Otherwise the last call uses the default, which overrides the previous
calls.
Test plan:
- Go to system settings - shortcuts
- Verify the category for layout switching is now correctly called
"Keyboard Layout Switcher"
CCBUG: 492019
xyz -> xy and xy -> xyz conversions have divisions in them, so we need to
handle the edge cases of the divisor being zero. This can happen if an ICC
profile is invalid, or if the XYZ color space is used.
Avoid using Window::frameGeometry() because its position is invalid.
In the future, the XdgToplevelWindow would need to run the placement
code before updating the Window::frameGeometry(). In order to prepare
for that, this change also makes placeSmart() pull the window size from
a cached QSizeF() object.
Avoid using Window::frameGeometry() because its position is invalid.
In the future, the XdgToplevelWindow would need to run the placement
code before updating the Window::frameGeometry(). In order to prepare
for that, this change also makes placeCascaded() pull the window size
from a cached QSizeF() object.
The main motivation behind this change is to ensure that every placeXyz()
function issues only one move() or moveResize() function so the placement
code is more refactorable, and in general, it's a good idea to avoid
changing the geometry until the final geometry is finally known.
Note that it changes the behavior of placeOnMainWindow() and
placeUnderMouse(). Those methods will not anymore potentially resize
the window. It was done to avoid a flicker glitch on Wayland. This is
the responsibility of the caller (X11Window) to further inspect whether
the window needs to be resized. However, for now, let's keep things
simple and revisit this if somebody actually complains about it.
Workspace::cascadeOffset() can be called by placement code before the
frame geometry has a valid position. That is wrong. In order to avoid
that, use the placement area that we are given.
When the user closes the active window, with separate screen focus disabled, a
window on the other screen might get activated, which also switches the active
screen to the other one. As this is quite unintuitive, and in my testing having
separate screen focus enabled didn't have any other unintuitive side effects,
this commit enables separate screen focus by default
Instead of just clipping when HDR content is brighter than the maximum luminance the
screen can show, when HDR metadata indicates this could happen, KWin now
- converts the rgb colors to ICtCp, to split luminance and color
- applies a tone mapping curve that maps the intensity component from
- [0, reference] to [0, newReference] linearly
- [reference, max content luminance] to [newReference, max display luminance] nonlinearly
- converts the resulting ICtCp color back to rgb
The result is that HDR content looks much, much better on SDR displays, at least when decent
HDR metadata is provided.
As wrong metadata could cause this tone mapping to wrongly kick in in games for example, the
environment variable KWIN_DISABLE_TONEMAPPING is provided to disable tone mapping and fall back
to clipping again instead.
If effects->grabKeyboard() fails and effects->ungrabKeyboard() is called
later, kwin will crash due to an assert in the ungrabKeyboard() function.
This matters only on X11.
It's more readable and it also makes code more refactorable. For example,
if somebody makes Workspace::stackingOrder() return a normal QList, KWin
won't crash at this code.
The synthetic ConfigureNotify event is not sent when the stack mode is
TopIf and BottomIf, i.e. the way the restackWindow() function handles
the "send_event" argument is inconsistent.
And the window manager doesn't have to send a synthetic configure event
when processing a _NET_RESTACK_WINDOW client message. It must do it only
after handling a ConfigureRequest request, which
the X11Window::configureRequestEvent() function already does.
So rather than fixing the handling of the "send_event" argument, this
change removes that boolean trap instead.
The first item in the `chain` list corresponds to the last item in
the focus chain. The last item in the `chain` list corresponds to
the first item in the focus chain.
Given that, we need to traverse the list from the start to the end in
order to find the first window owned by the same app as `reference`.
Whenever tearing is desired, this does an atomic test to figure out if the current
configuration can do tearing - if not, the backend just transparently falls back to
synchronous commits.
As the kernel (as of Linux 6.9) rejects all commits that are both async and modify
more than the primary plane FB_ID property, this disables the cursor plane and
IN_FENCE_FD usage, to make it more likely for the atomic commit to succeed.
Once these restrictions are loosened, these checks can be removed as well.
Boolean traps are a code smell that makes code harder to read and it
also usually makes the api more difficult.
The force == false is used by two places:
- in Workspace::restackWindowUnderActive()
- and in X11Window::restackWindow()
Technically, the restackWindow() function doesn't need the force == false
behavior because the stackBelow() function is used in code paths where an
explicit sibling is provided.
The restackWindowUnderActive() function is trickier, it is used in the case
when kwin rejects to raise a newly mapped window, so for that purpose, I
changed the function so it picks the lowest window that belongs to the same
application as m_activeWindow.
The result of this change is that the stackBelow() is simpler and its
behavior is much more predictable, which is important when analyzing the
code that updates the stack.
Above and Below modes should be handled as follows:
if (sibling) {
workspace_stack_below/above(window, sibling);
} else {
workspace_lower/raise(window);
}
But with the current code, it's difficult to see. This change rearranges
the code in the restackWindow() so it's more clear how the Above and the
Below modes are implemented.
It also moves workspace()->findClient(Predicate::WindowMatch, above); to
the top because all branches have it.
There are several ways how a window can be restacked relative to another:
either by placing it below the reference window or above the reference
window.
Currently, the restack() function places the given window below the
refererence window. But it's hard to decipher from the name, one needs
to take a look at the argument names to understand what's going on.
Another reason to rename is that it could be useful to have a stackAbove()
function to make X11Window::restackWindow() implementation simpler and
more straightforward.
The shape region is used to clip the window contents. But, in practice,
it's used by a few applications. The most notable is xeyes.
The APIs that the shape region requires are manageable, but it would be
much preferred if we had a much simpler design.
In terms of the shape region support on Wayland, it's not widely
supported across all wayland compositors, to my knowledge, only two
support it, some compositors even want to disable XSHAPE extension at all.
This change makes the Xwayland windows ignore the shape region to see if
any real world applications are affected by it. If not, then we could
safely simplify the scene bits later.
The Xorg session is unaffected by this change.
It's used only by the decoration renderer, but even it doesn't need it
because the atlas parts are padded.
From the API point of view, it's worth looking for alternative solutions,
like integrating the render target clear step in the render passes. And
texture uploading code usually doesn't need to clear the texture because
it is going to overwrite its contents anyway.
The shared memory buffer may need to install a SIGBUS handler if the
compositor _may_ access the data outside of the buffer.
However, signal handling requires great care. For the simplicity sake,
only one shared memory buffer is allowed to be accessed at the time.
But such a restricted access policy is inconvenient and requires us
making client buffer contents copies.
This change eases the restrictions on map() and unmap() functions so
several buffers can be accessed simulatenously.
Note that atomic pointers are used because a signal handler can be
invoked at any point in the main thread's execution. Even though
"a = b" is a single operation from the developer's pov, it can take
several steps for the CPU to store a new value to the variable. The
atomic pointers ensure that assignments to the next and s_accessedBuffers
happen atomically.
Also keep in mind that we cannot start removing copy() calls yet
because the ShmClientBuffer life time management requires further
changes, which I believe, are out of the scope of this patch.
