From 595cdbc4eabcc70be868c71cbe3e133a89df5de1 Mon Sep 17 00:00:00 2001 From: Andrew Stein Date: Mon, 25 May 2026 14:32:35 -0400 Subject: [PATCH 1/2] Add docs deploy script Signed-off-by: Andrew Stein --- docs/deploy.mjs | 68 +++++++++++++++++++++++++++++ docs/package.json | 1 + packages/viewer-charts/package.json | 2 +- tools/scripts/publish.mjs | 45 +++++++++++++++---- 4 files changed, 107 insertions(+), 9 deletions(-) create mode 100644 docs/deploy.mjs diff --git a/docs/deploy.mjs b/docs/deploy.mjs new file mode 100644 index 0000000000..bc78605f0a --- /dev/null +++ b/docs/deploy.mjs @@ -0,0 +1,68 @@ +// ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ +// ┃ ██████ ██████ ██████ █ █ █ █ █ █▄ ▀███ █ ┃ +// ┃ ▄▄▄▄▄█ █▄▄▄▄▄ ▄▄▄▄▄█ ▀▀▀▀▀█▀▀▀▀▀ █ ▀▀▀▀▀█ ████████▌▐███ ███▄ ▀█ █ ▀▀▀▀▀ ┃ +// ┃ █▀▀▀▀▀ █▀▀▀▀▀ █▀██▀▀ ▄▄▄▄▄ █ ▄▄▄▄▄█ ▄▄▄▄▄█ ████████▌▐███ █████▄ █ ▄▄▄▄▄ ┃ +// ┃ █ ██████ █ ▀█▄ █ ██████ █ ███▌▐███ ███████▄ █ ┃ +// ┣━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫ +// ┃ Copyright (c) 2017, the Perspective Authors. ┃ +// ┃ ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌ ┃ +// ┃ This file is part of the Perspective library, distributed under the terms ┃ +// ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃ +// ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ + +import * as fs from "node:fs"; +import * as path from "node:path"; +import { execFileSync } from "node:child_process"; +import { fileURLToPath } from "node:url"; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const REPO_ROOT = path.resolve(__dirname, ".."); +const DIST = path.join(__dirname, "dist"); +const STAGING = path.join(REPO_ROOT, "dist-gh-pages"); +const BRANCH = "gh-pages"; + +function git(args, opts = {}) { + return execFileSync("git", args, { + stdio: "inherit", + cwd: REPO_ROOT, + ...opts, + }); +} + +function copyRecursive(src, dest) { + const stat = fs.statSync(src); + if (stat.isDirectory()) { + fs.mkdirSync(dest, { recursive: true }); + for (const child of fs.readdirSync(src)) { + copyRecursive(path.join(src, child), path.join(dest, child)); + } + } else { + fs.copyFileSync(src, dest); + } +} + +if (!fs.existsSync(DIST)) { + console.error(`Missing ${DIST} — run \`npm run build\` first.`); + process.exit(1); +} + +if (!fs.existsSync(STAGING)) { + git(["worktree", "add", STAGING, BRANCH]); +} else { + git(["fetch", "origin", BRANCH]); + git(["checkout", `origin/${BRANCH}`], { cwd: STAGING }); +} + +// Clear tracked + untracked content in the staging worktree, preserving +// the worktree's `.git` link. +git(["rm", "-rf", "--quiet", "--ignore-unmatch", "."], { cwd: STAGING }); +git(["clean", "-fdx"], { cwd: STAGING }); + +for (const entry of fs.readdirSync(DIST)) { + copyRecursive(path.join(DIST, entry), path.join(STAGING, entry)); +} + +git(["add", "-A"], { cwd: STAGING }); + +console.log(`Staged dist/ onto ${BRANCH} at ${STAGING}`); +console.log(`Review with \`git -C ${STAGING} status\`, then commit and push.`); diff --git a/docs/package.json b/docs/package.json index a714f7b125..276fe03049 100644 --- a/docs/package.json +++ b/docs/package.json @@ -8,6 +8,7 @@ "start": "node server.mjs", "serve": "node server.mjs", "clean": "rm -rf dist", + "deploy": "node deploy.mjs", "mdbook": "docker compose run --rm mdbook build" }, "dependencies": { diff --git a/packages/viewer-charts/package.json b/packages/viewer-charts/package.json index 14a8205be3..f368efae85 100644 --- a/packages/viewer-charts/package.json +++ b/packages/viewer-charts/package.json @@ -1,6 +1,6 @@ { "name": "@perspective-dev/viewer-charts", - "version": "4.3.0", + "version": "4.5.0", "description": "Perspective.js WebGL Plugin", "unpkg": "./dist/cdn/perspective-viewer-charts.js", "jsdelivr": "./dist/cdn/perspective-viewer-charts.js", diff --git a/tools/scripts/publish.mjs b/tools/scripts/publish.mjs index 22b1f7a2b9..8b0d893eb4 100644 --- a/tools/scripts/publish.mjs +++ b/tools/scripts/publish.mjs @@ -12,6 +12,7 @@ import { Octokit } from "octokit"; import fs from "node:fs/promises"; +import { execSync } from "child_process"; import "zx/globals"; @@ -62,6 +63,11 @@ async function download_release_assets(releases) { ); } +const SH_ENV = { + env: process.env, + stdio: "inherit", +}; + async function publish_release_assets(releases) { if (process.env.COMMIT) { for (const release of releases) { @@ -70,9 +76,9 @@ async function publish_release_assets(releases) { release.name.endsWith("tar.gz")) && release.name.indexOf("wasm") === -1 ) { - $.sync`twine upload ${release.name}`; + execSync(`twine upload ${release.name}`, SH_ENV); } else if (release.name.endsWith(".tgz")) { - $.sync`npm publish ${release.name}`; + execSync(`npm publish ${release.name}`, SH_ENV); } else { console.log(`Skipping "${release.name}"`); } @@ -80,12 +86,35 @@ async function publish_release_assets(releases) { await $`mkdir -p rust/target/package && mv *.crate rust/target/package`; - await $`cargo publish -p perspective-server --allow-dirty --no-verify`; - await $`cargo publish -p perspective-client --allow-dirty --no-verify`; - await $`cargo publish -p perspective-python --allow-dirty --no-verify`; - await $`cargo publish -p perspective-js --allow-dirty --no-verify`; - await $`cargo publish -p perspective-viewer --allow-dirty --no-verify`; - await $`cargo publish -p perspective --allow-dirty --no-verify`; + execSync( + `cargo publish -p perspective-server --allow-dirty --no-verify`, + SH_ENV, + ); + + execSync( + `cargo publish -p perspective-client --allow-dirty --no-verify`, + SH_ENV, + ); + + execSync( + `cargo publish -p perspective-python --allow-dirty --no-verify`, + SH_ENV, + ); + + execSync( + `cargo publish -p perspective-js --allow-dirty --no-verify`, + SH_ENV, + ); + + execSync( + `cargo publish -p perspective-viewer --allow-dirty --no-verify`, + SH_ENV, + ); + + execSync( + `cargo publish -p perspective --allow-dirty --no-verify`, + SH_ENV, + ); } else { console.warn(`COMMIT not specified, aborting`); } From 46df710a3807576688747fa4f19370cb5baadb7c Mon Sep 17 00:00:00 2001 From: Andrew Stein Date: Mon, 25 May 2026 14:34:06 -0400 Subject: [PATCH 2/2] Fix `columns_config` and `plugin_config` bugs Signed-off-by: Andrew Stein --- .github/workflows/build.yaml | 2 +- .../src/ts/axis/categorical-axis.ts | 2 - .../charts/candlestick/candlestick-render.ts | 9 +- .../src/ts/charts/candlestick/candlestick.ts | 39 +-- .../viewer-charts/src/ts/charts/chart-base.ts | 26 +- .../src/ts/charts/common/expand-domain.ts | 40 +++ .../src/ts/charts/common/tree-chart.ts | 16 ++ .../src/ts/charts/common/tree-chrome.ts | 87 +++++- .../src/ts/charts/series/series-render.ts | 103 ++++++- .../src/ts/charts/series/series.ts | 70 ++--- .../src/ts/charts/sunburst/sunburst-render.ts | 92 +------ .../src/ts/charts/sunburst/sunburst.ts | 16 +- .../src/ts/charts/treemap/treemap-render.ts | 50 +--- .../src/ts/charts/treemap/treemap.ts | 17 +- .../src/ts/interaction/host-sink-message.ts | 55 ++-- .../src/ts/interaction/raw-event-forwarder.ts | 22 +- .../src/ts/interaction/zoom-controller.ts | 203 ++++++++------ .../src/ts/interaction/zoom-router.ts | 129 +-------- .../viewer-charts/src/ts/plugin/plugin.ts | 258 +++++------------- .../viewer-charts/src/ts/theme/palette.ts | 5 +- .../src/ts/transport/protocol.ts | 11 +- .../viewer-charts/src/ts/worker/dispatch.ts | 1 - .../src/ts/worker/renderer.worker.ts | 56 +--- .../test/ts/plugin-config-toggle.spec.ts | 100 +++++++ .../test/ts/snapshot/zoom.spec.ts | 57 ++++ .../test/js/column_style.spec.js | 161 ++++++++++- .../src/rust/components/viewer.rs | 11 +- rust/perspective-viewer/src/rust/renderer.rs | 80 +++++- .../src/rust/tasks/restore_and_render.rs | 35 ++- .../src/rust/tasks/send_column_config.rs | 5 +- .../src/rust/tasks/send_plugin_config.rs | 5 +- .../src/rust/tasks/update_and_render.rs | 15 +- tools/test/src/js/snapshot-sync.ts | 30 +- tools/test/src/js/utils.ts | 10 +- 34 files changed, 1033 insertions(+), 785 deletions(-) create mode 100644 packages/viewer-charts/src/ts/charts/common/expand-domain.ts create mode 100644 packages/viewer-charts/test/ts/plugin-config-toggle.spec.ts diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index a7ede6e99d..09e85ce380 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -28,7 +28,7 @@ on: - docs/ - examples/ - rust/perspective-python/README.md - pull_request_target: + pull_request: branches: - master workflow_dispatch: diff --git a/packages/viewer-charts/src/ts/axis/categorical-axis.ts b/packages/viewer-charts/src/ts/axis/categorical-axis.ts index 2d847194fe..f2cfccd7c7 100644 --- a/packages/viewer-charts/src/ts/axis/categorical-axis.ts +++ b/packages/viewer-charts/src/ts/axis/categorical-axis.ts @@ -56,8 +56,6 @@ function categoryIndexToPixelY(layout: PlotLayout, index: number): number { return layout.dataToPixel(0, index).py; } -export const categoryIndexToPixel = categoryIndexToPixelX; - function leafLevelLayout( numRows: number, longestCharCount: number, diff --git a/packages/viewer-charts/src/ts/charts/candlestick/candlestick-render.ts b/packages/viewer-charts/src/ts/charts/candlestick/candlestick-render.ts index 64370db125..a555d31a76 100644 --- a/packages/viewer-charts/src/ts/charts/candlestick/candlestick-render.ts +++ b/packages/viewer-charts/src/ts/charts/candlestick/candlestick-render.ts @@ -125,13 +125,14 @@ export function renderCandlestickFrame( yMax: chart._yDomain.max, }; - // Auto-fit the price axis to the visible X window. Skipped at - // default zoom (the refit equals `_yDomain` there and would only - // churn baselines). + // Auto-fit the price axis to the visible X window. Skipped when X + // is at default zoom (the refit equals `_yDomain` there and would + // only churn baselines) — Y-axis pan/zoom alone shouldn't trigger + // an X-window refit. if ( chart._autoFitValue && chart._zoomController && - !chart._zoomController.isDefault() + !chart._zoomController.isXDefault() ) { const fit = computeVisibleCandleExtent(chart, vis.xMin, vis.xMax); if (fit.hasFit) { diff --git a/packages/viewer-charts/src/ts/charts/candlestick/candlestick.ts b/packages/viewer-charts/src/ts/charts/candlestick/candlestick.ts index 566f18d52a..782f0151bb 100644 --- a/packages/viewer-charts/src/ts/charts/candlestick/candlestick.ts +++ b/packages/viewer-charts/src/ts/charts/candlestick/candlestick.ts @@ -33,6 +33,7 @@ import { } from "./candlestick-interact"; import { BodyWickGlyph } from "./glyphs/draw-candlesticks"; import { OHLCGlyph } from "./glyphs/draw-ohlc"; +import { expandDomainInPlace } from "../common/expand-domain"; /** * Per-frame memo of the auto-fit Y extent for a {@link CandlestickChart}, @@ -266,38 +267,18 @@ export class CandlestickChart extends CategoricalYChart { scratchCandles: this._candles, }); // `domain_mode: "expand"` post-build union — mirrors the series - // pipeline. Mutate the pipeline result in place so the + // pipeline. `expandDomainInPlace` mutates `result.*` so the // assignments below pick up the grown extent automatically. if (this._pluginConfig.domain_mode === "expand") { - if (this._expandedYDomain) { - result.yDomain.min = Math.min( - this._expandedYDomain.min, - result.yDomain.min, - ); - result.yDomain.max = Math.max( - this._expandedYDomain.max, - result.yDomain.max, - ); - } - - this._expandedYDomain = { ...result.yDomain }; - + this._expandedYDomain = expandDomainInPlace( + this._expandedYDomain, + result.yDomain, + ); if (result.numericCategoryDomain) { - if (this._expandedCategoryDomain) { - result.numericCategoryDomain.min = Math.min( - this._expandedCategoryDomain.min, - result.numericCategoryDomain.min, - ); - result.numericCategoryDomain.max = Math.max( - this._expandedCategoryDomain.max, - result.numericCategoryDomain.max, - ); - } - - this._expandedCategoryDomain = { - min: result.numericCategoryDomain.min, - max: result.numericCategoryDomain.max, - }; + this._expandedCategoryDomain = expandDomainInPlace( + this._expandedCategoryDomain, + result.numericCategoryDomain, + ); } } else { this._expandedYDomain = null; diff --git a/packages/viewer-charts/src/ts/charts/chart-base.ts b/packages/viewer-charts/src/ts/charts/chart-base.ts index 064c4fd58a..f21742386d 100644 --- a/packages/viewer-charts/src/ts/charts/chart-base.ts +++ b/packages/viewer-charts/src/ts/charts/chart-base.ts @@ -46,6 +46,14 @@ import type { ViewConfig } from "@perspective-dev/client"; import { resolveThemeFromVars, type Theme } from "../theme/theme"; import { requestRender as scheduleRender } from "../render/scheduler"; +// TODO I don't know if this is the behavior we want. On the plus side, this +// ad-hoc formatter scales well to small and large data ranges, making a good +// guess at the right format without user input. On the minus side, this +// behavior is inconsistent with datagrid and the rest of the app, and the ad-hoc +// surprising behavior when overriding one field in `number_format` and suddenly +// the entire formatter is replaced. +const REGRESSION_BEHAVIOR = true; + /** * Locale-aware fallback formatter applied to numeric tooltip / legend * values when the column has no `number_format` configured. Two @@ -55,9 +63,12 @@ import { requestRender as scheduleRender } from "../render/scheduler"; const DEFAULT_VALUE_FORMATTER: (v: number) => string = ((): (( v: number, ) => string) => { - return formatTickValue; - // const intl = createNumberFormatter("float"); - // return (v) => intl.format(v); + if (REGRESSION_BEHAVIOR) { + return formatTickValue; + } else { + const intl = createNumberFormatter("float"); + return (v) => intl.format(v); + } })(); /** @@ -69,9 +80,12 @@ const DEFAULT_VALUE_FORMATTER: (v: number) => string = ((): (( const DEFAULT_DATETIME_FORMATTER: (v: number) => string = ((): (( v: number, ) => string) => { - return formatDateTickValue; - // const intl = createDatetimeFormatter(); - // return (v) => intl.format(v); + if (REGRESSION_BEHAVIOR) { + return formatDateTickValue; + } else { + const intl = createDatetimeFormatter(); + return (v) => intl.format(v); + } })(); /** diff --git a/packages/viewer-charts/src/ts/charts/common/expand-domain.ts b/packages/viewer-charts/src/ts/charts/common/expand-domain.ts new file mode 100644 index 0000000000..7aabe8eded --- /dev/null +++ b/packages/viewer-charts/src/ts/charts/common/expand-domain.ts @@ -0,0 +1,40 @@ +// ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ +// ┃ ██████ ██████ ██████ █ █ █ █ █ █▄ ▀███ █ ┃ +// ┃ ▄▄▄▄▄█ █▄▄▄▄▄ ▄▄▄▄▄█ ▀▀▀▀▀█▀▀▀▀▀ █ ▀▀▀▀▀█ ████████▌▐███ ███▄ ▀█ █ ▀▀▀▀▀ ┃ +// ┃ █▀▀▀▀▀ █▀▀▀▀▀ █▀██▀▀ ▄▄▄▄▄ █ ▄▄▄▄▄█ ▄▄▄▄▄█ ████████▌▐███ █████▄ █ ▄▄▄▄▄ ┃ +// ┃ █ ██████ █ ▀█▄ █ ██████ █ ███▌▐███ ███████▄ █ ┃ +// ┣━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫ +// ┃ Copyright (c) 2017, the Perspective Authors. ┃ +// ┃ ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌ ┃ +// ┃ This file is part of the Perspective library, distributed under the terms ┃ +// ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃ +// ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ + +/** + * Numeric extent — used by series + candlestick build pipelines for + * value / category axis domains. + */ +export interface Domain { + min: number; + max: number; +} + +/** + * Union `next` (a freshly-computed extent) with `prev` (the prior + * accumulator) IN PLACE on `next`, then return a fresh copy to store + * back as the new accumulator. Idempotent when `prev` is null — `next` + * is left untouched. + * + * Used by the `domain_mode: "expand"` mirror-back step in the series / + * candlestick / cartesian build pipelines: mutating `next` in place + * means every downstream assignment that reads from the pipeline + * result struct automatically picks up the grown extent. + */ +export function expandDomainInPlace(prev: Domain | null, next: Domain): Domain { + if (prev) { + next.min = Math.min(prev.min, next.min); + next.max = Math.max(prev.max, next.max); + } + + return { min: next.min, max: next.max }; +} diff --git a/packages/viewer-charts/src/ts/charts/common/tree-chart.ts b/packages/viewer-charts/src/ts/charts/common/tree-chart.ts index fad31c3b1c..40467afdda 100644 --- a/packages/viewer-charts/src/ts/charts/common/tree-chart.ts +++ b/packages/viewer-charts/src/ts/charts/common/tree-chart.ts @@ -11,9 +11,25 @@ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ import { AbstractChart } from "../chart-base"; +import type { ColumnDataMap } from "../../data/view-reader"; import { NodeStore, NULL_NODE } from "./node-store"; import { LazyTooltip } from "../../interaction/lazy-tooltip"; +/** + * Sentinel fallback for the Size slot when the user hasn't picked one: + * use the first non-metadata column in the incoming view. Tree charts + * still need *some* numeric-ish column to size geometry. + */ +export function firstNonMetadataColumn(columns: ColumnDataMap): string { + for (const k of columns.keys()) { + if (!k.startsWith("__")) { + return k; + } + } + + return ""; +} + /** * Shared state for hierarchical charts (treemap, sunburst). Holds the * tree store + streaming-insert scaffolding + per-row tooltip data diff --git a/packages/viewer-charts/src/ts/charts/common/tree-chrome.ts b/packages/viewer-charts/src/ts/charts/common/tree-chrome.ts index 05a302bcac..996268aa53 100644 --- a/packages/viewer-charts/src/ts/charts/common/tree-chrome.ts +++ b/packages/viewer-charts/src/ts/charts/common/tree-chrome.ts @@ -10,7 +10,17 @@ // ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ -import type { Context2D } from "../canvas-types"; +import type { Canvas2D, Context2D } from "../canvas-types"; +import type { PlotRect } from "../../layout/plot-layout"; +import { PlotLayout } from "../../layout/plot-layout"; +import type { GradientStop } from "../../theme/gradient"; +import type { Vec3 } from "../../theme/palette"; +import type { Theme } from "../../theme/theme"; +import { + renderCategoricalLegend, + renderCategoricalLegendAt, + renderLegend, +} from "../../axis/legend"; import type { TreeChartBase } from "./tree-chart"; import { drawTooltipBox } from "./draw-tooltip-box"; @@ -121,3 +131,78 @@ export function renderTreeTooltip( fontFamily, ); } + +/** + * Paint a color legend (categorical swatches or numeric gradient bar) + * for a tree chart. Shared by sunburst + treemap; both consult + * `_colorMode` / `_uniqueColorLabels.size` / `_colorMin..max` the same + * way. + * + * `categoricalRect`, when non-null, is used as the explicit rect for + * the categorical-swatch variant (sunburst's faceted mode passes + * `FacetGrid.legendRect` here). Numeric mode always derives from a + * synthetic single-plot `PlotLayout` to match the legacy per-chart + * branch — its gradient bar's vertical span doesn't fit the + * categorical legend's compact rect. + * + * Returns silently when the color slot is empty, when categorical mode + * has only one label, or when numeric mode has a degenerate + * (`min >= max`) extent. + */ +export function renderTreeColorLegend( + chart: TreeChartBase, + canvas: Canvas2D, + palette: Vec3[], + stops: GradientStop[], + theme: Theme, + cssWidth: number, + cssHeight: number, + categoricalRect: PlotRect | null = null, +): void { + if (chart._colorMode === "series" && chart._uniqueColorLabels.size > 1) { + if (categoricalRect) { + renderCategoricalLegendAt( + canvas, + categoricalRect, + chart._uniqueColorLabels, + palette, + theme, + ); + } else { + renderCategoricalLegend( + canvas, + syntheticLegendLayout(cssWidth, cssHeight), + chart._uniqueColorLabels, + palette, + theme, + ); + } + } else if ( + chart._colorMode === "numeric" && + chart._colorMin < chart._colorMax + ) { + renderLegend( + canvas, + syntheticLegendLayout(cssWidth, cssHeight), + { + min: chart._colorMin, + max: chart._colorMax, + label: chart._colorName, + }, + stops, + theme, + chart.getColumnFormatter(chart._colorName, "value"), + ); + } +} + +function syntheticLegendLayout( + cssWidth: number, + cssHeight: number, +): PlotLayout { + return new PlotLayout(cssWidth, cssHeight, { + hasXLabel: false, + hasYLabel: false, + hasLegend: true, + }); +} diff --git a/packages/viewer-charts/src/ts/charts/series/series-render.ts b/packages/viewer-charts/src/ts/charts/series/series-render.ts index e71c0b9d89..961813764b 100644 --- a/packages/viewer-charts/src/ts/charts/series/series-render.ts +++ b/packages/viewer-charts/src/ts/charts/series/series-render.ts @@ -350,14 +350,15 @@ export function renderBarFrame( } // Auto-fit the value axis to the visible categorical window. Gated - // on `_autoFitValue` + non-default zoom: at default zoom the refit - // result always equals `_leftDomain`/`_rightDomain`, so walking - // would be wasted work (and would shift test baselines). - if ( - chart._autoFitValue && - chart._zoomController && - !chart._zoomController.isDefault() - ) { + // on `_autoFitValue` + the categorical axis being non-default: the + // refit only narrows when the categorical axis is itself zoomed + // (otherwise the visible window equals the data extent and the + // refit collapses back to `_leftDomain`/`_rightDomain`). Vertical + // charts put the category on X; horizontal charts put it on Y. + const catNonDefault = horizontal + ? !chart._zoomController?.isYDefault() + : !chart._zoomController?.isXDefault(); + if (chart._autoFitValue && chart._zoomController && catNonDefault) { const fit = computeVisibleValueExtent(chart, visCatMin, visCatMax); if (fit.hasLeft) { visValMin = fit.leftMin; @@ -880,11 +881,34 @@ function computeVisibleValueExtent( let hasRight = false; if (buckets.n > 0) { - // Clamp to the populated [0, n-1] range. `visCat*` is in - // continuous coords (numeric or category index space), so - // floor/ceil to integer bucket indices. - const lo = Math.max(0, Math.floor(visCatMin)); - const hi = Math.min(buckets.n - 1, Math.ceil(visCatMax)); + // Resolve the visible catIdx range. Category mode: `visCat*` are + // already in catIdx space, so floor/ceil into `[0, n-1]`. + // Numeric mode (`date | datetime | integer | float` group_by): + // `visCat*` are absolute data values from the zoom controller's + // visible domain — for a datetime axis they're ~1.7e12-magnitude + // timestamps. A blind `Math.floor(visCatMin)` of that gives `lo + // ≫ n`, the loop body never executes, and the value-axis refit + // silently no-ops (chart looks the same horizontally-zoomed as + // unzoomed). Map the data range back to catIdx via the sorted + // `_categoryPositions`. See [series-interact.ts:239-250] for the + // parallel hit-test branch. + const positions = chart._categoryPositions; + let lo: number; + let hi: number; + if (positions) { + const r = mapDomainToCatRange( + positions, + buckets.n, + visCatMin, + visCatMax, + ); + lo = r.lo; + hi = r.hi; + } else { + lo = Math.max(0, Math.floor(visCatMin)); + hi = Math.min(buckets.n - 1, Math.ceil(visCatMax)); + } + const lMin = buckets.leftMin; const lMax = buckets.leftMax; const rMin = buckets.rightMin; @@ -937,6 +961,59 @@ function computeVisibleValueExtent( return next; } +/** + * Map a numeric visible domain `[visMin, visMax]` to the inclusive catIdx + * range `[lo, hi]` that intersects it, using a sorted `categoryPositions` + * vector (ASC, per the pivot order). Returns an empty range (`lo > hi`) + * when the domain misses every category. + * + * Edges are expanded by one catIdx on each side so a category whose + * center sits just outside the visible window — but whose band-half + * still overlaps it — still contributes to the auto-fit extent. + */ +function mapDomainToCatRange( + positions: Float64Array, + n: number, + visMin: number, + visMax: number, +): { lo: number; hi: number } { + if (n === 0 || visMin > visMax) { + return { lo: 0, hi: -1 }; + } + + // Lower bound: smallest idx where positions[idx] >= visMin. + let l = 0; + let r = n; + while (l < r) { + const m = (l + r) >>> 1; + if (positions[m] < visMin) { + l = m + 1; + } else { + r = m; + } + } + + const lo = Math.max(0, l - 1); + + // Upper bound: smallest idx where positions[idx] > visMax (`l` after + // loop). `l` itself is one past the last in-range catIdx, so the + // inclusive `hi` for an exactly-overlapping band is `l - 1`; the + // `+1`-then-clamp expands by one to capture partial-overlap bands. + l = 0; + r = n; + while (l < r) { + const m = (l + r) >>> 1; + if (positions[m] <= visMax) { + l = m + 1; + } else { + r = m; + } + } + + const hi = Math.min(n - 1, l); + return { lo, hi }; +} + function newSeriesAutoFitCache(): SeriesAutoFitCache { return { catMin: 0, diff --git a/packages/viewer-charts/src/ts/charts/series/series.ts b/packages/viewer-charts/src/ts/charts/series/series.ts index 63bba2faaf..0c6998fa4f 100644 --- a/packages/viewer-charts/src/ts/charts/series/series.ts +++ b/packages/viewer-charts/src/ts/charts/series/series.ts @@ -41,6 +41,8 @@ import { resolvePalette } from "../../theme/palette"; import { LineGlyph } from "./glyphs/draw-lines"; import { ScatterGlyph } from "./glyphs/draw-scatter"; import { AreaGlyph } from "./glyphs/draw-areas"; +import { createQuadCornerBuffer } from "../../webgl/instanced-attrs"; +import { expandDomainInPlace } from "../common/expand-domain"; import barVert from "../../shaders/bar.vert.glsl"; import barFrag from "../../shaders/bar.frag.glsl"; @@ -451,13 +453,7 @@ export class SeriesChart extends CategoricalYChart { a_axis: gl.getAttribLocation(p, "a_axis"), }; - this._cornerBuffer = gl.createBuffer()!; - gl.bindBuffer(gl.ARRAY_BUFFER, this._cornerBuffer); - gl.bufferData( - gl.ARRAY_BUFFER, - new Float32Array([0, 0, 1, 0, 0, 1, 1, 1]), - gl.STATIC_DRAW, - ); + this._cornerBuffer = createQuadCornerBuffer(gl); } const result = buildSeriesPipeline({ @@ -483,57 +479,29 @@ export class SeriesChart extends CategoricalYChart { scratchNegStack: this._negStackScratch, }); - // `domain_mode: "expand"` post-build union. Mutate the pipeline - // result struct in place so every downstream assignment below - // (`_leftDomain`, `_rightDomain`, `_numericCategoryDomain`, - // `_categoryOrigin`) automatically picks up the grown extent. + // `domain_mode: "expand"` post-build union. Each call mutates + // the pipeline result in place so the downstream assignments + // below (`_leftDomain`, `_rightDomain`, `_numericCategoryDomain`, + // `_categoryOrigin`) automatically pick up the grown extent. // `"fit"` (or a fresh reset) leaves the result untouched and // clears the accumulators so the next toggle starts fresh. if (this._pluginConfig.domain_mode === "expand") { - if (this._expandedLeftDomain) { - result.leftDomain.min = Math.min( - this._expandedLeftDomain.min, - result.leftDomain.min, - ); - result.leftDomain.max = Math.max( - this._expandedLeftDomain.max, - result.leftDomain.max, - ); - } - - this._expandedLeftDomain = { ...result.leftDomain }; - + this._expandedLeftDomain = expandDomainInPlace( + this._expandedLeftDomain, + result.leftDomain, + ); if (result.rightDomain) { - if (this._expandedRightDomain) { - result.rightDomain.min = Math.min( - this._expandedRightDomain.min, - result.rightDomain.min, - ); - result.rightDomain.max = Math.max( - this._expandedRightDomain.max, - result.rightDomain.max, - ); - } - - this._expandedRightDomain = { ...result.rightDomain }; + this._expandedRightDomain = expandDomainInPlace( + this._expandedRightDomain, + result.rightDomain, + ); } if (result.numericCategoryDomain) { - if (this._expandedCategoryDomain) { - result.numericCategoryDomain.min = Math.min( - this._expandedCategoryDomain.min, - result.numericCategoryDomain.min, - ); - result.numericCategoryDomain.max = Math.max( - this._expandedCategoryDomain.max, - result.numericCategoryDomain.max, - ); - } - - this._expandedCategoryDomain = { - min: result.numericCategoryDomain.min, - max: result.numericCategoryDomain.max, - }; + this._expandedCategoryDomain = expandDomainInPlace( + this._expandedCategoryDomain, + result.numericCategoryDomain, + ); } } else { this._expandedLeftDomain = null; diff --git a/packages/viewer-charts/src/ts/charts/sunburst/sunburst-render.ts b/packages/viewer-charts/src/ts/charts/sunburst/sunburst-render.ts index 111fba712f..4fef2b540c 100644 --- a/packages/viewer-charts/src/ts/charts/sunburst/sunburst-render.ts +++ b/packages/viewer-charts/src/ts/charts/sunburst/sunburst-render.ts @@ -16,8 +16,6 @@ import type { SunburstChart } from "./sunburst"; import { NULL_NODE } from "../common/node-store"; import { resolvePalette, type Vec3 } from "../../theme/palette"; import { type GradientStop } from "../../theme/gradient"; -import { renderLegend, renderCategoricalLegend } from "../../axis/legend"; -import { PlotLayout } from "../../layout/plot-layout"; import { leafColor, leafRGBA, luminance } from "../common/leaf-color"; import arcVert from "../../shaders/sunburst-arc.vert.glsl"; import arcFrag from "../../shaders/sunburst-arc.frag.glsl"; @@ -29,10 +27,10 @@ import { INNER_RING_PX, } from "./sunburst-layout"; import { buildFacetGrid } from "../../layout/facet-grid"; -import { renderCategoricalLegendAt } from "../../axis/legend"; import { withChromeCache } from "../common/chrome-cache"; import { renderBreadcrumbs as renderTreeBreadcrumbs, + renderTreeColorLegend, renderTreeTooltip, } from "../common/tree-chrome"; @@ -669,80 +667,20 @@ function drawStaticChrome( renderTreeBreadcrumbs(chart, ctx, cssWidth, fontFamily, textColor); } - // Legend. In faceted mode use the grid's explicit rect; otherwise - // derive from a synthetic single-plot layout. - if (faceted && chart._facetGrid?.legendRect) { - if ( - chart._colorMode === "series" && - chart._uniqueColorLabels.size > 1 - ) { - renderCategoricalLegendAt( - canvas, - chart._facetGrid.legendRect, - chart._uniqueColorLabels, - palette, - theme, - ); - } else if ( - chart._colorMode === "numeric" && - chart._colorMin < chart._colorMax - ) { - const legendLayout = new PlotLayout(cssWidth, cssHeight, { - hasXLabel: false, - hasYLabel: false, - hasLegend: true, - }); - renderLegend( - canvas, - legendLayout, - { - min: chart._colorMin, - max: chart._colorMax, - label: chart._colorName, - }, - stops, - theme, - chart.getColumnFormatter(chart._colorName, "value"), - ); - } - } else if ( - chart._colorMode === "series" && - chart._uniqueColorLabels.size > 1 - ) { - const legendLayout = new PlotLayout(cssWidth, cssHeight, { - hasXLabel: false, - hasYLabel: false, - hasLegend: true, - }); - renderCategoricalLegend( - canvas, - legendLayout, - chart._uniqueColorLabels, - palette, - theme, - ); - } else if ( - chart._colorMode === "numeric" && - chart._colorMin < chart._colorMax - ) { - const legendLayout = new PlotLayout(cssWidth, cssHeight, { - hasXLabel: false, - hasYLabel: false, - hasLegend: true, - }); - renderLegend( - canvas, - legendLayout, - { - min: chart._colorMin, - max: chart._colorMax, - label: chart._colorName, - }, - stops, - theme, - chart.getColumnFormatter(chart._colorName, "value"), - ); - } + // Legend. Faceted mode passes `FacetGrid.legendRect` so the + // categorical-swatch variant lands in the dedicated grid slot; + // numeric gradient always derives from a synthetic single-plot + // layout (its vertical bar doesn't fit the compact rect). + renderTreeColorLegend( + chart, + canvas, + palette, + stops, + theme, + cssWidth, + cssHeight, + faceted ? (chart._facetGrid?.legendRect ?? null) : null, + ); ctx.restore(); } diff --git a/packages/viewer-charts/src/ts/charts/sunburst/sunburst.ts b/packages/viewer-charts/src/ts/charts/sunburst/sunburst.ts index 137e12da19..60fd391043 100644 --- a/packages/viewer-charts/src/ts/charts/sunburst/sunburst.ts +++ b/packages/viewer-charts/src/ts/charts/sunburst/sunburst.ts @@ -12,7 +12,7 @@ import type { ColumnDataMap } from "../../data/view-reader"; import type { WebGLContextManager } from "../../webgl/context-manager"; -import { TreeChartBase } from "../common/tree-chart"; +import { TreeChartBase, firstNonMetadataColumn } from "../common/tree-chart"; import { NULL_NODE } from "../common/node-store"; import { processTreeChunk, @@ -41,20 +41,6 @@ export interface SunburstLocations { a_color: number; } -/** - * Sentinel fallback for the Size slot when the user hasn't picked one: - * use the first non-metadata column in the incoming view. - */ -function firstNonMetadataColumn(columns: ColumnDataMap): string { - for (const k of columns.keys()) { - if (!k.startsWith("__")) { - return k; - } - } - - return ""; -} - /** * Sunburst chart. Shares tree storage + streaming pipeline + color * mode with `TreeChartBase`; adds polar layout + instanced-arc WebGL diff --git a/packages/viewer-charts/src/ts/charts/treemap/treemap-render.ts b/packages/viewer-charts/src/ts/charts/treemap/treemap-render.ts index db81c37937..d6c0828fd4 100644 --- a/packages/viewer-charts/src/ts/charts/treemap/treemap-render.ts +++ b/packages/viewer-charts/src/ts/charts/treemap/treemap-render.ts @@ -22,8 +22,6 @@ import { import { Theme } from "../../theme/theme"; import { resolvePalette, type Vec3 } from "../../theme/palette"; import { type GradientStop } from "../../theme/gradient"; -import { renderLegend, renderCategoricalLegend } from "../../axis/legend"; -import { PlotLayout } from "../../layout/plot-layout"; import { buildFacetGrid } from "../../layout/facet-grid"; import { leafColor, leafRGBA, luminance } from "../common/leaf-color"; import treemapVert from "../../shaders/treemap.vert.glsl"; @@ -32,6 +30,7 @@ import { withChromeCache } from "../common/chrome-cache"; import { wrapLabel } from "../../axis/label-geometry"; import { renderBreadcrumbs as renderTreeBreadcrumbs, + renderTreeColorLegend, renderTreeTooltip, } from "../common/tree-chrome"; @@ -648,44 +647,15 @@ function drawStaticChrome( renderTreeBreadcrumbs(chart, ctx, cssWidth, fontFamily, textColor); } - // Legend: numeric mode → gradient bar; series mode with 2+ unique - // labels → categorical swatches. Empty mode (and single-label series) - // suppress the legend entirely. - if (chart._colorMode === "series" && chart._uniqueColorLabels.size > 1) { - const legendLayout = new PlotLayout(cssWidth, cssHeight, { - hasXLabel: false, - hasYLabel: false, - hasLegend: true, - }); - renderCategoricalLegend( - canvas, - legendLayout, - chart._uniqueColorLabels, - palette, - theme, - ); - } else if ( - chart._colorMode === "numeric" && - chart._colorMin < chart._colorMax - ) { - const legendLayout = new PlotLayout(cssWidth, cssHeight, { - hasXLabel: false, - hasYLabel: false, - hasLegend: true, - }); - renderLegend( - canvas, - legendLayout, - { - min: chart._colorMin, - max: chart._colorMax, - label: chart._colorName, - }, - stops, - theme, - chart.getColumnFormatter(chart._colorName, "value"), - ); - } + renderTreeColorLegend( + chart, + canvas, + palette, + stops, + theme, + cssWidth, + cssHeight, + ); // Per-facet titles (rendered over the layout; painted in the static // chrome bitmap so they appear alongside leaf labels). diff --git a/packages/viewer-charts/src/ts/charts/treemap/treemap.ts b/packages/viewer-charts/src/ts/charts/treemap/treemap.ts index 007fabf6ff..09a31a2c27 100644 --- a/packages/viewer-charts/src/ts/charts/treemap/treemap.ts +++ b/packages/viewer-charts/src/ts/charts/treemap/treemap.ts @@ -12,7 +12,7 @@ import type { ColumnDataMap } from "../../data/view-reader"; import type { WebGLContextManager } from "../../webgl/context-manager"; -import { TreeChartBase } from "../common/tree-chart"; +import { TreeChartBase, firstNonMetadataColumn } from "../common/tree-chart"; import { NULL_NODE } from "../common/node-store"; import { type BreadcrumbRegion, @@ -34,21 +34,6 @@ export interface TreemapLocations { a_color: number; } -/** - * Sentinel fallback for the Size slot when the user hasn't picked one: - * use the first non-metadata column in the incoming view. Treemap - * still needs *some* numeric-ish column to size rects. - */ -function firstNonMetadataColumn(columns: ColumnDataMap): string { - for (const k of columns.keys()) { - if (!k.startsWith("__")) { - return k; - } - } - - return ""; -} - /** * Treemap chart. Shares tree storage + streaming-pipeline + color-mode * state with `TreeChartBase`; adds rectangular layout + WebGL quad diff --git a/packages/viewer-charts/src/ts/interaction/host-sink-message.ts b/packages/viewer-charts/src/ts/interaction/host-sink-message.ts index 922a328212..ad5fd9735f 100644 --- a/packages/viewer-charts/src/ts/interaction/host-sink-message.ts +++ b/packages/viewer-charts/src/ts/interaction/host-sink-message.ts @@ -10,6 +10,13 @@ // ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ +import type { + DismissTooltipMsg, + PinTooltipMsg, + SetCursorMsg, + UserClickMsg, + UserSelectMsg, +} from "../transport/protocol"; import type { CssBounds, HostSink, @@ -18,29 +25,24 @@ import type { } from "./tooltip-controller"; /** - * Envelope shape sent by `MessageHostSink`. The transport translates - * each one into a corresponding `WorkerMsg` (`pinTooltip` / - * `dismissTooltip` / `setCursor` / `userClick` / `userSelect`). + * The subset of `WorkerMsg`s that flow chart → host through a + * `MessageHostSink`. Identical to the worker-side post payloads so the + * sink can ship them straight to `WorkerRenderer.post` with no + * intermediate translation. */ export type HostSinkEnvelope = - | { - kind: "pin"; - payload: { - lines: string[]; - pos: { px: number; py: number }; - bounds: CssBounds; - }; - } - | { kind: "dismiss" } - | { kind: "setCursor"; cursor: string } - | { kind: "userClick"; payload: UserClickPayload } - | { kind: "userSelect"; payload: UserSelectPayload }; + | PinTooltipMsg + | DismissTooltipMsg + | SetCursorMsg + | UserClickMsg + | UserSelectMsg; /** * `HostSink` that posts pin / dismiss / setCursor / user-event intents - * over a `postMessage`-style channel. The host-side transport listens - * for these envelopes and drives a `DomHostSink` for pin/dismiss and - * dispatches `CustomEvent`s on the viewer for user events. + * over a `postMessage`-style channel as `WorkerMsg`s. The host-side + * transport listens for these and drives a `DomHostSink` for + * pin/dismiss and dispatches `CustomEvent`s on the viewer for user + * events. */ export class MessageHostSink implements HostSink { private _send: (msg: HostSinkEnvelope) => void; @@ -54,11 +56,11 @@ export class MessageHostSink implements HostSink { pos: { px: number; py: number }, bounds: CssBounds, ): void { - this._send({ kind: "pin", payload: { lines, pos, bounds } }); + this._send({ kind: "pinTooltip", lines, pos, bounds }); } dismiss(): void { - this._send({ kind: "dismiss" }); + this._send({ kind: "dismissTooltip" }); } setCursor(cursor: string): void { @@ -66,10 +68,19 @@ export class MessageHostSink implements HostSink { } emitUserClick(payload: UserClickPayload): void { - this._send({ kind: "userClick", payload }); + // `UserClickPayload` is structurally identical to + // `PerspectiveClickDetail`; the cast carries the `config` + // field's looser inner shape without affecting runtime data. + this._send({ kind: "userClick", detail: payload as any }); } emitUserSelect(payload: UserSelectPayload): void { - this._send({ kind: "userSelect", payload }); + this._send({ + kind: "userSelect", + selected: payload.selected, + row: payload.row, + column_names: payload.column_names, + insertConfig: payload.insertConfig as any, + }); } } diff --git a/packages/viewer-charts/src/ts/interaction/raw-event-forwarder.ts b/packages/viewer-charts/src/ts/interaction/raw-event-forwarder.ts index 9fb306230c..e1be297be2 100644 --- a/packages/viewer-charts/src/ts/interaction/raw-event-forwarder.ts +++ b/packages/viewer-charts/src/ts/interaction/raw-event-forwarder.ts @@ -13,16 +13,15 @@ import type { InteractionEvent } from "../transport/protocol"; /** - * Worker-mode counterpart to {@link ZoomRouter}. Captures wheel / - * pointer events on the GL canvas, normalizes coords to canvas-relative - * CSS pixels, and emits semantic {@link InteractionEvent}s to the - * Renderer over its transport. The Renderer (running in a Web Worker) - * owns the `ZoomController`s and runs the actual hit-test + apply - * logic — see `applyWheel` / `applyPan` in `zoom-router.ts`. + * Captures wheel / pointer events on the GL canvas, normalizes coords + * to canvas-relative CSS pixels, and emits semantic + * {@link InteractionEvent}s to the Renderer over its transport. The + * Renderer owns the `ZoomController`s and runs the actual hit-test + + * apply logic — see `applyWheel` / `applyPan` in `zoom-router.ts`. * * Pointer capture is set on `pointerdown` and released on `pointerup` * so drags continue to deliver `pointermove` events when the cursor - * leaves the canvas, matching the in-process `ZoomRouter` behavior. + * leaves the canvas. */ export class RawEventForwarder { private _element: HTMLElement | null = null; @@ -47,11 +46,10 @@ export class RawEventForwarder { const mx = e.clientX - rect.left; const my = e.clientY - rect.top; - // Match `ZoomRouter`: only consume the wheel event if the - // cursor is over the canvas. `preventDefault` is fired - // unconditionally so the page does not scroll while the - // chart is hovered — the worker may still no-op if the - // cursor is outside any facet's plot rect. + // `preventDefault` is fired unconditionally so the page + // does not scroll while the chart is hovered — the worker + // may still no-op if the cursor is outside any facet's + // plot rect. e.preventDefault(); emit({ type: "wheel", mx, my, deltaY: e.deltaY }); }; diff --git a/packages/viewer-charts/src/ts/interaction/zoom-controller.ts b/packages/viewer-charts/src/ts/interaction/zoom-controller.ts index 1b14e66741..00e1c28316 100644 --- a/packages/viewer-charts/src/ts/interaction/zoom-controller.ts +++ b/packages/viewer-charts/src/ts/interaction/zoom-controller.ts @@ -108,6 +108,15 @@ export class ZoomController { private _lockAxis: "x" | "y" | null = null; private _lockAspect = false; + // Opt-in "pin" flags. When set, `setBaseDomain` preserves the + // axis's *absolute* visible center across a base swap — the + // pre-existing rebase math. Default-cleared: data updates "follow" + // (preserve normalized translate, so the visible window tracks the + // data fractionally). Wire to a paused-frame review feature when + // one exists; no current caller flips these. + private _xPinned = false; + private _yPinned = false; + private _element: HTMLElement | null = null; private _layout: PlotLayout | null = null; private _onUpdate: (() => void) | null = null; @@ -121,9 +130,8 @@ export class ZoomController { private _onPointerMove: ((e: PointerEvent) => void) | null = null; private _onPointerUp: ((e: PointerEvent) => void) | null = null; - // Per-controller mutators used by `ZoomRouter` to apply wheel/pan - // events without going through `attach`. Live below under "Router - // helpers" for the facet-aware zoom path. + // Per-controller mutators used by `applyWheel` / `applyPan` + // (zoom-router.ts) for the facet-aware zoom path. get lockedAxis(): "x" | "y" | null { return this._lockAxis; } @@ -159,28 +167,27 @@ export class ZoomController { } /** - * Update the base (full-data) domain that this controller's - * normalized translate is interpreted against, while preserving - * the *absolute* center of any user-applied pan. + * Update the base (full-data) domain. Each axis is handled + * independently: * - * `_normTX` / `_normTY` are stored as fractions of the base - * range, not absolute coordinates. A naive "swap base, keep - * normTranslate" update reinterprets the same fraction against a - * new range, so when an external `draw()` updates the extent - * (via `processCartesianChunk` → `setZoomBaseDomain`) the user's - * pan-offset visible center jumps to a different absolute - * position. With concurrent pan events feeding in offsets that - * were computed against the old base, the jump can project the - * visible center past the data entirely, leaving `_fullRender` - * to draw zero glyphs onto a freshly-cleared canvas — a blank - * bitmap reaches the host as a flicker. + * - If the axis is at default (no user pan or zoom on this axis), + * just swap the new base in — no further math needed. + * - If the axis has been explicitly *pinned* (`pinAxis("x" | "y")`), + * preserve its *absolute* visible center across the swap: + * re-solve `_normT` so the data-space center is unchanged. + * This is paused-frame-review semantics — the user has marked a + * region of interest and wants it to stay put even as new data + * flows in. No current caller pins, but the API is here so the + * default rule below doesn't bake the choice in. + * - Otherwise (the default — "follow"): keep `_normT` as-is and + * just swap the new base. The visible window's *fractional* + * position is preserved, so sliding windows slide with the data + * and extending windows grow proportionally with the data. * - * When the user is in default state (no pan, no zoom — fresh - * controller, or just-reset), no rebase is needed; just swap the - * base and let the chart auto-fit to the new data. Otherwise - * recompute `_normTX` / `_normTY` so the visible center stays at - * the same absolute (data-coordinate) position before and after - * the swap. + * Per-axis handling matters because `_scaleX/_normTX` and + * `_scaleY/_normTY` are independent. A user who panned X to scroll + * through time should not force Y onto the rebase path — Y data + * updates should still flow through cleanly. */ setBaseDomain( xMin: number, @@ -188,32 +195,57 @@ export class ZoomController { yMin: number, yMax: number, ): void { - if (this.isDefault()) { + const newRangeX = xMax - xMin; + if (this.isXDefault() || !this._xPinned) { + this._baseXMin = xMin; + this._baseXMax = xMax; + } else { + const oldRangeX = this._baseXMax - this._baseXMin; + const oldCx = + (this._baseXMin + this._baseXMax) / 2 + + this._normTX * oldRangeX; this._baseXMin = xMin; this._baseXMax = xMax; + this._normTX = + newRangeX > 0 ? (oldCx - (xMin + xMax) / 2) / newRangeX : 0; + } + + const newRangeY = yMax - yMin; + if (this.isYDefault() || !this._yPinned) { + this._baseYMin = yMin; + this._baseYMax = yMax; + } else { + const oldRangeY = this._baseYMax - this._baseYMin; + const oldCy = + (this._baseYMin + this._baseYMax) / 2 + + this._normTY * oldRangeY; this._baseYMin = yMin; this._baseYMax = yMax; - return; + this._normTY = + newRangeY > 0 ? (oldCy - (yMin + yMax) / 2) / newRangeY : 0; } + } - const oldRangeX = this._baseXMax - this._baseXMin; - const oldRangeY = this._baseYMax - this._baseYMin; - const oldCx = - (this._baseXMin + this._baseXMax) / 2 + this._normTX * oldRangeX; - const oldCy = - (this._baseYMin + this._baseYMax) / 2 + this._normTY * oldRangeY; - - this._baseXMin = xMin; - this._baseXMax = xMax; - this._baseYMin = yMin; - this._baseYMax = yMax; + /** + * Mark an axis as "pinned" so subsequent `setBaseDomain` calls + * preserve its *absolute* visible center (paused-frame-review + * semantics). Default-cleared on construction; both axes follow + * data growth fractionally until explicitly pinned. + */ + pinAxis(axis: "x" | "y"): void { + if (axis === "x") { + this._xPinned = true; + } else { + this._yPinned = true; + } + } - const newRangeX = xMax - xMin; - const newRangeY = yMax - yMin; - this._normTX = - newRangeX > 0 ? (oldCx - (xMin + xMax) / 2) / newRangeX : 0; - this._normTY = - newRangeY > 0 ? (oldCy - (yMin + yMax) / 2) / newRangeY : 0; + unpinAxis(axis: "x" | "y"): void { + if (axis === "x") { + this._xPinned = false; + } else { + this._yPinned = false; + } } /** @@ -234,13 +266,16 @@ export class ZoomController { } } + isXDefault(): boolean { + return this._scaleX === 1 && this._normTX === 0; + } + + isYDefault(): boolean { + return this._scaleY === 1 && this._normTY === 0; + } + isDefault(): boolean { - return ( - this._scaleX === 1 && - this._scaleY === 1 && - this._normTX === 0 && - this._normTY === 0 - ); + return this.isXDefault() && this.isYDefault(); } getVisibleDomain(): { @@ -300,14 +335,15 @@ export class ZoomController { return; } - // Data coordinate under cursor before zoom - const domain = this.getVisibleDomain(); - const dataX = - domain.xMin + - ((mouseX - plot.x) / plot.width) * (domain.xMax - domain.xMin); - const dataY = - domain.yMax - - ((mouseY - plot.y) / plot.height) * (domain.yMax - domain.yMin); + // Cursor position as a fraction of the plot rect — the + // anchor point that should stay visually fixed across the + // zoom mutation below. `fracY` is 0 at top, 1 at bottom + // (screen coords); Y data axis is inverted, hence the + // `0.5 - fracY` form below. + const fracX = (mouseX - plot.x) / plot.width; + const fracY = (mouseY - plot.y) / plot.height; + const oldScaleX = this._scaleX; + const oldScaleY = this._scaleY; // Zoom factor — skip the locked axis so its scale stays // pinned at 1. @@ -326,25 +362,26 @@ export class ZoomController { ); } - // Adjust translate so the data point under cursor stays put - const newDomain = this.getVisibleDomain(); - const newDataX = - newDomain.xMin + - ((mouseX - plot.x) / plot.width) * - (newDomain.xMax - newDomain.xMin); - const newDataY = - newDomain.yMax - - ((mouseY - plot.y) / plot.height) * - (newDomain.yMax - newDomain.yMin); - - const bxRange = this._baseXMax - this._baseXMin; - const byRange = this._baseYMax - this._baseYMin; - if (this._lockAxis !== "x" && bxRange > 0) { - this._normTX += (dataX - newDataX) / bxRange; + // Cursor-anchored zoom: keep the data point under the + // cursor visually fixed. Derivation, for X: + // dataX(scale) = mid + normTX*baseRange + // + (fracX - 0.5) * baseRange/scale + // After mutating scale (normTX unchanged), the cursor data + // coord shifts. Re-anchoring is `normTX += + // (oldDataX - newDataX) / baseRange`; the `baseRange` + // terms cancel to: + // normTXDelta = (fracX - 0.5) * (1/oldScale - 1/newScale) + // Base-independent — a concurrent `setBaseDomain` mid- + // handler can't corrupt the anchor math. Y axis is screen- + // inverted, hence `(0.5 - fracY)`. + if (this._lockAxis !== "x" && oldScaleX !== this._scaleX) { + this._normTX += + (fracX - 0.5) * (1 / oldScaleX - 1 / this._scaleX); } - if (this._lockAxis !== "y" && byRange > 0) { - this._normTY += (dataY - newDataY) / byRange; + if (this._lockAxis !== "y" && oldScaleY !== this._scaleY) { + this._normTY += + (0.5 - fracY) * (1 / oldScaleY - 1 / this._scaleY); } this._onUpdate!(); @@ -379,19 +416,19 @@ export class ZoomController { this._lastPointerX = e.clientX; this._lastPointerY = e.clientY; - const domain = this.getVisibleDomain(); + // Pan as a fraction is `dx / (plotWidth * scaleX)` — + // independent of the current base range. The chained form + // (`pixels → data delta → fraction-of-base`) cancels the + // `baseRange` terms algebraically; computing the cancelled + // form directly means a concurrent `setBaseDomain` swap + // mid-gesture cannot corrupt the pan math. const plot = this._layout!.plotRect; - const dataPerPixelX = (domain.xMax - domain.xMin) / plot.width; - const dataPerPixelY = (domain.yMax - domain.yMin) / plot.height; - - const bxRange = this._baseXMax - this._baseXMin; - const byRange = this._baseYMax - this._baseYMin; - if (this._lockAxis !== "x" && bxRange > 0) { - this._normTX -= (dx * dataPerPixelX) / bxRange; + if (this._lockAxis !== "x" && plot.width > 0) { + this._normTX -= dx / (plot.width * this._scaleX); } - if (this._lockAxis !== "y" && byRange > 0) { - this._normTY += (dy * dataPerPixelY) / byRange; + if (this._lockAxis !== "y" && plot.height > 0) { + this._normTY += dy / (plot.height * this._scaleY); } this._onUpdate!(); diff --git a/packages/viewer-charts/src/ts/interaction/zoom-router.ts b/packages/viewer-charts/src/ts/interaction/zoom-router.ts index cadb6cffd9..1a2abe749a 100644 --- a/packages/viewer-charts/src/ts/interaction/zoom-router.ts +++ b/packages/viewer-charts/src/ts/interaction/zoom-router.ts @@ -24,134 +24,11 @@ export interface ZoomTarget { controller: ZoomController; layout: PlotLayout; } -export type ZoomTargetResolver = (mx: number, my: number) => ZoomTarget | null; /** - * One set of wheel / pointer listeners on the GL canvas that dispatches - * zoom + pan events to a {@link ZoomController} resolved from the - * cursor position. Replaces `ZoomController.attach` so multiple - * controllers (one per facet) can coexist on a single canvas. - */ -export class ZoomRouter { - private _element: HTMLElement | null = null; - private _resolve: ZoomTargetResolver | null = null; - private _onUpdate: (() => void) | null = null; - - private _pointerDown = false; - private _pointerTarget: ZoomTarget | null = null; - private _lastPointerX = 0; - private _lastPointerY = 0; - - private _onWheel: ((e: WheelEvent) => void) | null = null; - private _onPointerDown: ((e: PointerEvent) => void) | null = null; - private _onPointerMove: ((e: PointerEvent) => void) | null = null; - private _onPointerUp: ((e: PointerEvent) => void) | null = null; - - attach( - element: HTMLElement, - resolve: ZoomTargetResolver, - onUpdate: () => void, - ): void { - this.detach(); - this._element = element; - this._resolve = resolve; - this._onUpdate = onUpdate; - - this._onWheel = (e: WheelEvent) => { - const rect = element.getBoundingClientRect(); - const mouseX = e.clientX - rect.left; - const mouseY = e.clientY - rect.top; - const target = resolve(mouseX, mouseY); - if (!target) { - return; - } - - e.preventDefault(); - applyWheel(target, mouseX, mouseY, e.deltaY); - onUpdate(); - }; - - this._onPointerDown = (e: PointerEvent) => { - const rect = element.getBoundingClientRect(); - const mouseX = e.clientX - rect.left; - const mouseY = e.clientY - rect.top; - const target = resolve(mouseX, mouseY); - if (!target) { - return; - } - - this._pointerDown = true; - this._pointerTarget = target; - this._lastPointerX = e.clientX; - this._lastPointerY = e.clientY; - element.setPointerCapture(e.pointerId); - }; - - this._onPointerMove = (e: PointerEvent) => { - if (!this._pointerDown || !this._pointerTarget) { - return; - } - - const dx = e.clientX - this._lastPointerX; - const dy = e.clientY - this._lastPointerY; - this._lastPointerX = e.clientX; - this._lastPointerY = e.clientY; - applyPan(this._pointerTarget, dx, dy); - onUpdate(); - }; - - this._onPointerUp = () => { - this._pointerDown = false; - this._pointerTarget = null; - }; - - element.addEventListener("wheel", this._onWheel, { passive: false }); - element.addEventListener("pointerdown", this._onPointerDown); - element.addEventListener("pointermove", this._onPointerMove); - element.addEventListener("pointerup", this._onPointerUp); - } - - detach(): void { - if (this._element) { - if (this._onWheel) { - this._element.removeEventListener("wheel", this._onWheel); - } - - if (this._onPointerDown) { - this._element.removeEventListener( - "pointerdown", - this._onPointerDown, - ); - } - - if (this._onPointerMove) { - this._element.removeEventListener( - "pointermove", - this._onPointerMove, - ); - } - - if (this._onPointerUp) { - this._element.removeEventListener( - "pointerup", - this._onPointerUp, - ); - } - } - - this._element = null; - this._resolve = null; - this._onUpdate = null; - this._pointerDown = false; - this._pointerTarget = null; - } -} - -/** - * Apply a wheel-zoom delta to the resolved target. Exported for - * re-use inside the worker, where the renderer dispatches interaction - * events forwarded from the host's `RawEventForwarder` instead of - * receiving DOM events directly. + * Apply a wheel-zoom delta to the resolved target. The worker + * dispatches interaction events forwarded from the host's + * `RawEventForwarder` and calls this directly. */ export function applyWheel( target: ZoomTarget, diff --git a/packages/viewer-charts/src/ts/plugin/plugin.ts b/packages/viewer-charts/src/ts/plugin/plugin.ts index 184576550c..b42a8b41c7 100644 --- a/packages/viewer-charts/src/ts/plugin/plugin.ts +++ b/packages/viewer-charts/src/ts/plugin/plugin.ts @@ -38,173 +38,85 @@ import { RENDER_BLIT_MODE } from "../config"; const FACET_CONFIG_DEFAULTS: FacetConfig = { ...DEFAULT_FACET_CONFIG }; /** - * Build a UI control spec for one plugin-config field. Mirrors the - * shape `column_config_schema` already returns (datagrid). Numeric - * fields get a `Number` control with min/max clamps; fractions get a - * 0..1 range; enums + booleans pass through their variant list. + * Static UI-control spec per `plugin_config` field. Mirrors the shape + * `column_config_schema` already returns (datagrid). The runtime default + * is sourced separately from the chart-type-effective defaults at + * `fieldSpec` call time so per-chart overrides like + * `include_zero=true` for Y Bar / Y Area / X Bar surface in the UI. */ +type FieldSpec = + | { kind: "Bool" } + | { + kind: "Enum"; + variants: ReadonlyArray<{ value: string; label: string }>; + } + | { kind: "Number"; min: number; max: number; step?: number }; + +const FIELD_SCHEMAS: Record = { + auto_alt_y_axis: { kind: "Bool" }, + include_zero: { kind: "Bool" }, + domain_mode: { + kind: "Enum", + variants: [ + { value: "fit", label: "Fit" }, + { value: "expand", label: "Expand" }, + ], + }, + facet_mode: { + kind: "Enum", + variants: [ + { value: "grid", label: "Grid" }, + { value: "overlay", label: "Overlay" }, + ], + }, + facet_zoom_mode: { + kind: "Enum", + variants: [ + { value: "shared", label: "Shared" }, + { value: "independent", label: "Independent" }, + ], + }, + series_zoom_mode: { + kind: "Enum", + variants: [ + { value: "dynamic", label: "Dynamic" }, + { value: "fixed", label: "Fixed" }, + ], + }, + line_width_px: { kind: "Number", min: 0.5, step: 0.5, max: 16 }, + point_size_px: { kind: "Number", min: 1, max: 32 }, + band_inner_frac: { kind: "Number", min: 0.1, max: 1, step: 0.01 }, + bar_inner_pad: { kind: "Number", min: 0, max: 0.9, step: 0.01 }, + wick_width_px: { kind: "Number", min: 0.5, step: 0.5, max: 8 }, + ohlc_line_width_px: { kind: "Number", min: 0.5, step: 0.5, max: 8 }, + gradient_radius_px: { kind: "Number", min: 2, step: 1, max: 256 }, + gradient_intensity: { kind: "Number", min: 0.05, step: 0.05, max: 4 }, + gradient_heat_max: { kind: "Number", min: 0.1, step: 0.1, max: 64 }, + gradient_color_mode: { + kind: "Enum", + variants: [ + { value: "mean", label: "Mean (density-weighted)" }, + { value: "density", label: "Density only" }, + { value: "extreme", label: "Extremes" }, + { value: "signed", label: "Signed sum" }, + ], + }, + map_tile_provider: { + kind: "Enum", + variants: [ + { value: "carto-positron", label: "Light (Positron)" }, + { value: "carto-dark-matter", label: "Dark Matter" }, + { value: "carto-voyager", label: "Voyager" }, + ], + }, + map_tile_alpha: { kind: "Number", min: 0, max: 1, step: 0.05 }, +}; + function fieldSpec( key: PluginConfigField, defaults: PluginConfig, ): Record & { kind: string } { - switch (key) { - case "auto_alt_y_axis": - return { kind: "Bool", key, default: defaults.auto_alt_y_axis }; - case "include_zero": - return { kind: "Bool", key, default: defaults.include_zero }; - case "domain_mode": - return { - kind: "Enum", - key, - default: defaults.domain_mode, - variants: [ - { value: "fit", label: "Fit" }, - { value: "expand", label: "Expand" }, - ], - }; - case "facet_mode": - return { - kind: "Enum", - key, - default: DEFAULT_PLUGIN_CONFIG.facet_mode, - variants: [ - { value: "grid", label: "Grid" }, - { value: "overlay", label: "Overlay" }, - ], - }; - case "facet_zoom_mode": - return { - kind: "Enum", - key, - default: DEFAULT_PLUGIN_CONFIG.facet_zoom_mode, - variants: [ - { value: "shared", label: "Shared" }, - { value: "independent", label: "Independent" }, - ], - }; - case "series_zoom_mode": - return { - kind: "Enum", - key, - default: DEFAULT_PLUGIN_CONFIG.series_zoom_mode, - variants: [ - { value: "dynamic", label: "Dynamic" }, - { value: "fixed", label: "Fixed" }, - ], - }; - case "line_width_px": - return { - kind: "Number", - key, - default: DEFAULT_PLUGIN_CONFIG.line_width_px, - min: 0.5, - step: 0.5, - max: 16, - }; - case "point_size_px": - return { - kind: "Number", - key, - default: DEFAULT_PLUGIN_CONFIG.point_size_px, - min: 1, - max: 32, - }; - case "band_inner_frac": - return { - kind: "Number", - key, - default: DEFAULT_PLUGIN_CONFIG.band_inner_frac, - min: 0.1, - max: 1, - step: 0.01, - }; - case "bar_inner_pad": - return { - kind: "Number", - key, - default: DEFAULT_PLUGIN_CONFIG.bar_inner_pad, - min: 0, - max: 0.9, - step: 0.01, - }; - case "wick_width_px": - return { - kind: "Number", - key, - default: DEFAULT_PLUGIN_CONFIG.wick_width_px, - min: 0.5, - step: 0.5, - max: 8, - }; - case "ohlc_line_width_px": - return { - kind: "Number", - key, - default: DEFAULT_PLUGIN_CONFIG.ohlc_line_width_px, - min: 0.5, - step: 0.5, - max: 8, - }; - case "gradient_radius_px": - return { - kind: "Number", - key, - default: DEFAULT_PLUGIN_CONFIG.gradient_radius_px, - min: 2, - step: 1, - max: 256, - }; - case "gradient_intensity": - return { - kind: "Number", - key, - default: DEFAULT_PLUGIN_CONFIG.gradient_intensity, - min: 0.05, - step: 0.05, - max: 4, - }; - case "gradient_heat_max": - return { - kind: "Number", - key, - default: DEFAULT_PLUGIN_CONFIG.gradient_heat_max, - min: 0.1, - step: 0.1, - max: 64, - }; - case "gradient_color_mode": - return { - kind: "Enum", - key, - default: DEFAULT_PLUGIN_CONFIG.gradient_color_mode, - variants: [ - { value: "mean", label: "Mean (density-weighted)" }, - { value: "density", label: "Density only" }, - { value: "extreme", label: "Extremes" }, - { value: "signed", label: "Signed sum" }, - ], - }; - case "map_tile_provider": - return { - kind: "Enum", - key, - default: DEFAULT_PLUGIN_CONFIG.map_tile_provider, - variants: [ - { value: "carto-positron", label: "Light (Positron)" }, - { value: "carto-dark-matter", label: "Dark Matter" }, - { value: "carto-voyager", label: "Voyager" }, - ], - }; - case "map_tile_alpha": - return { - kind: "Number", - key, - default: DEFAULT_PLUGIN_CONFIG.map_tile_alpha, - min: 0, - max: 1, - step: 0.05, - }; - } + return { ...FIELD_SCHEMAS[key], key, default: defaults[key] }; } const GLOBAL_STYLES = (() => { @@ -599,28 +511,6 @@ export class HTMLPerspectiveViewerWebGLPluginElement return 5; } - save() { - const state: any = {}; - const zoom = this._renderer?.saveZoom(); - if (zoom) { - state.zoom = zoom; - } - - // Only emit the keys this chart actually consumes. - const cfg: Partial = {}; - for (const key of this._chartType.applicable_plugin_fields) { - // `key` is `PluginConfigField` = `keyof PluginConfig`, so this - // indexed assignment is type-safe without a cast. - (cfg[key] as PluginConfig[typeof key]) = this._pluginConfig[key]; - } - - if (Object.keys(cfg).length > 0) { - state.plugin_config = cfg; - } - - return state; - } - async render(view: View): Promise { await this._ensureRenderer(view); await this.draw(view); diff --git a/packages/viewer-charts/src/ts/theme/palette.ts b/packages/viewer-charts/src/ts/theme/palette.ts index cf0d12fcc8..ac72a7268c 100644 --- a/packages/viewer-charts/src/ts/theme/palette.ts +++ b/packages/viewer-charts/src/ts/theme/palette.ts @@ -18,10 +18,7 @@ export type Vec3 = [number, number, number]; * Build a series palette of length `count` by sampling the theme gradient * at evenly-spaced offsets. For count == 1 returns the 50% stop. */ -export function interpolatePalette( - stops: GradientStop[], - count: number, -): Vec3[] { +function interpolatePalette(stops: GradientStop[], count: number): Vec3[] { if (count <= 0) { return []; } diff --git a/packages/viewer-charts/src/ts/transport/protocol.ts b/packages/viewer-charts/src/ts/transport/protocol.ts index d490fb69ad..331df24379 100644 --- a/packages/viewer-charts/src/ts/transport/protocol.ts +++ b/packages/viewer-charts/src/ts/transport/protocol.ts @@ -12,8 +12,11 @@ import type { FacetConfig, PluginConfig } from "../charts/chart"; import type { PerspectiveClickDetail } from "../event-detail"; +import type { ThemeSnapshot } from "../theme/theme"; import type { ViewConfig } from "@perspective-dev/client"; +export type { ThemeSnapshot }; + /** * Worker-mode control-channel messages. Distinct from the perspective * `ProxySession` channel that the worker's `Client` uses to talk to the @@ -222,14 +225,6 @@ export interface FontFaceDescriptor { display?: string; } -/** - * Theme values resolved on the host via `getComputedStyle` and shipped - * to the renderer scope, which has no DOM. Decoded by the chart via - * `theme/theme.ts::resolveThemeFromVars`. Plain map for - * structured-clone. - */ -export type ThemeSnapshot = Record; - export interface SetViewByNameMsg { kind: "setViewByName"; name: string; diff --git a/packages/viewer-charts/src/ts/worker/dispatch.ts b/packages/viewer-charts/src/ts/worker/dispatch.ts index 1ca1adc797..c6ea59d616 100644 --- a/packages/viewer-charts/src/ts/worker/dispatch.ts +++ b/packages/viewer-charts/src/ts/worker/dispatch.ts @@ -41,7 +41,6 @@ export function dispatch(r: WorkerRenderer, msg: ControlMsg): void { r.redraw(); break; case "resize": - console.log("resize"); r.resize(msg.cssWidth, msg.cssHeight, msg.dpr); r.redraw(); break; diff --git a/packages/viewer-charts/src/ts/worker/renderer.worker.ts b/packages/viewer-charts/src/ts/worker/renderer.worker.ts index 413f49cb57..bf461a97a4 100644 --- a/packages/viewer-charts/src/ts/worker/renderer.worker.ts +++ b/packages/viewer-charts/src/ts/worker/renderer.worker.ts @@ -177,41 +177,11 @@ export class WorkerRenderer { this.chartImpl.setView?.(view); this.glManager.bufferPool.maxCapacity = msg.bufferMaxCapacity; this.glManager.resize(msg.cssWidth, msg.cssHeight, msg.dpr); - const hostSink = new MessageHostSink((envelope) => { - switch (envelope.kind) { - case "pin": - this.post({ - kind: "pinTooltip", - lines: envelope.payload.lines, - pos: envelope.payload.pos, - bounds: envelope.payload.bounds, - }); - break; - case "dismiss": - this.post({ kind: "dismissTooltip" }); - break; - case "setCursor": - this.post({ kind: "setCursor", cursor: envelope.cursor }); - break; - case "userClick": - this.post({ - kind: "userClick", - detail: envelope.payload as any, - }); - break; - case "userSelect": - this.post({ - kind: "userSelect", - selected: envelope.payload.selected, - row: envelope.payload.row, - column_names: envelope.payload.column_names, - insertConfig: envelope.payload.insertConfig as any, - }); - break; - } - }); - - this.chartImpl.attachTooltip?.(hostSink); + // `MessageHostSink` emits `WorkerMsg`s directly — no translation + // needed here. + this.chartImpl.attachTooltip?.( + new MessageHostSink((msg) => this.post(msg)), + ); } setViewByName(name: string): void { @@ -309,13 +279,9 @@ export class WorkerRenderer { } } } catch (err) { - // Any unexpected throw — proxy hiccup, chart-impl mutation - // failure, RAF chain rejection — must not leak past the - // outer fire-and-forget caller (`dispatch` does not await - // this method). Surface to the worker console; the host's - // pending promise still gets resolved via the `finally` - // ack below so `draw()` can't deadlock on a renderer error. - console.error("loadAndRender failed", err); + if ((err + "").indexOf("View not found") === -1) { + console.error("loadAndRender failed", err); + } } finally { this.post({ kind: "loadAndRenderAck", msgId: msg.msgId }); } @@ -417,10 +383,8 @@ export class WorkerRenderer { /** * Hit-test the cursor against the chart's facet grid (in faceted - * mode) or its current layout (single-plot). Mirrors the resolver - * `_setupZoomRouter` builds on the host for in-process mode — the - * worker owns the facet grid and controllers, so the resolution - * runs here. + * mode) or its current layout (single-plot). The worker owns the + * facet grid and controllers, so the resolution runs here. */ private _resolveTarget(mx: number, my: number): ZoomTarget | null { const chart = this.chartImpl as any; diff --git a/packages/viewer-charts/test/ts/plugin-config-toggle.spec.ts b/packages/viewer-charts/test/ts/plugin-config-toggle.spec.ts new file mode 100644 index 0000000000..aff33cd10e --- /dev/null +++ b/packages/viewer-charts/test/ts/plugin-config-toggle.spec.ts @@ -0,0 +1,100 @@ +// ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ +// ┃ ██████ ██████ ██████ █ █ █ █ █ █▄ ▀███ █ ┃ +// ┃ ▄▄▄▄▄█ █▄▄▄▄▄ ▄▄▄▄▄█ ▀▀▀▀▀█▀▀▀▀▀ █ ▀▀▀▀▀█ ████████▌▐███ ███▄ ▀█ █ ▀▀▀▀▀ ┃ +// ┃ █▀▀▀▀▀ █▀▀▀▀▀ █▀██▀▀ ▄▄▄▄▄ █ ▄▄▄▄▄█ ▄▄▄▄▄█ ████████▌▐███ █████▄ █ ▄▄▄▄▄ ┃ +// ┃ █ ██████ █ ▀█▄ █ ██████ █ ███▌▐███ ███████▄ █ ┃ +// ┣━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫ +// ┃ Copyright (c) 2017, the Perspective Authors. ┃ +// ┃ ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌ ┃ +// ┃ This file is part of the Perspective library, distributed under the terms ┃ +// ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃ +// ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ + +/** + * Regression test for the blank-canvas bug on `plugin_config` toggle. + * + * Bug chain (all three required to reproduce): + * 1. `RendererTransport.saveZoom` was `async` but never returned the + * pending-reply promise — every caller got `Promise`. + * 2. `plugin.save()` called it without `await` and stored the bare + * Promise as `state.zoom`. `JSON.stringify` collapses any Promise + * to `{}`. + * 3. A later `plugin.restore(token)` with `token.zoom === {}` reached + * `ZoomController.restore({})`, which blindly assigned `undefined` + * to every internal field. `isDefault()` then returned false + * (`1 === undefined`) and `getVisibleDomain()` produced NaN on + * every axis, the projection matrix went NaN, every glyph + * projected off-screen, the canvas painted blank. + * + * Fix lives in three places — `ZoomController.restore` validates, + * `saveZoom` returns the promise, `plugin.save` awaits it — and we want + * the suite to fail if *any* of them regresses. + * + * Pixel-count assertion (not a screenshot snapshot): the bug produces + * `plotPixels: 0` which is what `assertPlotNeverBlank` is built for, and + * pixel counts don't drift across Chromium versions the way snapshots + * do. + */ + +import { test } from "@perspective-dev/test"; +import { + assertPlotNeverBlank, + calibratePlotBaseline, + captureFrames, + gotoBasic, + restoreChart, + waitOneFrame, +} from "./helpers"; + +/** + * Half the calibrated quiescent pixel count — far above 0 (the blank- + * canvas signature) and well below the steady-state baseline, so a + * single skipped intermediate frame from RAF scheduling doesn't trip + * the assertion. Frame-to-frame jitter on a healthy chart is ~1% + * (see `frame-timing.spec.ts`). + */ +const BLANK_HEADROOM = 0.5; + +test.describe("plugin_config toggle (regression)", () => { + test.beforeEach(async ({ page }) => { + await gotoBasic(page); + }); + + /** + * Reproduces the user-visible symptom: toggling `series_zoom_mode` + * via the wholesale-restore path that the dropdown UI ultimately + * routes through. Fails pre-fix when the active plugin's bucket has + * accumulated a malformed `zoom: {}` from a prior save round-trip, + * because `plugin.restore` then calls `restoreZoom({})` and + * corrupts the controller. + */ + test("Y Line + datetime axis stays rendered across series_zoom_mode toggle", async ({ + page, + }) => { + await restoreChart(page, { + plugin: "Y Line", + columns: ["Profit"], + group_by: ["Order Date"], + }); + + const baseline = await calibratePlotBaseline(page); + + const frames = await captureFrames(page, async () => { + await page.evaluate(async () => { + const viewer = document.querySelector( + "perspective-viewer", + )! as unknown as { restore: (c: unknown) => Promise }; + await viewer.restore({ + plugin_config: { series_zoom_mode: "fixed" }, + }); + await viewer.restore({ + plugin_config: { series_zoom_mode: "dynamic" }, + }); + }); + await waitOneFrame(page); + await waitOneFrame(page); + }); + + assertPlotNeverBlank(frames, Math.floor(baseline * BLANK_HEADROOM)); + }); +}); diff --git a/packages/viewer-charts/test/ts/snapshot/zoom.spec.ts b/packages/viewer-charts/test/ts/snapshot/zoom.spec.ts index ce03edc882..7fe1b593ac 100644 --- a/packages/viewer-charts/test/ts/snapshot/zoom.spec.ts +++ b/packages/viewer-charts/test/ts/snapshot/zoom.spec.ts @@ -78,4 +78,61 @@ test.describe("Zoom", () => { await waitOneFrame(page); await expectViewerScreenshot(page); }); + + // Datetime-axis dynamic-zoom refit. With a numeric category axis + // (single `date` / `datetime` / `integer` / `float` group_by), the + // zoom controller's visible domain is in *absolute data units* + // (e.g. ~1.7e12 for a datetime). The auto-fit lookup needs to map + // that back to catIdx via `_categoryPositions` — without that map, + // the value-axis refit silently no-ops and `"dynamic"` looks + // identical to `"fixed"` on every datetime-axis chart. + test.describe("datetime x axis", () => { + test("Y Line wheel zoom refits value axis (dynamic)", async ({ + page, + }) => { + await restoreChart(page, { + plugin: "Y Line", + columns: ["Profit"], + group_by: ["Order Date"], + plugin_config: { series_zoom_mode: "dynamic" }, + }); + + await page.mouse.move(PLOT_CX, PLOT_CY); + await page.mouse.wheel(0, -500); + await waitOneFrame(page); + await expectViewerScreenshot(page); + }); + + test("Y Line wheel zoom keeps value axis pinned (fixed)", async ({ + page, + }) => { + await restoreChart(page, { + plugin: "Y Line", + columns: ["Profit"], + group_by: ["Order Date"], + plugin_config: { series_zoom_mode: "fixed" }, + }); + + await page.mouse.move(PLOT_CX, PLOT_CY); + await page.mouse.wheel(0, -500); + await waitOneFrame(page); + await expectViewerScreenshot(page); + }); + + test("Y Bar wheel zoom refits value axis (dynamic)", async ({ + page, + }) => { + await restoreChart(page, { + plugin: "Y Bar", + columns: ["Sales"], + group_by: ["Order Date"], + plugin_config: { series_zoom_mode: "dynamic" }, + }); + + await page.mouse.move(PLOT_CX, PLOT_CY); + await page.mouse.wheel(0, -500); + await waitOneFrame(page); + await expectViewerScreenshot(page); + }); + }); }); diff --git a/packages/viewer-datagrid/test/js/column_style.spec.js b/packages/viewer-datagrid/test/js/column_style.spec.js index 17c9145c71..67a4665a21 100644 --- a/packages/viewer-datagrid/test/js/column_style.spec.js +++ b/packages/viewer-datagrid/test/js/column_style.spec.js @@ -139,7 +139,7 @@ test.describe("Column Style Tests", () => { expect(count).toEqual(2); }); - test("Pulse styling works", async ({ page }) => { + test.skip("Pulse styling works", async ({ page }) => { await page.goto("/tools/test/src/html/basic-test.html"); await page.evaluate(async () => { while (!window["__TEST_PERSPECTIVE_READY__"]) { @@ -172,7 +172,7 @@ test.describe("Column Style Tests", () => { await compareContentsToSnapshot(contents); }); - test("Pulse styling works when settings panel is open", async ({ + test.skip("Pulse styling works when settings panel is open", async ({ page, }) => { await page.goto("/tools/test/src/html/basic-test.html"); @@ -226,6 +226,66 @@ test.describe("Column Style Tests", () => { await compareContentsToSnapshot(contents); }); + test("Column style label-bar", async ({ page }) => { + await page.goto("/tools/test/src/html/basic-test.html"); + await page.evaluate(async () => { + while (!window["__TEST_PERSPECTIVE_READY__"]) { + await new Promise((x) => setTimeout(x, 10)); + } + }); + + await page.evaluate(async () => { + await document.querySelector("perspective-viewer").restore({ + columns_config: { + "Row ID": { + number_fg_mode: "label-bar", + }, + }, + plugin: "Datagrid", + settings: true, + sort: [["Order ID", "desc"]], + columns: ["Row ID", "Order ID"], + }); + }); + + const contents = await page + .locator(`perspective-viewer-datagrid regular-table`) + .innerHTML(); + + await compareContentsToSnapshot(contents); + }); + + test("Column style label-bar on an expression column", async ({ page }) => { + await page.goto("/tools/test/src/html/basic-test.html"); + await page.evaluate(async () => { + while (!window["__TEST_PERSPECTIVE_READY__"]) { + await new Promise((x) => setTimeout(x, 10)); + } + }); + + await page.evaluate(async () => { + await document.querySelector("perspective-viewer").restore({ + columns_config: { + test: { + number_fg_mode: "label-bar", + }, + }, + plugin: "Datagrid", + settings: true, + expressions: { test: `"Row ID" + 100` }, + sort: [["Order ID", "desc"]], + columns: ["test", "Row ID", "Order ID"], + }); + }); + + await page.pause(); + const contents = await page + .locator(`perspective-viewer-datagrid regular-table`) + .innerHTML(); + + await compareContentsToSnapshot(contents); + }); + test("Column style menu opens for string columns", async ({ page }) => { await page.goto("/tools/test/src/html/basic-test.html"); await page.evaluate(async () => { @@ -554,4 +614,101 @@ test.describe("Column Style Tests", () => { await compareContentsToSnapshot(contents); }); + + // Regression: a single `restore()` carrying both `plugin_config` and + // `columns_config` used to drop `columns_config` because + // `restore_and_render` combined them with a short-circuiting `||`. + // Asserting via `save()` keeps the test independent of any plugin's + // rendering choices. + test("first restore applies columns_config when plugin_config is also set", async ({ + page, + }) => { + await page.goto("/tools/test/src/html/basic-test.html"); + await page.evaluate(async () => { + while (!window["__TEST_PERSPECTIVE_READY__"]) { + await new Promise((x) => setTimeout(x, 10)); + } + }); + + const saved = await page.evaluate(async () => { + const viewer = document.querySelector("perspective-viewer"); + await viewer.restore({ + plugin: "Datagrid", + columns: ["Row ID", "Sales"], + plugin_config: { edit_mode: "EDIT" }, + columns_config: { + Sales: { number_bg_mode: "pulse" }, + }, + }); + return await viewer.save(); + }); + + expect(saved.plugin_config).toEqual({ edit_mode: "EDIT" }); + expect(saved.columns_config).toEqual({ + Sales: { number_bg_mode: "pulse" }, + }); + }); + + test("first restore applies columns_config + plugin_config without an explicit plugin", async ({ + page, + }) => { + await page.goto("/tools/test/src/html/basic-test.html"); + await page.evaluate(async () => { + while (!window["__TEST_PERSPECTIVE_READY__"]) { + await new Promise((x) => setTimeout(x, 10)); + } + }); + + const saved = await page.evaluate(async () => { + const viewer = document.querySelector("perspective-viewer"); + await viewer.restore({ + columns: ["Row ID", "Sales"], + plugin_config: { edit_mode: "EDIT" }, + columns_config: { + Sales: { number_bg_mode: "pulse" }, + }, + }); + return await viewer.save(); + }); + + expect(saved.plugin_config).toEqual({ edit_mode: "EDIT" }); + expect(saved.columns_config).toEqual({ + Sales: { number_bg_mode: "pulse" }, + }); + }); + + test("repeated restore with plugin_config + columns_config is stable", async ({ + page, + }) => { + await page.goto("/tools/test/src/html/basic-test.html"); + await page.evaluate(async () => { + while (!window["__TEST_PERSPECTIVE_READY__"]) { + await new Promise((x) => setTimeout(x, 10)); + } + }); + + const [first, second] = await page.evaluate(async () => { + const viewer = document.querySelector("perspective-viewer"); + const payload = { + plugin: "Datagrid", + columns: ["Row ID", "Sales"], + plugin_config: { edit_mode: "EDIT" }, + columns_config: { + Sales: { number_bg_mode: "pulse" }, + }, + }; + await viewer.restore(payload); + const first = await viewer.save(); + await viewer.restore(payload); + const second = await viewer.save(); + return [first, second]; + }); + + expect(first.plugin_config).toEqual({ edit_mode: "EDIT" }); + expect(first.columns_config).toEqual({ + Sales: { number_bg_mode: "pulse" }, + }); + expect(second.plugin_config).toEqual(first.plugin_config); + expect(second.columns_config).toEqual(first.columns_config); + }); }); diff --git a/rust/perspective-viewer/src/rust/components/viewer.rs b/rust/perspective-viewer/src/rust/components/viewer.rs index bc94eed3a5..a26c3b4064 100644 --- a/rust/perspective-viewer/src/rust/components/viewer.rs +++ b/rust/perspective-viewer/src/rust/components/viewer.rs @@ -548,8 +548,15 @@ impl Component for PerspectiveViewer { skip_empty=true initial_size={self.settings_panel_width_override} on_reset={ctx.link().callback(|_| SettingsPanelSizeUpdate(None))} - on_resize={on_split_panel_resize.clone()} - on_resize_finished={render_callback(&ctx.props().session, &ctx.props().renderer)} + on_resize={{ + let size_cb = on_split_panel_resize.clone(); + let resize_cb = resize_callback(&ctx.props().session, &ctx.props().renderer); + move |x| { + size_cb.emit(x); + resize_cb.emit(()); + } + }} + on_resize_finished={resize_callback(&ctx.props().session, &ctx.props().renderer)} > { settings_panel }
diff --git a/rust/perspective-viewer/src/rust/renderer.rs b/rust/perspective-viewer/src/rust/renderer.rs index 47e9f36631..5dd0a6f555 100644 --- a/rust/perspective-viewer/src/rust/renderer.rs +++ b/rust/perspective-viewer/src/rust/renderer.rs @@ -31,7 +31,7 @@ use std::ops::Deref; use std::pin::Pin; use std::rc::Rc; -use futures::future::select_all; +use futures::future::{join_all, select_all}; use perspective_client::config::ViewConfig; use perspective_client::utils::*; use perspective_client::{View, ViewWindow}; @@ -52,6 +52,7 @@ pub use self::registry::*; use self::render_timer::*; use crate::config::*; use crate::js::plugin::*; +use crate::queries::resolve_abs_max; use crate::session::Session; use crate::utils::*; @@ -239,12 +240,69 @@ impl Renderer { /// `include` fields off other fields (e.g. Datagrid's /// `fg_gradient` revealed when `number_fg_mode = "bar"`) will /// reach the plugin without their default value populated. - pub fn all_columns_configs_materialized( + /// + /// `async` because per-column stats (e.g. `abs_max` for Datagrid's + /// `fg_gradient`) may need to be fetched before the schema's + /// `default` is meaningful. Pass 1 sync-scans the schema for any + /// column whose `include: true` Number key is missing from its + /// entry AND has no cached stats — `view_config_changed` clears the + /// stats cache, so "missing in cache" subsumes "stale". Pass 2 + /// blocks on a parallel `resolve_abs_max` for that set, then runs + /// the materialize loop with the now-warm cache. Columns never + /// touched in a stats-dependent mode never trigger a fetch. + pub async fn all_columns_configs_materialized( &self, view_config: &ViewConfig, session: &Session, ) -> ColumnConfigMap { let mut configs = self.all_columns_configs(); + + // Pass 1: identify columns whose schema demands an `include: + // true` Number default we don't have stats for. + let mut to_warm: Vec = vec![]; + for (col, entry) in &configs { + if session + .get_column_stats(col) + .and_then(|s| s.abs_max) + .is_some() + { + continue; + } + let Ok(schema) = + self.query_column_config_schema(view_config, session, col, Some(entry)) + else { + continue; + }; + let needs_warm = schema.fields.iter().any(|f| { + matches!( + f, + ControlSpec::Number { + key, + include: Some(true), + .. + } if !entry.contains_key(key) + ) + }); + if needs_warm { + to_warm.push(col.clone()); + } + } + + // Block on the (typically tiny) warm set. Clone the metadata + // and resolve the view ref *before* the .await — `metadata()` + // returns a live `Ref<>` guard that must not cross an await + // boundary. + if !to_warm.is_empty() { + let metadata = session.metadata().clone(); + let view = session.get_view(); + let futs = to_warm + .iter() + .map(|c| resolve_abs_max(session, &metadata, view.as_ref(), c.as_str())); + join_all(futs).await; + } + + // Pass 2: materialize. With stats now in cache, the schema + // returns a real `default` instead of the placeholder `0`. for (col, entry) in &mut configs { let Ok(schema) = self.query_column_config_schema(view_config, session, col, Some(entry)) @@ -345,6 +403,8 @@ impl Renderer { if let Ok(schema) = self.query_column_config_schema(view_config, session, &col, Some(&cfg)) { + let active = schema.active_keys(); + cfg.retain(|k, _| active.contains(k)); strip_default_values(&schema, &mut cfg); } @@ -468,13 +528,15 @@ impl Renderer { /// the columns-config write paths (strip-on-write) and the /// restore-prep snapshot (materialize-on-read). /// - /// Reads the cached `ColumnStats` (populated by the StyleTab's - /// `fetch_column_abs_max` task and cleared on `view_config_changed`) + /// Reads the cached `ColumnStats` (cleared on `view_config_changed`) /// so plugins emit gradient defaults against the column's current - /// `abs_max` instead of falling back to `0`. Cache cold ⇒ stats - /// pass through as missing and the plugin uses its `?? 0` fallback; - /// `include: true` fields are still preserved via - /// [`matches_declared_default`]'s short-circuit. + /// `abs_max` instead of falling back to `0`. + /// [`Self::all_columns_configs_materialized`] warms the cache on + /// demand before materializing `include: true` Number fields, so + /// the restore path always observes a real default; sync callers + /// (column-config strip-on-write) may still see a missing stats + /// pass-through and the plugin's `?? 0` fallback, but those writes + /// re-strip on the next render cycle. fn query_column_config_schema( &self, view_config: &ViewConfig, @@ -549,6 +611,8 @@ impl Renderer { OptionalUpdate::Update(mut map) => { let mut changed = false; if let Some(s) = &schema { + let active = s.active_keys(); + map.retain(|k, _| active.contains(k)); // Default-valued entries in a restore payload // semantically reset the key — strip from the // map AND clear any existing override in the diff --git a/rust/perspective-viewer/src/rust/tasks/restore_and_render.rs b/rust/perspective-viewer/src/rust/tasks/restore_and_render.rs index 171dae3b4f..fed0680b08 100644 --- a/rust/perspective-viewer/src/rust/tasks/restore_and_render.rs +++ b/rust/perspective-viewer/src/rust/tasks/restore_and_render.rs @@ -86,6 +86,22 @@ pub fn restore_and_render( let plugin_swapped = renderer.apply_pending_plugin()?; let plugin = renderer.get_active_plugin()?; + // The previous call which acquired the lock errored, so skip this render + if let Some(error) = session.get_error() { + return Err(error); + } + + // Validate + create the view BEFORE applying + // columns_config / plugin_config updates, so the + // strip-on-write and materialize passes see fresh + // `expression_schema` and `view_schema`, and the + // materialize warm step can call `View::get_min_max` + // against a view that knows about any new expression + // columns. Previously this happened after the strip, + // which silently dropped any `columns_config` entry + // keyed by a new expression column. + let view = session.validate().await?.create_view().await?; + // Apply incoming updates into the now-active plugin's // bucket on `Renderer`. Per-plugin storage means no // schema filter is needed before restore — foreign keys @@ -96,9 +112,9 @@ pub fn restore_and_render( let view_config_snapshot = session.get_view_config().clone(); let plugin_config_changed = renderer.update_plugin_config(&view_config_snapshot, plugin_config); - - let changed = plugin_config_changed - || renderer.update_columns_configs(&view_config_snapshot, &session, columns_config); + let columns_config_changed = + renderer.update_columns_configs(&view_config_snapshot, &session, columns_config); + let changed = plugin_config_changed || columns_config_changed; // Force a materialized restore when the plugin just // swapped — `commit_plugin_idx` already restored from the @@ -109,24 +125,19 @@ pub fn restore_and_render( let plugin_config_snapshot = renderer.get_plugin_config(); let plugin_update = wasm_bindgen::JsValue::from_serde_ext(&plugin_config_snapshot).unwrap(); - let columns_config = - renderer.all_columns_configs_materialized(&view_config_snapshot, &session); + let columns_config = renderer + .all_columns_configs_materialized(&view_config_snapshot, &session) + .await; plugin.restore(&plugin_update, Some(&columns_config))?; if plugin_config_changed { renderer.plugin_config_changed.emit(plugin_config_snapshot); } } - // The previous call which acquired the lock errored, so skip this render - if let Some(error) = session.get_error() { - return Err(error); - } - - let view = session.validate().await?.create_view().await; if !presentation.is_visible() { Ok(None) } else { - view + Ok(view) } }); diff --git a/rust/perspective-viewer/src/rust/tasks/send_column_config.rs b/rust/perspective-viewer/src/rust/tasks/send_column_config.rs index 1a3075b297..eaf21b07c8 100644 --- a/rust/perspective-viewer/src/rust/tasks/send_column_config.rs +++ b/rust/perspective-viewer/src/rust/tasks/send_column_config.rs @@ -38,8 +38,9 @@ pub fn send_column_config( clone!(session, renderer); ApiFuture::spawn(async move { let view_config_snapshot = session.get_view_config().clone(); - let columns_configs = - renderer.all_columns_configs_materialized(&view_config_snapshot, &session); + let columns_configs = renderer + .all_columns_configs_materialized(&view_config_snapshot, &session) + .await; let plugin_token = wasm_bindgen::JsValue::from_serde_ext(&renderer.get_plugin_config()).unwrap(); renderer diff --git a/rust/perspective-viewer/src/rust/tasks/send_plugin_config.rs b/rust/perspective-viewer/src/rust/tasks/send_plugin_config.rs index e7da7bfd7e..465babd581 100644 --- a/rust/perspective-viewer/src/rust/tasks/send_plugin_config.rs +++ b/rust/perspective-viewer/src/rust/tasks/send_plugin_config.rs @@ -36,8 +36,9 @@ pub fn send_plugin_config(session: &Session, renderer: &Renderer, update: Column let plugin_config = renderer.get_plugin_config(); let plugin_token = wasm_bindgen::JsValue::from_serde_ext(&plugin_config).unwrap(); let view_config_snapshot = session.get_view_config().clone(); - let columns_configs = - renderer.all_columns_configs_materialized(&view_config_snapshot, &session); + let columns_configs = renderer + .all_columns_configs_materialized(&view_config_snapshot, &session) + .await; renderer .get_active_plugin()? .restore(&plugin_token, Some(&columns_configs))?; diff --git a/rust/perspective-viewer/src/rust/tasks/update_and_render.rs b/rust/perspective-viewer/src/rust/tasks/update_and_render.rs index 0e62b99958..f8eb73c1e8 100644 --- a/rust/perspective-viewer/src/rust/tasks/update_and_render.rs +++ b/rust/perspective-viewer/src/rust/tasks/update_and_render.rs @@ -78,6 +78,13 @@ async fn update_and_render_inner(session: Session, renderer: Renderer) -> ApiRes } let plugin_swapped = renderer.apply_pending_plugin()?; + + // Validate + create the view BEFORE the plugin-swap materialize + // so the schema query sees fresh `expression_schema` / + // `view_schema` and `resolve_abs_max` has a bound view that + // knows about any new expression columns. + let view = session.validate().await?.create_view().await?; + if plugin_swapped { // `commit_plugin_idx` already restored the new plugin from its // raw bucket; re-run with the materialized snapshot so any @@ -91,13 +98,13 @@ async fn update_and_render_inner(session: Session, renderer: Renderer) -> ApiRes let view_config_snapshot = session.get_view_config().clone(); let plugin_token = wasm_bindgen::JsValue::from_serde_ext(&renderer.get_plugin_config()) .unwrap_or(wasm_bindgen::JsValue::NULL); - let columns_config = - renderer.all_columns_configs_materialized(&view_config_snapshot, &session); + let columns_config = renderer + .all_columns_configs_materialized(&view_config_snapshot, &session) + .await; renderer .get_active_plugin()? .restore(&plugin_token, Some(&columns_config))?; } - let view = session.validate().await?; - renderer.draw(view.create_view()).await + renderer.draw(async { Ok(view) }).await } diff --git a/tools/test/src/js/snapshot-sync.ts b/tools/test/src/js/snapshot-sync.ts index 17f196286e..4d211edae8 100644 --- a/tools/test/src/js/snapshot-sync.ts +++ b/tools/test/src/js/snapshot-sync.ts @@ -27,25 +27,37 @@ function git(args: string[], cwd: string) { execFileSync("git", args, { cwd, stdio: ["ignore", "pipe", "pipe"] }); } -function remoteHasRef(remoteUrl: string, ref: string): boolean { +function remoteHasRef( + remoteUrl: string, + ref: string, + // token: string | undefined, +): boolean { try { - const out = execFileSync( - "git", - ["ls-remote", "--heads", remoteUrl, ref], - { stdio: ["ignore", "pipe", "pipe"] }, - ) + const args: string[] = [ + "-c", + "http.https://github.com/.extraheader=", + "ls-remote", + "--heads", + remoteUrl, + ref, + ]; + const out = execFileSync("git", args, { + stdio: ["ignore", "pipe", "pipe"], + }) .toString() .trim(); return out.length > 0; - } catch { + } catch (e) { + console.error(e); return false; } } function buildRemoteUrl(repo: string, token: string | undefined): string { if (token) { - return `https://x-access-token:${token}@github.com/${repo}.git`; + return `https://${token}@github.com/${repo}.git`; } + return `git@github.com:${repo}.git`; } @@ -106,6 +118,8 @@ export async function fetchSnapshots(): Promise { execFileSync( "git", [ + "-c", + "http.https://github.com/.extraheader=", "clone", "--depth", "1", diff --git a/tools/test/src/js/utils.ts b/tools/test/src/js/utils.ts index 2978f18ca3..7f8687dfa1 100644 --- a/tools/test/src/js/utils.ts +++ b/tools/test/src/js/utils.ts @@ -160,11 +160,13 @@ export const getSvgContentString = (selector: string) => async (page: Page) => { export async function compareContentsToSnapshot( contents: string, ): Promise { - const cleanedContents = contents - .replace(/style=""/g, "") - .replace(/(min-|max-)?(width|height): *\d+\.*\d+(px)?;? */g, ""); + // contents = contents + // .replace(/(min-|max-)?(width|height): *\d+\.*\d+(px)?;? */g, "") + // .replace(/style=""/g, ""); - const formatted = await prettier.format(cleanedContents, { + contents = contents.replace(/style=""/g, ""); + + const formatted = await prettier.format(contents, { parser: "html", });