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.
ShmClientBuffer needs to hold a reference to the old memory
map while the buffer is mapped. Currently, it's not a problem
because unmap() must be called soon after map(). But in case
we relax that constraint, shm_pool.resize() can change the
memory map in the ShmPool with a new one, which can cause issues
for the SIGBUS handler because it won't be able to find the
right mapping object.
This is needed to ensure that the values passed to GL_UNPACK_ROW_LENGTH
are reasonable. It's not described in the spec but wlroots already does
the same.
The main motivation behind this change is to make the ItemRendererOpenGL
use a homogeneous coordinate space for texture coordinates in order to
simplify rendering code.
The device pixels have been chosen because they are more agnostic about
the graphics api.
The main motivation behind this change is to make the ItemRendererOpenGL
use a homogeneous coordinate space for texture coordinates in order to
simplify rendering code.
The device pixels have been chosen because they are more agnostic about
the graphics api.
On X11, the stack order of a window can be changed in multiple ways:
- Opposite
- TopIf
- BottomIf
- Above
- Below
You would pass either of those modes plus maybe the above window id. For
this crash, the relevant mode is Above.
Since the Workspace only has one restack function that places the given
window right under the reference window, the Above stack mode
implementation performs a quirk: it walks through the stack in order to
find a window right above the reference window and pass it to the
restack() function.
However, it could be that the window that wants to be restacked is already
above the reference window, so that same window would be passed as the
"under" window to the restack() function.
It's nothing but a miracle that we have not received major complaints
about this issue until now.
The restack() function doesn't like `window` and `under` to be the same
because of code like
unconstrained_stacking_order.removeAll(window);
unconstrained_stacking_order.insert(unconstrained_stacking_order.indexOf(under), window);
The removeAll() function would effectively remove both `window` and `under`
from the unconstrained stack, which breaks the next line because the
indexOf() would return -1, i.e.
unconstrained_stacking_order.insert(-1, window);
This is the reason why the unconstrained_stacking_order contains strange
values such as `0xe`.
In order to fix the crash, this change adds some code to short-circuit
the restack() function if the passed in window and the reference window
are the same. It would be great to clean up X11Window::restackWindow()
and also add ergonomic restack functions in the Workspace, but this can
be done later. The major blocker is lack of proper test coverage of
X11 bits at the moment.
Last but not least, I would like to express my profound gratitude to
Peter Strick for filing the crash report AND providing a VM image that
helped massively with reproducing the crash and finally fixing it.
BUG: 491618
It's leftover after the Deleted type. The unmanaged window will be
removed from the stack when all its references are dropped and
Workspace::removeDeleted() is called.
CCBUG: 491618
The image stride can be other than "width * bpp". In order to properly
handle such a case, we need to set GL_UNPACK_ROW_LENGTH to skip the right
amount of pixels between consecutive rows.
Co-authored-by: Gabriel Souza Franco <gabrielfrancosouza@gmail.com>
Some clients have two or more completely opaque surfaces stacked on top of each other,
optimizing the lower ones out makes direct scanout happen more often and more efficiently
when multiple planes are involved
This is done by
- fixing isFuzzyScalingOnly to not check the [3, 3] component, which is always 1
- making the comparison between transfer functions fuzzy, so small floating point
errors don't prevent two practically identical functions to be optimized out
- switching manual optimizations to use addMatrix instead, which removes the
matrix or replaces it with a multiplier
and the autotest is expanded to test transformations between color descriptions with
transfer functions and reference luminances that are just scaled versions of each
other
The systemd watchdog has three big problems:
- it wrongly restarts KWin in some cases after suspend
- it wrongly restarts KWin in some OOM situations
- it wrongly restarts KWin when doing a live upgrade from the watchdog being disabled to it being enabled
Each of those cases can cause users to lose data and in the upgrade case even require repairing
the system manually, which is not acceptable.
This commit keeps the watchdog code in place for now; the decision to remove it or what to
replace it with isn't as urgent.
This ensures a couple of things:
- avoid pointlessly binding and unbinding the texture
- if the image format needs to be changed, it will be done only once
Use the unpack code path by default, it lets us to simplify the code and
also fix texture uploading when the image stride is not
"width * bytes per pixel".
It's widely supported, and other wayland compositors already require it,
so the chances of breaking things should be minimal, although some embedded
GPUs might be affected, in which case kwin will fallback to software
rendering. With the unpack being always available, we can simplify and
also fix bugs in texture uploading.
This means that we prefer direct scanout over a specific presentation mode (tearing),
which usually just means we first engage direct scanout and program the relevant
properties, and then switch to tearing afterwards.
It also removes a hack for direct scanout with legacy, and is one step less for
implementing overlay plane support.
I'm confident in the implementation and (significant) upstream changes before the protocol
is released as wp-colormanagement-v1 are unlikely, so I think allowing apps to make use of
it in practice is okay at this point.
Once the upstream protocol is released, we'll still drop the xx variant though and apps will
have to use the wp protocol in order to make use of color management features.
The NVidia driver maps them to hardware planes, which means they do not
apply to the cursor because the cursor plane doesn't have these color
operations.
BUG: 491634
The stack layer of a transient doesn't depend on its transient parent.
It is not clear why this code exists, it dates back to a commit 20 years
ago, which is massive and difficult to analyze. But, with that being said,
Window::belongsToLayer() doesn't place transients in parent's stack
layer, stack constraints are used instead.
Black point compensation introduced a tiny bit increased error in the floating point
calculations, but the errors are still way too small to actually matter.
To avoid those unnecessary calculations causing performance issues and more rounding
errors, this commit changes the allowed error to 10^-5.
