Summary:
So far we were following a bit unique and rare doxygen comment style:
/**
* Contents of the comment.
**/
Doxygen comments with this style look balanced and neat, but many people
that contribute to KWin don't follow this style. Instead, they prefer
more traditional doxygen comment style, i.e.
/**
* Contents of the comment.
*/
Reviewing such changes has been a bit frustrating for me (so selfish!)
and for other contributors.
This change switches doxygen comment style in KWin to a more traditional
style. The main reason for doing this is to make code review process easier
for new contributors as well us.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D22812
Summary:
Currently only SVG-based decoration themes react to border size changes.
QML-based decoration themes on the other hand, for example Plastik, don't
react to such changes, only new windows will have decorations with the
new border size.
BUG: 408035
Reviewers: #kwin
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D22665
Summary:
kwin(28512) QPainter::begin|QPainter::QPainter|KWin::Decoration::Renderer::renderToImage QPainter::begin: Paint device returned engine == 0, type: 3
kwin(28512) QPainter::setRenderHints|QPainter::setRenderHint|KWin::Decoration::Renderer::renderToImage QPainter::setRenderHint: Painter must be active to set rendering hints
kwin(28512) QPainter::setWindow|KWin::Decoration::Renderer::renderToImage|?KWinX11Platform.so? QPainter::setWindow: Painter not active
kwin(28512) QPainter::setClipRect|KWin::Decoration::Renderer::renderToImage|?KWinX11Platform.so? QPainter::setClipRect: Painter not active
Test Plan: seems to happen on startup, at least (when restarting kwin)
Reviewers: graesslin, zzag
Reviewed By: zzag
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D9014
Summary:
Currently code base of kwin can be viewed as two pieces. One is very
ancient, and the other one is more modern, which uses new C++ features.
The main problem with the ancient code is that it was written before
C++11 era. So, no override or final keywords, lambdas, etc.
Quite recently, KDE compiler settings were changed to show a warning if
a virtual method has missing override keyword. As you might have already
guessed, this fired back at us because of that ancient code. We had
about 500 new compiler warnings.
A "solution" was proposed to that problem - disable -Wno-suggest-override
and the other similar warning for clang. It's hard to call a solution
because those warnings are disabled not only for the old code, but also
for new. This is not what we want!
The main argument for not actually fixing the problem was that git
history will be screwed as well because of human factor. While good git
history is a very important thing, we should not go crazy about it and
block every change that somehow alters git history. git blame allows to
specify starting revision for a reason.
The other argument (human factor) can be easily solved by using tools
such as clang-tidy. clang-tidy is a clang-based linter for C++. It can
be used for various things, e.g. fixing coding style(e.g. add missing
braces to if statements, readability-braces-around-statements check),
or in our case add missing override keywords.
Test Plan: Compiles.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, apol, romangg, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D22371
Summary:
Position and most importantly scaling will likely be the same on the next run,
This saves a flicker round at startup when scaling is different of 1 initialising the view at 1 then jumping at whatever the user requested.
Test Plan: Restarted my system several times
Reviewers: #kwin, #plasma, romangg
Reviewed By: #kwin, #plasma, romangg
Subscribers: davidedmundson, zzag, romangg, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D22292
Summary: QColor::dark() and QColor::light() are obsolete since long time ago.
Reviewers: #kwin, apol
Reviewed By: apol
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D22360
Summary:
QImage::byteCount() was deprecated in Qt 5.10. It is advised to use
QImage::sizeInBytes() method instead.
Reviewers: #kwin, apol
Reviewed By: apol
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D22355
Summary:
QRegion::rects was deprecated in Qt 5.11. It is advised to use begin()
and end() methods instead.
Reviewers: #kwin, romangg
Reviewed By: #kwin, romangg
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D22353
Summary:
To streamline Compositor code more remove the composite reset timer. The two
times it was used we can either use a singleshot timer instead or connect the
call to a different signal in the X11 backend.
Long term goal is to have a well structured init of the Compositor such that
we can call directly instead.
Test Plan: Manually in X and Wayland nested session.
Reviewers: #kwin, zzag
Reviewed By: #kwin, zzag
Subscribers: zzag, kwin
Tags: #kwin
Maniphest Tasks: T11071
Differential Revision: https://phabricator.kde.org/D22270
Summary:
Font scaling is based on the screen scale factor so we can just return
dpi of 96.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D22219
Summary:
The new class is responsible for parsing EDID blobs. It has pretty
straightforward API: you hand blob data to the constructor and after
that you're ready to query parsed data, e.g. physical size, etc.
The main reason to extract EDID parsing code into a class is to clean up
drm_output.cpp a bit.
Test Plan: Compiles, the DRM platform still works.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D22211
Summary:
Depending on whether the underlying platform supports offscreen surfaces,
QOffscreenSurface may create an invisible QWindow. In our case that's the
case, for each offscreen surface a native window is created. This may
lead to some funky results related to window decorations, see bug 407612.
There are several ways to implement offscreen surfaces - either use pbuffers
or utilize a surfaceless context extension. For the sake of simplicity
this change sticks with pbuffers, but it's a good idea to support both
methods.
CCBUG: 407612
Reviewers: #kwin, romangg
Reviewed By: #kwin, romangg
Subscribers: romangg, alexeymin, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D22150
Summary:
The color correction manager doesn't make any specific assumptions about
underlying platform, e.g. whether it's x11, etc. The platform just
has to be capable of setting gamma ramps. Given that, there are no any
significant technical blockers for making this feature work on x.
Reviewers: #kwin, davidedmundson, romangg
Reviewed By: #kwin, davidedmundson, romangg
Subscribers: romangg, neobrain, GB_2, filipf, davidedmundson, ngraham, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D21345
Summary:
Represent outputs in the X11 session via AbstractOutput. For that we
move all Wayland specific parts of AbstractOutput into a new subclass
AbstractWaylandOutput and let the outputs of our Wayland backends inherit
from there.
This should allow us to get rid of the Screens class later on.
Test Plan: Manually in X session.
Reviewers: #kwin, zzag, davidedmundson
Reviewed By: #kwin, zzag, davidedmundson
Subscribers: ngraham, zzag, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D19208
Summary:
To homogenize our backends and as another step to remove the Screens class
use the AbstractOutput class in the windowed X11 backend.
Test Plan: Manually in X session.
Reviewers: #kwin, zzag
Reviewed By: #kwin, zzag
Subscribers: davidedmundson, zzag, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D19207
Summary:
To homogenize our backends and as another step to remove the Screens class
use the AbstractOutput class in the framebuffer backend.
Test Plan: Manually on VT enforcing the framebuffer backend.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D19199
Summary:
So far KWin didn't send axis_source, axis_discrete, and axis_stop. Even
though most of those events are optional, clients need them to work as
expected. For example, one needs axis_source and axis_stop to implement
kinetic scrolling; Xwayland needs axis_discrete to prevent multiple
scroll events when the compositor sends axis deltas greater than 10, etc.
BUG: 404152
FIXED-IN: 5.17.0
Test Plan:
* Content of a webpage in Firefox is moved by one line per each mouse
wheel "click";
* Scrolled gedit using 2 fingers on GNOME Shell, sway, and KDE Plasma;
in all three cases wayland debug looked the same (except diagonal scroll
motions).
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D19000
Summary:
Returning a reference to a value on the stack is broken.
This caused kwin_wayland to crash in openQA with a nullptr deref.
BUG: 407199
Test Plan: Only build tested.
Reviewers: #kwin, zzag
Reviewed By: #kwin, zzag
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D21085
Summary:
Every time a display gets connected/disconnected, every output will be
reinitialized. We don't really need to reset the scaling to 1 every time
we don't know, especially since the setting has been set voluntarily.
Test Plan: When I disconnect the external display, my laptop doesn't fallback to scale=1.
Reviewers: #plasma, apol
Reviewed By: apol
Subscribers: davidedmundson, ngraham, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D20521
Summary:
This is the initial implementation of a DRM backend based on the EGLDevice,
EGLOutput, and EGLStream extensions, supporting NVIDIA graphics hardware using
their proprietary driver. The new backend will be used if the environment
variable KWIN_DRM_USE_EGL_STREAMS is set. On initialization, it will attempt to
create an EGLDevice based on the DRM device currently in use and create
EGLOutputs and EGLStreams for any attached displays. These are used to control
presentation of the final composited frame. Additionally, it will register the
wl_eglstream_controller Wayland interface so that native EGL windows created by
clients can be attached to an EGLStream allowing buffer contents to be shared
with the compositor as a GL texture.
At this time there are two known bugs in the NVIDIA driver's EGL implementation
affecting desktop functionality. The first can result in tooltip windows drawn
by plasmashell to contain incorrect contents. The second prevents KWayland from
being able to query the format of EGLStream-backed buffers which interferes
with the blur effect. Fixes for both of these are currently in development and
should appear in an upcoming NVIDIA driver release.
Additionally, hardware cursors are currently not supported with this backend.
Enabling them causes the desktop to intermittently hang for several seconds.
This is also likely a bug in the NVIDIA DRM-KMS implementation but the root
cause is still under investigation.
Test Plan:
On a system with an NVIDIA graphics card running a recent release of their
proprietary driver
* Ensure the nvidia_drm kernel module is loaded with the option "modeset=1"
("# cat /sys/module/nvidia_drm/parameters/modeset" should print "Y")
* Ensure EGL external platform support is installed
https://github.com/NVIDIA/eglexternalplatform
* Ensure KWin was build with the CMake option
KWIN_BUILD_EGL_STREAM_BACKEND=ON (this is the default)
* Start a plasma wayland session with the environment variable
KWIN_DRM_USE_EGL_STREAMS set
* Ensure output from KWin OpenGL initialization indicates the NVIDIA EGL
driver is in use (as opposed to Mesa / llvmpipe).
* Desktop should be fully functional and perform smoothly.
Reviewers: #kwin, romangg, davidedmundson
Reviewed By: #kwin, romangg, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D18570
Summary:
Since QPA only performs off-screen rendering, it does not require an EGLConfig
that supports any particular surface type. Currently it requests a config
supporting window surfaces, however the EGLDeviceEXT platform, which is the
basis of the EGLStream back-end used with the proprietary NVIDIA driver, does
not support such surfaces resulting in eglChooseConfig not returning any valid
configs when that back-end is in use. Since QPA doesn't actually create any
window surfaces, it should be fine to remove this restriction to allow it to
initialize properly with the EGLStream back-end.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kwin, davidedmundson
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D20455
Summary:
Currently there is no any good reason for keeping ScopedDrmPointer;
providing our own deleter for QScopedPointer would make more sense.
Given that we already have type that acts as a scoped pointer for drm
objects we can improve it a bit and make simpler, e.g.
DrmScopedPointer<drmModeConnector> connector;
is much simpler than
ScopedDrmPointer<_drmModeConnector, &drmModeFreeConnector> connector;
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: apol, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D19905
Summary:
Otherwise it thinks outputs couldn't be created and it leaves.
Problem is that after this fix it crashes when it actually does things with:
kwin_wayland: /home/apol/devel/frameworks/kwin/composite.cpp:646: void KWin::Compositor::aboutToSwapBuffers(): Assertion `!m_bufferSwapPending' failed.
Test Plan: As said, it proceeds correctly and crashes elsewhere
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D19921
Summary:
The NVIDIA implementation of glXSwapBuffers will, by default, queue up
to two frames for presentation before blocking. KWin's compositor,
however, assumes that calls to glXSwapBuffers will always block until
the next vblank when rendering double buffered. This assumption isn't
valid, as glXSwapBuffers is specified as being an implicit glFlush,
not an implicit glFinish, and so it isn't required to block. When this
assumption is violated, KWin's frame timing logic will
break. Specifically, there will be extraneous calls to
setCompositeTimer with a waitTime of 0 after the non-blocking buffer
swaps, dramatically reducing desktop responsiveness. To remedy this,
a call to glXWaitGL was added by Thomas Luebking after glXSwapBuffers
in 2015 (see bug 346275, commit
8bea96d701). That glXWaitGL call is
equivalent to a glFinish call in direct rendering, so it was a good
way to make glXSwapBuffers behave as though it implied a glFinish
call.
However, the NVIDIA driver will by default do a busy wait in glFinish,
for reduced latency. Therefore that change dramatically increased CPU
usage. GL_YIELD can be set to USLEEP (case insensitive) to change
the behavior and use usleep instead. When using the NVIDIA driver,
KWin will disable vsync entirely if GL_YIELD isn't set to USLEEP
(case sensitive, a bug in KWin).
However, the NVIDIA driver supports another environment variable,
__GL_MaxFramesAllowed, which can be used to control how many frames
may be queued by glXSwapBuffers. If this is set to 1 the function
will always block until retrace, in line with KWin's expectations.
This allows the now-unnecessary call to glXWaitGL to be removed along
with the logic to conditionally disable vsync, providing a better
experience on NVIDIA hardware.
Reviewers: #kwin, davidedmundson, zzag
Reviewed By: #kwin, davidedmundson, zzag
Subscribers: kwin, davidedmundson, zzag
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D19867
Summary:
This patch rewrites large parts of the Wayland platform plugin, in order to
facilitate the testing of multi output behavior in nested KWin sessions.
For that a new class WaylandOutput is introduced, which is based on
AbstractOutput and by that shares functionality with our virtual and DRM
platform plugins.
The EGL/GBM and QPainter backends have been remodelled after the DRM one,
sharing similiarities there as well now.
Pointer grabbing has been rewritten to support multiple outputs, now using
pointer locking instead of confining and drawing in this case onto a sub-
surface, which get dynamically recreated in between the different output
surfaces while the cursor is being moved.
Window resizing is possible if host supports xdg-shell, but currently the
mode size does not yet fill the new window size.
The number of outputs can be set by command line argument `--output-count`,
scaling is also supported by setting the argument `--scale`.
Further steps could be:
* Enabling automatic fill of resized windows via Wayland mode change
* Multiple diverging initial sizes and scale factors for mulitple outputs
**Watch it in action:** https://youtu.be/FYItn1jvkbI
Test Plan: Tested it in live session.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, zzag, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D18465
Summary:
If hardware cursor support is not available when using the drm backend for
Wayland compositing, the software cursor texture will not be updated when the
cursor image changes, and it will still be drawn when no cursor image is set
(such as when running a full-screen game). Furthermore, the drmModeSetCursor
and drmModeMoveCursor functions will still be unnecessarily called when the
cursor is moved or hidden.
To correct this, SceneOpenGL should connect Platform::cursorChanged as opposed
to Cursor::cursorChanged to its texture update function, as only the former
will be emitted when the cursor is updated and the compositor should check if
the cursor is hidden and the software cursor image is not null before rendering
it. DrmBackend::moveCursor and DrmBackend::hideCursor should also return
immediately if using a software cursor.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D18376
