feat(detection): in-browser ONNX/YOLO object detection (#902)#905
feat(detection): in-browser ONNX/YOLO object detection (#902)#905giswqs wants to merge 5 commits into
Conversation
Add an Object Detection toolbox that complements AI Segmentation. A user-supplied YOLO model exported to ONNX runs fully in the browser via onnxruntime-web against a chosen GeoTIFF; detected boxes are georeferenced with the raster's geotransform, reprojected to WGS84, and each class is added as its own GeoJSON layer. - packages/processing/object-detection.ts: letterbox preprocess, lazy single-threaded onnxruntime-web inference, YOLOv5/v8/v11 output decode + per-class NMS, returning source-pixel detections. - ObjectDetectionDialog: pick imagery + .onnx model, optional class names, confidence/IoU/input-size, georeference + reproject + per-class layers. - Wire store flag, Processing menu, command palette, lazy mount, and i18n. Fixes #902
✅ Deploy Preview for geolibre-app ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds client-side YOLO object detection to the desktop app. Users can open a new object detection dialog, choose a GeoTIFF and ONNX model, run inference, and create georeferenced GeoJSON layers for each detected class. ChangesObject detection feature
Sequence Diagram(s)Detection runsequenceDiagram
participant ObjectDetectionDialog
participant fetchDetectionModel
participant detectObjects
participant AppStore
participant mapControllerRef
ObjectDetectionDialog->>fetchDetectionModel: download ONNX bytes
ObjectDetectionDialog->>detectObjects: raster bytes, model bytes, options
detectObjects-->>ObjectDetectionDialog: Detection[]
ObjectDetectionDialog->>AppStore: add GeoJSON layer per class
ObjectDetectionDialog->>mapControllerRef: fit to first created layer
Opening the dialogsequenceDiagram
participant TopToolbar
participant ProcessingMenu
participant AppStore
participant DesktopShell
participant ObjectDetectionDialog
TopToolbar->>AppStore: setObjectDetectionOpen(true)
ProcessingMenu->>AppStore: setObjectDetectionOpen(true)
DesktopShell->>ObjectDetectionDialog: lazy load and mount under Suspense
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/geolibre-desktop/src/components/layout/toolbar/ProcessingMenu.tsx (1)
85-89: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInclude object detection in
showGeolibreActions.If a UI profile leaves
processing.objectDetectionas the only visible GeoLibre action,showGeolibrestays false and the whole parent submenu disappears, so the new item is unreachable despite its own visibility check below.Suggested fix
const showGeolibreActions = show("processing.geocode") || show("processing.modelBuilder") || - (!mobile && show("processing.segmentation")); + (!mobile && show("processing.segmentation")) || + show("processing.objectDetection");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/geolibre-desktop/src/components/layout/toolbar/ProcessingMenu.tsx` around lines 85 - 89, `showGeolibreActions` in `ProcessingMenu` is missing the `processing.objectDetection` visibility check, which can hide the entire GeoLibre submenu when object detection is the only enabled action. Update the `showGeolibreActions` condition to include the object detection flag alongside the existing geocode, modelBuilder, and segmentation checks, so `showGeolibre` stays true whenever the new action is available.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/geolibre-desktop/src/components/layout/toolbar/ProcessingMenu.tsx`:
- Around line 506-509: The ProcessingMenu object detection item is incorrectly
gated by the mobile check, which hides a browser/client-side feature that should
stay available on web-capable clients. Update the conditional around the
DropdownMenuItem in ProcessingMenu so it no longer depends on !mobile, while
keeping the existing show("processing.objectDetection") permission/feature gate
intact. Use the surrounding objectDetection menu entry and
setObjectDetectionOpen handler as the location to make this adjustment.
In `@apps/geolibre-desktop/src/components/processing/ObjectDetectionDialog.tsx`:
- Around line 414-419: The ObjectDetectionDialog inputSize handler only enforces
the lower bound, so user-entered values above the Input max can still reach
detectObjects and cause oversized allocations. Update the onChange logic in
ObjectDetectionDialog to mirror the confidence/IoU clamping pattern: validate
parsed input and reject or clamp values outside the full 32–4096 range before
calling setInputSize.
In `@packages/processing/package.json`:
- Line 37: The `onnxruntime-web` dependency and the WASM CDN URL used by
`object-detection.ts` must stay in sync, since the package range currently
allows newer 1.x versions while the runtime still fetches artifacts from
`onnxruntime-web@1.27.0`. Update `packages/processing/package.json` to pin
`onnxruntime-web` to `1.27.0`, or refactor `object-detection.ts` and the package
metadata to read from a shared version constant so both references always match.
In `@packages/processing/src/object-detection.ts`:
- Around line 98-107: The tensor normalization in object-detection.ts is still
using raw sampled band values, so RasterData.nodata and other invalid pixels can
skew scaling and leak into the model. Update the normalization path around the
sampling logic and the helper that prepares r/g/b so it skips non-finite values
and any nodata samples when computing max/divisor, and replaces invalid pixels
with the existing padding value before returning the tensor inputs. Make sure
the same fix is applied anywhere the sampled bands are normalized, including the
code path referenced by the duplicated logic near the later lines.
- Around line 313-315: Validate the detection options before tensor allocation
in the object-detection flow by adding explicit guards near the inputSize,
confidenceThreshold, and iouThreshold defaults in object-detection.ts. Ensure
inputSize is a finite positive safe integer within a reasonable upper bound, and
clamp or reject confidenceThreshold and iouThreshold unless they are finite
values in the [0, 1] range. Apply these checks before any use in tensor
shape/allocation logic so the detection pipeline fails fast with a clear
validation error instead of exhausting memory or producing invalid filtering
behavior.
---
Outside diff comments:
In `@apps/geolibre-desktop/src/components/layout/toolbar/ProcessingMenu.tsx`:
- Around line 85-89: `showGeolibreActions` in `ProcessingMenu` is missing the
`processing.objectDetection` visibility check, which can hide the entire
GeoLibre submenu when object detection is the only enabled action. Update the
`showGeolibreActions` condition to include the object detection flag alongside
the existing geocode, modelBuilder, and segmentation checks, so `showGeolibre`
stays true whenever the new action is available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6928ebb0-b455-4b9a-b490-39e629dc46d2
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
apps/geolibre-desktop/src/components/layout/DesktopShell.tsxapps/geolibre-desktop/src/components/layout/TopToolbar.tsxapps/geolibre-desktop/src/components/layout/toolbar/ProcessingMenu.tsxapps/geolibre-desktop/src/components/processing/ObjectDetectionDialog.tsxapps/geolibre-desktop/src/i18n/locales/en.jsonapps/geolibre-desktop/src/lib/ui-profile.tspackages/core/src/store.tspackages/processing/package.jsonpackages/processing/src/index.tspackages/processing/src/object-detection.ts
Code reviewReviewed Bugs
Performance
SecurityNo issues found. The model is user-supplied and parsed entirely within the ORT WASM sandbox in the browser, with no server-side surface. The CDN Quality
CLAUDE.md
Overall: The architecture is clean and the implementation follows the established pattern for client-side tools well. B1 (inputSize crash) and P1 (main-thread freeze) are the most important to address before shipping to users; the rest are lower-priority follow-ups. |
- object-detection: validate inputSize/confidence/IoU before allocating the inference tensor, fail fast instead of OOMing or returning nonsense. - object-detection: release the ORT session in a finally block so repeated runs do not leak WASM heap memory. - object-detection: throw with the available output names when the picked output tensor is missing (multi-output / NMS-baked exports). - object-detection: ignore NoData/non-finite pixels when choosing the normalisation divisor, scale higher-bit-depth data by its own max, and substitute the padding value for invalid samples (clamped to 0-1). - object-detection: prefer the v8 layout on a d1 === d2 tie (true v5 has d1 >> d2) and document the ambiguity. - ObjectDetectionDialog: clamp inputSize to the 4096 upper bound, not just the lower bound. - ProcessingMenu: drop the `!mobile` gate; detection runs client-side and should stay available on web/mobile clients. - pin onnxruntime-web to 1.27.0 to match the hardcoded WASM CDN version.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/geolibre-desktop/src/components/processing/ObjectDetectionDialog.tsx (1)
161-168: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDon’t clear
runningwhile a previous detection may still be active.If the user closes and reopens the dialog before
handleRunfinishes, Line 167 marks the dialog idle even though inference is still running. That enables a second run and can double the WASM/tensor memory pressure. Let the existingfinallyclearrunning.Proposed fix
- // Reset transient state so a re-opened dialog never shows a stale error, - // result, or a `running` spinner left over from a previous session. + // Reset transient messages so a re-opened dialog never shows stale feedback. + // Do not clear `running`: an async detection may still be in flight. setError(null); setResultMessage(null); - setRunning(false);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/geolibre-desktop/src/components/processing/ObjectDetectionDialog.tsx` around lines 161 - 168, The open-state reset in ObjectDetectionDialog’s useEffect is clearing the running flag too early, which can make a still-active detection look idle and allow a duplicate run. Update the dialog reset logic so it only clears transient UI state like error and resultMessage when open changes, and leave running to be managed exclusively by handleRun’s finally block. Use the ObjectDetectionDialog component and its useEffect/handleRun flow to locate the change.
♻️ Duplicate comments (1)
packages/processing/src/object-detection.ts (1)
109-119: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCompute the normalization divisor across all sampled channels.
Line 113 samples only
r, but Line 203 divides R/G/B by the same divisor. RGB rasters with a dark/NoData-heavy red band and bright green/blue bands will choosedivisor = 1, then clamp valid 8-bit green/blue values to1, silently distorting model input. Sampler,g, andbwhen choosingmax.Proposed fix
- const step = Math.max(1, Math.floor(r.length / 4096)); - for (let i = 0; i < r.length; i += step) { - const v = r[i]; - if (!Number.isFinite(v) || v === nodata) continue; - if (v > max) max = v; + const channels = [r, g, b]; + const step = Math.max(1, Math.floor(r.length / 4096)); + for (const band of channels) { + for (let i = 0; i < band.length; i += step) { + const v = band[i]; + if (!Number.isFinite(v) || v === nodata) continue; + if (v > max) max = v; + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/processing/src/object-detection.ts` around lines 109 - 119, The normalization divisor in object detection is currently derived only from the red band, which can produce the wrong scale when green or blue carry higher valid values. Update the sampling logic in the object-detection path to inspect all three channels used later in the same function (the RGB arrays in object-detection.ts) when computing max and divisor, so the chosen normalization matches the full image data before the R/G/B division step.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/geolibre-desktop/src/components/processing/ObjectDetectionDialog.tsx`:
- Around line 161-168: The open-state reset in ObjectDetectionDialog’s useEffect
is clearing the running flag too early, which can make a still-active detection
look idle and allow a duplicate run. Update the dialog reset logic so it only
clears transient UI state like error and resultMessage when open changes, and
leave running to be managed exclusively by handleRun’s finally block. Use the
ObjectDetectionDialog component and its useEffect/handleRun flow to locate the
change.
---
Duplicate comments:
In `@packages/processing/src/object-detection.ts`:
- Around line 109-119: The normalization divisor in object detection is
currently derived only from the red band, which can produce the wrong scale when
green or blue carry higher valid values. Update the sampling logic in the
object-detection path to inspect all three channels used later in the same
function (the RGB arrays in object-detection.ts) when computing max and divisor,
so the chosen normalization matches the full image data before the R/G/B
division step.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 50d4925a-633a-46c9-a583-4d8fa953071a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
apps/geolibre-desktop/src/components/layout/toolbar/ProcessingMenu.tsxapps/geolibre-desktop/src/components/processing/ObjectDetectionDialog.tsxpackages/processing/package.jsonpackages/processing/src/object-detection.ts
Code reviewReviewed Bugs
Quality
Security / Performance / CLAUDE.md
Overall the implementation is solid — the georeferencing math, NMS, letterbox inversion, class-per-layer split, and session lifecycle are all correct. Finding #1 is the only one that could silently break detection for a subset of real YOLOv5 models; the rest are quality improvements. |
Import onnxruntime-web from the `/wasm` subpath instead of the default entry. The default entry also bundles the ~26 MB WebGPU/jsep WASM, which exceeds Cloudflare Pages' 25 MiB per-file limit and failed the preview deploy. We force the WASM execution provider anyway, so the WebGPU artifact was dead weight; the CPU WASM is ~13 MB (under the limit) and shrinks every build. Verified detection still works end to end in the browser.
⚡ Cloudflare Pages preview
|
Code reviewPR adds a fully client-side ONNX/YOLO Object Detection toolbox. The architecture is clean (lazy-loaded dialog, store flag, proper ORT session teardown via Bugs
Performance
Maintainability
Quality / nits
What I checked and did not flag
|
Add a model-source toggle to the Object Detection dialog so users no longer have to supply their own .onnx file. The default is a built-in COCO model that downloads from jsDelivr (permissive CORS, pinned to immutable commit SHAs) and is cached via the Cache API so it is fetched once and works offline thereafter. - detection-models.ts: COCO class list, the built-in model catalog (YOLOv8n + YOLOv5n), and a cached fetchDetectionModel helper. - ObjectDetectionDialog: Built-in/Local source toggle, a built-in model dropdown that auto-fills the COCO class names, download-on-run with a spinner, and the local-file picker kept for custom/aerial models. - i18n strings for the new controls. Verified end to end: with only a GeoTIFF chosen and the built-in YOLOv8n selected, Detect downloads the model and adds named per-class layers (bus, person).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/geolibre-desktop/src/components/processing/ObjectDetectionDialog.tsx (1)
174-181:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the in-flight guard independent from the dialog open state.
Line 180 clears
runningevery time the dialog reopens. If the user closes and reopens while model download or inference is still awaiting, the Detect button becomes enabled again and a second run can start. Both runs then add layers independently.Suggested fix
+ const inFlightRef = useRef(false); + useEffect(() => { if (!open) return; // Reset transient state so a re-opened dialog never shows a stale error, // result, or a `running` spinner left over from a previous session. setError(null); setResultMessage(null); - setRunning(false); }, [open]); const handleRun = useCallback(async () => { + if (inFlightRef.current) return; + inFlightRef.current = true; setError(null); setResultMessage(null); if (!imageBytes) { setError(t("objectDetection.error.chooseImage")); + inFlightRef.current = false; return; } if (modelSource === "local" && !modelBytes) { setError(t("objectDetection.error.chooseModel")); + inFlightRef.current = false; return; } setRunning(true); try { // ... } finally { + inFlightRef.current = false; setRunning(false); } }, [/* ... */]);Also applies to: 217-229, 305-306
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/geolibre-desktop/src/components/processing/ObjectDetectionDialog.tsx` around lines 174 - 181, The transient reset in ObjectDetectionDialog’s open-state effect is clearing the in-flight guard too aggressively, which can re-enable Detect while a model download or inference is still running. Keep the running state independent from the dialog’s open/close lifecycle by only resetting error and result state on reopen, and preserve the guard used by the detect flow so the existing run in the handleDetect / model-loading / inference path cannot be started twice. Also update the related disable logic around the Detect button and any cleanup paths referenced by the same running state so they continue to respect an active in-flight operation even after the dialog is reopened.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/geolibre-desktop/src/lib/detection-models.ts`:
- Around line 79-97: The fetch and cache write are coupled in
detection-models.ts, so a successful download can be lost if cache.put() rejects
and the code falls back to fetching again. Split the logic in the model-loading
path so the network response from the first successful fetch in the
`fetch`/`caches.open` flow is returned directly even when `cache.put()` fails,
and only treat Cache API failures as non-fatal for caching while preserving the
already-downloaded `url` response.
---
Duplicate comments:
In `@apps/geolibre-desktop/src/components/processing/ObjectDetectionDialog.tsx`:
- Around line 174-181: The transient reset in ObjectDetectionDialog’s open-state
effect is clearing the in-flight guard too aggressively, which can re-enable
Detect while a model download or inference is still running. Keep the running
state independent from the dialog’s open/close lifecycle by only resetting error
and result state on reopen, and preserve the guard used by the detect flow so
the existing run in the handleDetect / model-loading / inference path cannot be
started twice. Also update the related disable logic around the Detect button
and any cleanup paths referenced by the same running state so they continue to
respect an active in-flight operation even after the dialog is reopened.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 20c6aa83-16b2-4dc7-9e25-bd94894487cb
📒 Files selected for processing (3)
apps/geolibre-desktop/src/components/processing/ObjectDetectionDialog.tsxapps/geolibre-desktop/src/i18n/locales/en.jsonapps/geolibre-desktop/src/lib/detection-models.ts
Code reviewReviewed the full diff for PR #905 (in-browser ONNX/YOLO object detection). The overall design is sound — lazy loading, store flag, per-class layer grouping, NMS, WASM single-thread path, and CDN SHA-pinning are all done correctly. The YOLO output decoder's v8/v5 heuristic is correct for any realistic class count. The fallback Suspense pattern for the lazy dialog matches the established codebase convention. Below are the findings grouped by category. Bugs
Performance
Quality
SecurityNothing flagged. The CDN URLs are pinned to immutable commit SHAs (good), the CORS fetch is explicit, the Cache API key is a fixed string, and user-supplied model bytes are passed directly to the ONNX runtime without any re-serialization. CLAUDE.mdNo violations. New i18n strings are in |
- object-detection: make loadOrt a promise singleton so concurrent calls cannot race on configuration. - object-detection: fall back to an 8-bit divisor when sampling finds no valid pixels (mostly-NoData raster), avoiding a divisor of 1 that feeds raw 0-255. - object-detection: unroll the bilinear NoData check to drop a per-sample array allocation in the hot loop. - object-detection: only apply the objectness gate for v5 (it was a no-op for v8/v11), and document that decode assumes sigmoid-activated output (standard Ultralytics exports) rather than double-activating the common case. - object-detection: note the O(inputSize^2) main-thread preprocessing cost. - object-detection: export ORT_VERSION and add a guard test asserting it equals the onnxruntime-web pin in package.json. - detection-models: separate Cache API ops from the network fetch so a 404/5xx surfaces instead of being swallowed and retried, and a successful download is never discarded when a cache write fails. - ObjectDetectionDialog: round inputSize to a multiple of 32 (YOLO stride); guard concurrent runs with an inferringRef that survives close/re-open so a re-opened dialog cannot spawn a duplicate inference; document the north-to-south row-order assumption. - ProcessingMenu: include objectDetection in showGeolibreActions so the GeoLibre submenu still renders when it is the only enabled action.
| // in a mostly-NoData raster), assume 8-bit range so the tensor receives 0-1 | ||
| // values rather than raw 0-255 counts. | ||
| if (!hasValid) max = 255; | ||
| const divisor = max <= 1.5 ? 1 : max <= 255 ? 255 : max; |
There was a problem hiding this comment.
Bug — wrong divisor for float rasters with sampled max in (1.5, 255)
The three-way branch is:
max ≤ 1.5→ divisor1(already-normalised float)1.5 < max ≤ 255→ divisor255(treated as 8-bit)max > 255→ divisormax(high bit-depth auto-scale)
Any GeoTIFF whose sampled red-band maximum falls in (1.5, 255) — multi-spectral reflectance products scaled 0–2, vegetation indices, DEMs with Celsius temperatures, or any float raster whose values happen to peak in that range — is silently treated as 8-bit. A pixel value of 2.0 becomes 2/255 ≈ 0.008 in the tensor; the model receives a near-black image and returns no detections.
A minimal fix for the float-vs-8-bit ambiguity is to auto-scale any raster where max is non-integer (floats never hit exact 255):
| const divisor = max <= 1.5 ? 1 : max <= 255 ? 255 : max; | |
| const divisor = max <= 1.5 ? 1 : (max <= 255 && Number.isInteger(max)) ? 255 : max; |
A more robust fix would expose the GeoTIFF bit-depth through RasterData and use that directly, but the Number.isInteger guard eliminates the false-positive for the most common failure case without changing existing 8-bit or 16-bit paths.
| features, | ||
| }; | ||
| if (epsg && epsg !== 4326) { | ||
| fc.crs = { type: "name", properties: { name: `EPSG:${epsg}` } }; |
There was a problem hiding this comment.
Bug — missing EPSG silently skips reprojection, placing boxes at wrong coordinates
When epsgFromGeoKeys returns null (user-defined CRS sentinel 32767, GeoTIFF 1.1 GeodeticCRSGeoKey, or any raster whose CRS is stored only as WKT with no EPSG authority code), no crs member is attached to the FeatureCollection.
reprojectFeatureCollectionToWgs84 in duckdb-vector-loader.ts checks sourceCrsFromGeoJson first (line 494):
if (!sourceCrs) return stripped as FeatureCollection;So the feature collection is returned unchanged, and its coordinates — which are in the raster's native projected CRS — are added to the map as if they were WGS84 lon/lat. For a UTM raster without a recognized EPSG code, detection boxes could be placed thousands of kilometres from the true location with no error shown to the user.
Consider surfacing a warning when epsg === null and the raster has non-trivial georeferencing (i.e., originX !== 0 || originY !== 0):
| fc.crs = { type: "name", properties: { name: `EPSG:${epsg}` } }; | |
| if (epsg && epsg !== 4326) { | |
| fc.crs = { type: "name", properties: { name: `EPSG:${epsg}` } }; | |
| } else if (!epsg) { | |
| console.warn( | |
| "objectDetection: no recognized EPSG code in GeoKeys — detections will not be reprojected and may appear at the wrong location.", | |
| ); | |
| } |
| try { | ||
| session = await ort.InferenceSession.create(new Uint8Array(modelBytes), { | ||
| executionProviders: ["wasm"], | ||
| }); |
There was a problem hiding this comment.
Performance — InferenceSession re-created on every call; model graph is re-parsed each run
fetchDetectionModel caches model bytes via the Cache API so there is no repeated download. But ort.InferenceSession.create re-parses those bytes into the WASM heap on each detectObjects() call. For the default yolov8n.onnx (~6 MB) this adds ~200–500 ms of parsing overhead on every run. A user who adjusts the confidence threshold and clicks "Detect" again pays the full re-parse cost each time.
The ortPromise singleton already avoids re-importing the ORT module; a session cache keyed on the model URL (or a stable hash of the bytes) would similarly amortise the model-graph cost:
const sessionCache = new Map<string, ort.InferenceSession>();
// in detectObjects(), before session.create:
let session = sessionCache.get(cacheKey);
if (!session) {
session = await ort.InferenceSession.create(...);
sessionCache.set(cacheKey, session);
}
// then omit session.release() from the finally block for cached sessionsThe same try/finally { session.release() } wrapper should be kept only for uncached / one-shot sessions. A page-unload handler (or WeakRef-based eviction) can clean up cached sessions to avoid holding WASM heap indefinitely.
| id: "yolov8n-coco", | ||
| label: "YOLOv8n (COCO, 80 classes)", | ||
| url: "https://cdn.jsdelivr.net/gh/Hyuto/yolov8-onnxruntime-web@fc4a52c466d15ad4519873a0cef22fbc935b93b6/public/model/yolov8n.onnx", | ||
| classNames: COCO_CLASSES, |
There was a problem hiding this comment.
CLAUDE.md — label strings bypass t() and can't be translated
The label fields are rendered directly in the dropdown ({model.label}) in ObjectDetectionDialog.tsx without going through t(). CLAUDE.md requires t() for new user-facing strings.
These are technical model identifiers (version + dataset + class count) that arguably don't need translation, but if you want to follow the convention strictly, the labels could be moved to en.json and referenced by key:
| classNames: COCO_CLASSES, | |
| label: "toolbar.command.objectDetectionModel.yolov8n", |
with a corresponding en.json entry:
"objectDetectionModel": {
"yolov8n": "YOLOv8n (COCO, 80 classes)",
"yolov5n": "YOLOv5n (COCO, 80 classes)"
}Alternatively, if the team considers model identifiers exempt from i18n, that exception should be documented.
| @@ -0,0 +1,450 @@ | |||
| /** | |||
There was a problem hiding this comment.
CLAUDE.md — multi-paragraph JSDoc blocks throughout this file (and detection-models.ts, ObjectDetectionDialog.tsx)
CLAUDE.md: "Never write multi-paragraph docstrings or multi-line comment blocks — one short line max."
All three new files contain multi-paragraph JSDoc blocks. The file-level module doc here is 21 lines; rgbBands, sampleBilinear, preprocess, nonMaxSuppression, decodeYolo, and detectObjects each have 6–14-line blocks. detection-models.ts opens with a 14-line module comment and a 12-line JSDoc on fetchDetectionModel. ObjectDetectionDialog.tsx has four multi-paragraph blocks on its helper functions.
The content of these comments often explains genuinely non-obvious constraints (YOLO layout heuristic, NoData NaN sentinel, single-threaded WASM rationale), which satisfies the "WHY is non-obvious" criterion — but the format rule is unconditional. Each of these blocks should collapse to at most one line, with the non-obvious constraint preserved and the rest removed.
| if (model) setClassNames(model.classNames.join(", ")); | ||
| }, []); | ||
|
|
||
| const handleRun = useCallback(async () => { |
There was a problem hiding this comment.
Quality — in-flight inference cannot be cancelled; dialog close produces a ghost run
inferringRef.current correctly prevents a second run from starting while one is in progress. But there is no AbortController passed to fetch() inside fetchDetectionModel, and no cancellation path exists for ort.InferenceSession.run(). If the user starts detection, closes the dialog while the model is downloading or inference is running, and then re-opens it:
- The ghost run continues to completion on the stale closure.
addGeoJsonLayeris called silently — a layer appears on the map with no result message visible.inferringRef.currentstaystrueuntil the ghost run finishes, making the re-opened dialog appear locked (the button is greyed out via therunningstate, butrunningwas reset on close... wait —runningis NOT reset on close, so the spinner shows). This is consistent, but a user who did not see the original spinner may be confused.
For a future iteration, consider passing an AbortSignal to fetchDetectionModel and polling a cancel flag between inference steps. At minimum, noting the limitation in a comment near inferringRef would help future contributors understand why no cancel UI is offered.
Code reviewThis PR adds a fully client-side YOLO/ONNX object detection toolbox: a new `@geolibre/processing` engine handles raster preprocessing, layout-auto-detected YOLOv8/v11 and YOLOv5 decoding, NMS, and back-projection; the `ObjectDetectionDialog` georeferences detected boxes, splits them by class into separate GeoJSON layers, and supports both downloadable built-in COCO models and user-supplied ONNX files. The store, toolbar, Processing menu, UI-profile catalog, i18n, and lazy-mount shell wiring are all extended consistently. Bugs
Performance
Quality
CLAUDE.md
What I checked and did not flag: the inferringRef early-return guard is correct (all returns after the ref is set are inside the outer try, so finally always resets it); the crs format {type:"name",properties:{name:"EPSG:…"}} matches what sourceCrsFromGeoJson expects; reprojectFeatureCollectionToWgs84 is concurrent-safe (monotonic sequence counter, per-call DuckDB connection); addGeoJsonLayer return value is synchronously available; the v8/v11 layout heuristic (d1 <= d2) is correct for standard Ultralytics ONNX exports (outputs are in input-pixel coordinates, not normalised, so the un-letterbox math is right); the ORT_VERSION test guard against version skew is a nice touch. |
|
Greatly thankful for taking action on this request. FYI This raster is too large for the in-browser engine. Use the sidecar (rasterio/GDAL) engine instead. |
|
Yeah, it is not ready yet. Need more testing. |
|
If it makes things easier(?) Apply detection on layer or visible (just as QGIS plugins) |
2 participants
What
Adds an Object Detection toolbox that complements the existing AI Segmentation, addressing the request in #902 for custom ONNX/YOLO detection models with multiple classes, where each class becomes a layer.
Unlike AI Segmentation (which proxies SAM3 through the Python sidecar), detection runs fully in the browser with
onnxruntime-web, so it works in both the web and desktop builds with no server.How it works
geotiff.js(reusingreadRasterData), letterboxed into the model input, and run client-side (single-threaded WASM, no cross-origin-isolation requirement).[1, 4+nc, anchors]and YOLOv5[1, anchors, 5+nc], auto-detected), followed by per-class non-maximum suppression.classandscoreproperties.Files
packages/processing/src/object-detection.ts- the in-browser detection engine (preprocess, lazy ORT inference, decode + NMS).apps/geolibre-desktop/src/components/processing/ObjectDetectionDialog.tsx- the dialog (georeference + reproject + per-class layers).Notes
onnxruntime-webloads its WASM from the jsdelivr CDN, which is already in the Tauri CSP allowlist.Verification
Driven in the real app with Playwright in both light and dark themes, using a real
yolov8n.onnx(COCO) over a georeferenced GeoTIFF. Detection produced the expected boxes, correctly georeferenced and reprojected onto the basemap, with one named layer per detected class.npm run build, the full frontend test suite (1637 passing), andpre-commit(eslint + build) are green.This is an initial MVP slice; follow-ups could add embedded class-name metadata parsing, draw-a-box prompts, and a model library.
Fixes #902
Summary by CodeRabbit