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

Merge functionality of blur-providers extension in to this one #39

Merged
merged 20 commits into from
Jan 25, 2022

Conversation

CorvetteCole
Copy link
Collaborator

@CorvetteCole CorvetteCole commented Apr 21, 2021

This draft PR will track the progress of work to port the existing application blurring functionality of the blur-provider extension to blur-my-shell.

Beyond simply porting existing functionality, several other changes need to be done. These have been summarized below:

  • Port existing application blurring on X flag functionality
  • Improve Wayland support (Wayland support CorvetteCole/blur-provider#2) (needs testing!)
    • Figure out a way for applications to request blur beyond X flags. (e.g. an application not running within XWayland or X
  • Add manual user application selection for blurring (Making blur persistent CorvetteCole/blur-provider#3)
    • Add application selection to the extension settings
  • Keep blur behind target actors in overview (right now it just kind of disappears)
  • After switching to a workspace and back, blur appears on top of the target actor until the screen is interacted with Fixed In Gnome 40
  • When switching focus rapidly between windows, blur can blink in front of all windows briefly Fixed In Gnome 40

@CorvetteCole
Copy link
Collaborator Author

This PR replaces #35 so I can use a better source branch name

@Dziban-dev
Copy link

this would be amazing!

@ckissane
Copy link

@CorvetteCole may I help

@CorvetteCole
Copy link
Collaborator Author

@ckissane you are welcome to take a look at my fork and at my blur-provider program. I'm not far from being done with the initial implementation of this so you may want to wait a few for my to finish that up. Further improvements definitely could use some help!!

@ckissane
Copy link

@CorvetteCole took me 1 hour but I have an initial merge as well, with application blurring. I can wait for yours though :)

@ckissane
Copy link

@CorvetteCole
Screenshot from 2021-04-29 08-48-54

@CorvetteCole
Copy link
Collaborator Author

@ckissane it would be super simple to just add my existing blur-provider code, but I'm actually going through it and thoroughly rewriting it to take advantage of new knowledge and some ideas I had. I want to be able to support it much easier in the future and wanted to use the existing infrastructure from this extension for managing connections

@CorvetteCole
Copy link
Collaborator Author

rewrite will allow me to fix bugs easier and implement hacky workarounds like vfunc paint overriding

@CorvetteCole
Copy link
Collaborator Author

I actually have quite a lot of work put in to this so far... just haven't pushed the commits. I'll do that when I get to my computer in a minute if you wanna see what in working on right now

@ckissane
Copy link

@CorvetteCole Ok I made a PR with cleaned up code that uses the connections

@ashy1227
Copy link

doin' the lord's work! thank you!

@SudoVanilla
Copy link
Contributor

Haven't seen much happening here in a while, any progress?

@CorvetteCole
Copy link
Collaborator Author

hello @KorbsStudio , to be honest I've had almost no time to work on personal projects this summer. Luckily, I'll be moving back in to my apartment for spring, winter, and fall and will finally have a consistent work schedule with plenty of personal time to work on this project (and all my others!!). I apologize, but it will get done, sooner rather than later. Check back in two months? Sorry!

@SudoVanilla
Copy link
Contributor

yay, something is happening! 😄

@CorvetteCole
Copy link
Collaborator Author

I'm happy to report that I've pushed a fully working preliminary version of this integration to my branch. It needs some testing and a little more code cleanup before I feel like it is fully ready, but give it a try! I will implement manual application blurring with a selection menu later on @KorbsStudio

@CorvetteCole
Copy link
Collaborator Author

(if you don't have any applications that natively set the mutter hint you can use xprop -f _MUTTER_HINTS 8s -set _MUTTER_HINTS blur-provider=${sigma_value}. The brightness value is set from extension settings. If your sigma value is out of range or null, it will use the sigma value in the blur my shell extension settings

@CorvetteCole
Copy link
Collaborator Author

CorvetteCole commented Jan 24, 2022

I think that wayland doesn't have these mutter hints, so implementing manual application blurring selection is a priority. I'm also a little fearful of performance with this and lots of applications requesting blur so I'd like to optimize it a bit more. This particular implementation of blur provider has some notable improvements over the original as well. It will apply blur to windows even if it is activated after the window exists for one

@CorvetteCole CorvetteCole marked this pull request as ready for review January 24, 2022 02:54
@SudoVanilla
Copy link
Contributor

Blur Provider is the only extension with good performance, since it only provides the blur if the app request it.
I only use Blur Provider when building my Electron application, which just so happens to use "Glasstron".

I've dealt with Blur Provider before on Wayland, "It Just Works" for me. Well, last time I checked on Intel graphics.

@CorvetteCole
Copy link
Collaborator Author

that is good to hear. it is just xprop then which is easily avoided. Manual application blurring will be a nice feature to have for applications that have transparency but don't support this.

@aunetx
Copy link
Owner

aunetx commented Jan 24, 2022

I'm reviewing this, it looks cool! I will change some things (mainly to better understand the code myself), but should soon merge this!

By the way, I don't think it would be a good idea to push this to extensions.gnome.org now, it still lacks some polish:

  • a very easy way to select which windows to blur, e.g. with a window selector in the preferences
  • a way to provide transparency automatically if the user wants it (should probably be a choice for each app, rather than global)
  • blur in the overview and overview gesture animation

I would love blur my shell to stay a very easy way to enhance the shell, so I don't think that pushing to most users too soon would be a good idea!
But once these problems are fixed, this will obviously make it to the end users :p

About it, do you think that merging this directly in master would be a good idea for the moment? If these three problems can be fixed soon (I will try to help too, but have some work beside sadly), then master would be good to avoid having to rebase again; but if it risks to stall more than some weeks more, then it would prevent me from updating BmS in a normal way (I will not be able to push to eog until this is fixed). So I guess you know the best way to do it better than me :)

@aunetx
Copy link
Owner

aunetx commented Jan 24, 2022

Multiple windows can't share the same blur effect, which prevents from blurring multiple windows by using xprop, fixing this rn!

@CorvetteCole
Copy link
Collaborator Author

I think most users will not notice or use this until we implement the listed features you mentioned, it simply is meant to provide a way for 3rd party applications to blur themselves. The whole xprop thing was always meant as a way for developers to test this functionality out for their own purposes.

I'm definitely gonna need help blurring behind windows in overview haha, I'm not familiar with that part of Gnome. I can work on a way to choose windows to blur. I do wonder if perhaps that is somewhat dependent on a settings rewrite though, the current one is quite clunky. Perhaps we can take inspiration from the preferences here: https://github.com/Schneegans/Burn-My-Windows?

Just some thoughts, I'm feeling motivated lately so will def work on this

@aunetx
Copy link
Owner

aunetx commented Jan 24, 2022

Yeah I know about xprop, but it's exactly that I don't think pushing the feature to all users if they can't use it effectively is a good idea, as it would probably make some of them try to use the app blur feature anyway -- even if it is not ready for the end-user managing it for the moment! And this could disappoint some, whereas just waiting some days to get all of this fixed would permit them to use it much more easily :)

I will see about the blur in overview, this may be somewhere I'm better!

And for the settings, preferences in this style would be... perfect :)
I've tried two times to port the preferences to a correct GTK4+libadwaita style, but each time I worked on something else instead; however this really would be a huge benefit for the extension.
Having panels for each component, and being able to control them individually, would make a lot of sense! But would you prefer to add a window selector, and the after that we port the preferences, or port the preferences first, and then the window selector?

And that's really cool for your motivation, I will do what I can do to help you on this :)

List of changes:
- a lot of comments added (to better understand the code myself, and because of my new resolution to explain code :p)
- added some logs, removed other, refactored some code
- main blur effect removed as it was wrongly shared by multiple actors, each window now has its own blur effect
- removed the user_extension_blur condition, instead directly set the extension's sigma value
- renamed methods not to use underscore (easier to read)
- use paint_signals when hacks level are 1 or 2, because the default is 1 and it caused too much artefacts to be used sanely
- renamed _update_blur to update_blur_effect and made it update the blur effect instead of creating a new one
- simplified the set_brightness and set_sigma mechanism
@aunetx
Copy link
Owner

aunetx commented Jan 25, 2022

It's complete for me!

This still needs some bug-hunting, especially about how the windows and blur actors are tracked -- I have the impression they are not correctly removed after the extension asks for blur removal, and this notably creates a bug -- that you can try by going to the extensions preferences with a blurred app aside, and toogling/untoogling the applications blur like 10/20 times. When closing the preferences window, the blur disappear, and the shell complaints about:

JS ERROR: TypeError: this.blurActorMap.get(...) is undefined
set_blur/<@/home/aunetx/.local/share/gnome-shell/extensions/blur-my-shell@aunetx/applications.js:163:35
JS ERROR: TypeError: this.blurActorMap.get(...) is undefined
set_blur/<@/home/aunetx/.local/share/gnome-shell/extensions/blur-my-shell@aunetx/applications.js:165:35

and also, directly when closing the preferences and when destroying the blurred window afterwards,

../gobject/gsignal.c:2732: instance '0x55d72766a720' has no handler with id '43017'

However, I feel like this is not an extremely important issue -- not enough to report the merge at least! So I opened an issue at #179 to track this. We should also open issues for the remaining things to do before we forget something.

I merge this, I think we can celebrate :p

@aunetx aunetx merged commit 7a61fd4 into aunetx:master Jan 25, 2022
@SudoVanilla
Copy link
Contributor

:O

@CorvetteCole
Copy link
Collaborator Author

great! I think we can also disable the applications toggle too for pushes to extensions.gnome.org (and add a "coming soon"). Would be the easiest way to continue to push updates there while keeping that code in the main branch. I do think my code is a little rough around the edges but I wanted to put it out there so it isn't just lost to the ether. At least in the public we can work on it :)

weblate pushed a commit to weblate/blur-my-shell that referenced this pull request Nov 9, 2023
This adds per-request application blur!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants