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

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

Merged
merged 3 commits into from
Jul 2, 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
14 changes: 11 additions & 3 deletions apps/theming/lib/Listener/BeforePreferenceListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@

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

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

public function __construct(
private IAppManager $appManager,
) {
Expand All @@ -38,15 +44,16 @@ public function handle(Event $event): void {
}

private function handleThemingValues(BeforePreferenceSetEvent|BeforePreferenceDeletedEvent $event): void {
$allowedKeys = ['shortcuts_disabled', 'primary_color'];

if (!in_array($event->getConfigKey(), $allowedKeys)) {
if (!in_array($event->getConfigKey(), self::ALLOWED_KEYS)) {
// Not allowed config key
return;
}

if ($event instanceof BeforePreferenceSetEvent) {
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;
Expand All @@ -56,6 +63,7 @@ private function handleThemingValues(BeforePreferenceSetEvent|BeforePreferenceDe
default:
$event->setValid(false);
}
return;
}

$event->setValid(true);
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 @@ -75,6 +75,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 @@ -7,48 +7,35 @@
*/
namespace OCA\Theming\Themes;

use OC\AppFramework\Http\Request;
use OCA\Theming\ImageManager;
use OCA\Theming\ITheme;
use OCA\Theming\ThemingDefaults;
use OCA\Theming\Util;
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 @@ -96,12 +83,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]);
szaimen marked this conversation as resolved.
Show resolved Hide resolved
// 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',
Fixed Show fixed Hide fixed

// 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/UserTheming.vue
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,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', 'Primary color')"
Expand Down Expand Up @@ -86,6 +94,7 @@ import UserPrimaryColor from './components/UserPrimaryColor.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 @@ -109,6 +118,8 @@ export default {
enforceTheme,
shortcutsDisabled,
isUserThemingDisabled,

enableBlurFilter,
}
},

Expand Down Expand Up @@ -223,6 +234,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 @@ -315,6 +315,7 @@ private function initThemes() {
$this->config,
$l10n,
$appManager,
null,
),
'light' => new LightTheme(
$util,
Expand All @@ -325,6 +326,7 @@ private function initThemes() {
$this->config,
$l10n,
$appManager,
null,
),
'dark' => new DarkTheme(
$util,
Expand All @@ -335,6 +337,7 @@ private function initThemes() {
$this->config,
$l10n,
$appManager,
null,
),
'light-highcontrast' => new HighContrastTheme(
$util,
Expand All @@ -345,6 +348,7 @@ private function initThemes() {
$this->config,
$l10n,
$appManager,
null,
),
'dark-highcontrast' => new DarkHighContrastTheme(
$util,
Expand All @@ -355,6 +359,7 @@ private function initThemes() {
$this->config,
$l10n,
$appManager,
null,
),
'opendyslexic' => new DyslexiaFont(
$util,
Expand All @@ -365,6 +370,7 @@ private function initThemes() {
$this->config,
$l10n,
$appManager,
null,
),
];
}
Expand Down
24 changes: 16 additions & 8 deletions apps/theming/tests/Settings/PersonalTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,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 @@ -106,17 +107,18 @@ public function testGetForm(string $enforcedTheme, $themesState) {
->method('getDefaultAppForUser')
->willReturn('forcedapp');

$this->initialStateService->expects($this->exactly(7))
$this->initialStateService->expects($this->exactly(8))
->method('provideInitialState')
->withConsecutive(
->willReturnMap([
['shippedBackgrounds', BackgroundService::SHIPPED_BACKGROUNDS],
['themingDefaults'],
['enableBlurFilter', ''],
['userBackgroundImage'],
['themes', $themesState],
['enforceTheme', $enforcedTheme],
['isUserThemingDisabled', false],
['navigationBar', ['userAppOrder' => [], 'enforcedDefaultApp' => 'forcedapp']],
);
]);

$expected = new TemplateResponse('theming', 'settings-personal');
$this->assertEquals($expected, $this->admin->getForm());
Expand Down Expand Up @@ -158,6 +160,7 @@ private function initThemes() {
$config,
$l10n,
$appManager,
null,
),
'light' => new LightTheme(
$util,
Expand All @@ -168,6 +171,7 @@ private function initThemes() {
$config,
$l10n,
$appManager,
null,
),
'dark' => new DarkTheme(
$util,
Expand All @@ -178,6 +182,7 @@ private function initThemes() {
$config,
$l10n,
$appManager,
null,
),
'light-highcontrast' => new HighContrastTheme(
$util,
Expand All @@ -188,6 +193,7 @@ private function initThemes() {
$config,
$l10n,
$appManager,
null,
),
'dark-highcontrast' => new DarkHighContrastTheme(
$util,
Expand All @@ -198,6 +204,7 @@ private function initThemes() {
$config,
$l10n,
$appManager,
null,
),
'opendyslexic' => new DyslexiaFont(
$util,
Expand All @@ -208,6 +215,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 @@ -101,6 +101,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 @@ -98,6 +98,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 @@ -102,6 +102,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 @@ -103,6 +103,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 @@ -101,6 +101,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