Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dual-kawase blur algorithm for new OpenGL backend #382

Merged
merged 5 commits into from
Aug 31, 2020

Conversation

tryone144
Copy link
Collaborator

@tryone144 tryone144 commented Apr 18, 2020

This PR implements the dual-filter kawase blur algorithm as found in KWin.

demo

Usable with --blur-method dual_kawase --blur-strength 7.

This is currently work-in-progress and open for comments. DO NOT MERGE.
Should be ready to merge now.

Previous discussion: #32
See also: #361

EDIT: Sorry, clicked submit too fast...

@tryone144 tryone144 changed the title dual-kawase blur algorithm for new OpenGL backend WIP: dual-kawase blur algorithm for new OpenGL backend Apr 18, 2020
@codecov
Copy link

codecov bot commented Apr 18, 2020

Codecov Report

Merging #382 into next will decrease coverage by 0.92%.
The diff coverage is 0.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next     #382      +/-   ##
==========================================
- Coverage   37.46%   36.54%   -0.93%     
==========================================
  Files          45       45              
  Lines        8671     8896     +225     
==========================================
+ Hits         3249     3251       +2     
- Misses       5422     5645     +223     
Impacted Files Coverage Δ
src/backend/backend_common.c 43.12% <0.00%> (-3.78%) ⬇️
src/backend/gl/gl_common.c 0.00% <0.00%> (ø)
src/backend/gl/gl_common.h 0.00% <ø> (ø)
src/backend/xrender/xrender.c 0.00% <0.00%> (ø)
src/config.c 41.32% <0.00%> (-0.88%) ⬇️
src/config.h 23.52% <ø> (ø)
src/picom.c 67.76% <0.00%> (-0.61%) ⬇️
src/options.c 21.63% <9.09%> (-0.38%) ⬇️
src/config_libconfig.c 58.00% <50.00%> (-0.05%) ⬇️
... and 1 more

src/config.c Show resolved Hide resolved
src/config_libconfig.c Outdated Show resolved Hide resolved
src/config_libconfig.c Outdated Show resolved Hide resolved
@tryone144
Copy link
Collaborator Author

Apart from the comments above:

  • Commit 46b48eb has to be removed prior to merging
  • Commit 209d9b6 might be better off in a separate PR. Merging this beforehand reduced the complexity of the blur-code somewhat.

@Shadorain
Copy link

Shadorain commented Apr 20, 2020

Hey guys, little bit off topic, but idea! I just got into tiling window managers and wouldnt survive without your fork. I was wondering if it is possible (maybe I missed something even after hours of searching) to have blur set up to be across all windows, but then increase the blur strength on the focused window, just an idea that I feel would cap my theme! Thanks!

@tryone144
Copy link
Collaborator Author

@Shadorain having different strength-levels similar to active/inactive opacity should be possible with a second set of parameters. But then you could also request per-window rules of blur-strength... I think it should be priority to get blur-algo in general merged first. 😉

Keep in mind, that strength-fading with this algorithm requires some hackery and will never be smooth due to the discrete nature of the internal sample-iterations. See #32 (comment) for an explanation and tryone144/compton#2 for some hacked samples.

@Shadorain
Copy link

@Shadorain having different strength-levels similar to active/inactive opacity should be possible with a second set of parameters. But then you could also request per-window rules of blur-strength... I think it should be priority to get blur-algo in general merged first. wink

Keep in mind, that strength-fading with this algorithm requires some hackery and will never be smooth due to the discrete nature of the internal sample-iterations. See #32 (comment) for an explanation and tryone144/compton#2 for some hacked samples.

to be honest, I'm not sure what I did to make it this way, but when i remade my entire configuration file by hand just now, somehow (not even trying) I was able to get a very strange effect that I'm not too sure if its a bug or not
But it kind of does what I wanted in my previous post (which doesn't really make the most sense lol) I'll upload my conf file, it's very clean with alot less lines which makes it easier to read and edit

Thank you for the quick reply! I will see what i can do about your comment too, how i can implement it

src/backend/gl/gl_common.c Outdated Show resolved Hide resolved
@absolutelynothelix
Copy link
Collaborator

absolutelynothelix commented Apr 21, 2020

hey, @yshui, since the pull request is here (though it still requires some work, but i don't think there is much to do), may i ask you: are you planning to merge it before releasing v8? would be amazing to have dual kawase blur in v8 (and, hopefully, rounded corners in v9) :)

@tryone144
Copy link
Collaborator Author

tryone144 commented Apr 22, 2020

I wouldn't want to rush things, especially in light of #385. Should be fine with releasing v8 first and then add these visual features in v9.

Except for the small changes outlined above, this code should be feature complete and seems to run without issues. The thing I am currently kinda stuck on is how to handle #349 as these changes have some implications on the rendering logic.

Edit: Well, I'm a bit slow. v8 is already out... 😆

@Philipp-M
Copy link

I've noted a small visual issue, if windows are next to each other.
Windows are borderless in the following example and have the same color and transparency.
I've used the default blur strength 5, no shadows, with only a wallpaper behind the windows.
screenshot-2020-04-22_17:41:21

@tryone144
Copy link
Collaborator Author

I've noted a small visual issue, if windows are next to each other.

This is to be expected and due to the way windows are rendered. Each window is rendered one by one and the current result is used for blur calculation (i.e. the top-most window includes the already rendered windows below into its calculations as one would expect).
The two windows in your example aren't on-top of each other strictly speaking, but are still rendered in order (most likely the active window as last).

It would be possible to only take the pixel directly below the window into account for bluring, but this will introduce artifacts when pixels directly at the window border change — mostly noticable when moving a floating window. See this comment for a more in-depth explanation.

This should however be fixable by something like #265, although the implementation in 6a3d135 seems to clip to little leading to artifacts on the edge of the windows as there are still parts of the underlying window sampled. This is a different issue though and has to be fixed separately.

@yshui
Copy link
Owner

yshui commented Apr 22, 2020

@tryone144 The clipping added in #265 clips the window directly under the transparent window's body, so it's expected to have windows around the transparent window included in the blur. I agree it's not pretty. We could probably use root_pixmap instead of the target buffer for blur when transparent-clipping is enabled, should be pretty easy to do.

Edit: This would require a change of the backend interface. So let's merge dual-kawase first.

@tryone144
Copy link
Collaborator Author

We could probably use root_pixmap instead of the target buffer for blur when transparent-clipping is enabled, should be pretty easy to do.

This sounds like a good solution. Actually, that's exactly what I thought you would implement when I read the posts. 😆
But I agree, fix the problems step-by-step.

@Philipp-M
Copy link

Another idea I just had regarding this issue should still allow visual overlapping of windows.

I guess currently the sorting of the windows is done by the window manager, and in this order the windows are drawn by the compositor.
I'm not too much in the internals of window managing/compositing with X, so I'm not sure if it's working:

Resort the window draw order, allowing multiple windows drawn at once / with the same background framebuffer based on how they're overlapping (maintaining the original order).
This should be possible with a simple rectangle intersection (or maybe even a fancier but more costly pixel-based intersection for non-rectangular windows probably using the gpu if possible)
If windows are not overlapping (like in the example above) or have the same windows beneath them, they are drawn in the same pass with the same background framebuffer.

This could possibly also solve problems with shadows overlapping neighboring windows (I'm pretty sure it's the same issue)

@absolutelynothelix
Copy link
Collaborator

absolutelynothelix commented Apr 24, 2020

image
looks like we need an ability to turn off background blurring for a wintype?

@tryone144
Copy link
Collaborator Author

<rant>
I really hate all those tooltips menus and splashscreens which think rendering their own shadow is an incredibly great idea... And typically these are the ones that don't set appropriate wintypes. 🤦
</rant>

Extending the blur-exclude to wintype options seems like a good idea, though as a separate change.

@absolutelynothelix
Copy link
Collaborator

absolutelynothelix commented Apr 24, 2020

yeah, i had this problem with all tooltip menus initially, but i was too lazy to dig in, so i just set blur-background-exclude to match all windows except the terminal windows, but terminal also has tooltip menus...

not sure what you meant with "extending the blur-exclude to wintype options", but we already have wintypes thing that allows to configure different things like shadows, focus for different wintypes and i want to have blur or blur-background option there, like:

wintypes: {
  tooltip = {
    blur-background = false;
  };
};

if we are going for this, would be good to implement this in a separate pull request as well and merge before this pull request.

@tryone144
Copy link
Collaborator Author

not sure what you meant with "extending the blur-exclude to wintype options", but we already have wintypes thing that allows to configure different things like shadows, focus for different wintypes and i want to have blur or blur-background option there
[snip]

Exactly this. 😆

@tryone144 tryone144 force-pushed the feature/dual_kawase branch from 209d9b6 to 9b4a6f0 Compare April 25, 2020 12:50
@tryone144
Copy link
Collaborator Author

Applied the changes discussed above and squashed some unnecessary commits.

  • Commit 46b48eb has to be removed prior to merging

Done.

  • Commit 209d9b6 might be better off in a separate PR. Merging this beforehand reduced the complexity of the blur-code somewhat.

Merged with #388 and #389.

Currently this leads to darkened blur around screen borders in the kernel blur implementation.
I'd suggest adopting the kernel blur shader to use texture2D() as well with correctly setup textures and provide normalized coordinates in x_rect_to_coords() when requested.

Will take a look into this next and then rebase on top of that PR.

@vn-ki
Copy link

vn-ki commented May 1, 2020

@tryone144

[ 05/01/2020 14:52:44.508 win_clear_flags WARN ] Flags cleared on a destroyed window 0x0040032e (vn-ki - File Manager)

Also segfaults when closing a window sometimes.

My config
# ROunded corners
corner-radius = 10;
# Shadow
shadow = true;
#shadow-radius = 30;
#shadow-offset-x = -30;
#shadow-offset-y = -30;
shadow-radius = 20;
shadow-offset-x = -20;
shadow-offset-y = -20;
log-level = "warn";
# log-file = "/path/to/your/log/file";
shadow-opacity = 0.8;
# shadow-red = 0.0;
# shadow-green = 0.0;
# shadow-blue = 0.0;
shadow-exclude = [
	"_COMPTON_NO_SHADOW@:c = 1",
    # for zoom screen share
    "name = 'cpt_frame_window'",

    #"class_g = 'Google-chrome-beta'",
#	"name = 'Notification'",
#	"class_g = 'Conky'",
#	"class_g ?= 'Notify-osd'",
#	"class_g = 'Cairo-clock'",
#	"_GTK_FRAME_EXTENTS@:c"
];
# shadow-exclude = "n:e:Notification";
# shadow-exclude-reg = "x10+0+0";
# xinerama-shadow-crop = true;

# Opacity
#inactive-opacity = 0.9;
# active-opacity = 0.8;
frame-opacity = 0.7;
inactive-opacity-override = false;
 inactive-dim = 0.2;
# inactive-dim-fixed = true;
#blur-background = true;
#blur-background-frame = true;
blur-method = "dual_kawase";
blur-strength=10;
# blur-kern = "3x3box";
# blur-kern = "5,5,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1";
blur-background-fixed = true;
blur-background-exclude = [
    # "window_type = 'dock'",
    "window_type = 'desktop'",
    "_GTK_FRAME_EXTENTS@:c"
];
opacity-rule = [ 
    "80:class_g = 'URxvt'",
    # "70:class_i = 'music.youtube.com'",
    # "90:class_i = 'discordapp.com__activity'",
];

# Fading
fading = true;
fade-delta = 5;
fade-in-step = 0.03;
fade-out-step = 0.03;
# no-fading-openclose = true;
# no-fading-destroyed-argb = true;
fade-exclude = [
#"class_g = 'Google-chrome-beta'",
];

# Other
#backend = "xr_glx_hybrid";
backend = "glx";

#backend = "xr_glx_hybrid";
#vsync-use-glfinish=true;

mark-wmwin-focused = true;
mark-ovredir-focused = true;
# use-ewmh-active-win = true;
detect-rounded-corners = true;
detect-client-opacity = true;
#refresh-rate = 60;
vsync = true;
#vsync = "opengl-swc";
#sw-opti = true;
#unredir-if-possible = true;
#unredir-if-possible-delay = 5000;
# unredir-if-possible-exclude = [ ];
focus-exclude = [ 
"class_g = 'Cairo-clock'", 
"class_g = 'Google-chrome-beta'" 
];
detect-transient = true;
detect-client-leader = true;
invert-color-include = [ ];
# resize-damage = 1;

# GLX backend
glx-no-stencil = true;
#glx-no-rebind-pixmap = true;
#xrender-sync-fence = true;
#use-damage = true;

# Window type settings
wintypes:
{
  tooltip = { fade = true; shadow = true; opacity = 0.75; focus = true; full-shadow = false; };
  dock = { shadow = false; }
  dnd = { shadow = false; }
  menu = { shadow = false; }
  #popup_menu = { opacity = 0.8; }
  popup_menu = {shadow = false; }
  #notify = {shadow = false; }
  dropdown_menu = { shadow = false; }
  # toolbar = { shadow = false; }
  # utility = { shadow = false; }
  # toolbar = { shadow = false; }
  # splash = { shadow = false; }
  dialog = { shadow = false; }
};

@tryone144
Copy link
Collaborator Author

[ 05/01/2020 14:52:44.508 win_clear_flags WARN ] Flags cleared on a destroyed window 0x0040032e (vn-ki - File Manager)

Also segfaults when closing a window sometimes.

This issue is not directly related to this PR and happens on next as well. @vn-ki please open a separate issue for this. At least a stack trace would be pretty useful to further investigate this.
I have run into this several times now while testing (both glx and xrender backend) and it seems to be related to some state housekeeping when windows rapidly map/unmap (really bad case: Memory pressure and "fading" vlc controls in fullscreen).

@yshui
Copy link
Owner

yshui commented May 1, 2020

Perhaps the same crash as #390?

@tryone144 tryone144 force-pushed the feature/dual_kawase branch from 9b4a6f0 to 72990cb Compare August 7, 2020 22:35
@tryone144 tryone144 force-pushed the feature/dual_kawase branch from 72990cb to 21ab70d Compare August 19, 2020 11:22
@tryone144 tryone144 changed the title WIP: dual-kawase blur algorithm for new OpenGL backend dual-kawase blur algorithm for new OpenGL backend Aug 19, 2020
@tryone144
Copy link
Collaborator Author

Running flawlessly on my setup since the last rebase (apart from the unrelated assertion failures in the experimental backends) and I haven't gotten any complaints, so it seems to be good for merging.

If there is enough demand, I can try to cleanly backport the dual_kawase code to the legacy backend as well.

@yshui
Copy link
Owner

yshui commented Aug 20, 2020

I will try to review this ASAP.

P.S. @tryone144, may I interest you in becoming a collaborator of the picom project? (In other words, can you help me review PRs, handle issue and maybe fix bugs for picom, please?)

@tryone144
Copy link
Collaborator Author

P.S. @tryone144, may I interest you in becoming a collaborator of the picom project? (In other words, can you help me review PRs, handle issue and maybe fix bugs for picom, please?)

I'll gladly help out with handling issues and PRs. 😄
I only have limited time to spend though, so I can't promise too much from my side. If this is okay with you, feel free to hit me up with more details.

@yshui
Copy link
Owner

yshui commented Aug 27, 2020

@tryone144 It's still better than just having me.

Basically I am going to add you as a collaborator on this project, so you will be able to push to this repository, create new branches here, and approve pull requests.

Although you will be able to push directly to the next branch in theory. For more complex changes, I still want you to open PRs so more people can look at it (I will do the same). For trivial changes, you can just push to next. I'll leave it to you (and to me) to decide what is "trivial", and hopefully we will hash out the details as time progresses.

@tryone144
Copy link
Collaborator Author

Sounds good to me. :)

@tryone144 tryone144 force-pushed the feature/dual_kawase branch 2 times, most recently from acf6935 to b698735 Compare August 30, 2020 20:58
Copy link
Owner

@yshui yshui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still going through the last commit.

A small nit.

src/backend/gl/gl_common.c Show resolved Hide resolved
src/backend/gl/gl_common.c Outdated Show resolved Hide resolved
src/backend/gl/gl_common.c Outdated Show resolved Hide resolved
src/backend/gl/gl_common.c Outdated Show resolved Hide resolved
@yshui
Copy link
Owner

yshui commented Aug 30, 2020

LGTM, only found some nits.

Support an arbitrary number of auxiliary textures and framebuffers for
blur. Preparation for the dual-filter kawase algorithm.
**Work-in-Progress**

Add `dual_kawase` to configuration and argument parsing. Allow `kawase`
for backward compatibility. Add `--blur-strength` parameter for
blur-method `dual_kawase`.

Update documentation to reflect the new blur-method and parameters.
**Work-in-Progress**

Generate suitable parameters for dual-filter kawase blur based on the
selected `blur-strength` or approximate a gauss blur with the selected
`blur-size` and std-deviation.
…e [WIP]

**Work-in-Progress**

Split-off kernel-blur specific initialization and rendering from common
OpenGL setup. Add stub functions for dual_kawase-blur initialization and
rendering.
Implement the dual-filter kawase blur algorithm for the new OpenGL backend
as seen in kwin [1]. Use with `--blur-method dual_kawase` and set the
desired strength with `--blur-strength level` (1-20).

The dual-filter kawase algorithm produces results close to a traditional
gaussian blur with higher performace, especially at high blur radii. The
supported strength levels provide an effect similar to gauss-radii between
4 and 500 pixels.

As this algorithm relies heavily on the texture-filtering units of a
GPU, there is no support for the xrender backend — at least for now.

[1](https://kwin.kde.narkive.com/aSqRYYw7/d9848-updated-the-blur-method-to-use-the-more-efficient-dual-kawase-blur-algorithm)
@tryone144 tryone144 force-pushed the feature/dual_kawase branch from b698735 to 5e0215a Compare August 31, 2020 12:16
@yshui
Copy link
Owner

yshui commented Aug 31, 2020

We can finally merge this after so long. Feels like there needs to be some celebration of some sort.

@yshui yshui merged commit 731bd61 into yshui:next Aug 31, 2020
@yshui
Copy link
Owner

yshui commented Aug 31, 2020

Also, huge thanks to @tryone144 for their amazing work, patience and perseverance.

nerbug added a commit to nerbug/arch-dotfiles-laptop that referenced this pull request Nov 30, 2020
Replaced picom with its experimental version from the AUR, which adds
the dual-filter kawase blur algorithm for background blurring.

PR: yshui/picom#382

When this gets merged into the stable version, I might reinstall the
stable version again, though the experimental version was just as stable
for me.
aw1cks added a commit to archlinux-ansible/ansible-arch_packages that referenced this pull request Sep 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants