Caching brush settings in mask editor

[benceruleanlu]

For #2190 and comfyanonymous/ComfyUI#6458

I think the mask editor needs a lot of refactoring, fixes, and features. I couldn't even navigate the file without resorting to ctrl + f and vscode's minimap.

I tested my changes and they work on my machine, but it might break on desktop? Not too sure how localStorage.setItem works with Electron.

┆Issue is synchronized with this Notion page by Unito

[benceruleanlu]

Also, getting this error on every pre-commit run, even trivial changes. I've just been --no-verify ing past it since it threw the same error even on style changes.

✖ vue-tsc --noEmit:
src/views/InstallView.vue(143,24): error TS2339: Property 'trackEvent' does not exist on type '{ readonly getDetectedGpu: () => Promise<GpuType>; readonly setWindowStyle: (style: "custom" | "default") => Promise<void>; readonly getWindowStyle: () => Promise<"custom" | "default">; }'.
src/views/InstallView.vue(182,24): error TS2339: Property 'trackEvent' does not exist on type '{ readonly getDetectedGpu: () => Promise<GpuType>; readonly setWindowStyle: (style: "custom" | "default") => Promise<void>; readonly getWindowStyle: () => Promise<"custom" | "default">; }'.
husky - pre-commit script failed (code 1)

[christian-byrne]

Tested and works for me.

[christian-byrne]

There's also this:

ComfyUI_frontend/src/scripts/utils.ts

Lines 167 to 175 in 7fd41ee

	export function getStorageValue(id: string) {
	const clientId = api.clientId ?? api.initialClientId
	return (
	(clientId && sessionStorage.getItem(`${id}:${clientId}`)) ??
	localStorage.getItem(id)
	)
	}
	export function setStorageValue(id: string, value: string) {

[webfiltered]

Also, getting this error on every pre-commit run

Does running npm i fix it?

[benceruleanlu]

Also, getting this error on every pre-commit run

Does running npm i fix it?

Running npm install does indeed fix it. Very funny!

[benceruleanlu]

There's also this:

ComfyUI_frontend/src/scripts/utils.ts

Lines 167 to 175 in 7fd41ee

	export function getStorageValue(id: string) {
	const clientId = api.clientId ?? api.initialClientId
	return (
	(clientId && sessionStorage.getItem(`${id}:${clientId}`)) ??
	localStorage.getItem(id)
	)
	}
	export function setStorageValue(id: string, value: string) {

ahhh thanks for letting me know, I didn't see this.

let me try and implement this.

[huchenlei]

Alternatively you can also do hidden setting for restoring these states, e.g.

ComfyUI_frontend/src/constants/coreSettings.ts

Lines 197 to 203 in 7fd41ee

	// Hidden setting used by the queue for how to fit images
	{
	id: 'Comfy.Queue.ImageFit',
	name: 'Queue image fit',
	type: 'hidden',
	defaultValue: 'cover'
	},

But that would require you debounce the setter calls, as each adjustment triggers a request to update it in the backend server.

[huchenlei]

Maybe we would want to debounce these cache calls? You can use https://lodash.com/docs/#debounce

[benceruleanlu]

that's a good point. i didn't think of that.

[huchenlei]

The current state lgtm. You can open a new PR if session/local storage does become an issue later.