Skip to content

Commit 34c00ad

Browse files
authored
fix(processing): let Whitebox Dissolve take a grouping field (#1997)
* fix(processing): let Whitebox Dissolve take a grouping field The Dissolve dialog rendered its `dissolve_field` parameter as a second layer picker with a file-path box, so the attribute to dissolve by could not be entered at all. The parameter kind came from geolibre-wasm's manifest, which inferred a parameter's type from its description -- and a column parameter's description describes the data it *indexes*, not the value the user types. "Optional attribute field used to dissolve polygons within groups" read as a polygon layer to open. Bump geolibre-wasm to 1.5.2, which carries three upstream fixes: - opengeos/whitebox-wasm#19 types a `*_field`/`*_attribute` parameter as the column name it is. 54 params across 28 tools stop asking for a file or a checkbox, so the dialog's attribute picker (GeoLibre#1459) now reaches them: Dissolve, join_tables, merge_table_with_csv, the route event family, and every network tool's `one_way_field`. - opengeos/whitebox-wasm#20 makes `dissolve` emit one feature per group. Parts of a group that shared a value but no boundary were separate features, so 290 polygons over 12 values dissolved to 48, not 12. - opengeos/whitebox-wasm#21 decodes GeoJSON/TopoJSON strings as UTF-8. Both parsers read each byte as a Latin-1 code point and re-encoded it, so a non-ASCII attribute gained a layer of mojibake on every pass through a tool. Also correct two comments that cited `join_tables.primary_key_field` as a field-named parameter that is legitimately a dataset input. It was one of the 40 the manifest mistyped, and it is a string now; the sidecar catalog's `classify_objects_svm.class_field` is a live example, so the scalar-string guard those comments explain still earns its place. Fixes #1977 * fix(processing): run Whitebox WASM tools off the main thread The WASI runner is a single synchronous `wasi.start()` with no yield points, so running it on the main thread freezes the whole UI -- no repaint, no input -- for as long as the tool takes. Dissolving the 290-polygon layer from #1977 takes ~60s, and a stall probe measured 58,573 ms without a single animation frame: the app looks hung. wasm-convert.ts already routed its tiling calls to a one-shot Worker for exactly this reason, and its worker script is generic. Lift that machinery into `wasm-tool-runner.ts`, rename the script to `wasm-tool.worker.ts` now that it serves both callers, and route `runWhiteboxToolWasm` through it. One implementation, so the two paths cannot drift. This freeze predates the dissolve fix -- every WASM tool blocked the main thread -- but a long-running Dissolve was unreachable until the grouping field became typeable, so it surfaces there first. Measured on the same run: max main-thread stall drops from 58,573 ms to 756 ms, and the output is unchanged at 12 features. * Address Claude review feedback - reuse WASM tool workers instead of discarding each after one run. A worker compiles the ~23 MB geolibre-cli.wasm in its own module scope, and the main thread's copy is not shared with it, so one worker per run made every run pay that again. Invisible next to a minutes-long tiling job, the only caller before this PR, but not next to the many Whitebox tools that finish in well under a second. An idle worker is taken when there is one and a new one spawned otherwise, so concurrent runs still overlap rather than serializing behind a single shared worker; a worker that fails at the worker level is terminated rather than parked, and listeners are removed so a reused worker does not accumulate them. Adds `releaseIdleWasmToolWorkers()` to free the warm workers, which test teardown needs so a parked worker is not handed to the next case. - fix the stale `wasm-convert.worker.ts` reference in the afterEach comment left by this PR's rename. * Address Claude review feedback - guard reuse of a parked worker with an acknowledgement. A worker killed out of band fires no `error` event and silently swallows `postMessage`, and this module deliberately puts no timeout on the run itself, so reusing one that died while idle would have left the run pending forever with nothing shown to the user. The worker now acks on receipt, before starting the run that blocks its thread; a reused worker that does not ack within 10s is terminated, replaced, and the request re-sent. Only reused workers are watched — a freshly spawned one has not had time to die, reports itself through `error`, and its module-graph startup can outlast any sensible ack window in a dev server. A false positive costs a respawn, never a failed run.
1 parent b4b1c17 commit 34c00ad

10 files changed

Lines changed: 350 additions & 116 deletions

File tree

apps/geolibre-desktop/src/components/processing/ProcessingDialog.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,9 @@ function isSubsetUrlParameter(tool: WhiteboxTool, param: WhiteboxToolParameter):
190190
// vector inputs (points_to_line's `line_field`/`sort_field`, and ~170 other
191191
// tools), so the dialog can offer the selected layer's attribute names instead
192192
// of asking the user to recall a column name (GeoLibre#1459). The kind check is
193-
// what keeps a same-named *dataset* param out (join_tables' `primary_key_field`
194-
// is a vector input): only a scalar string names a column.
193+
// what keeps a same-named *dataset* param out (the catalog types
194+
// classify_objects_svm's `class_field` as a LiDAR input): only a scalar string
195+
// names a column.
195196
function isFieldParameter(param: WhiteboxToolParameter): boolean {
196197
return parameterKind(param) === "string" && isFieldParameterName(param.name);
197198
}

apps/geolibre-desktop/src/lib/whitebox-field-params.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,12 @@ export const FIELD_PARAM_SUFFIX = /(^|_)(fields?|attributes?)$/i;
1717
/**
1818
* Whether a parameter name reads as an attribute-column name.
1919
*
20-
* Callers must also check the parameter is a scalar string: `join_tables`
21-
* exposes `primary_key_field` as a *dataset* input, and a dataset parameter
22-
* names a file, not a column.
20+
* Callers must also check the parameter is a scalar string: the sidecar's
21+
* catalog exposes `classify_objects_svm`'s `class_field` as a *dataset* input,
22+
* and a dataset parameter names a file, not a column. (The WASM manifests used
23+
* to mistype ~40 of these the same way, `dissolve_field` among them, until
24+
* opengeos/whitebox-wasm#19 taught the manifest inference that a `*_field` name
25+
* is a column; the sidecar catalog still carries a few.)
2326
*
2427
* @param name - The tool parameter's name.
2528
* @returns `true` when the name ends in a field/attribute suffix.

package-lock.json

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/processing/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
"@turf/voronoi": "^7.4.0",
3535
"dggal": "^0.0.6",
3636
"fflate": "^0.8.3",
37-
"geolibre-wasm": "^1.5.1",
37+
"geolibre-wasm": "^1.5.2",
3838
"geotiff": "^3.0.5",
3939
"onnxruntime-web": "1.27.0",
4040
"s2js": "^1.44.0"

packages/processing/src/wasm-client.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import type { FeatureCollection } from "geojson";
99
import { convertGeoTiffToCog } from "./cog-convert";
1010
import { normalizeVectorOutputFormat } from "./sidecar-client";
11+
import { runWasmToolInBackground } from "./wasm-tool-runner";
1112
import type {
1213
RunWhiteboxToolRequest,
1314
VectorOutputFormat,
@@ -651,7 +652,6 @@ export async function ensureWhiteboxRasterCog(bytes: Uint8Array): Promise<Uint8A
651652
* (Cloud Optimized GeoTIFF) for `raster_out` - never a server path.
652653
*/
653654
export async function runWhiteboxToolWasm(request: RunWhiteboxToolRequest): Promise<WhiteboxJob> {
654-
const { runTool } = await loadToolsModule();
655655
const encoder = new TextEncoder();
656656
const input: Record<string, Uint8Array> = {};
657657
const args: string[] = [];
@@ -770,7 +770,14 @@ export async function runWhiteboxToolWasm(request: RunWhiteboxToolRequest): Prom
770770
}
771771
}
772772

773-
const { exitCode, stdout, files } = await runTool(request.tool_id, { args, input });
773+
// Off the main thread: the WASI runner is one synchronous call with no yield
774+
// points, so running it here would freeze the UI for the tool's whole
775+
// duration (~60s for the 290-polygon dissolve in GeoLibre#1977).
776+
const { exitCode, stdout, files } = await runWasmToolInBackground({
777+
tool: request.tool_id,
778+
args,
779+
input,
780+
});
774781
if (exitCode !== 0) {
775782
return job(
776783
request.tool_id,

packages/processing/src/wasm-convert.ts

Lines changed: 3 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
// All three run entirely client-side, so the web build needs no Python sidecar
1515
// for them.
1616
import type { RunToolOptions, ToolResult } from "geolibre-wasm/tools";
17-
import type { WasmToolRequest, WasmToolResponse } from "./wasm-convert.worker";
17+
import { runWasmToolInBackground } from "./wasm-tool-runner";
1818

1919
/** The subset of `geolibre-wasm/tools` these converters use. */
2020
interface ConvertToolsModule {
@@ -59,61 +59,6 @@ export async function initConvertTools(
5959
await initTools(source);
6060
}
6161

62-
/**
63-
* Run a tool on a one-shot Web Worker and resolve with its result.
64-
*
65-
* No timeout: how long a tool runs is bounded by the data, not the clock (a
66-
* country-scale tile pyramid is minutes), and cutting off work that would have
67-
* finished is worse than waiting. `error`/`messageerror` still reject, so the
68-
* promise settles on every failure the worker can report.
69-
*/
70-
function runToolOnWorker(request: WasmToolRequest): Promise<ToolResult> {
71-
return new Promise((resolve, reject) => {
72-
const worker = new Worker(new URL("./wasm-convert.worker.ts", import.meta.url), {
73-
type: "module",
74-
});
75-
worker.addEventListener("message", (event: MessageEvent<WasmToolResponse>) => {
76-
worker.terminate();
77-
if (event.data.ok) resolve(event.data.result);
78-
else reject(new Error(event.data.error || `${request.tool} failed.`));
79-
});
80-
worker.addEventListener("error", (event) => {
81-
worker.terminate();
82-
reject(new Error(event.message || `The ${request.tool} worker failed.`));
83-
});
84-
// `error` does not fire when a posted message cannot be deserialized, which
85-
// would otherwise leave this promise pending forever.
86-
worker.addEventListener("messageerror", () => {
87-
worker.terminate();
88-
reject(new Error(`The ${request.tool} worker posted an undeserializable message.`));
89-
});
90-
// The input files are structured-cloned rather than transferred: these
91-
// wrappers do not otherwise take ownership of the caller's bytes, and a
92-
// neutered input array would be a trap the sibling converters don't set.
93-
try {
94-
worker.postMessage(request);
95-
} catch (error) {
96-
// A throw here (e.g. DataCloneError) rejects the promise on its own, but
97-
// the worker is already spawned and would leak without this.
98-
worker.terminate();
99-
reject(error instanceof Error ? error : new Error(String(error)));
100-
}
101-
});
102-
}
103-
104-
/**
105-
* Run a tool off the main thread where Workers exist, inline where they do not
106-
* (node, tests). The inline path is why {@link initConvertTools} still takes an
107-
* explicit wasm source: a worker resolves its own bundled copy instead.
108-
*/
109-
async function runToolInBackground(request: WasmToolRequest): Promise<ToolResult> {
110-
if (typeof Worker === "undefined") {
111-
const { runTool } = await loadToolsModule();
112-
return runTool(request.tool, { args: request.args, input: request.input });
113-
}
114-
return runToolOnWorker(request);
115-
}
116-
11762
/** An input file for a WASM conversion: its name (the extension drives format
11863
* detection) and its raw bytes. */
11964
export interface WasmConvertFile {
@@ -295,7 +240,7 @@ export interface VectorToPmtilesOptions {
295240
* `siblings`, exactly as in {@link convertVectorWithWasm}.
296241
*
297242
* Unlike its siblings here this runs on a Web Worker (see
298-
* {@link runToolInBackground}). Tiling is by far the heaviest of these tools —
243+
* {@link runWasmToolInBackground}). Tiling is by far the heaviest of these tools —
299244
* a US-wide layer to the default zoom 14 is millions of tiles and minutes of
300245
* uninterrupted WASM — so running it on the main thread would freeze the UI for
301246
* the whole conversion. The others finish quickly enough not to warrant the
@@ -322,7 +267,7 @@ export async function tileVectorToPmtiles(
322267
]);
323268
const files: Record<string, Uint8Array> = { [input.name]: input.data };
324269
for (const sibling of siblings) files[sibling.name] = sibling.data;
325-
const result = await runToolInBackground({
270+
const result = await runWasmToolInBackground({
326271
tool: "vector_to_pmtiles",
327272
args,
328273
input: files,

packages/processing/src/wasm-convert.worker.ts

Lines changed: 0 additions & 40 deletions
This file was deleted.

0 commit comments

Comments
 (0)