Configurable grid + snap-to-grid in the layout system #45
@@ -166,7 +166,9 @@ func (s *Service) DeleteGarden(ctx context.Context, actorID, gardenID int64) err
|
||||
|
||||
// gardenFromInput validates and normalizes input into a domain.Garden (without
|
||||
// identity/ownership fields). With applyDefaults, absent dimensions/unit are
|
||||
// filled in (create); without it, every field must be supplied (update).
|
||||
// filled in (create); without it, dimensions and unit must be supplied (update).
|
||||
// The grid size is the exception: 0 always means "unset" and is defaulted on both
|
||||
// paths, so an older client that omits it can't wipe the column to an invalid 0.
|
||||
func gardenFromInput(in GardenInput, applyDefaults bool) (*domain.Garden, error) {
|
||||
name := strings.TrimSpace(in.Name)
|
||||
if name == "" || len(name) > maxGardenNameLen {
|
||||
|
||||
@@ -157,15 +157,14 @@ export function GardenCanvas({
|
||||
if (!rect) return
|
||||
const world = screenToWorld({ x: e.clientX - rect.left, y: e.clientY - rect.top }, viewport)
|
||||
const local = worldToLocal(world, { x: focusedObject.xCm, y: focusedObject.yCm }, focusedObject.rotationDeg)
|
||||
const halfW = focusedObject.widthCm / 2
|
||||
const halfH = focusedObject.heightCm / 2
|
||||
// Snap the plant to the bed's grid when the bed opts in; snapLocalToBedGrid
|
||||
// also clamps, so the plain clamp covers the non-snapping case.
|
||||
const placed = focusedObject.snapToGrid
|
||||
? snapLocalToBedGrid(local, focusedObject.gridSizeCm, halfW, halfH)
|
||||
: { x: Math.max(-halfW, Math.min(halfW, local.x)), y: Math.max(-halfH, Math.min(halfH, local.y)) }
|
||||
const x = placed.x
|
||||
const y = placed.y
|
||||
// snapLocalToBedGrid clamps to the bed either way; step 0 (snapping off) makes
|
||||
// it a pure clamp, so both paths go through one helper.
|
||||
const { x, y } = snapLocalToBedGrid(
|
||||
local,
|
||||
focusedObject.snapToGrid ? focusedObject.gridSizeCm : 0,
|
||||
|
|
||||
focusedObject.widthCm / 2,
|
||||
focusedObject.heightCm / 2,
|
||||
)
|
||||
const radiusCm = Math.max(1.5 * armedPlant.spacingCm, 15)
|
||||
// Stay armed for repeat-placement; don't select (the placement sheet covers
|
||||
// the object, so a selection would be hidden until placement ends anyway).
|
||||
@@ -178,9 +177,16 @@ export function GardenCanvas({
|
||||
// Draw the bed's own grid inside a focused plantable bed that has snapping on,
|
||||
// so the user sees exactly where plants will land. Lines are in the bed's local
|
||||
// frame (origin at center), anchored to the corner to match snapLocalToBedGrid.
|
||||
const showBedGrid = !!focusedObject && focusedObject.plantable && focusedObject.snapToGrid
|
||||
const bedVLines = showBedGrid ? bedGridLines(halfFW, focusedObject.gridSizeCm) : []
|
||||
const bedHLines = showBedGrid ? bedGridLines(halfFH, focusedObject.gridSizeCm) : []
|
||||
// Memoized so a high-frequency plop drag (which re-renders the canvas on every
|
||||
// pointermove) doesn't rebuild the line arrays each frame. null when the focused
|
||||
// object isn't a snapping plantable bed.
|
||||
|
gitea-actions
commented
🟠 bedGridLines arrays recreated on every render without memoization performance · flagged by 3 models
🪰 Gadfly · advisory 🟠 **bedGridLines arrays recreated on every render without memoization**
_performance · flagged by 3 models_
- `web/src/editor/GardenCanvas.tsx:182` — `bedVLines` and `bedHLines` are computed inline without `useMemo`, so `bedGridLines` allocates fresh arrays on every render. `GardenCanvas` subscribes to `livePlanting` (and `liveObject`), so it re-renders on every pointermove during drag; the bed grid line arrays are recreated and diffed even though the focused bed's dimensions and grid size haven't changed. For a 200-line cap this is bounded, but it's still unnecessary garbage and React diff work on a…
<sub>🪰 Gadfly · advisory</sub>
|
||||
const bedGrid = useMemo(() => {
|
||||
if (!focusedObject || !focusedObject.plantable || !focusedObject.snapToGrid) return null
|
||||
return {
|
||||
v: bedGridLines(focusedObject.widthCm / 2, focusedObject.gridSizeCm),
|
||||
h: bedGridLines(focusedObject.heightCm / 2, focusedObject.gridSizeCm),
|
||||
}
|
||||
}, [focusedObject])
|
||||
|
||||
return (
|
||||
<div ref={containerRef} className="relative h-full w-full overflow-hidden rounded-xl border border-border bg-bg">
|
||||
@@ -210,12 +216,12 @@ export function GardenCanvas({
|
||||
</g>
|
||||
))}
|
||||
|
||||
{showBedGrid && focusedObject && (
|
||||
{focusedObject && bedGrid && (
|
||||
<g transform={objectTransform(focusedObject)} pointerEvents="none">
|
||||
{bedVLines.map((x) => (
|
||||
{bedGrid.v.map((x) => (
|
||||
<line key={`v${x}`} x1={x} y1={-halfFH} x2={x} y2={halfFH} stroke="#3f8f4f" strokeOpacity={0.3} strokeWidth={1} vectorEffect="non-scaling-stroke" />
|
||||
))}
|
||||
{bedHLines.map((y) => (
|
||||
{bedGrid.h.map((y) => (
|
||||
<line key={`h${y}`} x1={-halfFW} y1={y} x2={halfFW} y2={y} stroke="#3f8f4f" strokeOpacity={0.3} strokeWidth={1} vectorEffect="non-scaling-stroke" />
|
||||
))}
|
||||
</g>
|
||||
|
||||
@@ -63,11 +63,6 @@ export function PlopOverlay({
|
||||
}
|
||||
}
|
||||
|
||||
const clampLocal = (p: Point): Point => ({
|
||||
x: Math.max(-halfW, Math.min(halfW, p.x)),
|
||||
y: Math.max(-halfH, Math.min(halfH, p.y)),
|
||||
})
|
||||
|
||||
const begin = (
|
||||
e: ReactPointerEvent,
|
||||
onMove: (e: PointerEvent) => EditorPlanting,
|
||||
@@ -110,8 +105,9 @@ export function PlopOverlay({
|
||||
(ev) => {
|
||||
const p = ptr(ev)
|
||||
const moved: Point = { x: base.xCm + (p.x - start.x), y: base.yCm + (p.y - start.y) }
|
||||
// snapLocalToBedGrid clamps too, so the plain clamp covers the non-snap case.
|
||||
const next = snap ? snapLocalToBedGrid(moved, gridCm, halfW, halfH) : clampLocal(moved)
|
||||
// snapLocalToBedGrid clamps either way; step 0 (snapping off) makes it a
|
||||
// pure clamp, so both paths go through one helper.
|
||||
const next = snapLocalToBedGrid(moved, snap ? gridCm : 0, halfW, halfH)
|
||||
return { ...base, xCm: next.x, yCm: next.y }
|
||||
},
|
||||
(f) => ({ id: base.id, version: base.version, xCm: f.xCm, yCm: f.yCm }),
|
||||
|
||||
@@ -91,12 +91,12 @@ export function snapPoint(p: Point, step: number): Point {
|
||||
* clamped, not snapped.
|
||||
*/
|
||||
export function snapLocalToBedGrid(p: Point, step: number, halfW: number, halfH: number): Point {
|
||||
|
gitea-actions
commented
🟡 snapLocalToBedGrid duplicates the anchor-snap formula per axis instead of reusing snapValue-style factoring maintainability · flagged by 1 model
🪰 Gadfly · advisory 🟡 **snapLocalToBedGrid duplicates the anchor-snap formula per axis instead of reusing snapValue-style factoring**
_maintainability · flagged by 1 model_
- **`web/src/lib/geometry.ts:93` (`snapLocalToBedGrid`)** — The inline ternary `step > 0 ? -halfW + Math.round((p.x + halfW) / step) * step : p.x` interleaves snapping and passthrough per axis, duplicating the anchored-snap formula across both axes. `snapValue`/`snapPoint` already exist for the origin-anchored case; a `snapToCorner(v, origin, step)` helper (or `snapValue(p.x + halfW, step) - halfW`) would deduplicate and match the existing "anchor the snap, then clamp" shape. Verified by reading…
<sub>🪰 Gadfly · advisory</sub>
|
||||
const sx = step > 0 ? -halfW + Math.round((p.x + halfW) / step) * step : p.x
|
||||
const sy = step > 0 ? -halfH + Math.round((p.y + halfH) / step) * step : p.y
|
||||
return {
|
||||
x: Math.max(-halfW, Math.min(halfW, sx)),
|
||||
y: Math.max(-halfH, Math.min(halfH, sy)),
|
||||
}
|
||||
// Snap one axis to the grid anchored at -half (the bed's near edge), then clamp
|
||||
// to [-half, half]. step<=0 skips the snap, so callers can pass step 0 to get a
|
||||
// pure clamp — the non-snapping placement/move path — through the same helper.
|
||||
const snapAxis = (v: number, half: number) =>
|
||||
Math.max(-half, Math.min(half, step > 0 ? -half + Math.round((v + half) / step) * step : v))
|
||||
return { x: snapAxis(p.x, halfW), y: snapAxis(p.y, halfH) }
|
||||
}
|
||||
|
||||
/** Linear interpolation. */
|
||||
|
||||
Reference in New Issue
Block a user
🟠 Duplicated inline clamp logic for bed bounds
maintainability · flagged by 2 models
web/src/editor/GardenCanvas.tsx:166— The non-snapping branch inonPlacemanually inlines the same clamp logic that already exists inPlopOverlay.tsx'sclampLocalhelper and insidesnapLocalToBedGrid. Extract a sharedclampLocal(point, halfW, halfH)intogeometry.tsand use it in both overlays so bed-bounds clamping lives in one place and future changes (e.g., inset margins) only need one edit.🪰 Gadfly · advisory