Bug 360549 - Visible 1px space between window content and title bar when zooming
Summary: Visible 1px space between window content and title bar when zooming
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: effects-various (show other bugs)
Version: 5.5.5
Platform: openSUSE Linux
: NOR minor
Target Milestone: ---
Assignee: Vlad Zahorodnii
URL:
Keywords:
: 393149 414693 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-03-15 08:58 UTC by Fabian Vogt
Modified: 2020-01-09 13:13 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.18.0


Attachments
Screenshot (475.31 KB, image/png)
2016-03-15 08:58 UTC, Fabian Vogt
Details
Screenshot (840.39 KB, patch)
2016-03-15 13:43 UTC, Fabian Vogt
Details
Screenshot (840.39 KB, image/png)
2016-03-15 13:43 UTC, Fabian Vogt
Details
Texture bleeding in all its glory (108.57 KB, image/png)
2018-06-05 15:12 UTC, Vlad Zahorodnii
Details
Half pixel correction (2.75 KB, patch)
2018-06-06 09:32 UTC, Vlad Zahorodnii
Details
Zoom with half pixel correction (19.93 KB, image/png)
2018-06-06 09:33 UTC, Vlad Zahorodnii
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Fabian Vogt 2016-03-15 08:58:04 UTC
When zooming into the desktop (any of the effects),
there is a tiny space between the window content and the title bar.

(I couldn't set anything lower then "minor" as severity, I'd have chosen "miniscule" otherwise :) )

Reproducible: Always
Comment 1 Fabian Vogt 2016-03-15 08:58:40 UTC
Created attachment 97904 [details]
Screenshot
Comment 2 Martin Flöser 2016-03-15 13:35:37 UTC
Looks correct to me. The borders can be blurry due to the way how zoom works (especially the 1 px outline in the breeze style). But to me it doesn't look on the screenshot as if there were a 1 px gap.
Comment 3 Fabian Vogt 2016-03-15 13:43:00 UTC
Created attachment 97910 [details]
Screenshot

Attached a new screenshot with window not being focused.
It looks as if the decoration and content are drawn separately and do not connect properly (half pixel offset?)
Comment 4 Fabian Vogt 2016-03-15 13:43:46 UTC
Created attachment 97911 [details]
Screenshot
Comment 5 Martin Flöser 2016-03-15 13:47:44 UTC
(In reply to Fabian Vogt from comment #3)
> It looks as if the decoration and content are drawn separately

yes that is the case

> and do not connect properly (half pixel offset?)

this they should and with a disabled look I can reproduce (on Wayland), thus I mark as CONFIRMED.
Comment 6 Vlad Zahorodnii 2018-06-04 21:51:39 UTC
*** Bug 393149 has been marked as a duplicate of this bug. ***
Comment 7 Vlad Zahorodnii 2018-06-04 22:41:00 UTC
Isn't it related to 178508?
Comment 8 Fabian Vogt 2018-06-05 06:44:11 UTC
(In reply to Vlad Zagorodniy from comment #7)
> Isn't it related to 178508?

I don't think so, I'm not using XRender.
Comment 9 Vlad Zahorodnii 2018-06-05 08:44:32 UTC
(In reply to Fabian Vogt from comment #8) 
> ... XRender.

Well, that wasn't my point.

Anyway, I suggest to move this bug to scene-opengl component because
zoom effect(and pretty much all other effects that scale/rotate windows)
does nothing wrong.

I think what we see is the texture bleeding (decorations are stored in
a texture atlas with 1px gap, IIRC).
Comment 10 Fabian Vogt 2018-06-05 09:11:40 UTC
(In reply to Vlad Zagorodniy from comment #9)
> (In reply to Fabian Vogt from comment #8) 
> > ... XRender.
> 
> Well, that wasn't my point.
> 
> Anyway, I suggest to move this bug to scene-opengl component because
> zoom effect(and pretty much all other effects that scale/rotate windows)
> does nothing wrong.

> I think what we see is the texture bleeding (decorations are stored in
> a texture atlas with 1px gap, IIRC).

Maybe - it's not clear from the screenshots whether the edges are sharp or blurry. If they are sharp, it's that the vertices don't have the same coordinates, if not, it's bleeding.

My initial suspicion was the usual OpenGL half-pixel issue: The center of a pixel is at (x+0.5|y+0.5).
Comment 11 Vlad Zahorodnii 2018-06-05 10:46:54 UTC
(In reply to Fabian Vogt from comment #10)
> Maybe - it's not clear from the screenshots whether the edges are sharp or
> blurry. If they are sharp, it's that the vertices don't have the same
> coordinates, if not, it's bleeding.

Here's example window quads:

contents: left = 4 top = 29 right = 885 bottom = 632
decoration: left = 0 top = 29 right = 4 bottom = 632
decoration: left = 0 top = 0 right = 889 bottom = 29
decoration: left = 885 top = 29 right = 889 bottom = 632
decoration: left = 0 top = 632 right = 889 bottom = 636
shadow: left = -93 top = -59 right = 99 bottom = 133
shadow: left = 99 top = -59 right = 790 bottom = 133
shadow: left = 790 top = -59 right = 982 bottom = 133
shadow: left = 790 top = 133 right = 982 bottom = 571
shadow: left = 790 top = 571 right = 982 bottom = 763
shadow: left = 99 top = 571 right = 790 bottom = 763
shadow: left = -93 top = 571 right = 99 bottom = 763
shadow: left = -93 top = 133 right = 99 bottom = 571

as you see vertices are totally fine (if we scale a window
the quads will stay the same).

Anyway, I still suggest to move this bug to scene-opengl component.

> My initial suspicion was the usual OpenGL half-pixel issue: The center of a
> pixel is at (x+0.5|y+0.5).
Comment 12 Vlad Zahorodnii 2018-06-05 15:12:55 UTC
Created attachment 113096 [details]
Texture bleeding in all its glory

Yep, that's texture bleeding (I've slightly changed
decoration renderer so it initially fills the texture
atlas with red color).
Comment 13 Martin Flöser 2018-06-05 17:20:50 UTC
I would keep the bug in this component. I doubt the scene can do anything about it. But the effect could by rendering the unzoomed window into an fbo and then zoom the fbo texture. That should prevent the bleading issue I think.
Comment 14 Vlad Zahorodnii 2018-06-05 17:35:00 UTC
(In reply to Martin Flöser from comment #13)
> I would keep the bug in this component. I doubt the scene can do anything
> about it. But the effect could by rendering the unzoomed window into an fbo
> and then zoom the fbo texture. That should prevent the bleading issue I
> think.

So, it would work like the Magnifier effect, right?
Comment 15 Martin Flöser 2018-06-05 20:06:08 UTC
(In reply to Vlad Zagorodniy from comment #14)
> (In reply to Martin Flöser from comment #13)
> > I would keep the bug in this component. I doubt the scene can do anything
> > about it. But the effect could by rendering the unzoomed window into an fbo
> > and then zoom the fbo texture. That should prevent the bleading issue I
> > think.
> 
> So, it would work like the Magnifier effect, right?

Does magnifier use fbo? I know looking glass does.
Comment 16 Vlad Zahorodnii 2018-06-05 20:26:26 UTC
(In reply to Martin Flöser from comment #15)
> Does magnifier use fbo? I know looking glass does.

Well, yeah, as far as I understand it uses blitFromFramebuffer
to magnify a portion of the screen (size of the source rect is
smaller than size of the destination rect + linear filter).

So, maybe, we could do something similar in zoom effect?
Comment 17 Martin Flöser 2018-06-06 04:16:20 UTC
Yeah
Comment 18 Fabian Vogt 2018-06-06 07:21:30 UTC
What about fixing the texture bleeding instead?

AFAICT it is the half-pixel issue. If you can give me a pointer where to look I can give it a try.
Comment 19 Vlad Zahorodnii 2018-06-06 09:32:12 UTC
Created attachment 113108 [details]
Half pixel correction

(In reply to Fabian Vogt from comment #18)
> What about fixing the texture bleeding instead?
> 
> AFAICT it is the half-pixel issue. If you can give me a pointer where to
> look I can give it a try.

See the attached patch. I hope it will help you.

What I don't like about that approach is that if there is mipmapping,
it won't help pretty much.
Comment 20 Vlad Zahorodnii 2018-06-06 09:33:14 UTC
Created attachment 113109 [details]
Zoom with half pixel correction
Comment 21 Fabian Vogt 2018-06-06 10:57:47 UTC
(In reply to Vlad Zagorodniy from comment #19)
> Created attachment 113108 [details]
> Half pixel correction
> 
> (In reply to Fabian Vogt from comment #18)
> > What about fixing the texture bleeding instead?
> > 
> > AFAICT it is the half-pixel issue. If you can give me a pointer where to
> > look I can give it a try.
> 
> See the attached patch. I hope it will help you.
> 
> What I don't like about that approach is that if there is mipmapping,
> it won't help pretty much.

Indeed. Mipmapping with texture atlases can't ever work correctly, the only workaround is to increase padding there.
Comment 22 Martin Flöser 2018-06-06 16:46:15 UTC
on the other hand mipmapping hardly makes sense in KWin as we cannot provide mipmaps. We can use the generate call, but that's not better than doing smart things in the shader...
Comment 23 Vlad Zahorodnii 2019-12-02 13:11:16 UTC
Git commit ac4dce1c20a1cbfd3974bf8da6dd7557d35a4ca1 by Vlad Zahorodnii.
Committed on 02/12/2019 at 13:08.
Pushed by vladz into branch 'master'.

[scene] Fix decoration texture bleeding

Summary:
Quite long time ago, window decorations were painted on real X11 windows.
The nicest thing about that approach is that we get both contents of the
client and the frame window at the same time. However, somewhere around
KDE 4.2 - 4.3 times, decoration rendering architecture had been changed
to what we have now.

I've mentioned the previous decoration rendering design because it didn't
have a problem that the new design has, namely the texture bleeding issue.

In the name of better performance, opengl scene puts all decoration parts
to an atlas. This is totally reasonable, however we must be super cautious
about things such as the GL_LINEAR filter.

The GL_LINEAR filter may need to sample a couple of neighboring texels
in order to produce the final texel value. However, since all decoration
parts now live in a single texture, we have to make sure that we don't
sample texels that belong to another decoration part.

This patch fixes the texture bleeding problem by padding each individual
decoration part in the atlas. There is another solution for this problem
though. We could render a window into an offscreen texture and then map
that texture on the transformed window geometry. This would work well and
we definitely need an offscreen rendering path in the opengl scene,
however it's not feasible at the moment since we need to break the window
quads API. Also, it would be great to have as less as possible stuff going
on between invocation of Scene::Window::performPaint() and getting the
corresponding pixel data on the screen.

There is a good chance that the new padding stuff may make you vomit. If
it does so, I'm all ears for the suggestions how to make the code more
nicer.
Related: bug 257566, bug 412573
FIXED-IN: 5.18.0

Reviewers: #kwin

Subscribers: fredrik, kwin, fvogt

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D25611

M  +6    -1    decorations/decorationrenderer.cpp
M  +1    -0    decorations/decorationrenderer.h
M  +108  -9    plugins/scenes/opengl/scene_opengl.cpp
M  +11   -4    scene.cpp

https://commits.kde.org/kwin/ac4dce1c20a1cbfd3974bf8da6dd7557d35a4ca1
Comment 24 Vlad Zahorodnii 2019-12-02 18:00:01 UTC
Git commit 6e000314b34820760cd08593d18708e1f8905cc9 by Vlad Zahorodnii.
Committed on 02/12/2019 at 17:45.
Pushed by vladz into branch 'master'.

Revert the fix for the texture bleeding issue

This reverts commit 9151bb7b9e71fdfeb3d3682ee8a42d796fa315e9.
This reverts commit ac4dce1c20a1cbfd3974bf8da6dd7557d35a4ca1.
This reverts commit 754b72d155820a6c8a9ba94b2c0960da1b2f86ce.

In order to make the fix work, we need to redirect the client window
instead of the frame window. However, we cannot to do that because
Xwayland expects the toplevel window(in our case, the frame window)
to be redirected.

Another solution to the texture bleeding issue must be found.
Related: bug 257566

M  +6    -0    composite.cpp
M  +1    -6    decorations/decorationrenderer.cpp
M  +0    -1    decorations/decorationrenderer.h
M  +6    -1    deleted.cpp
M  +2    -1    deleted.h
M  +2    -3    effects.cpp
M  +1    -1    events.cpp
M  +54   -50   geometry.cpp
M  +16   -109  plugins/scenes/opengl/scene_opengl.cpp
M  +18   -17   scene.cpp
M  +13   -1    scene.h
M  +8    -9    toplevel.cpp
M  +10   -1    toplevel.h
M  +56   -8    x11client.cpp
M  +6    -22   x11client.h

https://commits.kde.org/kwin/6e000314b34820760cd08593d18708e1f8905cc9
Comment 25 Nate Graham 2019-12-02 23:05:52 UTC
*** Bug 414693 has been marked as a duplicate of this bug. ***
Comment 26 David Edmundson 2020-01-09 13:04:05 UTC
Git commit d1cfcf4c975e1dfe6f54c470f9c2ead2548afacf by David Edmundson.
Committed on 09/01/2020 at 13:03.
Pushed by davidedmundson into branch 'master'.

Avoid texture bleed rendering X11 window

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.
Related: 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

M  +14   -3    libkwineffects/kwinglutils.cpp
M  +2    -0    libkwineffects/kwinglutils.h
M  +40   -13   plugins/scenes/opengl/scene_opengl.cpp

https://commits.kde.org/kwin/d1cfcf4c975e1dfe6f54c470f9c2ead2548afacf
Comment 27 Vlad Zahorodnii 2020-01-09 13:13:40 UTC
Git commit af71763be53054925f27b7614fc992e6380a1d02 by Vlad Zahorodnii.
Committed on 09/01/2020 at 13:13.
Pushed by vladz into branch 'master'.

[scene] Fix decoration texture bleeding

Summary:
Quite long time ago, window decorations were painted on real X11 windows.
The nicest thing about that approach is that we get both contents of the
client and the frame window at the same time. However, somewhere around
KDE 4.2 - 4.3 times, decoration rendering architecture had been changed
to what we have now.

I've mentioned the previous decoration rendering design because it didn't
have a problem that the new design has, namely the texture bleeding issue.

In the name of better performance, opengl scene puts all decoration parts
to an atlas. This is totally reasonable, however we must be super cautious
about things such as the GL_LINEAR filter.

The GL_LINEAR filter may need to sample a couple of neighboring texels
in order to produce the final texel value. However, since all decoration
parts now live in a single texture, we have to make sure that we don't
sample texels that belong to another decoration part.

This patch fixes the texture bleeding problem by padding each individual
decoration part in the atlas. There is another solution for this problem
though. We could render a window into an offscreen texture and then map
that texture on the transformed window geometry. This would work well and
we definitely need an offscreen rendering path in the opengl scene,
however it's not feasible at the moment since we need to break the window
quads API. Also, it would be great to have as less as possible stuff going
on between invocation of Scene::Window::performPaint() and getting the
corresponding pixel data on the screen.

There is a good chance that the new padding stuff may make you vomit. If
it does so, I'm all ears for the suggestions how to make the code more
nicer.
Related: bug 257566, bug 412573
FIXED-IN: 5.18.0

Reviewers: #kwin

Subscribers: fredrik, kwin, fvogt

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D25611

M  +6    -1    decorations/decorationrenderer.cpp
M  +1    -0    decorations/decorationrenderer.h
M  +108  -9    plugins/scenes/opengl/scene_opengl.cpp
M  +11   -4    scene.cpp

https://commits.kde.org/kwin/af71763be53054925f27b7614fc992e6380a1d02