Skip to content

GeecsPanels: crosshairs and the analysis ROI — drawn by the panel, dragged to move, written through an allow-list - #1133

Merged
skbarber merged 6 commits into
masterfrom
feat/panels-crosshairs
Oct 23, 2026
Merged

skbarber merged 6 commits into
masterfrom
feat/panels-crosshairs

Conversation

@skbarber

Copy link
Copy Markdown
Collaborator

Stacked PR, step 2b of the panels arc. Base: feat/panels-viewer (#1132) ← #1131 ← #1130. The stack merges in order at the end of the arc (maintainer's call: evaluate the arc as a unit).

What

The IOC stores both crosshairs (OVER1:1/2) and the analysis ROI (ROI1): shared by every viewer and autosaved. The panel draws them over the colour-mapped image and edits them. This is the design note's §5 ruling: colour crosshairs drawn client-side, with the stream left plain greyscale.

  • overlay.js, a layer on the viewer's new hooks (draw, grab, hover, snapshot via window.GeecsCameraView). It zooms and pans with the image.
    • Drag a crosshair's centre, or the ROI's edges and corners, with resize and move cursors on hover.
    • An edit stays on screen until the IOC's readback agrees, so it never jumps back mid-drag.
  • Rail controls:
    • Per crosshair: on/off, colour, x/y, To centroid. An overlay that was never configured becomes a visible cross with a visible colour.
    • ROI: x/y/width/height and Full frame.
  • Writes (the first in the panel):
    • POST /camera/{d}/put: allow-listed (camera.WRITABLE: OVER1:1/2 fields and ROI1 Min/Size only, nothing a scan saves), with values coerced to the PV's type.
    • /crosshair/{n}/colour: r/g/b, range-checked by the model.
    • /crosshair/{n}/to-centroid: the STAT centroid converted from ROI to image pixels on the server (coords.py, binning and reversal included).
    • --read-only refuses every write and disables the controls.
  • Fit leaves a 16 px margin. Found in the live test: a full-frame ROI's corner sat exactly on the viewport's edge and couldn't be grabbed.

1013 added, 19 removed.

Tests

  • GeecsPanels: 90 passed. New test_writes.py covers:
    • Nothing outside the allow-list is writable, including the driver and HDF1.
    • Value coercion and refusal.
    • Coordinates with binning and reversal.
    • Each route: success, a 422 refusal that puts nothing, read-only 403, an IOC failure as a 502, to-centroid's conversion, a stale centroid as a 409, and colour.
  • GeecsWebTheme: 199 passed (overlay.js added to the guard).
  • Lint OK.

Hardware verification (done on ADTEST_CAM1)

Through the panel's routes and real mouse events in Chrome over CDP:

  • Crosshair 1 moved and recoloured.
  • cam1:AcquireTime and HDF1:Capture refused; Capture_RBV stayed Done.
  • An out-of-range colour refused.
  • To-centroid on crosshair 2 put (696, 552).
  • Crosshair 2 turned on from its checkbox: Shape → Cross, size 205.
  • Dragging crosshair 1: the IOC stored exactly the image pixel under the drop point, (717, 553).
  • Dragging the ROI corner: ROI1 became 822 × 719, and STAT's centroid recomputed inside it.
  • The test camera was restored afterwards and checked by readback: crosshair 1 (640, 512) green, crosshair 2 off as a 1-px rectangle, ROI full frame.

🤖 Generated with Claude Code

skbarber and others added 6 commits October 22, 2026 22:21
…collapsible Live strip, and frames fetched only when the camera makes one

- The image fills the panel; wheel zooms at the cursor, drag pans,
  double-click or Fit fits, 1:1 shows camera pixels (unsmoothed when
  magnified). The colour-mapped frame is kept apart from the view, so later
  overlays zoom and pan with it (toView/toImage).
- Scale: linear / square root / log, in the same lookup table as the levels.
- Live folds into a native disclosure whose summary keeps centroid and max.
- FrameSource reuses a frame exactly while MJPG1's counter stands still and
  sends it as X-Frame-Counter; the page skips decoding a frame it already
  shows. A 1 Hz camera polled at 10 Hz is now one fetch and one draw a second.
- Fixed while measuring: the reuse window was timed from the fetch's reply,
  so a poll at the cap's period was handed the old frame a third of the
  time (5.5 fps shown of 10). Now timed from the request, and the counter
  decides whenever it is known. The page also fetches the next frame while
  drawing the last. Measured: 9.5-10 fps at the 10 fps cap.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ers, a reuse limit, per-fetch frame tags, decode errors, pixel density, the 1:1 floor

F1: the min_interval floor applies whatever the counter says, so N viewers
cost the IOC one fetch per floor (a 30 Hz camera and 5 viewers was ~50/s).
F2: a frame is never reused past max_reuse (1 s) under an unchanged
counter, and each fetch gets its own tag (X-Frame-Tag) so the page redraws
a refetch. ffmpegServer stores the JPEG before posting its counter, so this
is a net, not a known failure.
F3: a change of devicePixelRatio (window moved between screens) re-sizes.
F4: a frame that fails to decode shows the error and is retried.
F5: 1:1 reaches 100% on a frame smaller than the view.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…decode-error box hides only after a frame draws; two comments brought up to date

Review round 2 on #1132 (both low).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…agged to move, written through an allow-list

Build step 2b. The IOC stores both crosshairs (OVER1:1/2) and the analysis
ROI (ROI1); the panel draws them over the colour-mapped image (overlay.js,
a layer on the viewer's new hooks) and edits them:
- drag a crosshair's centre, or the ROI's edges and corners; the edit is
  held on screen until the IOC's readback agrees
- per crosshair: on/off (an unconfigured overlay becomes a visible cross
  with a visible colour), colour, x/y, to-centroid (converted ROI -> image
  pixels on the server, coords.py, binning and reversal included)
- ROI x/y/width/height and Full frame
Writes: POST put (allow-listed: OVER1:1/2 fields, ROI1 Min/Size — nothing a
scan saves; values coerced to the PV's type), a colour route, a to-centroid
route; --read-only refuses all. Fit now leaves a 16 px margin so a
full-frame ROI's handles can be grabbed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…[object Object]

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… value ranges, no grabs before a frame, whole-bin reversal

F1: a crosshair is drawn and hit-tested on its pixel's centre, a drag
writes the pixel under the pointer (floor, as the cursor readout names it),
and to-centroid rounds half up rather than half-to-even.
F2: a cleared or non-numeric field writes nothing; the allow-list carries
ranges (pixels 0..65535, ROI sizes >= 1, colour 0..255).
F3: no hover or grab before the first frame (an invisible ROI could be
collapsed).
F4: coords uses NDROI's whole-bin span, so a reversed ROI whose size is not
a multiple of the binning maps to the right pixel.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@skbarber

Copy link
Copy Markdown
Collaborator Author

Adversarial review (fresh-context reviewer, two rounds)

The reviewer found no way around the allow-list or --read-only: every write route passes _Writer.device() (403/404) and check_write's exact-key match. Injection values (text for a number, true for a pixel, NaN or Infinity, an enum outside its choices) are refused. The ROI write order (both Mins, then both Sizes) is safe against NDROI's clamping.

# Finding Disposition
F1 Crosshairs were drawn and written on a pixel's corner, not its centre. That put them half a pixel off STAT and OVER1's convention: 20 screen px at maximum zoom. round() rounded half to even. Fixed in d86bcd6a3: drawn and hit-tested at the pixel centre, a drag writes the pixel the cursor readout names, to-centroid rounds half up. Confirmed (a mutation reverting the rounding fails the test).
F2 A cleared number field wrote 0 (ROI1:SizeX 0 collapses the analysis ROI), and /put had no ranges. Fixed: an empty field writes nothing. WRITABLE carries Whole(lo, hi) ranges: pixels 0..65535, ROI size ≥ 1, colour 0..255. Confirmed (removing the ranges fails 4 tests).
F3 Handles were grabbable before the first frame, which could collapse an invisible ROI. Related: positions computed in JPEG pixels. Fixed (no hover or grab before a frame). Confirmed. The JPEG-size part was also found live: ffmpegServer scales a Rot90 1280×1024 frame to 960×1200, so drags would land about 6% off. The fix is in #1134 (2c): the JPEG is stretched to MJPG1:ArraySize0/1_RBV. ⚠️ Do not deploy this PR without #1134. The stack merges together at the end of the arc.
F4 A reversed ROI whose size isn't whole bins mapped one pixel wrong. Fixed: NDROI's whole-bin span. A new non-divisible test fails on the old formula. Confirmed. ROI1:EnableX/Y isn't handled: noted, since the chain contract keeps ROI1 enabled.
F5 The write routes can be requested cross-site (a form POST to to-centroid, a no-CORS JSON put), though only within the allow-list. Noted, not fixed here. An Origin/Sec-Fetch-Site check belongs in geecs_web_theme.web for every surface; none has one today.

Below the floor, fixed in #1134: the allow-list comment sat above the wrong definition.

@skbarber skbarber left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed this PR in the #1130–#1135 stack, including the existing review fixes. No additional actionable findings in this PR. Validation at stack head dc8ac07: panel suite plus Core EPICS bootstrap/light-import tests, 143 passed (including JavaScript syntax checks). Targeted offline probes reproduced the findings on #1134/#1135. No hardware writes or live camera testing performed. The documented dependency on #1134 for scaled-image coordinates still applies.

@skbarber
skbarber changed the base branch from feat/panels-viewer to master October 23, 2026 23:10
@skbarber
skbarber merged commit 70b168d into master Oct 23, 2026
12 checks passed
@skbarber
skbarber deleted the feat/panels-crosshairs branch October 23, 2026 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant