Skip to content

Anchored smooth zoom: verify the existing blit and fix the sy divisor #435

Description

@turner

Follow-up to #431, which closed as Option B (delete the dead anchored-zoom path). Option A — anchored smooth zoom — was deferred to a feature request; this is it.

Reading the code to write this up turned up something that changes the framing, so please read the finding before scheduling the work.

Background

#431 removed a call that passed (anchorPx, anchorPy, scaleFactor) into contactMatrixView.zoomIn(), which declares no parameters. The stated intent of Option A was to make wheel and pinch zoom feel anchored on the cursor rather than recentred on the view.

Finding: the blit already tracks the anchor

The issue body of #431 asserted that zoomIn() "recentres on the view rather than on the anchor point the caller supplies." That looks wrong on inspection.

zoomIn() (js/contactMatrixView.js:240) derives its source rectangle from browser.state after State.panWithZoom has run, and panWithZoom (js/hicState.js:208-216) already does the anchor-preservation math — it solves for the x/y that hold the genomic position under (anchorPx, anchorPy) invariant across the pixelSize change. this.genomicExtent is the extent captured at the end of the previous repaint().

So sx/sy are the offset of the new (anchored) genomic window inside the old one. That is an anchored blit. The anchor reaches the view implicitly through State rather than as an argument — which is why the explicit parameters were never needed and never implemented.

If that reading holds, Option A is mostly already in place, and the remaining work is the bug below rather than a new feature.

Actual defect: sy divides by width

js/contactMatrixView.js:261-264:

const sx = ((newGenomicExtent.x - this.genomicExtent.x) / this.genomicExtent.w) * viewportWidth;
const sy = ((newGenomicExtent.y - this.genomicExtent.y) / this.genomicExtent.w) * viewportHeight;
const sWidth  = (newGenomicExtent.w / this.genomicExtent.w) * viewportWidth;
const sHeight = (newGenomicExtent.h / this.genomicExtent.h) * viewportHeight;

sy divides the vertical genomic offset by this.genomicExtent.w. It should be .h. sHeight on the next line uses .h correctly, so the two disagree.

This is invisible whenever the viewport is square (w === h), which is the common juicebox layout — likely why it has gone unnoticed. In a non-square viewport the vertical source offset is scaled by the wrong extent, so the zoom-in frame slides vertically before the repaint lands and corrects it. Symptom would be a vertical "pop" during wheel/pinch zoom, worse the further the aspect ratio is from 1.

Suggested work

  1. Confirm or refute the reading above — ideally by watching wheel zoom with the cursor well off-centre, in a deliberately non-square viewport.
  2. Fix sy to divide by this.genomicExtent.h.
  3. If anchoring still feels wrong after that fix, then consider passing the anchor explicitly and reopening the Option A design question.

Notes

  • Nothing about zoomIn() ignores its arguments: anchored smooth-zoom was wired up but never implemented #431's change forecloses any of this; the parameters can be reintroduced if step 3 turns out to be needed.
  • zoomIn() early-returns on zoom-out (newGenomicExtent.w > this.genomicExtent.w), so this path only ever runs zooming in.
  • Labelled needs-triage rather than ready-for-agent because step 1 is a judgement call about perceived interaction feel, which wants a human at the controls.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions