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

Add blur API v2 #224

Open
wants to merge 51 commits into
base: master
from

Conversation

Projects
None yet
@donadigo
Copy link
Contributor

commented Apr 13, 2018

This is a follow up to #151 to add a blur behind API for other applications.

This branch is largely based off of the work done by Alex Nemeth for KDE here: https://phabricator.kde.org/D9848

Here's a code that provides a demo for this branch: https://gist.github.com/donadigo/6e93d4730b66f1a6f35100d43317fe18

The code itself is actually much more different than it was for the previous method and should be much more performant. We use gradual downsampling and upsampling of the texture and on every iteration we apply the corresponding shader. The algorithm itself was described by Marius Bjørge here: https://community.arm.com/cfs-file/__key/communityserver-blogs-components-weblogfiles/00-00-00-20-66/siggraph2015_2D00_mmg_2D00_marius_2D00_notes.pdf

Unlike the previous branch, it doesn't support multiple blur amounts for individual windows - this is because of the fact that all texture pipeline is initialized statically, not on every paint (which was really expensive).

This is ready to review.

donadigo added some commits Feb 19, 2018

Fix texture position when scaled; BlurActor class cleanup; add test f…
…or enabling blur for a specific window
Always blur only the relevant rectangle within the FBO; properly hand…
…le dock windows; use framebuffer_* functions instead of old Cogl push and pop

@donadigo donadigo requested a review from cassidyjames Apr 16, 2018

@cassidyjames

This comment has been minimized.

Copy link
Member

commented Apr 16, 2018

Impressive demo! However, my previous issues still stand:

  1. This is not part of the elementary visual language at all. It feels super out of place to me with the rest of our design, and I'd prefer not to have broad design changes driven by tech demos.

  2. This has a very, very limited set of uses; it's difficult to get a window that would be contrast-compliant in the worst scenarios (think: a full white, full black, or a large white/black checkerboard pattern), and turning up the opacity enough to do so makes the blur effect almost completely lost and unnecessary.

  3. I have strong concerns about performance cost to visual benefit. Even if it's a minor cost, it's still increased power consumption for something I don't think we visually want or need in the first place.

  4. It's giving developers a toy to do something we really don't want them to do. If we merge this into Gala and apps can use it, I feel like we'll start to get more and more more macOS UI lookalikes instead of things that look right according to the elementary HIG. Plus means we have built them a feature that is far too easy to abuse for poor contrast.

@donadigo

This comment has been minimized.

Copy link
Contributor Author

commented Apr 24, 2018

@cassidyjames okay so here are my thoughts so other people can follow this PR along:

  1. I understand, it's something that we can pick on whenever we want to e.g next cycle. We will just need to evaluate if this can be useful for other apps / shell, and if yes, how (modal dialogs, panels, terminal etc.)

  2. Yes, I experimented with the effect a little bit here and I came into conclusion that the contents behind should be a little bit more saturated and brightened - which I did locally (didn't push this yet), and I will say it actually works better with more opaque windows and in general.

  3. I'm open for suggestions here. We should probably turn off the effect completely for systems that don't have any GPU at all, while keeping it enabled for all integrated (e.g Intel) and dedicated GPU's. Not sure about if the user should control this setting or not (accesibility settings?).

  4. That's also something that I thought about. Yes, we would need strong do / do not when giving app developers this API so they don't over use it (making mac OS look alikes and what not), however I see this API being useful in apps such as Desktop Folder to give background protection. We should also probably set some contrast guidelines like that if you use this effect, the window's opacity should be around 0.8 or something along those lines.

@danrabbit

This comment has been minimized.

Copy link
Member

commented Apr 25, 2018

Just a technical comment, not a design comment. It looks like the blur isn’t applied in multitasking view

@danrabbit

This comment has been minimized.

Copy link
Member

commented Apr 25, 2018

Design comment: something I remember being mentioned in Slack is we could restrict the API to being accessible to Panel and Dock type windows only. That would cater to apps like the desktop folder and wingpanel without exposing ourselves to people doing macOS style sidebars and such

@alex285

This comment has been minimized.

Copy link

commented Apr 25, 2018

@danrabbit

without exposing ourselves to people doing macOS style sidebars and such

Just an opinion. If elementary can have such functionality, why not let developers to use it if they want to, and see what they can create? They may come with good ideas. And if they dont like it, they simple wont use it

Just some feedback. AWESOME WORK!!!

donadigo added some commits Apr 25, 2018

Make the blur appear also in the workspace and multitasking overview;…
… add little saturation and brightness to final blur texture
@donadigo

This comment has been minimized.

Copy link
Contributor Author

commented on src/BlurActor.vala in 0f83b3a Jun 22, 2018

@jantrushar I don't really have a preference, comments are also good, appreciate your reviews :)
Let me test that here.

This comment has been minimized.

Copy link
Contributor Author

replied Jun 22, 2018

@jantrushar can you describe what exactly is happening? I don't have any problems with either max () call.

@ghost

This comment has been minimized.

Copy link

commented Jun 22, 2018

Updated my system and retestet (I am on arch right now). It is working perfectly now. Sorry for false report.

@donadigo

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2018

@jantrushar thank you for investigating. This made me reconsider the dock calculations in the copy_target_texture method and they're much simpler now.

@MasonLeeBack

This comment has been minimized.

Copy link

commented Jun 23, 2018

Graphics drivers are fine, however, removing those lines seemed to do the trick.

I'll get to integrating this into my fork of wingpanel to see how it looks.

screenshot from 2018-06-22 20-58-07

@guidedlinux

This comment has been minimized.

Copy link

commented Jun 23, 2018

Hi, I really like the idea of having this blur algorithm implemented in Gala, but the demo isn't working for me.
I'm new to vala and elementary but I wanted to try this out, but it seems like the CSS rules are being ignored. I don't get a tinted or transparent window. I am using this example. I get no error messages whatsoever.
I don't know if this issue is even related to this PR, sorry if it isn't. I am using an Intel Core i7-6700 and a GTX 950 with proprietary drivers.

screenshot from 2018-06-23 18 04 53

@donadigo

This comment has been minimized.

Copy link
Contributor Author

commented Jun 23, 2018

@guidedlinux I have updated the gist for the demo, but I'm not sure if that'll work for you. It looks like the effect is applied, it's just not visible because the window is fully opaque. You can try the new demo version at the same link: https://gist.github.com/donadigo/6e93d4730b66f1a6f35100d43317fe18

@ghost

This comment has been minimized.

Copy link

commented Jun 23, 2018

@donadigo it is running very good for me. Great job.

float width = blur_clip_rect.width > 0 ? blur_clip_rect.width : rect.width;
float height = blur_clip_rect.height > 0 ? blur_clip_rect.height : rect.height;

Maybe change this to > -1 so you can distinguish between "0 wanted for clip" and "don't use clip".

One missing piece for the future is the possibility to add blur for non-recangular windows and also to not blur behind shadows (e.g. menu popups with shadows and round corners).

@donadigo

This comment has been minimized.

Copy link
Contributor Author

commented Jun 23, 2018

@jantrushar From the API comment:

If you want to exclusively constrain only the position or size of the blur effect you can pass 0's
for all other values you do not want to constrain, e.g: making the blur effect appear with an always
fixed height can be achieved by passing 0's to x, y and width parameters and the
requested value for the height parameter.

So basically, here 0 is your -1. I chose to do this since if you pass 0 to either width or height, the effect won't be drawn at all anyway.

Regarding setting a non-rectangular blur area, this would require more work (how to encode the area with DBus params?) and also in the effect itself. I've already considered implementing this, but I'm focusing on getting the effect itself work correctly.

@guidedlinux

This comment has been minimized.

Copy link

commented Jun 24, 2018

Thanks for your answer @donadigo :)
Maybe I have some outdated versions of the required dependencies installed (as I said, I'm new to vala) so it wasn't working. I have changed "window" to "GtkWindow" in the CSS and now it's working fine (same for the button). I really love the effect and the performance is also good. Thanks again for your help.

screenshot from 2018-06-24 07 35 30

@cassidyjames

This comment has been minimized.

Copy link
Member

commented Jul 4, 2018

@donadigo hey following up from Slack.

So I finally just was able to vocalize some sort of potential human interface guidelines around blur…

If something is already transparent or translucent (and would generally make sense to be without any blur), then blur can be used to enhance contrast.

If something is explicitly being de-prioritized, i.e. the background in a system-modal context, it could be blurred to further draw attention to the non-blurred foreground (like lowering brightness of the background). But do NOT use it in a foreground context where no transparency/blur would be just as effective.

Like, I think it works fine in this sort of context: https://appcenter.elementary.io/com.github.cassidyjames.principles/ (see the background behind the dialog)

This is effectively a modal, where the background is deprioritized and blurred out. But it would be dumb to reverse it, making the background normal and the dialog a super blurred out "glass" dialog.

(I was just looking at macOS and Windows blurs recently, and they both do the latter, which makes it literally nauseating and completely unnecessary)

donadigo and others added some commits Jul 4, 2018

@gabeps88

This comment has been minimized.

Copy link

commented Oct 4, 2018

Is not working for me, I can compile it but the blur is not working, I got this error in terminal

** (BlurWindow:5575): CRITICAL **: file /home/user/BlurWindow.vala.c: line 457: uncaught error: GDBus.Error:org.freedesktop.DBus.Error.UnknownMethod: No such method 'EnableBlurBehind' (g-dbus-error-quark, 19)

@gabeps88

This comment has been minimized.

Copy link

commented Oct 4, 2018

screenshot from 2018-10-03 20 50 26

@donadigo

This comment has been minimized.

Copy link
Contributor Author

commented Oct 4, 2018

@gabeps88 you need to install and restart gala to be able to actually see the demo.
Before you launch the demo, do sudo ninja install and run (gala -r &) &. Then run the demo.

@gabeps88

This comment has been minimized.

Copy link

commented Oct 6, 2018

@donadigo it didn't work
screenshot from 2018-10-05 21 00 42

@donadigo

This comment has been minimized.

Copy link
Contributor Author

commented Oct 6, 2018

@gabeps88 you have to be in the directory that you cloned this repo to. So cd into it before those commands. Did you even
clone and compile this gala branch? The demo isn't the entire thing, it just uses functions from this branch.

@ciampolo

This comment has been minimized.

Copy link

commented Nov 3, 2018

Any chance you will implement the blur effect for the context menus as well? Would be really awesome setting context menus to like 50% transparency and then the background.

@naaando

This comment has been minimized.

Copy link

commented Feb 14, 2019

Looks good with lyrics 😎
P.S.: I know this is against elementary's HIG, was just a experiment.

blurred lyrics

@alex47

This comment has been minimized.

Copy link

commented Feb 14, 2019

sorry for being offtopic again

@naaando are you sure about that?

untitled

@donadigo

This comment has been minimized.

Copy link
Contributor Author

commented Feb 14, 2019

@alex47 he has probably used a lower opacity for the effect in the DBus call, @naaando for going full opaque you should use 255 for opacity.

@naaando

This comment has been minimized.

Copy link

commented Feb 14, 2019

Yep, I was using lower opacity.
Using full opacity on blur

On content

Lyrics white theme: #fffffff, 20% opacity
lyrics-white-blurred-on-content
Lyrics dark theme: #000000, 20% opacity
lyrics-dark-blurred-on-content

On background

Lyrics dark theme: #000000, 20% opacity
lyrics-dark-blurred-on-bg

@elementary elementary locked as off topic and limited conversation to collaborators Feb 15, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.