Skip to content

Commit 5a0aa7c

Browse files
authored
Merge pull request #3190 from perspective-dev/viewer-css-update
Fix `viewer-charts` context sharing and misc bugs
2 parents 82eaa40 + 4508577 commit 5a0aa7c

117 files changed

Lines changed: 2485 additions & 1120 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/actions/install-deps/action.yaml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,17 @@ runs:
6464
with:
6565
cmake-version: "3.29.6"
6666

67+
- name: Setup MSVC environment (Windows)
68+
if: ${{ runner.os == 'Windows' }}
69+
uses: ilammy/msvc-dev-cmd@v1
70+
with:
71+
arch: ${{ inputs.arch == 'aarch64' && 'amd64_arm64' || 'x64' }}
72+
73+
- name: Force Ninja generator (Windows)
74+
if: ${{ runner.os == 'Windows' }}
75+
shell: bash
76+
run: echo "CMAKE_GENERATOR=Ninja" >> "$GITHUB_ENV"
77+
6778
- name: Install pnpm
6879
uses: pnpm/action-setup@v3
6980
with:

Cargo.lock

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

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
"llvm": "17.0.6",
1818
"pyodide": "0.29.3",
1919
"engines": {
20-
"node": ">=16 <23"
20+
"node": ">=16 <24"
2121
},
2222
"workspaces": [
2323
"tools/test",

packages/viewer-charts/src/ts/config.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,27 @@ export const RUNTIME_MODE: "worker" | "inprocess" = "worker";
3333
* `ImageBitmap` over the control channel; the host blits the bitmap
3434
* into the visible canvas via `drawImage`.
3535
*/
36-
export const RENDER_BLIT_MODE: "direct" | "blit" = "direct";
36+
export const RENDER_BLIT_MODE: "direct" | "blit" = "blit";
37+
38+
/**
39+
* Number of shared WebGL contexts in pooled blit mode.
40+
*
41+
* The browser caps live contexts per agent (~16 in Chromium) and
42+
* force-loses the oldest past that cap, so a page with more charts than
43+
* the cap cannot give each its own context. When `> 0` *and*
44+
* `RENDER_BLIT_MODE === "blit"`, every chart borrows one of this many
45+
* shared contexts (round-robin, sticky for the chart's lifetime) instead
46+
* of allocating its own — decoupling live-context count from chart
47+
* count. N charts render through K = this many contexts; the scheduler
48+
* serializes renders that land on the same context.
49+
*
50+
* `0` disables pooling (every chart gets its own context — the original
51+
* behavior). Pooling never applies to `"direct"` mode, which renders
52+
* into the host's transferred visible canvas and is permanently 1:1 with
53+
* a context; keep pages that need more than ~16 simultaneous charts on
54+
* `"blit"`.
55+
*/
56+
export const RENDER_CONTEXT_POOL_SIZE: number = 4;
3757

3858
/**
3959
* Strict-mode validation for `BufferPool.upload`.

packages/viewer-charts/src/ts/plugin/plugin.ts

Lines changed: 76 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,22 @@ const GLOBAL_STYLES = (() => {
125125
return [sheet];
126126
})();
127127

128+
/**
129+
* Process-global GL presentation strategy, shared by *every* chart-type
130+
* plugin in this renderer. Seeded from the build-time {@link RENDER_BLIT_MODE}
131+
* and overridable at runtime via
132+
* {@link HTMLPerspectiveViewerWebGLPluginElement.setBlitMode}.
133+
*
134+
* Module scope (not per-instance) on purpose: blit-vs-direct is a
135+
* whole-renderer decision — in `"blit"` mode the worker shares a pool of
136+
* GL contexts across all charts ([webgl/context-pool.ts]), so a page
137+
* can't sensibly mix strategies per chart. Every per-chart-type subclass
138+
* registered in [index.ts] reads this one value when it builds its
139+
* renderer, so setting it once (before the first chart renders) applies
140+
* to all of them.
141+
*/
142+
let BLIT_MODE: "direct" | "blit" = RENDER_BLIT_MODE;
143+
128144
export class HTMLPerspectiveViewerWebGLPluginElement
129145
extends HTMLElement
130146
implements IPerspectiveViewerPlugin
@@ -139,7 +155,6 @@ export class HTMLPerspectiveViewerWebGLPluginElement
139155
private _rendererPromise: Promise<RendererTransport> | null = null;
140156
private _rawEventForwarder: RawEventForwarder | null = null;
141157
private _generation = 0;
142-
private _renderBlitMode: "direct" | "blit" = RENDER_BLIT_MODE;
143158
private _resetClickAbort: AbortController | null = null;
144159

145160
/**
@@ -257,13 +272,41 @@ export class HTMLPerspectiveViewerWebGLPluginElement
257272
return this._rendererPromise;
258273
}
259274

260-
this._rendererPromise = this._buildRenderer(view).then((r) => {
261-
this._renderer = r;
262-
this._setupInteraction(r);
263-
return r;
264-
});
275+
// `_buildRenderer` is async — it awaits `getClient`, `getTable`,
276+
// and a full worker handshake — so a `disconnectedCallback`
277+
// (toggle / chart-type switch) frequently lands *before* this
278+
// `.then` runs. At that point `this._renderer` is still null,
279+
// so `delete()`'s `if (this._renderer)` teardown is skipped and
280+
// the freshly-built transport (and its WebGL context) would be
281+
// assigned to a detached element and leak — one context per
282+
// raced toggle, until the browser evicts the oldest.
283+
//
284+
// `delete()` sets `_rendererPromise = null`, so promise identity
285+
// is the dispose signal: if `this._rendererPromise` no longer
286+
// points at *this* build when it resolves, the element was
287+
// deleted (or a reconnect started a newer build) and we destroy
288+
// the orphan instead of adopting it. Promise identity — not
289+
// `_generation`, which every `draw`/`update` bumps — is the
290+
// right token: a rapid draw→update must NOT tear down the
291+
// single in-flight build it shares via this memoized promise.
292+
const p: Promise<RendererTransport> = this._buildRenderer(view).then(
293+
(r) => {
294+
if (this._rendererPromise !== p) {
295+
r.destroy();
296+
throw new Error("renderer disposed during init");
297+
}
298+
299+
this._renderer = r;
300+
this._setupInteraction(r);
301+
return r;
302+
},
303+
);
265304

266-
return this._rendererPromise;
305+
// Swallow the dispose rejection so it doesn't surface as an
306+
// unhandled rejection; `_drawImpl` catches it and bails.
307+
p.catch(() => {});
308+
this._rendererPromise = p;
309+
return p;
267310
}
268311

269312
/**
@@ -350,15 +393,28 @@ export class HTMLPerspectiveViewerWebGLPluginElement
350393
},
351394
pluginConfig: this._pluginConfig,
352395
defaultChartType: this._chartType.default_chart_type,
353-
renderBlitMode: this._renderBlitMode,
396+
renderBlitMode: BLIT_MODE,
354397
});
355398

356399
return transport;
357400
}
358401

359-
setBlitMode(mode: "direct" | "blit") {
360-
console.assert(this._initialized, "Already initialized");
361-
this._renderBlitMode = mode;
402+
/**
403+
* Select the GL presentation strategy for *all* chart-type plugins
404+
* in this renderer. Static + process-global: the value is shared by
405+
* every per-chart-type subclass, so it must be set once before the
406+
* charts that should use it build their renderers (a renderer reads
407+
* {@link BLIT_MODE} at construction in `_buildRenderer`; charts
408+
* already built keep their mode until torn down and rebuilt).
409+
*
410+
* - `"direct"` — each chart owns a GL context 1:1 with its visible
411+
* canvas (lowest latency; bounded by the browser's ~16-context
412+
* cap).
413+
* - `"blit"` — charts render off-screen and share a pool of GL
414+
* contexts ([webgl/context-pool.ts]), so a page can exceed the cap.
415+
*/
416+
static setBlitMode(mode: "direct" | "blit") {
417+
BLIT_MODE = mode;
362418
}
363419

364420
get_static_config(): PluginStaticConfig {
@@ -499,7 +555,15 @@ export class HTMLPerspectiveViewerWebGLPluginElement
499555

500556
private async _drawImpl(view: View): Promise<void> {
501557
const gen = ++this._generation;
502-
const renderer = await this._ensureRenderer(view);
558+
let renderer: RendererTransport;
559+
try {
560+
renderer = await this._ensureRenderer(view);
561+
} catch {
562+
// Renderer was disposed mid-init (element disconnected
563+
// during the async build) — nothing to draw.
564+
return;
565+
}
566+
503567
if (this._generation !== gen) {
504568
return;
505569
}

0 commit comments

Comments
 (0)