fix: address coderabbit review on the layer editor UX batch

Author: jtydhr88

src/renderer/extensions/compositor/components/WidgetCompositor.test.ts

@@ -1,6 +1,7 @@
 import { render, screen } from '@testing-library/vue'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import { ref } from 'vue'
+import { createI18n } from 'vue-i18n'
 
 import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
 import { toNodeId } from '@/types/nodeId'
@@ -40,17 +41,33 @@ vi.mock(
     })
   })
 )
-vi.mock('vue-i18n', () => ({
-  useI18n: () => ({ t: (key: string) => key })
-}))
+const i18n = createI18n({
+  legacy: false,
+  locale: 'en',
+  messages: {
+    en: {
+      compositor: {
+        empty: 'Run the workflow to generate a composite',
+        open: 'Open Compositor',
+        runWorkflowFirst: 'Run the workflow once to load input images',
+        downloadPsd: 'Download PSD'
+      }
+    }
+  }
+})
 
 const nodeId = toNodeId(9)
 const graphNode = { id: nodeId, graph: null } as unknown as LGraphNode
 
 function renderWidget() {
   return render(WidgetCompositor, {
     props: { nodeId },
-    global: { stubs: { Button: { template: '<button v-bind="$attrs" />' } } }
+    global: {
+      plugins: [i18n],
+      stubs: {
+        Button: { template: '<button v-bind="$attrs"><slot /></button>' }
+      }
+    }
   })
 }
 
@@ -64,8 +81,11 @@ describe('WidgetCompositor', () => {
   it('renders the empty state when the node is not in the graph (search preview)', () => {
     renderWidget()
 
-    expect(screen.getByTestId('compositor-empty')).toBeTruthy()
+    expect(screen.getByTestId('compositor-empty').textContent).toContain(
+      'Run the workflow to generate a composite'
+    )
     const open = screen.getByTestId('compositor-open-button')
+    expect(open.textContent).toContain('Open Compositor')
     expect(open.hasAttribute('disabled')).toBe(true)
   })
 

src/renderer/extensions/layerEditor/components/PropertyNumberField.test.ts

@@ -1,6 +1,7 @@
 import userEvent from '@testing-library/user-event'
 import { render, screen } from '@testing-library/vue'
 import { describe, expect, it } from 'vitest'
+import { nextTick } from 'vue'
 
 import PropertyNumberField from './PropertyNumberField.vue'
 
@@ -28,6 +29,7 @@ describe('PropertyNumberField', () => {
   it('reverts an emptied field instead of committing 0', async () => {
     const { emitted } = renderField()
     const input = await commitValue('')
+    await nextTick()
     expect(emitted('commit')).toBeUndefined()
     expect(input.value).toBe('40')
   })

src/renderer/extensions/layerEditor/composables/useLayerEditorSession.test.ts

@@ -24,6 +24,7 @@ import type {
   RasterData
 } from '@/renderer/extensions/layerEditor/engine/node'
 
+import { reorderDropIndex } from './layerPanelDnd'
 import { useLayerEditorSession } from './useLayerEditorSession'
 
 class FakeCompositor implements Compositor {
@@ -383,6 +384,39 @@ describe('useLayerEditorSession', () => {
       expect(session.imageLayers.value.map((n) => n.name)).toEqual(['A', 'B'])
     })
 
+    it('drag-drop math lands the dragged layer around the target in both directions', async () => {
+      const { session } = makeSession()
+      await session.loadImages(['a.png', 'b.png', 'c.png'], ['A', 'B', 'C'])
+      const names = () => session.imageLayers.value.map((n) => n.name)
+      const idOf = (name: string) =>
+        session.imageLayers.value.find((n) => n.name === name)!.id
+      const drop = (
+        dragged: string,
+        target: string,
+        pos: 'above' | 'below'
+      ) => {
+        const toIndex = reorderDropIndex(
+          session.imageLayers.value.map((n) => n.id),
+          idOf(target),
+          pos,
+          1
+        )
+        session.moveLayerTo(idOf(dragged), toIndex!)
+      }
+
+      drop('A', 'C', 'above')
+      expect(names()).toEqual(['B', 'C', 'A'])
+
+      drop('A', 'B', 'below')
+      expect(names()).toEqual(['A', 'B', 'C'])
+
+      drop('C', 'B', 'below')
+      expect(names()).toEqual(['A', 'C', 'B'])
+
+      drop('A', 'B', 'below')
+      expect(names()).toEqual(['C', 'A', 'B'])
+    })
+
     it('moveLayerTo refuses to drop below the background fill', async () => {
       const { session } = await loadedSession()
       const [a] = session.imageLayers.value

src/renderer/extensions/layerEditor/engine/history.test.ts

@@ -215,6 +215,19 @@ describe('History — dirty tracking and eviction', () => {
     expect(h.labels().undo).toHaveLength(3)
   })
 
+  it('a branch that discards the save point stays dirty', () => {
+    const h = new History()
+    h.push(new TestCommand('a'))
+    h.markSaved()
+    h.undo()
+    h.push(new TestCommand('b'))
+    expect(h.dirty()).toBe(true)
+    h.undo()
+    expect(h.dirty()).toBe(true)
+    h.markSaved()
+    expect(h.dirty()).toBe(false)
+  })
+
   it('an evicted history stays dirty until the next markSaved', () => {
     const h = new History({ byteBudget: 1, minSteps: 1 })
     h.push(new TestCommand('a', Dirty.DRAWABLE, 64))

src/renderer/extensions/layerEditor/engine/history.ts

@@ -115,6 +115,9 @@ export class History {
     this.undoStack.push(cmd)
     this.sizes.set(cmd, cmd.sizeBytes())
     this.undoBytes += this.sizes.get(cmd) ?? 0
+    if (this.redoStack.length > 0 && this.dirtyCount < 0) {
+      this.cleanReachable = false
+    }
     this.redoStack = []
     this.bumpDirty()
     this.evict()

src/renderer/extensions/layerEditor/engine/render/renderStack.test.ts

@@ -171,6 +171,26 @@ describe('renderDocument', () => {
     expect(c.freed).toEqual([c.allocated[0].id])
   })
 
+  it('frees every allocated target when a group composite throws mid-build', () => {
+    class ExplodingCompositor extends FakeCompositor {
+      targetComposites = 0
+      override composite(
+        inputs: CompositeInput[],
+        target?: FBOHandle | null,
+        region?: Rect
+      ) {
+        if (target && ++this.targetComposites === 2) throw new Error('boom')
+        super.composite(inputs, target, region)
+      }
+    }
+    const c = new ExplodingCompositor()
+    const first = group([leaf(1)], { id: 'g1' })
+    const second = group([leaf(0.9)], { id: 'g2' })
+    expect(() => renderDocument(doc([first, second]), deps(c))).toThrow('boom')
+    expect(c.allocated).toHaveLength(2)
+    expect([...c.freed].sort()).toEqual(c.allocated.map((h) => h.id).sort())
+  })
+
   it('splices a pass-through group directly into the parent stack (no isolation target)', () => {
     const c = new FakeCompositor()
     const g = group([leaf(1), leaf(1)], { passThrough: true })

src/renderer/extensions/layerEditor/engine/render/renderStack.ts

@@ -227,41 +227,52 @@ function buildInputs(
     devicePixelRatio: deps.devicePixelRatio ?? 1
   }
 
-  for (const node of group.children) {
-    if (!node.visible || node.opacity <= 0) continue
+  try {
+    for (const node of group.children) {
+      if (!node.visible || node.opacity <= 0) continue
 
-    if (node.kind === 'group') {
-      const g = node as GroupData
-      const sub = buildInputs(g, doc, deps, used)
-      if (g.passThrough) {
-        inputs.push(...sub.inputs)
-        cleanups.push(sub.cleanup)
+      if (node.kind === 'group') {
+        const g = node as GroupData
+        const sub = buildInputs(g, doc, deps, used)
+        if (g.passThrough) {
+          inputs.push(...sub.inputs)
+          cleanups.push(sub.cleanup)
+          continue
+        }
+        const handle = deps.compositor.allocTarget(doc.width, doc.height)
+        try {
+          deps.compositor.composite(sub.inputs, handle)
+        } catch (err) {
+          deps.compositor.freeTarget(handle)
+          sub.cleanup()
+          throw err
+        }
+        sub.cleanup()
+        cleanups.push(() => deps.compositor.freeTarget(handle))
+        const groupTexture = deps.compositor.targetTexture(handle)
+        if (groupTexture) {
+          inputs.push({
+            texture: { source: groupTexture, rect: region, linear: true },
+            opacity: node.opacity,
+            mode: resolveMode(node.mode),
+            mask: renderMaskTexture(node, region, deps, placed, used)
+          })
+        }
         continue
       }
-      const handle = deps.compositor.allocTarget(doc.width, doc.height)
-      deps.compositor.composite(sub.inputs, handle)
-      sub.cleanup()
-      cleanups.push(() => deps.compositor.freeTarget(handle))
-      const groupTexture = deps.compositor.targetTexture(handle)
-      if (groupTexture) {
-        inputs.push({
-          texture: { source: groupTexture, rect: region, linear: true },
-          opacity: node.opacity,
-          mode: resolveMode(node.mode),
-          mask: renderMaskTexture(node, region, deps, placed, used)
-        })
-      }
-      continue
-    }
 
-    const texture = renderLeafTexture(node, ctx, deps, used)
-    if (!texture) continue
-    inputs.push({
-      texture,
-      opacity: node.opacity,
-      mode: resolveMode(node.mode),
-      mask: renderMaskTexture(node, region, deps, placed, used)
-    })
+      const texture = renderLeafTexture(node, ctx, deps, used)
+      if (!texture) continue
+      inputs.push({
+        texture,
+        opacity: node.opacity,
+        mode: resolveMode(node.mode),
+        mask: renderMaskTexture(node, region, deps, placed, used)
+      })
+    }
+  } catch (err) {
+    cleanups.forEach((fn) => fn())
+    throw err
   }
 
   return { inputs, cleanup: () => cleanups.forEach((fn) => fn()) }