Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,15 @@ jobs:
- name: Install Electron dependencies
run: npm ci

# Electron 42 dropped the postinstall binary download: `npm ci` now leaves
# node_modules/electron/dist empty and the ~100 MB fetch happens lazily on
# the first `require('electron')` — which, here, is inside the first spec
# that launches the app. A slow or failed download would surface as a
# mystery test timeout instead of an install failure, so pull it here where
# it can fail loudly on its own line.
- name: Install the Electron binary
run: npx install-electron

- name: Cache Playwright browsers
uses: actions/cache@v4
with:
Expand Down
2 changes: 2 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ Python deps are in `pyproject.toml`. Key non-PyPI deps from custom forks (check

Frontend deps (Electron, React, electron-vite, Playwright) are in `electron/package.json`.

**Electron is pinned EXACTLY, in `package.json` AND `electron/package.json`, and the two must agree.** This repo is an npm workspace, so `electron` hoists to the root and `electron/node_modules/electron` no longer exists; electron-builder then cannot read the installed version and falls back to parsing the spec, which it *refuses* if it is a range. That failure is release-only (nothing else runs electron-builder), so a widened range passes typecheck, unit and e2e and only breaks the tag build. Since Electron 42 there is also **no postinstall binary download** — `npm ci` leaves `node_modules/electron/dist` empty and the ~100 MB fetch happens lazily on the first `require('electron')`. CI pulls it explicitly (`npx install-electron`) so a failed download is an install error rather than a mystery test timeout.

Supported file extensions: `.hspy`, `.zspy`, `.mrc`, `.tif`, `.tiff`, `.de5`, `.csb` (see `SUPPORTED_EXTS` in `backend/_session_files.py`, re-exported from `session.py`). Adding one means updating that tuple **and** the Open-dialog `filters` in `electron/src/main/index.ts` (two places: the File menu and the `spyde:open-file` IPC handler) — the dialog will not offer an extension the tuple accepts.

## Architecture
Expand Down
2 changes: 1 addition & 1 deletion electron/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"@types/react": "^18.3.0",
"@types/react-dom": "^18.3.0",
"@vitejs/plugin-react": "^4.3.0",
"electron": "34.5.8",
"electron": "44.0.0",
"electron-builder": "^26.15.3",
"electron-vite": "^3.0.0",
"typescript": "^5.4.0",
Expand Down
22 changes: 17 additions & 5 deletions electron/src/main/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/
import {
app, BrowserWindow, dialog, ipcMain, Menu, shell, nativeTheme, net, protocol,
clipboard, nativeImage, powerMonitor,
clipboard, ClipboardItem, nativeImage, powerMonitor,
} from 'electron'
import { join, basename, resolve } from 'path'
import { pathToFileURL } from 'url'
Expand Down Expand Up @@ -228,9 +228,14 @@ function createWindow(): BrowserWindow {

// Tee renderer + figure-IFRAME console messages to THIS terminal so a JS error
// (or a [TILEDBG-JS] tile-render log) is visible without opening DevTools and
// switching frame context. level: 0=log 1=warning 2=error 3=info. We surface
// switching frame context. level: 0=verbose 1=info 2=warning 3=error. We surface
// warnings/errors always, and any message tagged [TILEDBG] so the tile diagnostics
// come through. `line`/`sourceId` pinpoint where a JS error was thrown.
//
// The positional arguments are deprecated in favour of the event object but are
// still emitted; the shell's own tee (packages/shell-main/src/window.ts) reads
// whichever shape it is handed, because when this goes wrong it fails SILENTLY —
// the tee just stops teeing.
win.webContents.on('console-message', (_e, level, message, line, sourceId) => {
const isTag = message.includes('[TILEDBG')
// A genuine JS error is level>=2 AND not one of our own [TILEDBG] warns (which
Expand Down Expand Up @@ -917,10 +922,11 @@ ipcMain.handle('report:export-pdf', async (_e, htmlPath: string, pdfPath: string
// Let images/fonts referenced by the report settle before rasterizing.
await new Promise((resolve) => setTimeout(resolve, 250))

// No `margins`: Electron 44 dropped `marginType`, and the 'default' it
// named is now what you get by omitting the key — 1cm on every side.
const pdfBuffer = await pdfWin!.webContents.printToPDF({
printBackground: true,
pageSize: 'A4',
margins: { marginType: 'default' },
})
writeFileSync(pdfPath, pdfBuffer)
return { ok: true as const }
Expand Down Expand Up @@ -951,7 +957,7 @@ const CLIPBOARD_PNG_MAX_DATA_URL_LEN = Math.ceil((CLIPBOARD_PNG_MAX_BYTES * 4) /
* images up front — `nativeImage.createFromDataURL` decodes synchronously on
* the main process's only thread, so an unbounded image would block the
* whole app (every window, every IPC reply) for however long the decode takes. */
ipcMain.handle('clipboard:write-png', (_e, dataUrl: string) => {
ipcMain.handle('clipboard:write-png', async (_e, dataUrl: string) => {
if (typeof dataUrl !== 'string' || !dataUrl.startsWith('data:image/png')) {
return { ok: false, error: 'expected a data:image/png URL' }
}
Expand All @@ -961,7 +967,13 @@ ipcMain.handle('clipboard:write-png', (_e, dataUrl: string) => {
try {
const image = nativeImage.createFromDataURL(dataUrl)
if (image.isEmpty()) return { ok: false, error: 'failed to decode PNG data URL' }
clipboard.writeImage(image)
// Electron 44 replaced writeImage with the W3C shape: MIME-typed items,
// async. The decode above still earns its place as the validity check —
// it is what rejects a well-formed URL carrying junk before the junk
// reaches the user's clipboard.
await clipboard.write([
new ClipboardItem({ 'image/png': new Blob([image.toPNG()], { type: 'image/png' }) }),
])
return { ok: true }
} catch (err) {
return { ok: false, error: (err as Error)?.message ?? String(err) }
Expand Down
40 changes: 38 additions & 2 deletions electron/src/renderer/src/components/FloatingToolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ const HIDDEN_ACTIONS = new Set(['Reset', 'Zoom In', 'Zoom Out'])

export const BAR_H = 38 // bar box height (30px buttons + 2×3px padding + border)
export const BAR_GAP = 6 // gap between the window's bottom edge and the bar
export type Rect = { x: number; y: number; w: number; h: number }

const CARET_GAP = 10 // gap between the bar/window edge and an open caret
type CaretPlacement = 'below' | 'right' | 'left'

Expand Down Expand Up @@ -96,6 +98,11 @@ interface Props {
/** True when the bar should sit INSIDE the window's bottom edge (maximized /
* no room below the window). */
inside?: boolean
/** The open caret's rect in MDI-area coords, or null when none is open, so
* MDIArea can keep a NEW window from being placed on top of it. Nothing
* about z-order changes: a window still comes to the front over the caret
* when it overlaps one, it just does not spawn there. */
onCaretRectChange?: (rect: Rect | null) => void
}

function defaultsOf(parameters: Record<string, ParamSpec>): Record<string, unknown> {
Expand All @@ -111,7 +118,7 @@ const hasPopout = (a: ToolbarAction) => hasParams(a) || hasSubs(a)

export function FloatingToolbar({
actions, windowId, onAction, visible = true, onHoverShow, onHoverHide,
winRect, areaSize, inside = false,
winRect, areaSize, inside = false, onCaretRectChange,
}: Props) {
const { state, sendAction } = useSpyDE()
const [openName, setOpenName] = React.useState<string | null>(null)
Expand Down Expand Up @@ -141,9 +148,25 @@ export function FloatingToolbar({
const area = areaSize ?? { w: 100000, h: 100000 }
// Held in a ref so the ResizeObserver below always runs the LATEST closure —
// `wr`/`area` change on every window move and resize.
// Last rect handed to MDIArea. Reported through a ref + a tolerance so the
// sub-pixel churn of a live drag doesn't re-render the whole MDI area.
const lastCaretRect = React.useRef<Rect | null>(null)
const reportCaretRect = (r: Rect | null) => {
const prev = lastCaretRect.current
if (!r && !prev) return
if (r && prev
&& Math.abs(r.x - prev.x) < 2 && Math.abs(r.y - prev.y) < 2
&& Math.abs(r.w - prev.w) < 2 && Math.abs(r.h - prev.h) < 2) return
lastCaretRect.current = r
onCaretRectChange?.(r)
}
// Report null on unmount too — a window closed with its caret open would
// otherwise leave a phantom no-go area behind.
React.useEffect(() => () => { if (lastCaretRect.current) onCaretRectChange?.(null) }, [])

const place = React.useRef<() => void>(() => {})
place.current = () => {
if (!openName) return
if (!openName) { reportCaretRect(null); return }
const el = caretWrapRef.current?.firstElementChild as HTMLElement | null
if (el) {
const r = el.getBoundingClientRect()
Expand All @@ -157,6 +180,19 @@ export function FloatingToolbar({
next = wr.x + wr.w + CARET_GAP + cw <= area.w ? 'right' : 'left'
}
setPlacement(p => (p === next ? p : next))
// Mirror the placement the CSS above produces, so the MDI area knows which
// patch of itself the caret occupies. Approximate for the side placements
// (they anchor near the window's top) — it only has to be good enough to
// steer a new window's first-fit search away, and erring large is safe.
reportCaretRect(
next === 'below'
? { x: Math.round(wr.x + wr.w / 2 - cw / 2), y: Math.round(belowTop),
w: Math.round(cw), h: Math.round(ch) }
: next === 'right'
? { x: Math.round(wr.x + wr.w + CARET_GAP), y: Math.round(wr.y),
w: Math.round(cw), h: Math.round(ch) }
: { x: Math.round(wr.x - CARET_GAP - cw), y: Math.round(wr.y),
w: Math.round(cw), h: Math.round(ch) })
// The WIDTH has to be state, not just the ref: the side placements clamp
// with it (see `caretPos`), and the ref is written in a layout effect. If
// the placement itself does not change there is no re-render, so the
Expand Down
18 changes: 18 additions & 0 deletions electron/src/renderer/src/components/MDIArea.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,19 @@ export function MDIArea() {
setActiveWindow(parseInt(id, 10))
}, [setActiveWindow])

// Where each window's OPEN caret currently sits, in area coords. A new
// window's first-fit search treats these as occupied, so an action's own
// result windows stop landing on the panel that launched them. Z-order is
// untouched: a window still comes to the front over a caret it overlaps —
// it just is not PLACED there. Kept in a ref because it only ever feeds the
// placement pass below, which already runs on every render; putting it in
// state would re-render the whole area on each caret resize for nothing.
const caretRectsRef = useRef<Map<string, Rect>>(new Map())
const setCaretRect = useCallback((id: string, rect: Rect | null) => {
if (rect) caretRectsRef.current.set(id, rect)
else caretRectsRef.current.delete(id)
}, [])

const getZ = (id: string) => {
// Unfocused windows sit at a low base; focused windows are always above,
// most-recently-focused highest. (A single focused window must beat
Expand Down Expand Up @@ -405,6 +418,10 @@ export function MDIArea() {
taken.push({ x: placed.x, y: placed.y, w, h })
placements.set(id, placed)
}
// Open carets are obstacles too. Without this a fit/DPC/strain run drops its
// own result window straight onto the caret that started it, and the window's
// figure iframe then swallows every click meant for the panel.
for (const caret of caretRectsRef.current.values()) taken.push(caret)
// Read the LIVE area size (the `areaSize` state can still be the default when
// the first windows arrive); `areaSize` just forces a re-render on resize.
const areaW = areaRef.current?.clientWidth || areaSize.w
Expand Down Expand Up @@ -488,6 +505,7 @@ export function MDIArea() {
onResize={handleResize}
onAction={handleAction}
zIndex={getZ(id)}
onCaretRectChange={(r: Rect | null) => setCaretRect(id, r)}
hidden={minimized.has(id)}
acceptSignalDrop={win.isNavigator}
onSignalDrop={(srcId) =>
Expand Down
6 changes: 5 additions & 1 deletion electron/src/renderer/src/components/SubWindow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ interface Props {
// Bumped by MDIArea's Tile action: a new `gen` forces this rect to be
// adopted even if the user had manually moved/resized the window.
forced?: { gen: number; rect: Rect }
/** Relayed from FloatingToolbar: where this window's open caret sits, so new
* windows are not PLACED on top of it. */
onCaretRectChange?: (rect: Rect | null) => void
}

export interface Rect { x: number; y: number; w: number; h: number }
Expand Down Expand Up @@ -135,7 +138,7 @@ export function SubWindow({
toolbarActions, onClose, onFocus, onMinimize, onResize, onAction,
zIndex, windowId, children, hidden = false,
acceptSignalDrop = false, onSignalDrop,
areaSize, otherRects, onLiveRect, forced,
areaSize, otherRects, onLiveRect, forced, onCaretRectChange,
}: Props) {
const [maximized, setMaximized] = useState(false)
const [dropHover, setDropHover] = useState(false)
Expand Down Expand Up @@ -395,6 +398,7 @@ export function SubWindow({
winRect={rect}
areaSize={{ w: areaW, h: areaH }}
inside={barInside}
onCaretRectChange={onCaretRectChange}
/>
)}

Expand Down
30 changes: 21 additions & 9 deletions electron/tests/_harness.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -513,19 +513,31 @@ function navWindows(page) {
* viz_main_impl.cc "Exiting GPU process", command_buffer_proxy_impl.cc)
* and is infrastructure noise, not a SpyDE error. Python backend lines
* never match that shape, so real errors still fail the audit.
* - "Failed to create WebGPU Context Provider" — Chromium emits this from the
* figure iframe whenever the runner has no usable WebGPU adapter (every
* hosted CI runner under xvfb). anyplotlib falls back to Canvas2D and the
* render is still correct (the GPU render math is covered separately in
* anyplotlib's own --enable-unsafe-webgpu suite), so it is benign here. A
* real backend error is a Python traceback, never this renderer line.
* - NO USABLE WebGPU ADAPTER, in either wording. Chromium emits this from the
* figure iframe on every hosted CI runner under xvfb. anyplotlib falls back
* to Canvas2D and the render is still correct (the GPU render math is
* covered separately in anyplotlib's own --enable-unsafe-webgpu suite), so
* it is benign here. A real backend error is a Python traceback, never this
* renderer line. The wording is version-dependent and has already changed
* once: Chromium 132 said "Failed to create WebGPU Context Provider",
* Chromium 152 says "No available adapters". Match BOTH — an Electron bump
* must not turn a missing GPU back into a fake backend error, which is
* exactly how the 34 -> 44 upgrade first reddened these audits.
*/
function backendErrorLines(backend) {
return backend.logBuffer.filter((l) =>
function backendErrorLines(backendOrLines) {
// Accepts the backend or a plain array of lines, so a spec holding its own
// snapshot can still use THIS filter instead of copying it. ipf_perf kept a
// copy and it went stale exactly as you would expect: it knew only the old
// `bus.cc(406)` shape and the old WebGPU wording, so the Electron 44 bump
// turned dbus noise back into "backend errors during IPF render".
const lines = Array.isArray(backendOrLines)
? backendOrLines
: backendOrLines.logBuffer
return lines.filter((l) =>
/ERROR|Traceback/i.test(l)
&& !/Security Warning|Content.Security.Policy|Content Security/i.test(l)
&& !/willReadFrequently/i.test(l)
&& !/Failed to create WebGPU Context Provider/i.test(l)
&& !/Failed to create WebGPU Context Provider|No available adapters/i.test(l)
// Both Chromium stderr shapes: older `bus.cc(405)` and the newer
// full-path colon form `dbus/bus.cc:405]` (format changed upstream, so an
// Electron bump must not silently turn infrastructure noise back into
Expand Down
96 changes: 96 additions & 0 deletions electron/tests/caret_placement.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/**
* caret_placement.spec.ts — a new window must not be PLACED on top of an open
* caret.
*
* Z-order is deliberately NOT the subject here. A window that overlaps a caret
* still comes to the front over it, and the caret stays open underneath — that
* is the intended behaviour and this spec must not be read as forbidding it.
* What it forbids is the app *choosing* to drop a brand-new window onto the
* panel the user is working in.
*
* Why it matters: `findFreeSlot` packs a new window into the first spot that
* collides with no existing WINDOW, and it used to know nothing about carets.
* So a fit / DPC / strain / orientation run would place its own result window
* squarely over the caret that launched it, and because a caret lives inside
* its window's stacking context (SubWindow's root is positioned WITH a
* z-index), it cannot paint above that new window whatever z-index it takes —
* measured: a caret at z-index 1002 lost to a window at 11. The result window's
* figure iframe then swallowed every click meant for the caret, which is what
* reddened ~18 e2e tests on the Electron 34 -> 44 upgrade.
*/
import { test, expect } from '@playwright/test'
const { launchApp, backendAction, waitForSubwindowCount } = require('./_harness.cjs')

let ctx: Awaited<ReturnType<typeof launchApp>>

test.setTimeout(180_000)

test.beforeAll(async () => {
ctx = await launchApp({ dask: false, env: { SPYDE_LOG_LEVEL: 'WARNING' } })
await backendAction(ctx.page, 'load_test_data_si_grains')
await waitForSubwindowCount(ctx.page, 2, 120_000)
})
test.afterAll(async () => { await ctx?.app?.close() })

/** Every subwindow's rect, in viewport coords. */
async function windowRects(page: any) {
return page.evaluate(() =>
[...document.querySelectorAll('[data-testid="subwindow"]')].map((el) => {
const r = el.getBoundingClientRect()
return { x: r.x, y: r.y, w: r.width, h: r.height }
}))
}

const overlapArea = (a: any, b: any) =>
Math.max(0, Math.min(a.x + a.w, b.x + b.w) - Math.max(a.x, b.x))
* Math.max(0, Math.min(a.y + a.h, b.y + b.h) - Math.max(a.y, b.y))

test('windows opened while a caret is up are not placed on top of it', async () => {
const { page } = ctx

const sig = page.getByTestId('subwindow').filter({
has: page.getByTestId('window-breadcrumb').filter({ hasText: /^S-/ }),
}).first()
await sig.getByTestId('subwindow-title').click()
await sig.getByTestId('subwindow-titlebar').hover()
await sig.getByTestId('action-btn-Center Zero Beam').click()
const wizard = page.getByTestId('center-zero-beam-wizard')
await expect(wizard).toBeVisible()

const cb = (await wizard.boundingBox())!
const caret = { x: cb.x, y: cb.y, w: cb.width, h: cb.height }
const before = (await windowRects(page)).length

// Open more windows while the caret is up — the situation every staged action
// creates when it publishes its result.
await backendAction(page, 'load_test_data_si_grains')
await waitForSubwindowCount(page, before + 2, 120_000)
await page.waitForTimeout(500)

// The caret must still be open and still be what a click at its centre hits.
await expect(wizard).toBeVisible()
const after = await windowRects(page)
const worst = after
.map((w: any) => overlapArea(w, caret))
.sort((a: number, b: number) => b - a)[0] ?? 0
const caretArea = caret.w * caret.h

console.log(`[caret-placement] caret ${Math.round(caretArea)}px², `
+ `worst window overlap ${Math.round(worst)}px² `
+ `(${Math.round((worst / caretArea) * 100)}%)`)

// A sliver of overlap is tolerable (the search steps in 26px increments and
// the caret rect handed to it is approximate for side placements); burying
// the panel is not.
expect(worst / caretArea,
'a newly opened window was placed over the open caret — findFreeSlot is not '
+ 'treating the caret as occupied space').toBeLessThan(0.25)

const hit = await page.evaluate(([x, y]) => {
const el = document.elementFromPoint(x as number, y as number)
return el?.closest('[data-testid="center-zero-beam-wizard"]') ? 'in-caret'
: `${el?.tagName}:${el?.getAttribute('data-testid') ?? ''}`
}, [Math.round(caret.x + caret.w / 2), Math.round(caret.y + 12)])
expect(hit, 'the caret is covered, so its controls cannot be clicked')
.toBe('in-caret')
})
Loading
Loading