fix: address the multi-agent review on the layer editor UX batch

Author: jtydhr88

src/renderer/extensions/layerEditor/components/LayerPanel.vue

@@ -8,7 +8,7 @@
       </span>
     </div>
 
-    <div class="min-h-0 flex-1 overflow-y-auto">
+    <div class="min-h-0 flex-1 overflow-y-auto" @dragleave="onListDragLeave">
       <div
         v-for="node in rows"
         :key="node.id"
@@ -177,7 +177,7 @@ function isRowSelected(id: string): boolean {
 
 function rowClass(selected: boolean, extra?: string): string {
   return cn(
-    'group flex cursor-pointer items-center gap-2 border-border-default px-2 py-1.5 select-none',
+    'group relative flex cursor-pointer items-center gap-2 border-border-default px-2 py-1.5 select-none',
     'hover:bg-secondary-background-hover',
     selected && 'bg-secondary-background-selected',
     extra
@@ -208,20 +208,19 @@ function endDrag(): void {
 
 function dropHintClass(id: string): string {
   const hint = dropHint.value
-  if (hint?.id !== id) return 'relative'
-  return cn(
-    'relative',
-    hint.pos === 'above'
-      ? "before:absolute before:inset-x-0 before:top-0 before:h-0.5 before:bg-base-foreground before:content-['']"
-      : "after:absolute after:inset-x-0 after:bottom-0 after:h-0.5 after:bg-base-foreground after:content-['']"
-  )
+  if (hint?.id !== id) return ''
+  return hint.pos === 'above'
+    ? "before:absolute before:inset-x-0 before:top-0 before:h-0.5 before:bg-base-foreground before:content-['']"
+    : "after:absolute after:inset-x-0 after:bottom-0 after:h-0.5 after:bg-base-foreground after:content-['']"
 }
 
 function onRowDragStart(id: string, e: DragEvent): void {
   dragId.value = id
   if (e.dataTransfer) {
     e.dataTransfer.effectAllowed = 'move'
-    e.dataTransfer.setData('text/plain', id)
+    // Firefox refuses to start the drag without data; empty keeps text
+    // inputs from receiving the layer id on a stray drop.
+    e.dataTransfer.setData('text/plain', '')
   }
 }
 
@@ -237,21 +236,28 @@ function onRowDragOver(id: string, e: DragEvent): void {
 }
 
 function onRowDrop(id: string, e: DragEvent): void {
-  e.preventDefault()
   const dragged = dragId.value
   const hint = dropHint.value
-  if (dragged && hint?.id === id) {
-    const toIndex = reorderDropIndex(
-      imageLayers.value.map((n) => n.id),
-      id,
-      hint.pos,
-      backgroundLayer.value ? 1 : 0
-    )
-    if (toIndex !== null) session.moveLayerTo(dragged, toIndex)
+  if (!dragged || hint?.id !== id) {
+    endDrag()
+    return
   }
+  e.preventDefault()
+  const toIndex = reorderDropIndex(
+    imageLayers.value.map((n) => n.id),
+    id,
+    hint.pos,
+    backgroundLayer.value ? 1 : 0
+  )
+  if (toIndex !== null) session.moveLayerTo(dragged, toIndex)
   endDrag()
 }
 
+function onListDragLeave(e: DragEvent): void {
+  const container = e.currentTarget as HTMLElement
+  if (!container.contains(e.relatedTarget as Node | null)) dropHint.value = null
+}
+
 function onRowClick(id: string, e: MouseEvent): void {
   if (e.ctrlKey || e.metaKey || e.shiftKey) {
     const current = selectedNodeIds.value

src/renderer/extensions/layerEditor/composables/layerPanelDnd.ts

@@ -4,6 +4,12 @@ export function dropPositionFor(ratio: number): LayerDropPos {
   return ratio < 0.5 ? 'above' : 'below'
 }
 
+/**
+ * Target index in root.children for a panel drag-drop.
+ * @param bottomUpIds ids ordered z=0 (bottom) first - the inverse of the
+ *   panel's display order, so visually "above" means a higher index.
+ * @param offset reserved bottom slots (1 when a background fill is pinned).
+ */
 export function reorderDropIndex(
   bottomUpIds: readonly string[],
   targetId: string,

src/renderer/extensions/layerEditor/engine/editor/pickOps.test.ts

@@ -79,6 +79,24 @@ describe('layerOpacityAt', () => {
     expect(layerOpacityAt(node, { x: 20, y: 100 }, content)).toBe(0)
     expect(layerOpacityAt(node, { x: 200, y: 100 }, content)).toBe(1)
   })
+  it('scales a group pick by the group opacity', () => {
+    const child = mkRaster('a', 0, 0, 100, 100)
+    const content = fakeContent({ a: fakeCanvas(10, 10, () => 1) })
+    const group = {
+      id: 'g',
+      kind: 'group',
+      name: 'g',
+      visible: true,
+      opacity: 0.2,
+      mode: {},
+      transform: { x: 0, y: 0, w: 100, h: 100, rotation: 0 },
+      locks: { content: false, position: false, visibility: false },
+      children: [child],
+      passThrough: false
+    } as unknown as SceneNode
+    expect(layerOpacityAt(group, { x: 50, y: 50 }, content)).toBeCloseTo(0.2)
+    expect(pickLayerAt([group], { x: 50, y: 50 }, content)).toBeNull()
+  })
   it('returns 0 outside the transform and for invisible layers', () => {
     const node = mkRaster('a', 0, 0, 100, 100)
     const content = fakeContent({ a: fakeCanvas(10, 10, () => 1) })

src/renderer/extensions/layerEditor/engine/editor/pickOps.ts

@@ -10,24 +10,16 @@ export type AlphaSampler = (
   y: number
 ) => number
 
-const sampleCache = new WeakMap<
-  HTMLCanvasElement,
-  CanvasRenderingContext2D | null
->()
-
 function defaultAlphaSampler(
   canvas: HTMLCanvasElement,
   x: number,
   y: number
 ): number {
-  let ctx = sampleCache.get(canvas)
-  if (ctx === undefined) {
-    try {
-      ctx = canvas.getContext('2d', { willReadFrequently: true })
-    } catch {
-      ctx = null
-    }
-    sampleCache.set(canvas, ctx)
+  let ctx: CanvasRenderingContext2D | null
+  try {
+    ctx = canvas.getContext('2d', { willReadFrequently: true })
+  } catch {
+    ctx = null
   }
   if (!ctx) return 1
   try {
@@ -78,7 +70,7 @@ export function layerOpacityAt(
         best = Math.max(best, layerOpacityAt(child, pt, content, sample))
         if (best >= 1) break
       }
-      return best
+      return best * node.opacity
     }
     case 'raster':
       return rasterAlphaAt(node as RasterData, pt, content, sample)

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

@@ -173,6 +173,18 @@ describe('History — merging', () => {
     expect(h.dirty()).toBe(false)
   })
 
+  it('redo after undoing a post-save merge reads dirty again', () => {
+    const h = new History()
+    h.push(new MergingCommand('opacity'))
+    h.push(new MergingCommand('opacity'))
+    h.markSaved()
+    h.push(new MergingCommand('opacity'))
+    h.undo()
+    expect(h.dirty()).toBe(false)
+    h.redo()
+    expect(h.dirty()).toBe(true)
+  })
+
   it('never merges across a save point, so the new edit stays undoable to clean', () => {
     const h = new History()
     const first = new MergingCommand('opacity')

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

@@ -105,6 +105,7 @@ export class History {
     ) {
       this.undoBytes += top.sizeBytes() - (this.sizes.get(top) ?? 0)
       this.sizes.set(top, top.sizeBytes())
+      // Merged edits share the first commit's dirty step - no bumpDirty() here.
       this.emit(cmd.dirtyMask)
       return
     }
@@ -194,6 +195,7 @@ export class History {
     ) {
       const dropped = this.undoStack.shift()
       if (!dropped) break
+      if (dropped === this.mergeBarrier) this.mergeBarrier = null
       this.undoBytes -= this.sizes.get(dropped) ?? 0
       this.cleanReachable = false
     }

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

@@ -191,6 +191,24 @@ describe('renderDocument', () => {
     expect([...c.freed].sort()).toEqual(c.allocated.map((h) => h.id).sort())
   })
 
+  it('frees nested targets when a later allocTarget throws', () => {
+    class RationedCompositor extends FakeCompositor {
+      allocs = 0
+      override allocTarget(width: number, height: number): FBOHandle {
+        if (++this.allocs === 2) throw new Error('out of targets')
+        return super.allocTarget(width, height)
+      }
+    }
+    const c = new RationedCompositor()
+    const inner = group([leaf(1)], { id: 'inner' })
+    const outer = group([inner], { id: 'outer' })
+    expect(() => renderDocument(doc([outer]), deps(c))).toThrow(
+      'out of targets'
+    )
+    expect(c.allocated).toHaveLength(1)
+    expect(c.freed).toEqual([c.allocated[0].id])
+  })
+
   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

@@ -1,4 +1,9 @@
-import type { Compositor, CompositeInput, NodeTexture } from '../compositor'
+import type {
+  Compositor,
+  CompositeInput,
+  FBOHandle,
+  NodeTexture
+} from '../compositor'
 import type { ContentStore } from '../content'
 import type { Document } from '../document'
 import { resolveMode } from '../mode'
@@ -239,7 +244,13 @@ function buildInputs(
           cleanups.push(sub.cleanup)
           continue
         }
-        const handle = deps.compositor.allocTarget(doc.width, doc.height)
+        let handle: FBOHandle
+        try {
+          handle = deps.compositor.allocTarget(doc.width, doc.height)
+        } catch (err) {
+          sub.cleanup()
+          throw err
+        }
         try {
           deps.compositor.composite(sub.inputs, handle)
         } catch (err) {