Address Gadfly review on #9: memoize shapes, fades, unique ids, robustness

Fixes from the PR #29 adversarial review (considered; not graded).

Performance / correctness
- ObjectShape is memo'd, so a pan/zoom (which only mutates the world <g>
  transform) no longer re-renders every object.
- 'circle' objects render as an <ellipse> (rx/ry), honoring both dims — a
  circle when width == height — instead of ignoring heightCm.
- The 1 m grid now actually fades in (opacity ramps as cells grow past the
  visibility threshold), matching its description.
- Unique SVG pattern id (useId) so multiple canvases can't collide.
- The canvas re-fits when the garden id changes (not just once), so #11
  switching gardens reframes.

Robustness
- geometry.zoomToFitRect takes rect size by magnitude; wheel/pinch ignore
  non-finite deltas; ObjectShape clamps dimensions to >= 0 (no invalid SVG).

Maintainability
- Renamed the Size params from `viewport` to `canvasSize` (4 models); moved
  easeInOutCubic/lerp into geometry.ts; renamed toLocal → clientToCanvas;
  named constants for the label/corner factors; DEFAULT_FILL const;
  EditorGarden.unitPref imports UnitPref; the Fit control uses the shared
  Button. focusedObjectId/plantable are intentionally reserved for #11/#15.

tsc + 17 vitest tests green; re-verified in a browser (ellipses render,
unique pattern id, wheel zoom + Fit).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01JdQpdYYsTgtkJBxbcpAszi

web/src/editor/GardenCanvas.tsx

@@ -1,4 +1,5 @@
-import { useEffect, useMemo, useRef, useState } from 'react'
+import { useEffect, useId, useMemo, useRef, useState } from 'react'
+import { Button } from '@/components/ui/Button'
 import type { Size } from '@/lib/geometry'
 import { ObjectShape } from './ObjectShape'
 import { useEditorStore } from './store'
@@ -6,19 +7,21 @@ import { useViewport } from './useViewport'
 import type { EditorGarden, EditorObject } from './types'
 
 const GRID_CM = 100 // 1 m grid
-const GRID_MIN_CELL_PX = 6 // hide the grid when cells get this small (zoomed out)
+const GRID_MIN_CELL_PX = 6 // grid is invisible below this cell size (zoomed out)
+const GRID_MAX_OPACITY = 0.18
 
 /**
  * The editor canvas: a full-size SVG with a single world <g> that pan/zoom/pinch
- * transforms. Renders a fading 1 m grid, the garden boundary, and its objects.
+ * transforms. Renders a 1 m grid that fades in as you zoom, the garden boundary,
- * Interactions beyond viewport + basic select (move/resize/rotate, palette) land
+ * and its objects. Interactions beyond viewport + basic select (move/resize/
- * in #11; this is the foundation, driven by mock data until then.
+ * rotate, palette) land in #11; this is the foundation, driven by mock data.
  */
 export function GardenCanvas({ garden, objects }: { garden: EditorGarden; objects: EditorObject[] }) {
   const svgRef = useRef<SVGSVGElement>(null)
   const containerRef = useRef<HTMLDivElement>(null)
   const [size, setSize] = useState<Size>({ w: 0, h: 0 })
-  const fittedRef = useRef(false)
+  const fittedForRef = useRef<number | null>(null)
+  const gridId = 'garden-grid-' + useId().replace(/:/g, '')
 
   const viewport = useEditorStore((s) => s.viewport)
   const selectedId = useEditorStore((s) => s.selectedId)
@@ -41,15 +44,20 @@ export function GardenCanvas({ garden, objects }: { garden: EditorGarden; object
     return () => ro.disconnect()
   }, [])
 
-  // Frame the garden the first time we know the canvas size.
+  // Frame the garden once the canvas size is known, and again if we switch to a
+  // different garden.
   useEffect(() => {
-    if (!fittedRef.current && size.w > 0 && size.h > 0) {
+    const usable = size.w > 0 && size.h > 0 && garden.widthCm > 0 && garden.heightCm > 0
-      fittedRef.current = true
+    if (usable && fittedForRef.current !== garden.id) {
+      fittedForRef.current = garden.id
       fitToRect(gardenRect, size)
     }
-  }, [size, gardenRect, fitToRect])
+  }, [size, garden.id, garden.widthCm, garden.heightCm, gardenRect, fitToRect])
+
+  // Grid fades in as cells grow from GRID_MIN_CELL_PX to 2× that.
+  const cellPx = GRID_CM * viewport.scale
+  const gridOpacity = Math.max(0, Math.min(GRID_MAX_OPACITY, ((cellPx - GRID_MIN_CELL_PX) / GRID_MIN_CELL_PX) * GRID_MAX_OPACITY))
 
-  const showGrid = GRID_CM * viewport.scale >= GRID_MIN_CELL_PX
   const ordered = useMemo(() => [...objects].sort((a, b) => a.zIndex - b.zIndex), [objects])
 
   return (
@@ -63,21 +71,21 @@ export function GardenCanvas({ garden, objects }: { garden: EditorGarden; object
         onPointerDown={() => select(null)}
       >
         <g transform={`translate(${viewport.tx} ${viewport.ty}) scale(${viewport.scale})`}>
-          {showGrid && (
+          {gridOpacity > 0.005 && (
             <>
               <defs>
-                <pattern id="garden-grid" width={GRID_CM} height={GRID_CM} patternUnits="userSpaceOnUse">
+                <pattern id={gridId} width={GRID_CM} height={GRID_CM} patternUnits="userSpaceOnUse">
                   <path
                     d={`M ${GRID_CM} 0 L 0 0 0 ${GRID_CM}`}
                     fill="none"
                     stroke="#808080"
-                    strokeOpacity={0.18}
+                    strokeOpacity={gridOpacity}
                     strokeWidth={1}
                     vectorEffect="non-scaling-stroke"
                   />
                 </pattern>
               </defs>
-              <rect x={0} y={0} width={garden.widthCm} height={garden.heightCm} fill="url(#garden-grid)" />
+              <rect x={0} y={0} width={garden.widthCm} height={garden.heightCm} fill={`url(#${gridId})`} />
             </>
           )}
 
@@ -104,15 +112,13 @@ export function GardenCanvas({ garden, objects }: { garden: EditorGarden; object
       <div className="pointer-events-none absolute left-3 top-3 rounded-md bg-surface/80 px-2 py-1 text-xs text-muted backdrop-blur">
         {viewport.scale.toFixed(2)} px/cm
       </div>
-      <div className="absolute bottom-3 right-3">
+      <Button
-        <button
+        variant="ghost"
-          type="button"
+        onClick={() => size.w > 0 && fitToRect(gardenRect, size)}
-          onClick={() => size.w > 0 && fitToRect(gardenRect, size)}
+        className="absolute bottom-3 right-3 bg-surface/90 py-1.5 shadow-sm backdrop-blur"
-          className="rounded-md border border-border bg-surface/90 px-3 py-1.5 text-sm font-medium text-fg shadow-sm outline-none backdrop-blur transition-colors hover:bg-border/50 focus-visible:ring-2 focus-visible:ring-accent/40"
+      >
-        >
+        Fit
-          Fit
+      </Button>
-        </button>
-      </div>
     </div>
   )
 }

web/src/editor/ObjectShape.tsx

@@ -1,6 +1,8 @@
-import type { PointerEvent } from 'react'
+import { memo, type PointerEvent } from 'react'
 import type { EditorObject } from './types'
 
+const DEFAULT_FILL = '#8a8a8a'
+
 // Default fills by kind (overridable per object via object.color). Muted, earthy
 // tones so plops (added in #15) read clearly on top.
 const kindColors: Record<string, string> = {
@@ -10,21 +12,29 @@ const kindColors: Record<string, string> = {
   in_ground: '#7a6a4a',
   tree: '#4f7a4f',
   path: '#b8b0a0',
-  structure: '#8a8a8a',
+  structure: DEFAULT_FILL,
 }
 
+// Label font size = this fraction of the object's smaller side, clamped to a
+// readable cm range. Corner radius = this fraction of the smaller half-side.
+const LABEL_FONT_FACTOR = 0.28
+const LABEL_FONT_MIN_CM = 8
+const LABEL_FONT_MAX_CM = 40
+const CORNER_RADIUS_FACTOR = 0.06
+
 function fillFor(o: EditorObject): string {
-  return o.color ?? kindColors[o.kind] ?? '#8a8a8a'
+  return o.color ?? kindColors[o.kind] ?? DEFAULT_FILL
 }
 
 /**
- * One garden object in world (cm) space: a centered rect or circle, rotated
+ * One garden object in world (cm) space: a centered rect or ellipse (a circle
- * about its center, with the object's name. The parent world <g> applies the
+ * when width == height), rotated about its center, with the object's name. The
- * viewport scale, so geometry is authored in cm; strokes use
+ * parent world <g> applies the viewport scale, so geometry is authored in cm and
- * vector-effect=non-scaling-stroke to stay a constant pixel width at any zoom.
+ * strokes use vector-effect=non-scaling-stroke to stay a constant pixel width at
- * Full move/resize/rotate interactions come in #11.
+ * any zoom. memo'd so a pan/zoom (which only changes the world <g> transform)
+ * doesn't re-render every object. Full move/resize/rotate come in #11.
  */
-export function ObjectShape({
+export const ObjectShape = memo(function ObjectShape({
   object,
   selected,
   onSelect,
@@ -34,16 +44,21 @@ export function ObjectShape({
   onSelect: (id: number) => void
 }) {
   const fill = fillFor(object)
-  const halfW = object.widthCm / 2
+  const halfW = Math.max(0, object.widthCm / 2)
-  const halfH = object.heightCm / 2
+  const halfH = Math.max(0, object.heightCm / 2)
-  // Label size scales with the object but is clamped to a sensible cm range.
+  const fontCm = Math.max(
-  const fontCm = Math.max(8, Math.min(40, Math.min(object.widthCm, object.heightCm) * 0.28))
+    LABEL_FONT_MIN_CM,
+    Math.min(LABEL_FONT_MAX_CM, Math.min(object.widthCm, object.heightCm) * LABEL_FONT_FACTOR),
+  )
 
   function handleDown(e: PointerEvent) {
     e.stopPropagation() // don't let the canvas treat this as an empty-space pan/deselect
     onSelect(object.id)
   }
 
+  const stroke = selected ? '#2f7a3e' : '#00000033'
+  const strokeWidth = selected ? 2 : 1
+
   return (
     <g
       transform={`translate(${object.xCm} ${object.yCm}) rotate(${object.rotationDeg})`}
@@ -51,27 +66,28 @@ export function ObjectShape({
       style={{ cursor: 'pointer' }}
     >
       {object.shape === 'circle' ? (
-        <circle
+        <ellipse
           cx={0}
           cy={0}
-          r={halfW}
+          rx={halfW}
+          ry={halfH}
           fill={fill}
           fillOpacity={0.85}
-          stroke={selected ? '#2f7a3e' : '#00000033'}
+          stroke={stroke}
-          strokeWidth={selected ? 2 : 1}
+          strokeWidth={strokeWidth}
           vectorEffect="non-scaling-stroke"
         />
       ) : (
         <rect
           x={-halfW}
           y={-halfH}
-          width={object.widthCm}
+          width={halfW * 2}
-          height={object.heightCm}
+          height={halfH * 2}
-          rx={Math.min(halfW, halfH) * 0.06}
+          rx={Math.min(halfW, halfH) * CORNER_RADIUS_FACTOR}
           fill={fill}
           fillOpacity={0.85}
-          stroke={selected ? '#2f7a3e' : '#00000033'}
+          stroke={stroke}
-          strokeWidth={selected ? 2 : 1}
+          strokeWidth={strokeWidth}
           vectorEffect="non-scaling-stroke"
         />
       )}
@@ -91,4 +107,4 @@ export function ObjectShape({
       )}
     </g>
   )
-}
+})

web/src/editor/types.ts

@@ -1,6 +1,8 @@
 // The object shape the canvas renders. A subset of the server's garden object
 // (#10); #11 maps /full's objects onto this. Positioned by center in garden cm.
 
+import type { UnitPref } from '@/lib/units'
+
 export type ObjectShapeKind = 'rect' | 'circle'
 
 export interface EditorObject {
@@ -24,5 +26,5 @@ export interface EditorGarden {
   name: string
   widthCm: number
   heightCm: number
-  unitPref: 'metric' | 'imperial'
+  unitPref: UnitPref
 }

web/src/editor/useViewport.ts

@@ -1,15 +1,20 @@
 import { useCallback, useEffect, useRef, type RefObject } from 'react'
 import { useGesture } from '@use-gesture/react'
-import { zoomToFitRect, zoomViewportAt, type Point, type Rect, type Size } from '@/lib/geometry'
+import {
+  easeInOutCubic,
+  lerp,
+  zoomToFitRect,
+  zoomViewportAt,
+  type Point,
+  type Rect,
+  type Size,
+} from '@/lib/geometry'
 import { MAX_SCALE, MIN_SCALE, useEditorStore } from './store'
 
 const WHEEL_SENSITIVITY = 0.0015 // exponential zoom per wheel delta unit
 const FIT_PADDING = 32 // px margin around a fitted rect
 const FIT_DURATION_MS = 350
 
-const easeInOutCubic = (t: number) => (t < 0.5 ? 4 * t * t * t : 1 - Math.pow(-2 * t + 2, 3) / 2)
-const lerp = (a: number, b: number, t: number) => a + (b - a) * t
-
 /**
  * Wires pan/zoom/pinch gestures onto the svg (via @use-gesture) and returns an
  * animated `fitToRect`. Wheel and pinch zoom toward the pointer; drag on empty
@@ -28,7 +33,8 @@ export function useViewport(svgRef: RefObject<SVGSVGElement | null>) {
     }
   }, [])
 
-  const toLocal = useCallback(
+  // Client (page) coords → coords within the svg, for anchoring zoom.
+  const clientToCanvas = useCallback(
     (clientX: number, clientY: number): Point => {
       const rect = svgRef.current?.getBoundingClientRect()
       return rect ? { x: clientX - rect.left, y: clientY - rect.top } : { x: clientX, y: clientY }
@@ -49,12 +55,14 @@ export function useViewport(svgRef: RefObject<SVGSVGElement | null>) {
       onWheelStart: cancelAnim,
       onWheel: ({ event, delta: [, dy] }) => {
         event.preventDefault()
-        const p = toLocal(event.clientX, event.clientY)
+        if (!Number.isFinite(dy)) return // never let a bad delta corrupt the viewport
+        const p = clientToCanvas(event.clientX, event.clientY)
         setViewport((vp) => zoomViewportAt(vp, p, vp.scale * Math.exp(-dy * WHEEL_SENSITIVITY), MIN_SCALE, MAX_SCALE))
       },
       onPinchStart: cancelAnim,
       onPinch: ({ origin: [ox, oy], offset: [scale] }) => {
-        const p = toLocal(ox, oy)
+        if (!Number.isFinite(scale)) return
+        const p = clientToCanvas(ox, oy)
         setViewport((vp) => zoomViewportAt(vp, p, scale, MIN_SCALE, MAX_SCALE))
       },
     },
@@ -71,10 +79,10 @@ export function useViewport(svgRef: RefObject<SVGSVGElement | null>) {
   )
 
   const fitToRect = useCallback(
-    (rect: Rect, viewport: Size) => {
+    (rect: Rect, canvasSize: Size) => {
       cancelAnim()
       const from = useEditorStore.getState().viewport
-      const to = zoomToFitRect(rect, viewport, FIT_PADDING, MIN_SCALE, MAX_SCALE)
+      const to = zoomToFitRect(rect, canvasSize, FIT_PADDING, MIN_SCALE, MAX_SCALE)
       const start = performance.now()
       const step = (now: number) => {
         const t = Math.min(1, (now - start) / FIT_DURATION_MS)

web/src/lib/geometry.ts

@@ -68,6 +68,16 @@ export function clampScale(scale: number, min: number, max: number): number {
   return Math.min(max, Math.max(min, scale))
 }
 
+/** Linear interpolation. */
+export function lerp(a: number, b: number, t: number): number {
+  return a + (b - a) * t
+}
+
+/** Cubic ease-in-out for viewport tweens. */
+export function easeInOutCubic(t: number): number {
+  return t < 0.5 ? 4 * t * t * t : 1 - Math.pow(-2 * t + 2, 3) / 2
+}
+
 /**
  * Re-scale the viewport while keeping the world point currently under
  * `screenPoint` stationary — the math behind wheel-zoom-to-cursor and pinch. The
@@ -90,28 +100,29 @@ export function zoomViewportAt(
 }
 
 /**
- * The viewport that frames `rect` centered in a viewport of `viewport` pixels,
+ * The viewport that frames `rect` centered in a canvas of `canvasSize` pixels,
  * with `padding` px of margin on every side. Scale is clamped to [minScale,
- * maxScale] so a tiny/huge rect still lands in a sane zoom.
+ * maxScale] so a tiny/huge rect still lands in a sane zoom. Rect dimensions are
+ * taken by magnitude, so a degenerate (0/negative) size can't corrupt the math.
  */
 export function zoomToFitRect(
   rect: Rect,
-  viewport: Size,
+  canvasSize: Size,
   padding: number,
   minScale: number,
   maxScale: number,
 ): Viewport {
-  const availW = Math.max(1, viewport.w - padding * 2)
+  const availW = Math.max(1, canvasSize.w - padding * 2)
-  const availH = Math.max(1, viewport.h - padding * 2)
+  const availH = Math.max(1, canvasSize.h - padding * 2)
-  const rectW = Math.max(rect.w, 1e-6)
+  const rectW = Math.max(Math.abs(rect.w), 1e-6)
-  const rectH = Math.max(rect.h, 1e-6)
+  const rectH = Math.max(Math.abs(rect.h), 1e-6)
   const scale = clampScale(Math.min(availW / rectW, availH / rectH), minScale, maxScale)
 
   const centerX = rect.x + rect.w / 2
   const centerY = rect.y + rect.h / 2
   return {
     scale,
-    tx: viewport.w / 2 - centerX * scale,
+    tx: canvasSize.w / 2 - centerX * scale,
-    ty: viewport.h / 2 - centerY * scale,
+    ty: canvasSize.h / 2 - centerY * scale,
   }
 }