Skip to content

fix(theming): Conitionally disable blur filter for performance - #45395

Merged
susnux merged 3 commits into
masterfrom
fix/blur-chromium
Jul 2, 2024
Merged

fix(theming): Conitionally disable blur filter for performance#45395
susnux merged 3 commits into
masterfrom
fix/blur-chromium

Conversation

@susnux

@susnuxsusnux commented May 17, 2024

Copy link
Copy Markdown
Contributor

Summary

As an alternative for #45049 but this one is not removing the blurry background for all users but only for chrome + edge by default.
User can force enable if it works for them (e.g. they have hardware acceleration) or force disable if they suffer performance issues.

Firefox and Safari are not affected by the performance issues, and Android should also be ok as they have proper hardware acceleration.
Mostly Windows is affected and Linux with faulty GPU drivers.

Screencast

a.mp4

Checklist

@susnux
susnux requested a review from szaimenMay 17, 2024 12:39
Comment threadapps/theming/lib/Themes/DefaultTheme.php Fixed
Comment threadapps/theming/lib/Themes/DefaultTheme.php Fixed
Comment threadapps/theming/lib/Themes/DefaultTheme.php Fixed
@susnux
susnuxforce-pushed the fix/blur-chromium branch from 1466ff4 to 2b43786CompareMay 17, 2024 12:45
Comment threadapps/theming/lib/Themes/DefaultTheme.php Fixed
Comment threadapps/theming/lib/Themes/DefaultTheme.php Fixed
@susnux
susnuxforce-pushed the fix/blur-chromium branch from 2b43786 to cff7df4CompareMay 17, 2024 12:56
Comment threadapps/theming/lib/Themes/DefaultTheme.php Fixed
Comment threadapps/theming/lib/Themes/DefaultTheme.php Fixed
Comment threadapps/theming/lib/Themes/DefaultTheme.php
@susnux
susnuxforce-pushed the fix/blur-chromium branch from cff7df4 to e6724d7CompareMay 17, 2024 14:54
@susnuxsusnux added bug 3. to review Waiting for reviews labels May 17, 2024
@susnux
susnux requested review from a team, Pytal, jancborchardt, solracsf and sorbaugh and removed request for a teamMay 17, 2024 14:57
Comment threadapps/theming/lib/Themes/DefaultTheme.php Fixed
@susnux
susnux marked this pull request as ready for review May 17, 2024 15:01
@susnux
susnux requested review from a team, icewind1991, sorbaugh and yemkareems and removed request for a team and sorbaughMay 17, 2024 15:01
@susnux
susnuxforce-pushed the fix/blur-chromium branch from e6724d7 to d0373e2CompareMay 17, 2024 15:06
@susnuxsusnux added this to the Nextcloud 30 milestone May 17, 2024
@susnux

Copy link
Copy Markdown
ContributorAuthor

To be discussed by @nextcloud/designers

@kesselb

Copy link
Copy Markdown
Contributor

Thanks for working on it 👍

Apparently my chromium has no hardware acceleration any longer (it's a snap package now, maybe that's related).

It's basically impossible to use Nextcloud and especially navigation and dashboard.

Screencast.from.2024-05-18.16-46-36.webm
Screencast.from.2024-05-18.16-55-12.webm

The auto-detection worked.
If you selected yes or no once, you cannot go back to "auto" but that's acceptable, I guess.
The left navigation looks definitely cleaner with the blurred background.

@marcoambrosinimarcoambrosini left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I like this approach in general :)
One thing: leaving the sharp edges of a picture below the text can make it hard to read. I think that when we disable the blur we should also add a solid background, not transparent. We can use an average of the background similar to the auto primary? But lighter or darker depending on the theme.
Can we also narrow this down to windows && chromium users only?

@susnux

Copy link
Copy Markdown
ContributorAuthor

/backport to stable28

@jancborchardtjancborchardt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@susnux with this pull request, most people will not see the visual effect of blurring (however it is achieved) anymore, as far as I understand? Which browsers is it default enabled and which default disabled for?

I was under the impression we would go for #45452 as an inbetween solution instead. cc @AndyScherzinger since removing blur will make stuff look less nice.

@marcoambrosini

Copy link
Copy Markdown
Member

If it's impossible to find a solution I would remove transparency + blur everywhere and work from there to achieve a nice design for everyone, without settings

@susnux

Copy link
Copy Markdown
ContributorAuthor

I was under the impression we would go for #45452 as an inbetween solution instead. cc @AndyScherzinger since removing blur will make stuff look less nice.

Yes I hope we get this for Nextcloud 30. But we need a backportable solution for 28 and 29 as currently on those machines you can not use Nextcloud.
If you prefer I remove the autodetect part and only add the option switch.

For 30 we can then use @marcoambrosini solution

@susnux

Copy link
Copy Markdown
ContributorAuthor

Which browsers is it default enabled and which default disabled for?

Currently in this PR:
✅ everything not Chromium based
✅ Chromium based on mobile
✅ Chromium based on MacOS
❌ Chromium based on Windows
❌ Chromium based on Linux

The first 3 never suffer the issue
For Windows and Linux it depends on your GPU driver

@marcoambrosini

Copy link
Copy Markdown
Member

we need a backportable solution

Got it

If you prefer I remove the autodetect part and only add the option switch

I think it's fine then to leave the autodetect on

Comment threadapps/theming/lib/Themes/DefaultTheme.php Outdated

@jancborchardtjancborchardt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the explanation @susnux in your comment #45395 (comment)

Then I agree with you we should get this in very soon to backport, and get Marco’s solution done for 30: #45452

@sorbaugh

@susnux
susnuxforce-pushed the fix/blur-chromium branch from ce9eb83 to 40bbb5bCompareJuly 2, 2024 12:18
@susnux
susnux requested a review from szaimenJuly 2, 2024 12:24
@susnux
susnuxforce-pushed the fix/blur-chromium branch from 40bbb5b to 8c87589CompareJuly 2, 2024 12:33

@szaimenszaimen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tested and works :)

However it is a bit weird to show this state by default which will first allow to disable it even though it is already disabled:
image

susnux added 3 commits July 2, 2024 17:31
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@szaimen

Copy link
Copy Markdown
Contributor

🎉🎉🎉🎉🎉

@szaimen

Copy link
Copy Markdown
Contributor

Which browsers is it default enabled and which default disabled for?

Currently in this PR: ✅ everything not Chromium based ✅ Chromium based on mobile ✅ Chromium based on MacOS ❌ Chromium based on Windows ❌ Chromium based on Linux

The first 3 never suffer the issue For Windows and Linux it depends on your GPU driver

@susnux I guess we should document this somewhere?

@szaimenszaimen added the pending documentation This pull request needs an associated documentation update label Sep 17, 2025
@szaimen

Copy link
Copy Markdown
Contributor

I suppose on windows and linux with chromium based browser the checkbox should also be disabled? Apparently @peterwebdesign ran into a weird bug with it...

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviewsbugpending documentationThis pull request needs an associated documentation update

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

scrolling in dashboard is laggy CSS blur filter seriously impacts performance in Chrome

9 participants

@susnux@kesselb@marcoambrosini@ShGKme@netzpolitikorg@szaimen@jancborchardt@solracsf@github-advanced-security