Summary:
KWin only renders into the default framebuffer, which is for example an EGL
surface.
To prepare a post-processing step with a different framebuffer allow the
framebuffer to be changable. For that KWin's current framebuffer must be
communicated to the GLRenderTarget class, which otherwise does not set it back
to KWin's current one when a render target is disabled again.
Test Plan: Compiles, with other patches for Gl based screen rotation
Reviewers: #kwin
Subscribers: fredrik, zzag, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D25904
Summary:
We currently see a gap on transformed windows between the window and the
top decoration.
This is partly the atlas bleed on the decoration, and partly a bleed on
the window content itself.
On X11, the window we composite is the frame window - which is a larger
texture containing a transparent border where the frame normally would
be. When we sample with a linear filter we include these texels. Hence
GL_CLAMP_TO_EDGE doesn't work.
Vlad's patch to composite the correct window, not the frame was my
preferred approach, but we had to revert it as it caused an issue with
xwayland :(
Half pixel correction nearly worked, but caused blurry fonts.
This patch resolves it in the fragment shader used by effects doing
transforms. We pass the real texture geometry of the window to the
client with a half pixel correction. Any samples outside the outer half
pixel are then clamped within bounds.
Arguably a hack, but solves the problem in a comparatively
non-invasive way.
BUG: 360549
BUG: 257566
Test Plan:
X11:
Using Vlad's atlas padding for decoration
Slowed animations, wobbled a dark window over a light background
No artifacts
Wayland:
This isn't needed. Now tested that everything still renders the same.
Reviewers: #kwin, zzag
Reviewed By: #kwin, zzag
Subscribers: zzag, jgrulich, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D25737
Summary:
Won't make things go much faster since everything that was
being passed by value is refcounted but still const & is a bit faster
than refcounting
For shared pointers instead of adding const & we move them into the
destination variable saving some cpu usage but at the same time making
clear the pointer is being stored by not being const &
Reviewers: zzag
Reviewed By: zzag
Subscribers: zzag, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D25022
Summary:
Because KWin is a very old project, we use three kinds of null pointer
literals: 0, NULL, and nullptr. Since C++11, it's recommended to use
nullptr keyword.
This change converts all usages of 0 and NULL literal to nullptr. Even
though it breaks git history, we need to do it in order to have consistent
code as well to ease code reviews (it's very tempting for some people to
add unrelated changes to their patches, e.g. converting NULL to nullptr).
Test Plan: Compiles.
Reviewers: #kwin, davidedmundson, romangg
Reviewed By: #kwin, davidedmundson, romangg
Subscribers: romangg, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D23618
Summary:
Switch to Q_ASSERT in order to make code a bit more consistent. We have
places where both assert and Q_ASSERT are used next to each other. Also,
distributions like Ubuntu don't strip away assert(), let's hope that
things are a bit different with Q_ASSERT.
Test Plan: Compiles.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: romangg, davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D23605
Summary:
If s_renderTargets is not empty, GLRenderTarget::pushRenderTargets starts
to do pretty heavy things: it deletes head of the targets param in a while loop.
There is no need to do that. Because QStack inherits QVector, we can use
append method to push new render targets in a more efficient way.
Test Plan: Background behind Konsole is still blurred.
Reviewers: #kwin, mart
Reviewed By: #kwin, mart
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D13823
Summary:
There are several spaces that have to be considered in `GLRenderTarget::blitFromFramebuffer`:
* KWin logical space: the origin is located at the global top-left corner
* display space: the origin is located at the top-left corner of monitor/display
* OpenGL screen space: the origin is located at the bottom-left corner of monitor/display
Given `s`, which is in the KWin logical space, we have to transform it to the display space, then to the OpenGL screen space:
* KWin logical space -> display space: `y' = s.y() - s_virtualScreenGeometry.y()`
* display space -> OpenGL screen space: `y'' = s_virtualScreenGeometry.height() - y'`
Overall, `srcY0` and `srcY1` should be written as follows:
```
srcY0 = s_virtualScreenGeometry.height() - (s.y() - s_virtualScreenGeometry.y() + s.height())
srcY1 = s_virtualScreenGeometry.height() - (s.y() - s_virtualScreenGeometry.y())
```
Test Plan:
Tweak background contrast effect to use GLRenderTarget::blitFromFramebuffer
```
diff --git a/effects/backgroundcontrast/contrast.cpp b/effects/backgroundcontrast/contrast.cpp
index f920fcd88..5247d83b8 100644
--- a/effects/backgroundcontrast/contrast.cpp
+++ b/effects/backgroundcontrast/contrast.cpp
@@ -447,11 +447,10 @@ void ContrastEffect::doContrast(EffectWindow *w, const QRegion& shape, const QRe
GLTexture scratch(GL_RGBA8, r.width() * scale, r.height() * scale);
scratch.setFilter(GL_LINEAR);
scratch.setWrapMode(GL_CLAMP_TO_EDGE);
- scratch.bind();
- const QRect sg = GLRenderTarget::virtualScreenGeometry();
- glCopyTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, (r.x() - sg.x()) * scale, (sg.height() - sg.y() - r.y() - r.height()) * scale,
- scratch.width(), scratch.height());
+ GLRenderTarget scratchTarget(scratch);
+ scratchTarget.blitFromFramebuffer(r);
+ scratch.bind();
// Draw the texture on the offscreen framebuffer object, while blurring it horizontally
```
GLRenderTarget::blitFromFramebuffer without this change:
{F5817883, layout=center, size=full}
Reviewers: #kwin, fredrik, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kpiwowarski, davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D12452
Summary:
There are several spaces that have to be considered in `GLRenderTarget::blitFromFramebuffer`:
* KWin logical space: the origin is located at the global top-left corner
* display space: the origin is located at the top-left corner of monitor/display
* OpenGL screen space: the origin is located at the bottom-left corner of monitor/display
Given `s`, which is in the KWin logical space, we have to transform it to the display space, then to the OpenGL screen space:
* KWin logical space -> display space: `y' = s.y() - s_virtualScreenGeometry.y()`
* display space -> OpenGL screen space: `y'' = s_virtualScreenGeometry.height() - y'`
Overall, `srcY0` and `srcY1` should be written as follows:
```
srcY0 = s_virtualScreenGeometry.height() - (s.y() - s_virtualScreenGeometry.y() + s.height())
srcY1 = s_virtualScreenGeometry.height() - (s.y() - s_virtualScreenGeometry.y())
```
Test Plan:
Tweak background contrast effect to use GLRenderTarget::blitFromFramebuffer
```
diff --git a/effects/backgroundcontrast/contrast.cpp b/effects/backgroundcontrast/contrast.cpp
index f920fcd88..5247d83b8 100644
--- a/effects/backgroundcontrast/contrast.cpp
+++ b/effects/backgroundcontrast/contrast.cpp
@@ -447,11 +447,10 @@ void ContrastEffect::doContrast(EffectWindow *w, const QRegion& shape, const QRe
GLTexture scratch(GL_RGBA8, r.width() * scale, r.height() * scale);
scratch.setFilter(GL_LINEAR);
scratch.setWrapMode(GL_CLAMP_TO_EDGE);
- scratch.bind();
- const QRect sg = GLRenderTarget::virtualScreenGeometry();
- glCopyTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, (r.x() - sg.x()) * scale, (sg.height() - sg.y() - r.y() - r.height()) * scale,
- scratch.width(), scratch.height());
+ GLRenderTarget scratchTarget(scratch);
+ scratchTarget.blitFromFramebuffer(r);
+ scratch.bind();
// Draw the texture on the offscreen framebuffer object, while blurring it horizontally
```
GLRenderTarget::blitFromFramebuffer without this change:
{F5817883, layout=center, size=full}
Reviewers: #kwin, fredrik, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kpiwowarski, davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D12452
Summary:
Updated the old and outdated blur method to use the much more efficient dual kawase blur method.
Now with this we can do virtually infinite blur with very very little performance cost.
The dual kawase blur method is basically downscaling and upscaling an image, but combined with the kawase blur shader.
Comparison: https://i.imgur.com/mh6Cw61.png
Left is old, right is new.
Comparison was done with the strongest blur setting in a VM running on an Intel i7-4790 and a GTX980
We can see here that the performance is even better with this new method.
Reviewers: #plasma, #kwin, graesslin, fredrik
Reviewed By: fredrik
Subscribers: hein, dos, luebking, broulik, romangg, zzag, anthonyfieroni, mart, davidedmundson, fredrik, ngraham, plasma-devel, kwin, #kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D9848
This code path is executed a lot during animations (e.g. maximize, minimize).
Saves creating a temporary QVector.
Differential Revision: https://phabricator.kde.org/D9082
Summary: D8479 fixes viewport for real hardware, but there is still problem with screenshots. This patch fixes that.
Test Plan:
- Have three monitors (like in bug 385655)
- Press PrtSc and then Enter
- Screenshot is ok
Before:
{F5450056}
After:
{F5450058}
Reviewers: #kwin, #plasma, davidedmundson
Reviewed By: #plasma, davidedmundson
Subscribers: davidedmundson, plasma-devel, kwin, #kwin
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D8485
Summary:
glScissor works on window co-ordinates. i.e not scaled by the viewport
We need to multiply by the scale there.
This fixes a minor visual glitch in animations when using hardware
clipping.
Test Plan:
Hardcoded my minimise animation to be really slow.
Now every frame looks perfect
Reviewers: #kwin
Subscribers: kpiwowarski, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D8490
Summary:
The GL_CONTEXT_LOST flag is not reset when calling glGetError. This of
course bites with: "Thus, glGetError should always be called in a loop,
until it returns GL_NO_ERROR, if all error flags are to be reset."
(see https://www.khronos.org/registry/OpenGL-Refpages/gl4/html/glGetError.xhtml)
As KWin calls checkGLError from init call it can result in a freeze of
KWin. This is a regression reported multiple times since the release of
5.10.3 which enabled GLX_NV_robustness_video_memory_purge extension.
Please note that I am not able to test this change. I do not have an
NVIDIA card and are not hiting the problem. I have no way to simulate
the code. I do not know whether the change will fix the problem, it is
based on what others do. Inspiration for this change is mostly from
mutter: https://git.gnome.org/browse/mutter/commit/?id=d4d2bf0f6c1737256b921c4f1dedd3a95138cab9
BUG: 381870
FIXED-IN: 5.10.3.1
Test Plan: See above, I can only compile check the change
Reviewers: #kwin, #plasma
Subscribers: plasma-devel, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D6464
Summary:
We are getting reports about the shader self test failing in a false
positive way on Mesa 17. We already have such false positve results
for NVIDIA quadro and went with disabling the self test.
Most likely our self test has a problem or the rendering of the first
frame has a problem. But disabling compositing because our rendering of
the first frame is broken, is too much.
Let's better be pro user here and disable the shader self test till we
fixed the root problem.
BUG: 376801
FIXED-IN: 5.10.4
Test Plan: It compiles.
Reviewers: #kwin, #plasma
Subscribers: plasma-devel, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D6426
Summary:
The backend is responsible for setting the viewport, the GLRenderTarget
should save and restore the viewport actually set by the backend.
Test Plan:
Ran Blur effect which uses this method, under scaling the viewport "restored"
differed from the one set by the backend.
Reviewers: #plasma
Subscribers: plasma-devel, kwin, #kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D4952
Summary:
This method already translates to s_virtualScreenGeometry as the source
co-ordinates are in compostior space we should take scaling into account here too.
This method already supports resizing if source and target sizes do not match.
Calling funcitons can either double the size of the target if they want native resolution
or leave as-is if they want the result in standard DPI.
This implements scaling in the screenshot, magnifier and simple blur effect.
Test Plan:
Tested screenshot effect using spectacle.
I hit an (unrelated, was reproducible on master) crash whilst using this, so it is not fully tested
Ran magnifier effect manually
Reviewers: #plasma
Subscribers: plasma-devel, kwin, #kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D4951
Summary:
GLRenderTarget already stores the translation from global compositor
space to the render target co-ordinates, we also need a scale factor for
the exact same reasons.
Test Plan: Used in various effects after this patch and it works there.
Reviewers: #plasma
Subscribers: plasma-devel, kwin, #kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D4948
Summary:
The feature has always been considered experimental. Unfortunately it is
completely unmaintained and hasn't seen any commits in years. It
requires kolor-manager to function, but that has not seen a release
based on frameworks yet. This makes it difficult to maintain. In fact I
have never been able from the introduction till now to setup a color
corrected system. One needs kolor-manager and oyranos and especially the
latter is hardly available on any linux distribution (e.g. not on the
Debian/Ubuntu systems).
Due to being unmaintained color correction in KWin did not keep up with
recent changes. Neither did it see any updates during the xlib->xcb
port, nor during the Wayland port. Especially the Wayland port with the
rendering changes make it unlikely to function correctly. E.g. Wayland
introduced a proper per-screen rendering, while color correction did a
"fake" per screen rendering. How that is going to work in combination is
something nobody ever tried. Now after the introduction of proper
per-screen rendering the solution would be to port color correction to
the new api, but that never happened.
Color correction also modified the shaders, but a newer shader API got
introduced some time ago. Whether the color correction shader support
that or not, is unknown to me. Also which shader language versions are
supported. I know it was based on 3d texture support, which back on
introduction was partially lacking in OpenGL ES. Nowadays that changed,
but color correction didn't update.
Last but not least it is completely X11 based and there is no work on
how to make it work with Wayland.
Given all the problems, especially the fact that it is unmaintained and
cannot be setup on my system, means to me that the only solution is to
remove it.
I'm open to having it reintroduced in future, but only if the
availability on Linux distributions gets addressed before. As long as
major linux distributions do not ship this feature, it should not be in
KWin. Given that I must say that it was a mistake to add it in the first
place and I need to point out that I was against the merge back then.
Reviewers: #kwin, #plasma
Subscribers: plasma-devel, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D3402
Summary:
Not used anywhere in KWin and annotated that they should be removed. As
we have an ABI break in 5.9 we can remove them now.
Reviewers: #kwin, #plasma
Subscribers: plasma-devel, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D3398
Summary:
Instead of having the egl extensions in the global kwinglutils lib it
becomes private to the AbstractEglBackend. Just like on glx the
glxextensions are moved into the platform.
The extensions are queried from initEglAPI, that is as early as possible
after initializing the EGLDisplay. This ensures that any implementing
subclass can access the extensions early enough.
As a note: the EglOnXBackend had a potentially wrong sequence for
initializing the buffer age extension. It is now moved to the correct
place where the result is needed for the first time.
From the global API eglExtensions are removed from hasGLExtension and
the eglExtensions function is dropped. As by that initEGL did not do
anything it is also dropped.
Test Plan:
Tested nested kwin on Wayland, still works, extensions shown
in debug console
Reviewers: #kwin, #plasma
Subscribers: plasma-devel, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D3396
Summary:
Method doesn't do anything. If at some point we want to resolve egl
functions again, it should be done in the Platform, just like
glxResolveFunctions is nowadays done in the platform.
Reviewers: #kwin, #plasma
Subscribers: plasma-devel, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D3391
Summary:
It's unused in KWin and absolutely not needed as the call to
eglInitialize, which every platform does, also gives the version for
free. So no need to do any caching in a global method.
Reviewers: #kwin, #plasma
Subscribers: plasma-devel, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D3390
Summary:
KWin still resolves some OpenGL function pointers. For that it needs to
use either eglGetProcAddress or glxGetProcAddress. With other words the
method to resolve needs to know whether it is egl or glx and needs both
a dependency to egl and glx. Especially the dependency to glx is ugly as
that pulls in XLib into our library.
The way so far was to pass an enum value to the initGL method to know
whether it's EGL or GLX. With this change the enum value is removed and
replaced by a function pointer to resolve the methods.
This simplifies the resolve code and allows to completely remove the glx
variant we still had in the library. Thus kwinglutils library is now glx
and XLib free.
Test Plan: nested KWin with OpenGL/EGL still works
Reviewers: #kwin, #plasma_on_wayland
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D3336
Summary:
No need to resolve glx methods through the shared lib. At the moment
this duplicates some code, but will be cleaned up with a follow up
change.
Reviewers: #kwin, #plasma_on_wayland
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D3335
Summary:
Glx extensions are only interesting to the glxbackend. Given that
querying can be moved there.
In order to simplify the extensions can be stored in the OpenGLBackend
which also provides the convenience check as before.
The egl platforms should also be adjusted to query in that way and
remove it from the kwinglutils.
There is still a usage of the glxextensions inside kwinglutils to
resolve one function. That should also be moved into the platform.
Reviewers: #kwin, #plasma_on_wayland
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D3332
Summary:
Glx version is only required once in the standalone x11 platform. No
need to have that in the shared kwinglutils library.
Removes one xlib usage from shared kwin.
Reviewers: #kwin, #plasma_on_wayland
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D3329
Summary:
The viewport needs adjustment in the per-output rendering case. This
change ensures the viewport is setup like in the platforms which do per
output rendering. For the X11 case (multiple outputs in one render pass)
the values are the same as previously.
Reviewers: #kwin, #plasma_on_wayland
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D3099
Summary:
This is similar to ff6e042c7f just for
GLRenderTarget instead of GLVertexBuffer. The reasoning is the same: on
Wayland KWin has one native window per screen and needs to adjust the
blit target depending on the area.
Reviewers: #kwin, #plasma_on_wayland
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D3056
Summary:
On Wayland in multi-screen a "window" is used to render to per screen.
In OpenGL every "window" is at 0/0 and scissoring is in respect to the
window geometry. The rendering geometry passed to VBO rendering, which
performs the scissoring, is in virtual combined screen geometry space.
Thus the scissoring always removes any rendering on the second screen.
This results in e.g. wobbly windows not working on any additional screen
on Wayland.
This change makes the VBO rendering multi-screen aware by passing the
current virtual window geometry in each rendering pass. That replaces
the so far used virtual screen size. For single screen rendering or
multi-screen rendering on X11 this doesn't change anything as there is
always only one OpenGL window and the screen geoemetry starts at 0/0.
Thus the subtracting of x/y geometry do not affect the scissoring at
all.
Reviewers: #kwin, #plasma_on_wayland
Subscribers: plasma-devel, kwin
Tags: #plasma_on_wayland, #kwin
Differential Revision: https://phabricator.kde.org/D2955
Summary:
The self test fails with NVDIDA 370.23 or newer on Quadro hardware.
Most likely there is a bug in our code as the same things work later on.
But without the hardware we are not able to reproduce and investigate
properly. Given that all we currently can do is to skip the self test.
We encourage users to investigate this properly and to help us to
identify the root issue, so that we can fix it.
CCBUG: 367766
Reviewers: #kwin
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D2744
Summary: No need to parse the OpenGL version twice with different algorithms.
Reviewers: #kwin, bshah
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D2414
Summary:
The new tab shows the information from GLPlatform similar to how it is
shown in the supportInformation. In addition it also lists all available
openGL and EGL/GLX extensions. For that kwinglutils is extended by
functions to return the lists of extensions.
Reviewers: #kwin, #plasma, bshah
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D2385
The idea was to block shaders that operate on
Texture2D while Texture2DRectangle was use, but
that's indeed controlled by TextureNPOT while
LimitedNPOT indicates Texture2D but without support
for mipmapping and exotic clamping
FIXED-IN: 5.6
REVIEW: 126959
One resource is used for shader version 1.10 and one for version 1.40.
The ideas behind this change is to remove the locating of the shader
sources and also to fix that user provided shaders could be loaded
instead of the original ones (possible attack vector on Wayland).
To simplify the ShaderManager provides a new method call to load the
shader from the resource. This means the effects don't need to
duplicate the check for the shader version any more and also don't
need to duplicate the file reading functionality.
REVIEW: 126905
This method allows to generate a shader with ShaderTraits but one
part (e.g. vertex and/or fragment) to be replaced by custom shader
code.
Thus it can be used as a replacement for the load*(ShaderType) variants
and allows more effects to transition to shaders using traits.
The way nearestPowerOfTwo is currently defined allows for the possibility of
left-shifting an int by 32 bits or more, which is undefined behavior on
platforms where int is 32 bits, and is something that can happen here if `s`
is equal to 31 by the end of the loop. Noted by Coverity as CID 1291191.
This patch takes an algorithm to perform the same operation from Hank Warren
Jr.'s book "Hacker's Delight", which should avoid UB.
REVIEW:126540
Roman Gilg
David Edmundson
Laurent Montel
Yuri Chornoivan
Albert Astals Cid
Vlad Zahorodnii
Alex Nemeth
Alex Nemeth
Kai Uwe Broulik
Kamil Piwowarski
Martin Flöser
Thomas Lübking
Michael Pyne
