Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions packages/core/src/components/publish/publish-compositor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,35 @@ describe('publish-compositor', () => {
expect(result).toBe(mockCanvas);
});

// The figure editor relies on this passthrough to force the default
// (unzoomed) view (#294); guard it so a dropped forward is caught.
it('forwards resetView to captureAtResolution', () => {
const mockCanvas = document.createElement('canvas');
const seen: Array<{ resetView?: boolean }> = [];
const plotEl = document.createElement('div') as HTMLElement & {
captureAtResolution?: (
w: number,
h: number,
opts: { resetView?: boolean },
) => HTMLCanvasElement;
};
plotEl.captureAtResolution = (_w, _h, opts) => {
seen.push(opts);
return mockCanvas;
};

capturePlotCanvas(plotEl, {
width: 800,
height: 400,
backgroundColor: '#ffffff',
resetView: true,
});
capturePlotCanvas(plotEl, { width: 800, height: 400, backgroundColor: '#ffffff' });

expect(seen[0].resetView).toBe(true);
expect(seen[1].resetView).toBeUndefined();
});

/**
* jsdom doesn't implement canvas 2d. Instead of pixel sampling, spy on
* the output canvas's 2d context and assert drawImage's call signature.
Expand Down
7 changes: 6 additions & 1 deletion packages/core/src/components/publish/publish-compositor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ interface CaptureOptions {
width: number;
height: number;
backgroundColor: string;
/** Render the default fit-all view, ignoring any live zoom/pan. The figure
* editor always captures the unzoomed view so a zoomed plot doesn't leak
* into the figure (and the zoom-inset mapping stays correct). */
resetView?: boolean;
}

/**
Expand All @@ -52,7 +56,7 @@ export function capturePlotCanvas(
captureAtResolution?: (
w: number,
h: number,
opts: { dpr?: number; backgroundColor?: string },
opts: { dpr?: number; backgroundColor?: string; resetView?: boolean },
) => HTMLCanvasElement;
},
opts: CaptureOptions,
Expand All @@ -61,6 +65,7 @@ export function capturePlotCanvas(
return plotEl.captureAtResolution(opts.width, opts.height, {
dpr: 1,
backgroundColor: opts.backgroundColor,
resetView: opts.resetView,
});
}
// Fallback: grab whatever canvas the component has
Expand Down
13 changes: 10 additions & 3 deletions packages/core/src/components/publish/publish-modal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -819,10 +819,14 @@ describe('<protspace-publish-modal> plot cache key', () => {
});

it('invalidates the plot cache when background toggles white ↔ transparent', async () => {
const captures: Array<{ bg: string }> = [];
const captures: Array<{ bg: string; resetView?: boolean }> = [];
const fakePlotEl = {
captureAtResolution: (w: number, h: number, opts: { backgroundColor?: string }) => {
captures.push({ bg: opts.backgroundColor ?? '' });
captureAtResolution: (
w: number,
h: number,
opts: { backgroundColor?: string; resetView?: boolean },
) => {
captures.push({ bg: opts.backgroundColor ?? '', resetView: opts.resetView });
const c = document.createElement('canvas');
c.width = w;
c.height = h;
Expand Down Expand Up @@ -865,6 +869,9 @@ describe('<protspace-publish-modal> plot cache key', () => {

expect(captures.some((c) => c.bg === '#ffffff')).toBe(true);
expect(captures.some((c) => c.bg === 'rgba(0,0,0,0)')).toBe(true);
// #294: the editor preview always captures the default (unzoomed) view.
expect(captures.length).toBeGreaterThan(0);
expect(captures.every((c) => c.resetView === true)).toBe(true);
modal.remove();
} finally {
HTMLCanvasElement.prototype.getContext = origGetContext;
Expand Down
8 changes: 8 additions & 0 deletions packages/core/src/components/publish/publish-modal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ interface CaptureablePlotElement extends HTMLElement {
backgroundColor?: string;
dataDomain?: { xMin: number; xMax: number; yMin: number; yMax: number };
pointSizeReference?: { width: number; height: number };
resetView?: boolean;
},
) => HTMLCanvasElement;
getDataExtent?: (options?: {
Expand Down Expand Up @@ -372,6 +373,8 @@ export class ProtspacePublishModal extends LitElement {
width: plotRect.w,
height: plotRect.h,
backgroundColor: bgColor,
// Always render the unzoomed, fit-all view in the editor (#294).
resetView: true,
});
this._plotCacheKey = cacheKey;
}
Expand Down Expand Up @@ -616,6 +619,9 @@ export class ProtspacePublishModal extends LitElement {
width: plotRect.w * pointScale,
height: plotRect.h * pointScale,
},
// dataDomain is derived from the full (default-view) extent, so the
// live zoom must not be re-applied on top of it (#294).
resetView: true,
});
this._insetRenderCache.set(key, canvas);
this._lastInsetCanvases[i] = canvas;
Expand Down Expand Up @@ -722,6 +728,8 @@ export class ProtspacePublishModal extends LitElement {
width: plotRect.w,
height: plotRect.h,
backgroundColor: bgColor,
// Export the unzoomed, fit-all view, matching the live preview (#294).
resetView: true,
});

// Per-inset geometric renders for the export.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/** @vitest-environment jsdom */
import { describe, it, expect } from 'vitest';
import { zoomIdentity } from 'd3';
import { DuplicateStackOverlayController } from './duplicate-stack-overlay-controller';

/**
* #294: captureBadges renders the duplicate-stack badges at a caller-supplied
* transform (the figure editor passes identity) into a fresh off-screen canvas.
* When the overlay is disabled or no stacks are in view it returns null, so the
* caller (scatter-plot.captureAtResolution) skips compositing rather than
* pasting a blank or stale canvas onto the unzoomed render.
*/
function makeController(opts: { isEnabled?: boolean } = {}) {
const config = {
width: 800,
height: 600,
margin: { top: 20, right: 20, bottom: 20, left: 20 },
};
const deps = {
getOverlayGroup: () => null,
getBadgesCanvas: () => undefined,
getTransform: () => zoomIdentity,
getConfig: () => config,
getScales: () => null,
getPlotData: () => ({}),
getQuadtree: () => ({}),
isEnabled: () => opts.isEnabled ?? true,
isSelectionMode: () => false,
getColor: () => '#000000',
onPointActivate: () => {},
onHover: () => {},
onHoverEnd: () => {},
};
return new DuplicateStackOverlayController(
deps as unknown as ConstructorParameters<typeof DuplicateStackOverlayController>[0],
);
}

describe('DuplicateStackOverlayController.captureBadges (#294)', () => {
it('returns null when the overlay is disabled', () => {
expect(makeController({ isEnabled: false }).captureBadges(zoomIdentity)).toBeNull();
});

it('returns null when there are no duplicate stacks in view', () => {
// Fresh controller: no viewport compute has run, so there is nothing to draw.
expect(makeController({ isEnabled: true }).captureBadges(zoomIdentity)).toBeNull();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,44 @@ export class DuplicateStackOverlayController {
});
}

/**
* Render the current duplicate-stack badges into a fresh off-screen canvas
* using `transform` instead of the live zoom, leaving the on-screen badge
* canvas untouched. The figure editor passes `d3.zoomIdentity` so the captured
* badges line up with the unzoomed, fit-all points (#294) — the live badge
* canvas is positioned for the live zoom/pan and would otherwise be composited
* at the wrong place. The canvas is sized exactly like the live badge canvas
* (config size × devicePixelRatio) so the caller's existing draw-image stretch
* behaves identically. Returns null (so the caller skips compositing) when the
* overlay is disabled or no stacks are currently in view.
*
* Coverage note: badges reflect the stacks computed for the last live viewport
* (`ensureForViewport` queries only the visible region for perf), so a capture
* taken while the live plot is zoomed in shows that region's badges — the same
* coverage as the live overlay, now drawn at their correct fit-all positions.
*/
captureBadges(transform: ZoomTransform): HTMLCanvasElement | null {
if (!this.deps.isEnabled()) return null;
const config = this.deps.getConfig();
const win = computeViewportWindow(transform, config, DUPLICATE_BADGES_VIEWPORT_PADDING);
const stacksToRender = cullAndCapStacks(this.stacks, win, this.expandedKey, this.byKey);
if (stacksToRender.length === 0) return null;

const dpr = window.devicePixelRatio || 1;
const target = document.createElement('canvas');
target.width = Math.max(1, Math.floor(config.width * dpr));
target.height = Math.max(1, Math.floor(config.height * dpr));

const renderer = new DuplicateBadgesCanvasRenderer({
getCanvas: () => target,
getTransform: () => transform,
getSize: () => ({ width: config.width, height: config.height }),
getExpandedKey: () => this.expandedKey,
});
renderer.render(stacksToRender);
return target;
}

// ----- Public surface (1:1 with the old private methods on the host) -----

updateSelectionOverlays(options: { duplicateImmediate?: boolean } = {}): void {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/**
* @vitest-environment jsdom
*
* #294: the figure editor captures the scatter-plot with `resetView: true` so
* the points render at the default fit-all view. The duplicate-stack badges are
* a separate Canvas2D overlay whose live `_badgesCanvas` is positioned for the
* live zoom/pan — compositing it as-is onto an unzoomed render would paste the
* badges at their zoomed positions. These tests pin the fix: on the resetView
* (non-inset) path, captureAtResolution re-renders the badges at the identity
* transform via the overlay controller instead of using the live canvas.
*
* Constructed via createElement without appending (no connectedCallback), so the
* WebGL/canvas init never runs under jsdom. ResizeObserver is stubbed before the
* element module is imported (the constructor news one up).
*/
import { vi, describe, it, expect, beforeEach } from 'vitest';

vi.hoisted(() => {
if (!('ResizeObserver' in globalThis)) {
(globalThis as unknown as { ResizeObserver: unknown }).ResizeObserver = class {
observe() {}
unobserve() {}
disconnect() {}
};
}
});

import './scatter-plot';

type CaptureInternals = HTMLElement & {
_webglRenderer: unknown;
_dupOverlay: { captureBadges(transform: { x: number; y: number; k: number }): unknown };
captureAtResolution(
width: number,
height: number,
options?: {
resetView?: boolean;
dataDomain?: { xMin: number; xMax: number; yMin: number; yMax: number };
},
): HTMLCanvasElement;
};

describe('scatter-plot captureAtResolution — badges respect resetView (#294)', () => {
let el: CaptureInternals;

beforeEach(() => {
el = document.createElement('protspace-scatterplot') as CaptureInternals;
// Stub the WebGL renderer so capture proceeds to the badge-composite step
// without a real WebGL2 context (unavailable under jsdom).
el._webglRenderer = { renderToCanvas: vi.fn(() => document.createElement('canvas')) };
});

it('re-renders badges at the identity transform when resetView is true', () => {
const captureBadges = vi.spyOn(el._dupOverlay, 'captureBadges').mockReturnValue(null);

el.captureAtResolution(800, 600, { resetView: true });

expect(captureBadges).toHaveBeenCalledTimes(1);
const transform = captureBadges.mock.calls[0][0];
// d3.zoomIdentity — the default, unzoomed view.
expect(transform.x).toBe(0);
expect(transform.y).toBe(0);
expect(transform.k).toBe(1);
});

it('uses the live badge canvas (no re-render) when resetView is not requested', () => {
const captureBadges = vi.spyOn(el._dupOverlay, 'captureBadges').mockReturnValue(null);

el.captureAtResolution(800, 600);

expect(captureBadges).not.toHaveBeenCalled();
});

it('does not re-render badges on the inset (dataDomain) path even with resetView', () => {
const captureBadges = vi.spyOn(el._dupOverlay, 'captureBadges').mockReturnValue(null);

el.captureAtResolution(200, 200, {
resetView: true,
dataDomain: { xMin: 0.1, xMax: 0.4, yMin: 0.1, yMax: 0.4 },
});

expect(captureBadges).not.toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ describe('scatter-plot isolation render-refresh sequence', () => {
_reprocessAndRefresh(): void;
isolateSelection(): void;
resetIsolation(): void;
resetZoom(): void;
};

function buildData(): VisualizationData {
Expand Down Expand Up @@ -317,4 +318,39 @@ describe('scatter-plot isolation render-refresh sequence', () => {
el.resetIsolation();
expect(spy).toHaveBeenCalledTimes(2);
});

// #297: zooming into a region and then isolating should snap back to the full
// view of the isolated subset, not keep the stale pre-isolation zoom transform.
it('isolateSelection resets the zoom to the full view', () => {
const el = makeEl();
el.selectedProteinIds = ['p1', 'p3'];
const resetZoom = vi.spyOn(el, 'resetZoom');

el.isolateSelection();

expect(resetZoom).toHaveBeenCalledTimes(1);
});

it('does not reset the zoom when isolateSelection bails (no valid selection)', () => {
const el = makeEl();
el.selectedProteinIds = [];
const resetZoom = vi.spyOn(el, 'resetZoom');

el.isolateSelection();

expect(resetZoom).not.toHaveBeenCalled();
});

// Symmetry with isolateSelection: exiting isolation restores the full dataset,
// so the view should also snap back to the full extent (#297).
it('resetIsolation resets the zoom to the full view', () => {
const el = makeEl();
el._isolationMode = true;
el._isolationHistory = [['p1', 'p3']];
const resetZoom = vi.spyOn(el, 'resetZoom');

el.resetIsolation();

expect(resetZoom).toHaveBeenCalledTimes(1);
});
});
Loading
Loading