From 1b450c7a4489ee1ed0d7a9564dc0b00e6b4454bd Mon Sep 17 00:00:00 2001 From: psychedelicious <4822129+psychedelicious@users.noreply.github.com> Date: Thu, 4 Sep 2025 18:50:26 +1000 Subject: [PATCH 1/3] tidy(ui): remove unused x/y coords from params slice --- .../controlLayers/store/paramsSlice.ts | 103 ++++++++++-------- .../src/features/controlLayers/store/types.ts | 15 +-- .../nodes/util/graph/graphBuilderUtils.ts | 3 +- 3 files changed, 65 insertions(+), 56 deletions(-) diff --git a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.ts b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.ts index 232f88b0364..e6b88fd5427 100644 --- a/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.ts +++ b/invokeai/frontend/web/src/features/controlLayers/store/paramsSlice.ts @@ -4,6 +4,7 @@ import type { RootState } from 'app/store/store'; import type { SliceConfig } from 'app/store/types'; import { deepClone } from 'common/util/deepClone'; import { roundDownToMultiple, roundToMultiple } from 'common/util/roundDownToMultiple'; +import { isPlainObject } from 'es-toolkit'; import { clamp } from 'es-toolkit/compat'; import type { AspectRatioID, ParamsState, RgbaColor } from 'features/controlLayers/store/types'; import { @@ -52,6 +53,7 @@ import type { import { getGridSize, getIsSizeOptimal, getOptimalDimension } from 'features/parameters/util/optimalDimension'; import { modelConfigsAdapterSelectors, selectModelConfigsQuery } from 'services/api/endpoints/models'; import { isNonRefinerMainModelConfig } from 'services/api/types'; +import { assert } from 'tsafe'; const slice = createSlice({ name: 'params', @@ -122,8 +124,8 @@ const slice = createSlice({ state.dimensions.aspectRatio.isLocked = true; state.dimensions.aspectRatio.value = 1; state.dimensions.aspectRatio.id = '1:1'; - state.dimensions.rect.width = 1024; - state.dimensions.rect.height = 1024; + state.dimensions.width = 1024; + state.dimensions.height = 1024; } applyClipSkip(state, model, state.clipSkip); @@ -247,26 +249,26 @@ const slice = createSlice({ sizeRecalled: (state, action: PayloadAction<{ width: number; height: number }>) => { const { width, height } = action.payload; const gridSize = getGridSize(state.model?.base); - state.dimensions.rect.width = Math.max(roundDownToMultiple(width, gridSize), 64); - state.dimensions.rect.height = Math.max(roundDownToMultiple(height, gridSize), 64); - state.dimensions.aspectRatio.value = state.dimensions.rect.width / state.dimensions.rect.height; + state.dimensions.width = Math.max(roundDownToMultiple(width, gridSize), 64); + state.dimensions.height = Math.max(roundDownToMultiple(height, gridSize), 64); + state.dimensions.aspectRatio.value = state.dimensions.width / state.dimensions.height; state.dimensions.aspectRatio.id = 'Free'; state.dimensions.aspectRatio.isLocked = true; }, widthChanged: (state, action: PayloadAction<{ width: number; updateAspectRatio?: boolean; clamp?: boolean }>) => { const { width, updateAspectRatio, clamp } = action.payload; const gridSize = getGridSize(state.model?.base); - state.dimensions.rect.width = clamp ? Math.max(roundDownToMultiple(width, gridSize), 64) : width; + state.dimensions.width = clamp ? Math.max(roundDownToMultiple(width, gridSize), 64) : width; if (state.dimensions.aspectRatio.isLocked) { - state.dimensions.rect.height = roundToMultiple( - state.dimensions.rect.width / state.dimensions.aspectRatio.value, + state.dimensions.height = roundToMultiple( + state.dimensions.width / state.dimensions.aspectRatio.value, gridSize ); } if (updateAspectRatio || !state.dimensions.aspectRatio.isLocked) { - state.dimensions.aspectRatio.value = state.dimensions.rect.width / state.dimensions.rect.height; + state.dimensions.aspectRatio.value = state.dimensions.width / state.dimensions.height; state.dimensions.aspectRatio.id = 'Free'; state.dimensions.aspectRatio.isLocked = false; } @@ -274,17 +276,17 @@ const slice = createSlice({ heightChanged: (state, action: PayloadAction<{ height: number; updateAspectRatio?: boolean; clamp?: boolean }>) => { const { height, updateAspectRatio, clamp } = action.payload; const gridSize = getGridSize(state.model?.base); - state.dimensions.rect.height = clamp ? Math.max(roundDownToMultiple(height, gridSize), 64) : height; + state.dimensions.height = clamp ? Math.max(roundDownToMultiple(height, gridSize), 64) : height; if (state.dimensions.aspectRatio.isLocked) { - state.dimensions.rect.width = roundToMultiple( - state.dimensions.rect.height * state.dimensions.aspectRatio.value, + state.dimensions.width = roundToMultiple( + state.dimensions.height * state.dimensions.aspectRatio.value, gridSize ); } if (updateAspectRatio || !state.dimensions.aspectRatio.isLocked) { - state.dimensions.aspectRatio.value = state.dimensions.rect.width / state.dimensions.rect.height; + state.dimensions.aspectRatio.value = state.dimensions.width / state.dimensions.height; state.dimensions.aspectRatio.id = 'Free'; state.dimensions.aspectRatio.isLocked = false; } @@ -299,55 +301,55 @@ const slice = createSlice({ state.dimensions.aspectRatio.isLocked = false; } else if ((state.model?.base === 'imagen3' || state.model?.base === 'imagen4') && isImagenAspectRatioID(id)) { const { width, height } = IMAGEN_ASPECT_RATIOS[id]; - state.dimensions.rect.width = width; - state.dimensions.rect.height = height; - state.dimensions.aspectRatio.value = state.dimensions.rect.width / state.dimensions.rect.height; + state.dimensions.width = width; + state.dimensions.height = height; + state.dimensions.aspectRatio.value = state.dimensions.width / state.dimensions.height; state.dimensions.aspectRatio.isLocked = true; } else if (state.model?.base === 'chatgpt-4o' && isChatGPT4oAspectRatioID(id)) { const { width, height } = CHATGPT_ASPECT_RATIOS[id]; - state.dimensions.rect.width = width; - state.dimensions.rect.height = height; - state.dimensions.aspectRatio.value = state.dimensions.rect.width / state.dimensions.rect.height; + state.dimensions.width = width; + state.dimensions.height = height; + state.dimensions.aspectRatio.value = state.dimensions.width / state.dimensions.height; state.dimensions.aspectRatio.isLocked = true; } else if (state.model?.base === 'gemini-2.5' && isGemini2_5AspectRatioID(id)) { const { width, height } = GEMINI_2_5_ASPECT_RATIOS[id]; - state.dimensions.rect.width = width; - state.dimensions.rect.height = height; - state.dimensions.aspectRatio.value = state.dimensions.rect.width / state.dimensions.rect.height; + state.dimensions.width = width; + state.dimensions.height = height; + state.dimensions.aspectRatio.value = state.dimensions.width / state.dimensions.height; state.dimensions.aspectRatio.isLocked = true; } else if (state.model?.base === 'flux-kontext' && isFluxKontextAspectRatioID(id)) { const { width, height } = FLUX_KONTEXT_ASPECT_RATIOS[id]; - state.dimensions.rect.width = width; - state.dimensions.rect.height = height; - state.dimensions.aspectRatio.value = state.dimensions.rect.width / state.dimensions.rect.height; + state.dimensions.width = width; + state.dimensions.height = height; + state.dimensions.aspectRatio.value = state.dimensions.width / state.dimensions.height; state.dimensions.aspectRatio.isLocked = true; } else { state.dimensions.aspectRatio.isLocked = true; state.dimensions.aspectRatio.value = ASPECT_RATIO_MAP[id].ratio; const { width, height } = calculateNewSize( state.dimensions.aspectRatio.value, - state.dimensions.rect.width * state.dimensions.rect.height, + state.dimensions.width * state.dimensions.height, state.model?.base ); - state.dimensions.rect.width = width; - state.dimensions.rect.height = height; + state.dimensions.width = width; + state.dimensions.height = height; } }, dimensionsSwapped: (state) => { state.dimensions.aspectRatio.value = 1 / state.dimensions.aspectRatio.value; if (state.dimensions.aspectRatio.id === 'Free') { - const newWidth = state.dimensions.rect.height; - const newHeight = state.dimensions.rect.width; - state.dimensions.rect.width = newWidth; - state.dimensions.rect.height = newHeight; + const newWidth = state.dimensions.height; + const newHeight = state.dimensions.width; + state.dimensions.width = newWidth; + state.dimensions.height = newHeight; } else { const { width, height } = calculateNewSize( state.dimensions.aspectRatio.value, - state.dimensions.rect.width * state.dimensions.rect.height, + state.dimensions.width * state.dimensions.height, state.model?.base ); - state.dimensions.rect.width = width; - state.dimensions.rect.height = height; + state.dimensions.width = width; + state.dimensions.height = height; state.dimensions.aspectRatio.id = ASPECT_RATIO_MAP[state.dimensions.aspectRatio.id].inverseID; } }, @@ -359,25 +361,25 @@ const slice = createSlice({ optimalDimension * optimalDimension, state.model?.base ); - state.dimensions.rect.width = width; - state.dimensions.rect.height = height; + state.dimensions.width = width; + state.dimensions.height = height; } else { state.dimensions.aspectRatio = deepClone(DEFAULT_ASPECT_RATIO_CONFIG); - state.dimensions.rect.width = optimalDimension; - state.dimensions.rect.height = optimalDimension; + state.dimensions.width = optimalDimension; + state.dimensions.height = optimalDimension; } }, syncedToOptimalDimension: (state) => { const optimalDimension = getOptimalDimension(state.model?.base); - if (!getIsSizeOptimal(state.dimensions.rect.width, state.dimensions.rect.height, state.model?.base)) { + if (!getIsSizeOptimal(state.dimensions.width, state.dimensions.height, state.model?.base)) { const bboxDims = calculateNewSize( state.dimensions.aspectRatio.value, optimalDimension * optimalDimension, state.model?.base ); - state.dimensions.rect.width = bboxDims.width; - state.dimensions.rect.height = bboxDims.height; + state.dimensions.width = bboxDims.width; + state.dimensions.height = bboxDims.height; } }, paramsReset: (state) => resetState(state), @@ -488,7 +490,18 @@ export const paramsSliceConfig: SliceConfig = { schema: zParamsState, getInitialState: getInitialParamsState, persistConfig: { - migrate: (state) => zParamsState.parse(state), + migrate: (state) => { + assert(isPlainObject(state)); + + if (!('_version' in state)) { + // v0 -> v1, add _version and remove x/y from dimensions, lifting width/height to top level + state._version = 1; + state.dimensions.width = state.dimensions.rect.width; + state.dimensions.height = state.dimensions.rect.height; + } + + return zParamsState.parse(state); + }, }, }; @@ -600,8 +613,8 @@ export const selectRefinerScheduler = createParamsSelector((params) => params.re export const selectRefinerStart = createParamsSelector((params) => params.refinerStart); export const selectRefinerSteps = createParamsSelector((params) => params.refinerSteps); -export const selectWidth = createParamsSelector((params) => params.dimensions.rect.width); -export const selectHeight = createParamsSelector((params) => params.dimensions.rect.height); +export const selectWidth = createParamsSelector((params) => params.dimensions.width); +export const selectHeight = createParamsSelector((params) => params.dimensions.height); export const selectAspectRatioID = createParamsSelector((params) => params.dimensions.aspectRatio.id); export const selectAspectRatioValue = createParamsSelector((params) => params.dimensions.aspectRatio.value); export const selectAspectRatioIsLocked = createParamsSelector((params) => params.dimensions.aspectRatio.isLocked); diff --git a/invokeai/frontend/web/src/features/controlLayers/store/types.ts b/invokeai/frontend/web/src/features/controlLayers/store/types.ts index 23b1b968861..36381441f00 100644 --- a/invokeai/frontend/web/src/features/controlLayers/store/types.ts +++ b/invokeai/frontend/web/src/features/controlLayers/store/types.ts @@ -558,18 +558,13 @@ const zBboxState = z.object({ }); const zDimensionsState = z.object({ - // TODO(psyche): There is no concept of x/y coords for the dimensions state here... It's just width and height. - // Remove the extraneous data. - rect: z.object({ - x: z.number().int(), - y: z.number().int(), - width: zParameterImageDimension, - height: zParameterImageDimension, - }), + width: zParameterImageDimension, + height: zParameterImageDimension, aspectRatio: zAspectRatioConfig, }); export const zParamsState = z.object({ + _version: z.literal(1), maskBlur: z.number(), maskBlurMethod: zParameterMaskBlurMethod, canvasCoherenceMode: zParameterCanvasCoherenceMode, @@ -617,6 +612,7 @@ export const zParamsState = z.object({ }); export type ParamsState = z.infer; export const getInitialParamsState = (): ParamsState => ({ + _version: 1, maskBlur: 16, maskBlurMethod: 'box', canvasCoherenceMode: 'Gaussian Blur', @@ -661,7 +657,8 @@ export const getInitialParamsState = (): ParamsState => ({ clipGEmbedModel: null, controlLora: null, dimensions: { - rect: { x: 0, y: 0, width: 512, height: 512 }, + width: 512, + height: 512, aspectRatio: deepClone(DEFAULT_ASPECT_RATIO_CONFIG), }, }); diff --git a/invokeai/frontend/web/src/features/nodes/util/graph/graphBuilderUtils.ts b/invokeai/frontend/web/src/features/nodes/util/graph/graphBuilderUtils.ts index 78ddb6232fc..a98faea93a5 100644 --- a/invokeai/frontend/web/src/features/nodes/util/graph/graphBuilderUtils.ts +++ b/invokeai/frontend/web/src/features/nodes/util/graph/graphBuilderUtils.ts @@ -130,8 +130,7 @@ export const getOriginalAndScaledSizesForTextToImage = (state: RootState) => { const scaledSize = ['auto', 'manual'].includes(canvas.bbox.scaleMethod) ? canvas.bbox.scaledSize : originalSize; return { originalSize, scaledSize, aspectRatio }; } else if (tab === 'generate') { - const { rect, aspectRatio } = params.dimensions; - const { width, height } = rect; + const { width, height, aspectRatio } = params.dimensions; return { originalSize: { width, height }, scaledSize: { width, height }, From ec87e9b025e55f498e4f1169ed8514a36e216826 Mon Sep 17 00:00:00 2001 From: psychedelicious <4822129+psychedelicious@users.noreply.github.com> Date: Thu, 4 Sep 2025 19:14:33 +1000 Subject: [PATCH 2/3] fix(ui): rehydration + redux migration design issue Certain items in redux are ephemeral and omitted from persisted slices. On rehydration, we need to inject these values back into the slice. But there was an issue taht could prevent slice migrations from running during rehydration. The migrations look for the `_version` key in state and migrate the slice accordingly. The logic that merged in the ephemeral values accidentally _also_ merged in the `_version` key if it didn't already exist. This happened _before_ migrations are run. This causes problems for slices that didn't have a `_version` key and then have one added via migration. For example, the params slice didn't have a `_version` key until the previous commit, which added `_version` and changed some other parts of state in a migration. On first load of the updated code, we have a catch-22 kinda situation: - The persisted params slice is the old version. It needs to have both `_version` and some other data added to it. - We deserialize the state and then merge in ephemeral values. This inadvertnetly also merged in the `_version` key. - We run the slice migration. It sees there is a `_version` key and thinks it doesn't need to run. The extra data isn't added to the slice. The slice is parsed against its zod schema and fails because the new data is missing. - Because the parse failed, we treat the user's persisted data as invalid and overwrite it with initial state, potentially causing data loss. The fix is to be more selective when merging in the ephemeral state before migration - this is now done by checking which keys are on the persist denylist and only adding those key. --- invokeai/frontend/web/src/app/store/store.ts | 21 ++++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/invokeai/frontend/web/src/app/store/store.ts b/invokeai/frontend/web/src/app/store/store.ts index 40cd78a3e45..12fcfa5a406 100644 --- a/invokeai/frontend/web/src/app/store/store.ts +++ b/invokeai/frontend/web/src/app/store/store.ts @@ -18,7 +18,8 @@ import { addModelsLoadedListener } from 'app/store/middleware/listenerMiddleware import { addSetDefaultSettingsListener } from 'app/store/middleware/listenerMiddleware/listeners/setDefaultSettings'; import { addSocketConnectedEventListener } from 'app/store/middleware/listenerMiddleware/listeners/socketConnected'; import { deepClone } from 'common/util/deepClone'; -import { keys, mergeWith, omit, pick } from 'es-toolkit/compat'; +import { merge } from 'es-toolkit'; +import { omit, pick } from 'es-toolkit/compat'; import { changeBoardModalSliceConfig } from 'features/changeBoardModal/store/slice'; import { canvasSettingsSliceConfig } from 'features/controlLayers/store/canvasSettingsSlice'; import { canvasSliceConfig } from 'features/controlLayers/store/canvasSlice'; @@ -133,16 +134,14 @@ const unserialize: UnserializeFunction = (data, key) => { const initialState = getInitialState(); const parsed = JSON.parse(data); - // strip out old keys - const stripped = pick(deepClone(parsed), keys(initialState)); - /* - * Merge in initial state as default values, covering any missing keys. You might be tempted to use _.defaultsDeep, - * but that merges arrays by index and partial objects by key. Using an identity function as the customizer results - * in behaviour like defaultsDeep, but doesn't overwrite any values that are not undefined in the migrated state. - */ - const unPersistDenylisted = mergeWith(stripped, initialState, (objVal) => objVal); - // run (additive) migrations - const migrated = persistConfig.migrate(unPersistDenylisted); + // We need to inject non-persisted values from initial state into the rehydrated state. These values always are + // required to be in the state, but won't be in the persisted data. Build an object that consists of only these + // values, then merge it with the rehydrated state. + const nonPersistedSubsetOfState = pick(initialState, persistConfig.persistDenylist ?? []); + const stateToMigrate = merge(deepClone(parsed), nonPersistedSubsetOfState); + + // Run migrations to bring old state up to date with the current version. + const migrated = persistConfig.migrate(stateToMigrate); log.debug( { From 2f466642e7fa001b009e299d348a295171fe0603 Mon Sep 17 00:00:00 2001 From: psychedelicious <4822129+psychedelicious@users.noreply.github.com> Date: Thu, 4 Sep 2025 19:23:35 +1000 Subject: [PATCH 3/3] gh: update pr template --- .github/pull_request_template.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index cc78635071b..84633de6ce1 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -18,5 +18,6 @@ - [ ] _The PR has a short but descriptive title, suitable for a changelog_ - [ ] _Tests added / updated (if applicable)_ +- [ ] _❗Changes to a redux slice have a corresponding migration_ - [ ] _Documentation added / updated (if applicable)_ - [ ] _Updated `What's New` copy (if doing a release after this PR)_