feat(ui): address feedback

Author: psychedelicious

invokeai/frontend/web/src/features/controlLayers/konva/CanvasEntity/CanvasEntityTransformer.ts

@@ -9,6 +9,7 @@ import {
   getEmptyRect,
   getKonvaNodeDebugAttrs,
   getPrefixedId,
+  offsetCoord,
 } from 'features/controlLayers/konva/util';
 import { selectSelectedEntityIdentifier } from 'features/controlLayers/store/selectors';
 import type { Coordinate, Rect, RectWithRotation } from 'features/controlLayers/store/types';
@@ -548,18 +549,33 @@ export class CanvasEntityTransformer extends CanvasModuleBase {
     }
 
     const pixelRect = this.$pixelRect.get();
-    this.nudgePosition(-pixelRect.x, -pixelRect.y);
-  };
 
-  nudgePosition = (x: number, y: number) => {
-    // Nudge the position by (x, y) pixels
     const position = {
-      x: this.konva.proxyRect.x() + x,
-      y: this.konva.proxyRect.y() + y,
+      x: this.konva.proxyRect.x() - pixelRect.x,
+      y: this.konva.proxyRect.y() - pixelRect.y,
     };
-    // Push state to redux
-    this.manager.stateApi.setEntityPosition({ entityIdentifier: this.parent.entityIdentifier, position });
+
     this.log.trace({ position }, 'Position changed');
+    this.manager.stateApi.setEntityPosition({ entityIdentifier: this.parent.entityIdentifier, position });
+  };
+
+  nudgeBy = (offset: Coordinate) => {
+    // We can immediately move both the proxy rect and layer objects so we don't have to wait for a redux round-trip,
+    // which can take up to 2ms in my testing. This is optional, but can make the interaction feel more responsive,
+    // especially on lower-end devices.
+    // Get the relative position of the layer's objects, according to konva
+    const position = this.konva.proxyRect.position();
+    // Offset the position by the nudge amount
+    const newPosition = offsetCoord(position, offset);
+    // Set the new position of the proxy rect - this doesn't move the layer objects - only the outline rect
+    this.konva.proxyRect.setAttrs(newPosition);
+    // Sync the layer objects with the proxy rect - moves them to the new position
+    this.syncObjectGroupWithProxyRect();
+
+    // Push to redux. The state change will do a round-trip, and eventually make it back to the canvas classes, at
+    // which point the layer will be moved to the new position.
+    this.manager.stateApi.moveEntityBy({ entityIdentifier: this.parent.entityIdentifier, offset });
+    this.log.trace({ offset }, 'Nudged');
   };
 
   syncObjectGroupWithProxyRect = () => {

invokeai/frontend/web/src/features/controlLayers/konva/CanvasStateApiModule.ts

@@ -20,7 +20,8 @@ import {
   controlLayerAdded,
   entityBrushLineAdded,
   entityEraserLineAdded,
-  entityMoved,
+  entityMovedBy,
+  entityMovedTo,
   entityRasterized,
   entityRectAdded,
   entityReset,
@@ -40,7 +41,8 @@ import type {
   EntityBrushLineAddedPayload,
   EntityEraserLineAddedPayload,
   EntityIdentifierPayload,
-  EntityMovedPayload,
+  EntityMovedByPayload,
+  EntityMovedToPayload,
   EntityRasterizedPayload,
   EntityRectAddedPayload,
   Rect,
@@ -139,8 +141,15 @@ export class CanvasStateApiModule extends CanvasModuleBase {
   /**
    * Updates an entity's position, pushing state to redux.
    */
-  setEntityPosition = (arg: EntityMovedPayload) => {
-    this.store.dispatch(entityMoved(arg));
+  setEntityPosition = (arg: EntityMovedToPayload) => {
+    this.store.dispatch(entityMovedTo(arg));
+  };
+
+  /**
+   * Moves an entity by the give offset, pushing state to redux.
+   */
+  moveEntityBy = (arg: EntityMovedByPayload) => {
+    this.store.dispatch(entityMovedBy(arg));
   };
 
   /**

invokeai/frontend/web/src/features/controlLayers/konva/CanvasTool/CanvasMoveToolModule.ts

@@ -3,8 +3,22 @@ import type { CanvasManager } from 'features/controlLayers/konva/CanvasManager';
 import { CanvasModuleBase } from 'features/controlLayers/konva/CanvasModuleBase';
 import type { CanvasToolModule } from 'features/controlLayers/konva/CanvasTool/CanvasToolModule';
 import { getPrefixedId } from 'features/controlLayers/konva/util';
+import type { Coordinate } from 'features/controlLayers/store/types';
 import type { Logger } from 'roarr';
 
+type CanvasMoveToolModuleConfig = {
+  /**
+   * The number of pixels to nudge the entity by when moving with the arrow keys.
+   */
+  NUDGE_PX: number;
+};
+
+const DEFAULT_CONFIG: CanvasMoveToolModuleConfig = {
+  NUDGE_PX: 1,
+};
+
+type NudgeKey = 'ArrowLeft' | 'ArrowRight' | 'ArrowUp' | 'ArrowDown';
+
 export class CanvasMoveToolModule extends CanvasModuleBase {
   readonly type = 'move_tool';
   readonly id: string;
@@ -13,6 +27,9 @@ export class CanvasMoveToolModule extends CanvasModuleBase {
   readonly manager: CanvasManager;
   readonly log: Logger;
 
+  config: CanvasMoveToolModuleConfig = DEFAULT_CONFIG;
+  nudgeOffsets: Record<NudgeKey, Coordinate>;
+
   constructor(parent: CanvasToolModule) {
     super();
     this.id = getPrefixedId(this.type);
@@ -21,6 +38,17 @@ export class CanvasMoveToolModule extends CanvasModuleBase {
     this.path = this.manager.buildPath(this);
     this.log = this.manager.buildLogger(this);
     this.log.debug('Creating module');
+
+    this.nudgeOffsets = {
+      ArrowLeft: { x: -this.config.NUDGE_PX, y: 0 },
+      ArrowRight: { x: this.config.NUDGE_PX, y: 0 },
+      ArrowUp: { x: 0, y: -this.config.NUDGE_PX },
+      ArrowDown: { x: 0, y: this.config.NUDGE_PX },
+    };
+  }
+
+  isNudgeKey(key: string): key is NudgeKey {
+    return this.nudgeOffsets[key as NudgeKey] !== undefined;
   }
 
   syncCursorStyle = () => {
@@ -33,26 +61,44 @@ export class CanvasMoveToolModule extends CanvasModuleBase {
     }
   };
 
-  onKeyDown = (e: KeyboardEvent) => {
-    // Support moving via arrow keys
-    const OFFSET = 1; // How much to move, in px
-    const offsets: Record<string, { x: number; y: number }> = {
-      ArrowLeft: { x: -OFFSET, y: 0 },
-      ArrowRight: { x: OFFSET, y: 0 },
-      ArrowUp: { x: 0, y: -OFFSET },
-      ArrowDown: { x: 0, y: OFFSET },
-    };
-    const { key } = e;
+  nudge = (nudgeKey: NudgeKey) => {
+    if ($focusedRegion.get() !== 'canvas') {
+      return;
+    }
+
     const selectedEntity = this.manager.stateApi.getSelectedEntityAdapter();
-    const { x: offsetX = 0, y: offsetY = 0 } = offsets[key] || {};
+
+    if (!selectedEntity) {
+      return;
+    }
 
     if (
-      !(selectedEntity && selectedEntity.$isInteractable.get() && $focusedRegion.get() === 'canvas') ||
-      (offsetX === 0 && offsetY === 0)
+      selectedEntity.$isDisabled.get() ||
+      selectedEntity.$isEmpty.get() ||
+      selectedEntity.$isLocked.get() ||
+      selectedEntity.$isEntityTypeHidden.get()
     ) {
-      return; // Early return if no entity is selected or it is disabled or canvas is not focused
+      return;
+    }
+
+    const isBusy = this.manager.$isBusy.get();
+    const isMoveToolSelected = this.parent.$tool.get() === 'move';
+    const isThisEntityTransforming = this.manager.stateApi.$transformingAdapter.get() === selectedEntity;
+
+    if (isBusy) {
+      // When the canvas is busy, we shouldn't allow nudging - except when the canvas is busy transforming the selected
+      // entity. Nudging is allowed during transformation, regardless of the selected tool.
+      if (!isThisEntityTransforming) {
+        return;
+      }
+    } else {
+      // Otherwise, the canvas is not busy, and we should only allow nudging when the move tool is selected.
+      if (!isMoveToolSelected) {
+        return;
+      }
     }
 
-    selectedEntity.transformer.nudgePosition(offsetX, offsetY);
+    const offset = this.nudgeOffsets[nudgeKey];
+    selectedEntity.transformer.nudgeBy(offset);
   };
 }

invokeai/frontend/web/src/features/controlLayers/konva/CanvasTool/CanvasToolModule.ts

@@ -532,12 +532,9 @@ export class CanvasToolModule extends CanvasModuleBase {
       return;
     }
 
-    switch (
-      this.$tool.get() // before repeat, as we may want to catch repeating keys
-    ) {
-      case 'move':
-        this.tools.move.onKeyDown(e);
-        break;
+    // Handle nudging - must be before repeat, as we may want to catch repeating keys
+    if (this.tools.move.isNudgeKey(e.key)) {
+      this.tools.move.nudge(e.key);
     }
 
     if (e.repeat) {

invokeai/frontend/web/src/features/controlLayers/store/canvasSlice.ts

@@ -18,6 +18,7 @@ import type {
   CanvasEntityType,
   CanvasInpaintMaskState,
   CanvasMetadata,
+  EntityMovedByPayload,
   FillStyle,
   RegionalGuidanceReferenceImageState,
   RgbColor,
@@ -51,7 +52,7 @@ import type {
   EntityBrushLineAddedPayload,
   EntityEraserLineAddedPayload,
   EntityIdentifierPayload,
-  EntityMovedPayload,
+  EntityMovedToPayload,
   EntityRasterizedPayload,
   EntityRectAddedPayload,
   IPMethodV2,
@@ -1201,7 +1202,7 @@ export const canvasSlice = createSlice({
       }
       entity.fill.style = style;
     },
-    entityMoved: (state, action: PayloadAction<EntityMovedPayload>) => {
+    entityMovedTo: (state, action: PayloadAction<EntityMovedToPayload>) => {
       const { entityIdentifier, position } = action.payload;
       const entity = selectEntity(state, entityIdentifier);
       if (!entity) {
@@ -1212,6 +1213,20 @@ export const canvasSlice = createSlice({
         entity.position = position;
       }
     },
+    entityMovedBy: (state, action: PayloadAction<EntityMovedByPayload>) => {
+      const { entityIdentifier, offset } = action.payload;
+      const entity = selectEntity(state, entityIdentifier);
+      if (!entity) {
+        return;
+      }
+
+      if (!isRenderableEntity(entity)) {
+        return;
+      }
+
+      entity.position.x += offset.x;
+      entity.position.y += offset.y;
+    },
     entityRasterized: (state, action: PayloadAction<EntityRasterizedPayload>) => {
       const { entityIdentifier, imageObject, position, replaceObjects, isSelected } = action.payload;
       const entity = selectEntity(state, entityIdentifier);
@@ -1505,7 +1520,8 @@ export const {
   entityIsLockedToggled,
   entityFillColorChanged,
   entityFillStyleChanged,
-  entityMoved,
+  entityMovedTo,
+  entityMovedBy,
   entityDuplicated,
   entityRasterized,
   entityBrushLineAdded,

invokeai/frontend/web/src/features/controlLayers/store/types.ts

@@ -439,7 +439,8 @@ export type EntityIdentifierPayload<
       entityIdentifier: CanvasEntityIdentifier<U>;
     } & T;
 
-export type EntityMovedPayload = EntityIdentifierPayload<{ position: Coordinate }>;
+export type EntityMovedToPayload = EntityIdentifierPayload<{ position: Coordinate }>;
+export type EntityMovedByPayload = EntityIdentifierPayload<{ offset: Coordinate }>;
 export type EntityBrushLineAddedPayload = EntityIdentifierPayload<{
   brushLine: CanvasBrushLineState | CanvasBrushLineWithPressureState;
 }>;