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

Added automatic saving for settings and persistent storage for image filter settings #9032

Merged
merged 14 commits into from
Feb 13, 2025
Merged
8 changes: 8 additions & 0 deletions changelog.d/20250203_125000_klakhov_refactor_settings.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
### Changed

- Client settings are now saved automatically
(<https://github.com/cvat-ai/cvat/pull/9032>)

### Added
- Gamma filter settings are now automatically saved and restored upon reload
(<https://github.com/cvat-ai/cvat/pull/9032>)
113 changes: 113 additions & 0 deletions cvat-ui/src/actions/settings-actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,17 @@
//
// SPDX-License-Identifier: MIT

import _ from 'lodash';
import { AnyAction } from 'redux';
import { ThunkAction } from 'utils/redux';
import {
GridColor, ColorBy, SettingsState, ToolsBlockerState,
CombinedState,
} from 'reducers';
import { ImageFilter, ImageFilterAlias } from 'utils/image-processing';
import { conflict, conflictDetector } from 'utils/conflict-detector';
import GammaCorrection, { GammaFilterOptions } from 'utils/fabric-wrapper/gamma-correciton';
import { shortcutsActions } from './shortcuts-actions';

export enum SettingsActionTypes {
SWITCH_ROTATE_ALL = 'SWITCH_ROTATE_ALL',
Expand Down Expand Up @@ -409,3 +415,110 @@ export function resetImageFilters(): AnyAction {
payload: {},
};
}

export function restoreSettingsAsync(): ThunkAction {
return async (dispatch, getState): Promise<void> => {
const state: CombinedState = getState();
Copy link
Member

@bsekachev bsekachev Feb 11, 2025

Choose a reason for hiding this comment

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

I understand that you just copypasted the code, but the current implementation looks mind blowing. So, if you have ideas how it can be refactored, you are welcome

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I refactored parts related to the player, workspace, and image filter settings restoration—now the code looks cleaner.

As for the part that is handling shortcuts, where we currently have four nested loops—yes, it definitely needs refactoring. Ideally, conflictDetector should find all conflicts without requiring additional iterations. However, tackling this now feels a bit overwhelming. I'll work on it in a separate PR.

For now, let's focus on merging this customer request.

const { settings, shortcuts } = state;

dispatch(shortcutsActions.setDefaultShortcuts(structuredClone(shortcuts.keyMap)));
const newSettings = { ..._.pick(settings, 'player', 'workspace'), imageFilters: [] as ImageFilter[] };
const settingsString = localStorage.getItem('clientSettings') as string;
if (!settingsString) return;
const loadedSettings = JSON.parse(settingsString);
for (const [sectionKey, section] of Object.entries(newSettings)) {
for (const [key, value] of Object.entries(section)) {
let settedValue = value;
if (sectionKey in loadedSettings && key in loadedSettings[sectionKey]) {
settedValue = loadedSettings[sectionKey][key];
Object.defineProperty(newSettings[(sectionKey as 'player' | 'workspace')], key, { value: settedValue });
}
}
}

newSettings.imageFilters = [];
if ('imageFilters' in loadedSettings) {
for (const filter of loadedSettings.imageFilters) {
switch (filter.alias) {
case ImageFilterAlias.GAMMA_CORRECTION: {
const modifier = new GammaCorrection(filter.params as GammaFilterOptions);
newSettings.imageFilters.push({
modifier,
alias: ImageFilterAlias.GAMMA_CORRECTION,
});
break;
}
default: {
break;
}
}
}
}

dispatch(setSettings(newSettings));
if ('shortcuts' in loadedSettings) {
const updateKeyMap = structuredClone(shortcuts.keyMap);
for (const key of Object.keys(loadedSettings.shortcuts.keyMap)) {
const value = loadedSettings.shortcuts.keyMap[key];
if (key in updateKeyMap) {
updateKeyMap[key].sequences = value.sequences;
}
}
for (const key of Object.keys(updateKeyMap)) {
const currValue = {
[key]: { ...updateKeyMap[key] },
};
const conflictingShortcuts = conflictDetector(currValue, shortcuts.keyMap);
if (conflictingShortcuts) {
for (const conflictingShortcut of Object.keys(conflictingShortcuts)) {
for (const sequence of currValue[key].sequences) {
for (const conflictingSequence of conflictingShortcuts[conflictingShortcut].sequences) {
if (conflict(sequence, conflictingSequence)) {
updateKeyMap[conflictingShortcut].sequences = [
...updateKeyMap[conflictingShortcut].sequences.filter(
(s: string) => s !== conflictingSequence,
),
];
}
}
}
}
}
}
dispatch(shortcutsActions.registerShortcuts(updateKeyMap));
}
};
}

export function updateCachedSettings(settings: CombinedState['settings'], shortcuts: CombinedState['shortcuts']) {
const settingsForSaving: any = {
shortcuts: {
keyMap: {},
},
imageFilters: [],
};
const supportedImageFilters = [ImageFilterAlias.GAMMA_CORRECTION];
for (const [key, value] of Object.entries(settings)) {
if (['player', 'workspace'].includes(key)) {
settingsForSaving[key] = value;
}
if (key === 'imageFilters') {
const filters = [];
for (const filter of value) {
if (supportedImageFilters.includes(filter.alias)) {
filters.push(filter.modifier.toJSON());
}
}
settingsForSaving.imageFilters = filters;
}
}
for (const [key] of Object.entries(shortcuts.keyMap)) {
if (key in shortcuts.defaultState) {
settingsForSaving.shortcuts.keyMap[key] = {
sequences: shortcuts.keyMap[key].sequences,
};
}
}

localStorage.setItem('clientSettings', JSON.stringify(settingsForSaving));
}
155 changes: 35 additions & 120 deletions cvat-ui/src/components/header/settings-modal/settings-modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,20 @@
// SPDX-License-Identifier: MIT

import './styles.scss';
import _ from 'lodash';
import React, { useCallback, useEffect } from 'react';
import React, { useEffect, useState } from 'react';
import { useSelector, useDispatch } from 'react-redux';
import Tabs from 'antd/lib/tabs';
import Text from 'antd/lib/typography/Text';
import Modal from 'antd/lib/modal/Modal';
import Button from 'antd/lib/button';
import notification from 'antd/lib/notification';
import Tooltip from 'antd/lib/tooltip';
import { PlayCircleOutlined, LaptopOutlined, BuildOutlined } from '@ant-design/icons';

import { setSettings } from 'actions/settings-actions';
import { shortcutsActions } from 'actions/shortcuts-actions';
import { restoreSettingsAsync, updateCachedSettings } from 'actions/settings-actions';
import WorkspaceSettingsContainer from 'containers/header/settings-modal/workspace-settings';
import PlayerSettingsContainer from 'containers/header/settings-modal/player-settings';
import ShortcutsSettingsContainer from 'containers/header/settings-modal/shortcuts-settings';
import { CombinedState } from 'reducers';
import { conflict, conflictDetector } from 'utils/conflict-detector';

interface SettingsModalProps {
visible: boolean;
Expand All @@ -33,92 +29,49 @@ function SettingsModal(props: SettingsModalProps): JSX.Element {

const settings = useSelector((state: CombinedState) => state.settings);
const shortcuts = useSelector((state: CombinedState) => state.shortcuts);
const [settingsInitialized, setSettingsInitialized] = useState(false);
const dispatch = useDispatch();

const onSaveSettings = useCallback(() => {
const settingsForSaving: any = {
shortcuts: {
keyMap: {},
},
};
for (const [key, value] of Object.entries(settings)) {
if (['player', 'workspace'].includes(key)) {
settingsForSaving[key] = value;
}
}
for (const [key] of Object.entries(shortcuts.keyMap)) {
if (key in shortcuts.defaultState) {
settingsForSaving.shortcuts.keyMap[key] = {
sequences: shortcuts.keyMap[key].sequences,
};
}
}

localStorage.setItem('clientSettings', JSON.stringify(settingsForSaving));
notification.success({
message: 'Settings were successfully saved',
className: 'cvat-notification-notice-save-settings-success',
});
useEffect(() => {
if (!settingsInitialized) return;

onClose();
}, [onClose, settings, shortcuts]);
updateCachedSettings(settings, shortcuts);
}, [setSettingsInitialized, settings, shortcuts]);

useEffect(() => {
try {
dispatch(shortcutsActions.setDefaultShortcuts(structuredClone(shortcuts.keyMap)));
const newSettings = _.pick(settings, 'player', 'workspace');
const settingsString = localStorage.getItem('clientSettings') as string;
if (!settingsString) return;
const loadedSettings = JSON.parse(settingsString);
for (const [sectionKey, section] of Object.entries(newSettings)) {
for (const [key, value] of Object.entries(section)) {
let settedValue = value;
if (sectionKey in loadedSettings && key in loadedSettings[sectionKey]) {
settedValue = loadedSettings[sectionKey][key];
Object.defineProperty(newSettings[(sectionKey as 'player' | 'workspace')], key, { value: settedValue });
}
}
}
dispatch(setSettings(newSettings));
if ('shortcuts' in loadedSettings) {
const updateKeyMap = structuredClone(shortcuts.keyMap);
for (const key of Object.keys(loadedSettings.shortcuts.keyMap)) {
const value = loadedSettings.shortcuts.keyMap[key];
if (key in updateKeyMap) {
updateKeyMap[key].sequences = value.sequences;
}
}
for (const key of Object.keys(updateKeyMap)) {
const currValue = {
[key]: { ...updateKeyMap[key] },
};
const conflictingShortcuts = conflictDetector(currValue, shortcuts.keyMap);
if (conflictingShortcuts) {
for (const conflictingShortcut of Object.keys(conflictingShortcuts)) {
for (const sequence of currValue[key].sequences) {
for (const conflictingSequence of conflictingShortcuts[conflictingShortcut].sequences) {
if (conflict(sequence, conflictingSequence)) {
updateKeyMap[conflictingShortcut].sequences = [
...updateKeyMap[conflictingShortcut].sequences.filter(
(s: string) => s !== conflictingSequence,
),
];
}
}
}
}
}
}
dispatch(shortcutsActions.registerShortcuts(updateKeyMap));
}
dispatch(restoreSettingsAsync());
} catch {
notification.error({
message: 'Failed to load settings from local storage',
className: 'cvat-notification-notice-load-settings-fail',
});
} finally {
setSettingsInitialized(true);
}
}, []);

const tabItems = [
{
key: 'player',
label: <Text>Player</Text>,
icon: <PlayCircleOutlined />,
children: <PlayerSettingsContainer />,
},
{
key: 'workspace',
label: <Text>Workspace</Text>,
icon: <LaptopOutlined />,
children: <WorkspaceSettingsContainer />,
},
{
key: 'shortcuts',
label: <Text>Shortcuts</Text>,
icon: <BuildOutlined />,
children: <ShortcutsSettingsContainer />,
},
];

return (
<Modal
title='Settings'
Expand All @@ -127,51 +80,13 @@ function SettingsModal(props: SettingsModalProps): JSX.Element {
width={800}
className='cvat-settings-modal'
footer={(
<>
<Tooltip title='Will save settings to restore them after the app is reopened'>
<Button className='cvat-save-settings-button' type='primary' onClick={onSaveSettings}>
Save
</Button>
</Tooltip>
<Button className='cvat-close-settings-button' type='default' onClick={onClose}>
Close
</Button>
</>
<Button className='cvat-close-settings-button' type='default' onClick={onClose}>
Close
</Button>
)}
>
<div className='cvat-settings-tabs'>
<Tabs
type='card'
tabBarStyle={{ marginBottom: '0px', marginLeft: '-1px' }}
items={[{
key: 'player',
label: (
<span>
<PlayCircleOutlined />
<Text>Player</Text>
</span>
),
children: <PlayerSettingsContainer />,
}, {
key: 'workspace',
label: (
<span>
<LaptopOutlined />
<Text>Workspace</Text>
</span>
),
children: <WorkspaceSettingsContainer />,
}, {
key: 'shortcuts',
label: (
<span>
<BuildOutlined />
<Text>Shortcuts</Text>
</span>
),
children: <ShortcutsSettingsContainer />,
}]}
/>
<Tabs defaultActiveKey='player' type='card' items={tabItems} />
</div>
</Modal>
);
Expand Down
10 changes: 10 additions & 0 deletions cvat-ui/src/utils/fabric-wrapper/gamma-correciton.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// SPDX-License-Identifier: MIT

import { fabric } from 'fabric';
import { ImageFilterAlias, SerializedImageFilter } from 'utils/image-processing';
import FabricFilter from './fabric-wrapper';

export interface GammaFilterOptions {
Expand Down Expand Up @@ -34,6 +35,15 @@ export default class GammaCorrection extends FabricFilter {
this.#gamma = newGamma;
}

public toJSON(): SerializedImageFilter {
return {
alias: ImageFilterAlias.GAMMA_CORRECTION,
params: {
gamma: this.#gamma,
},
};
}

get gamma(): number {
return this.#gamma[0];
}
Expand Down
9 changes: 9 additions & 0 deletions cvat-ui/src/utils/image-processing.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,17 @@
import { fabric } from 'fabric';

export type ConfigurableFilterType = fabric.IBaseFilter;
export interface SerializedImageFilter {
alias: ImageFilterAlias;
params: object;
}
export interface ImageProcessing {
filter: ConfigurableFilterType | null;
currentProcessedImage: number | null;

processImage: (src: ImageData, frameNumber: number) => ImageData;
configure: (options: object) => void;
toJSON: () => SerializedImageFilter;
}

/* eslint @typescript-eslint/no-unused-vars: ["error", { "argsIgnorePattern": "^_" }] */
Expand All @@ -23,6 +28,10 @@ export class BaseImageFilter implements ImageProcessing {
}

configure(_options: object): void {}

toJSON(): SerializedImageFilter {
throw new Error('Method is not implemented');
}
}

export interface ImageFilter {
Expand Down
Loading
Loading