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

[stable29] fix(theming): Conitionally disable blur filter for performance #46258

Merged
merged 3 commits into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions apps/theming/lib/Listener/BeforePreferenceListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@

/** @template-implements IEventListener<BeforePreferenceDeletedEvent|BeforePreferenceSetEvent> */
class BeforePreferenceListener implements IEventListener {

/**
* @var string[]
*/
private const ALLOWED_KEYS = ['force_enable_blur_filter', 'shortcuts_disabled'];

public function __construct(
private IAppManager $appManager,
) {
Expand All @@ -55,13 +61,22 @@ public function handle(Event $event): void {
}

private function handleThemingValues(BeforePreferenceSetEvent|BeforePreferenceDeletedEvent $event): void {
if ($event->getConfigKey() !== 'shortcuts_disabled') {
if (!in_array($event->getConfigKey(), self::ALLOWED_KEYS)) {
// Not allowed config key
return;
}

if ($event instanceof BeforePreferenceSetEvent) {
$event->setValid($event->getConfigValue() === 'yes');
switch ($event->getConfigKey()) {
case 'force_enable_blur_filter':
$event->setValid($event->getConfigValue() === 'yes' || $event->getConfigValue() === 'no');
break;
case 'shortcuts_disabled':
$event->setValid($event->getConfigValue() === 'yes');
break;
default:
$event->setValid(false);
}
return;
}

Expand Down
1 change: 1 addition & 0 deletions apps/theming/lib/Settings/Personal.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ public function getForm(): TemplateResponse {
$this->initialStateService->provideInitialState('themes', array_values($themes));
$this->initialStateService->provideInitialState('enforceTheme', $enforcedTheme);
$this->initialStateService->provideInitialState('isUserThemingDisabled', $this->themingDefaults->isUserThemingDisabled());
$this->initialStateService->provideInitialState('enableBlurFilter', $this->config->getUserValue($this->userId, 'theming', 'force_enable_blur_filter', ''));
$this->initialStateService->provideInitialState('navigationBar', [
'userAppOrder' => json_decode($this->config->getUserValue($this->userId, 'core', 'apporder', '[]'), true, flags:JSON_THROW_ON_ERROR),
'enforcedDefaultApp' => $forcedDefaultApp
Expand Down
58 changes: 31 additions & 27 deletions apps/theming/lib/Themes/DefaultTheme.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
*/
namespace OCA\Theming\Themes;

use OC\AppFramework\Http\Request;
use OCA\Theming\ImageManager;
use OCA\Theming\ITheme;
use OCA\Theming\Service\BackgroundService;
Expand All @@ -33,41 +34,27 @@
use OCP\App\IAppManager;
use OCP\IConfig;
use OCP\IL10N;
use OCP\IRequest;
use OCP\IURLGenerator;
use OCP\IUserSession;

class DefaultTheme implements ITheme {
use CommonThemeTrait;

public Util $util;
public ThemingDefaults $themingDefaults;
public IUserSession $userSession;
public IURLGenerator $urlGenerator;
public ImageManager $imageManager;
public IConfig $config;
public IL10N $l;
public IAppManager $appManager;

public string $defaultPrimaryColor;
public string $primaryColor;

public function __construct(Util $util,
ThemingDefaults $themingDefaults,
IUserSession $userSession,
IURLGenerator $urlGenerator,
ImageManager $imageManager,
IConfig $config,
IL10N $l,
IAppManager $appManager) {
$this->util = $util;
$this->themingDefaults = $themingDefaults;
$this->userSession = $userSession;
$this->urlGenerator = $urlGenerator;
$this->imageManager = $imageManager;
$this->config = $config;
$this->l = $l;
$this->appManager = $appManager;

public function __construct(
public Util $util,
public ThemingDefaults $themingDefaults,
public IUserSession $userSession,
public IURLGenerator $urlGenerator,
public ImageManager $imageManager,
public IConfig $config,
public IL10N $l,
public IAppManager $appManager,
private ?IRequest $request,
) {
$this->defaultPrimaryColor = $this->themingDefaults->getDefaultColorPrimary();
$this->primaryColor = $this->themingDefaults->getColorPrimary();

Expand Down Expand Up @@ -120,12 +107,29 @@ public function getCSSVariables(): array {
$colorSuccess = '#2d7b41';
$colorInfo = '#0071ad';

$user = $this->userSession->getUser();
// Chromium based browsers currently (2024) have huge performance issues with blur filters
$isChromium = $this->request !== null && $this->request->isUserAgent([Request::USER_AGENT_CHROME, Request::USER_AGENT_MS_EDGE]);
// Ignore MacOS because they always have hardware accelartion
$isChromium = $isChromium && !$this->request->isUserAgent(['/Macintosh/']);
// Allow to force the blur filter
$forceEnableBlur = $user === null ? false : $this->config->getUserValue(
$user->getUID(),
'theming',
'force_enable_blur_filter',
);
$workingBlur = match($forceEnableBlur) {
'yes' => true,
'no' => false,
default => !$isChromium
};

$variables = [
'--color-main-background' => $colorMainBackground,
'--color-main-background-rgb' => $colorMainBackgroundRGB,
'--color-main-background-translucent' => 'rgba(var(--color-main-background-rgb), .97)',
'--color-main-background-blur' => 'rgba(var(--color-main-background-rgb), .8)',
'--filter-background-blur' => 'blur(25px)',
'--filter-background-blur' => $workingBlur ? 'blur(25px)' : 'none',

// to use like this: background-image: linear-gradient(0, var('--gradient-main-background));
'--gradient-main-background' => 'var(--color-main-background) 0%, var(--color-main-background-translucent) 85%, transparent 100%',
Expand Down
27 changes: 27 additions & 0 deletions apps/theming/src/UserThemes.vue
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@
type="font"
@change="changeFont" />
</div>

<h3>{{ t('theming', 'Misc accessibility options') }}</h3>
<NcCheckboxRadioSwitch type="checkbox"
:checked="enableBlurFilter === 'yes'"
:indeterminate="enableBlurFilter === ''"
@update:checked="changeEnableBlurFilter">
{{ t('theming', 'Enable blur background filter (may increase GPU load)') }}
</NcCheckboxRadioSwitch>
</NcSettingsSection>

<NcSettingsSection :name="t('theming', 'Background')"
Expand Down Expand Up @@ -93,6 +101,7 @@ import UserAppMenuSection from './components/UserAppMenuSection.vue'
const availableThemes = loadState('theming', 'themes', [])
const enforceTheme = loadState('theming', 'enforceTheme', '')
const shortcutsDisabled = loadState('theming', 'shortcutsDisabled', false)
const enableBlurFilter = loadState('theming', 'enableBlurFilter', '')

const isUserThemingDisabled = loadState('theming', 'isUserThemingDisabled')

Expand All @@ -115,6 +124,8 @@ export default {
enforceTheme,
shortcutsDisabled,
isUserThemingDisabled,

enableBlurFilter,
}
},

Expand Down Expand Up @@ -240,6 +251,22 @@ export default {
}
},

async changeEnableBlurFilter() {
this.enableBlurFilter = this.enableBlurFilter === 'no' ? 'yes' : 'no'
await axios({
url: generateOcsUrl('apps/provisioning_api/api/v1/config/users/{appId}/{configKey}', {
appId: 'theming',
configKey: 'force_enable_blur_filter',
}),
data: {
configValue: this.enableBlurFilter,
},
method: 'POST',
})
// Refresh the styles
this.$emit('update:background')
},

updateBodyAttributes() {
const enabledThemesIDs = this.themes.filter(theme => theme.enabled === true).map(theme => theme.id)
const enabledFontsIDs = this.fonts.filter(font => font.enabled === true).map(font => font.id)
Expand Down
6 changes: 6 additions & 0 deletions apps/theming/tests/Service/ThemesServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ private function initThemes() {
$this->config,
$l10n,
$appManager,
null,
),
'light' => new LightTheme(
$util,
Expand All @@ -342,6 +343,7 @@ private function initThemes() {
$this->config,
$l10n,
$appManager,
null,
),
'dark' => new DarkTheme(
$util,
Expand All @@ -352,6 +354,7 @@ private function initThemes() {
$this->config,
$l10n,
$appManager,
null,
),
'light-highcontrast' => new HighContrastTheme(
$util,
Expand All @@ -362,6 +365,7 @@ private function initThemes() {
$this->config,
$l10n,
$appManager,
null,
),
'dark-highcontrast' => new DarkHighContrastTheme(
$util,
Expand All @@ -372,6 +376,7 @@ private function initThemes() {
$this->config,
$l10n,
$appManager,
null,
),
'opendyslexic' => new DyslexiaFont(
$util,
Expand All @@ -382,6 +387,7 @@ private function initThemes() {
$this->config,
$l10n,
$appManager,
null,
),
];
}
Expand Down
30 changes: 20 additions & 10 deletions apps/theming/tests/Settings/PersonalTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,15 @@
use OCP\IL10N;
use OCP\IURLGenerator;
use OCP\IUserSession;
use PHPUnit\Framework\MockObject\MockObject;
use Test\TestCase;

class PersonalTest extends TestCase {
private IConfig $config;
private ThemesService $themesService;
private IInitialState $initialStateService;
private ThemingDefaults $themingDefaults;
private IAppManager $appManager;
private IConfig|MockObject $config;
private ThemesService|MockObject $themesService;
private IInitialState|MockObject $initialStateService;
private ThemingDefaults|MockObject $themingDefaults;
private IAppManager|MockObject $appManager;
private Personal $admin;

/** @var ITheme[] */
Expand Down Expand Up @@ -116,23 +117,26 @@ public function testGetForm(string $enforcedTheme, $themesState) {
->with('enforce_theme', '')
->willReturn($enforcedTheme);

$this->config->expects($this->once())
$this->config->expects($this->exactly(2))
->method('getUserValue')
->with('admin', 'core', 'apporder')
->willReturn('[]');
->willReturnMap([
['admin', 'core', 'apporder', '[]'],
['admin', 'theming', 'force_enable_blur_filter', ''],
]);

$this->appManager->expects($this->once())
->method('getDefaultAppForUser')
->willReturn('forcedapp');

$this->initialStateService->expects($this->exactly(4))
->method('provideInitialState')
->withConsecutive(
->willReturnMap([
['themes', $themesState],
['enableBlurFilter', ''],
['enforceTheme', $enforcedTheme],
['isUserThemingDisabled', false],
['navigationBar', ['userAppOrder' => [], 'enforcedDefaultApp' => 'forcedapp']],
);
]);

$expected = new TemplateResponse('theming', 'settings-personal');
$this->assertEquals($expected, $this->admin->getForm());
Expand Down Expand Up @@ -174,6 +178,7 @@ private function initThemes() {
$config,
$l10n,
$appManager,
null,
),
'light' => new LightTheme(
$util,
Expand All @@ -184,6 +189,7 @@ private function initThemes() {
$config,
$l10n,
$appManager,
null,
),
'dark' => new DarkTheme(
$util,
Expand All @@ -194,6 +200,7 @@ private function initThemes() {
$config,
$l10n,
$appManager,
null,
),
'light-highcontrast' => new HighContrastTheme(
$util,
Expand All @@ -204,6 +211,7 @@ private function initThemes() {
$config,
$l10n,
$appManager,
null,
),
'dark-highcontrast' => new DarkHighContrastTheme(
$util,
Expand All @@ -214,6 +222,7 @@ private function initThemes() {
$config,
$l10n,
$appManager,
null,
),
'opendyslexic' => new DyslexiaFont(
$util,
Expand All @@ -224,6 +233,7 @@ private function initThemes() {
$config,
$l10n,
$appManager,
null,
),
];
}
Expand Down
1 change: 1 addition & 0 deletions apps/theming/tests/Themes/DarkHighContrastThemeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ protected function setUp(): void {
$this->config,
$this->l10n,
$this->appManager,
null,
);

parent::setUp();
Expand Down
1 change: 1 addition & 0 deletions apps/theming/tests/Themes/DarkThemeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ protected function setUp(): void {
$this->config,
$this->l10n,
$this->appManager,
null,
);

parent::setUp();
Expand Down
1 change: 1 addition & 0 deletions apps/theming/tests/Themes/DefaultThemeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ protected function setUp(): void {
$this->config,
$this->l10n,
$this->appManager,
null,
);

parent::setUp();
Expand Down
1 change: 1 addition & 0 deletions apps/theming/tests/Themes/DyslexiaFontTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ protected function setUp(): void {
$this->config,
$this->l10n,
$this->appManager,
null,
);

parent::setUp();
Expand Down
1 change: 1 addition & 0 deletions apps/theming/tests/Themes/HighContrastThemeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ protected function setUp(): void {
$this->config,
$this->l10n,
$this->appManager,
null,
);

parent::setUp();
Expand Down
4 changes: 2 additions & 2 deletions dist/theming-personal-theming.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/theming-personal-theming.js.map

Large diffs are not rendered by default.

Loading