-
Notifications
You must be signed in to change notification settings - Fork 199
Added Crumble Zoom+pan & scroll #1051
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
1d5f03b
d81ded2
ded0af5
7b49ef5
db19363
9b84c6a
8047eec
2f76e0f
df008c3
8b1f2c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,8 +17,14 @@ class StateSnapshot { | |
| * @param {!number} mouseDownX | ||
| * @param {!number} mouseDownY | ||
| * @param {!Array<![!number, !number]>} boxHighlightPreview | ||
| * @param {!number} viewportX | ||
| * @param {!number} viewportY | ||
| * @param {!number} viewportZoom | ||
| * @param {!number} timelineScrollY | ||
| * @param {!number} curMouseScreenX | ||
| * @param {!number} curMouseScreenY | ||
| */ | ||
| constructor(circuit, curLayer, focusedSet, timelineSet, curMouseX, curMouseY, mouseDownX, mouseDownY, boxHighlightPreview) { | ||
| constructor(circuit, curLayer, focusedSet, timelineSet, curMouseX, curMouseY, mouseDownX, mouseDownY, boxHighlightPreview, viewportX=0, viewportY=0, viewportZoom=1, timelineScrollY=0, curMouseScreenX=undefined, curMouseScreenY=undefined) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are these new parameters defaulting to values, when the old ones weren't?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed default assignments |
||
| this.circuit = circuit.copy(); | ||
| this.curLayer = curLayer; | ||
| this.focusedSet = new Map(focusedSet.entries()); | ||
|
|
@@ -28,6 +34,12 @@ class StateSnapshot { | |
| this.mouseDownX = mouseDownX; | ||
| this.mouseDownY = mouseDownY; | ||
| this.boxHighlightPreview = [...boxHighlightPreview]; | ||
| this.viewportX = viewportX; | ||
| this.viewportY = viewportY; | ||
| this.viewportZoom = viewportZoom; | ||
| this.timelineScrollY = timelineScrollY; | ||
| this.curMouseScreenX = curMouseScreenX; | ||
| this.curMouseScreenY = curMouseScreenY; | ||
|
|
||
| while (this.circuit.layers.length <= this.curLayer) { | ||
| this.circuit.layers.push(new Layer()); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,13 @@ | ||
| import {Circuit} from "./circuit/circuit.js" | ||
| import {minXY} from "./circuit/layer.js" | ||
| import {pitch} from "./draw/config.js" | ||
| import {MAX_QUBIT_COORDINATE, MAX_ZOOM, MIN_ZOOM, pitch} from "./draw/config.js" | ||
| import {GATE_MAP} from "./gates/gateset.js" | ||
| import {EditorState} from "./editor/editor_state.js"; | ||
| import {initUrlCircuitSync} from "./editor/sync_url_to_state.js"; | ||
| import {draw} from "./draw/main_draw.js"; | ||
| import {drawToolbox} from "./keyboard/toolbox.js"; | ||
| import {Operation} from "./circuit/operation.js"; | ||
| import {make_mpp_gate} from './gates/gateset_mpp.js'; | ||
| import {PropagatedPauliFrames} from './circuit/propagated_pauli_frames.js'; | ||
|
|
||
| const OFFSET_X = -pitch + Math.floor(pitch / 4) + 0.5; | ||
| const OFFSET_Y = -pitch + Math.floor(pitch / 4) + 0.5; | ||
|
|
@@ -39,6 +38,14 @@ txtStimCircuit.addEventListener('keydown', ev => ev.stopPropagation()); | |
|
|
||
| let editorState = /** @type {!EditorState} */ new EditorState(document.getElementById('cvn')); | ||
|
|
||
| function toWorldMouseX(screenX) { | ||
| return (screenX - editorState.viewportX) / editorState.viewportZoom + OFFSET_X; | ||
| } | ||
|
|
||
| function toWorldMouseY(screenY) { | ||
| return (screenY - editorState.viewportY) / editorState.viewportZoom + OFFSET_Y; | ||
| } | ||
|
|
||
| btnExport.addEventListener('click', _ev => { | ||
| exportCurrentState(); | ||
| }); | ||
|
|
@@ -144,8 +151,10 @@ function exportCurrentState() { | |
| } | ||
|
|
||
| editorState.canvas.addEventListener('mousemove', ev => { | ||
| editorState.curMouseX = ev.offsetX + OFFSET_X; | ||
| editorState.curMouseY = ev.offsetY + OFFSET_Y; | ||
| editorState.curMouseScreenX = ev.offsetX; | ||
| editorState.curMouseScreenY = ev.offsetY; | ||
| editorState.curMouseX = toWorldMouseX(ev.offsetX); | ||
| editorState.curMouseY = toWorldMouseY(ev.offsetY); | ||
|
|
||
| // Scrubber. | ||
| let w = editorState.canvas.width / 2; | ||
|
|
@@ -159,10 +168,12 @@ editorState.canvas.addEventListener('mousemove', ev => { | |
|
|
||
| let isInScrubber = false; | ||
| editorState.canvas.addEventListener('mousedown', ev => { | ||
| editorState.curMouseX = ev.offsetX + OFFSET_X; | ||
| editorState.curMouseY = ev.offsetY + OFFSET_Y; | ||
| editorState.mouseDownX = ev.offsetX + OFFSET_X; | ||
| editorState.mouseDownY = ev.offsetY + OFFSET_Y; | ||
| editorState.curMouseScreenX = ev.offsetX; | ||
| editorState.curMouseScreenY = ev.offsetY; | ||
| editorState.curMouseX = toWorldMouseX(ev.offsetX); | ||
| editorState.curMouseY = toWorldMouseY(ev.offsetY); | ||
| editorState.mouseDownX = toWorldMouseX(ev.offsetX); | ||
| editorState.mouseDownY = toWorldMouseY(ev.offsetY); | ||
|
|
||
| // Scrubber. | ||
| let w = editorState.canvas.width / 2; | ||
|
|
@@ -179,14 +190,80 @@ editorState.canvas.addEventListener('mouseup', ev => { | |
| let highlightedArea = editorState.currentPositionsBoxesByMouseDrag(ev.altKey); | ||
| editorState.mouseDownX = undefined; | ||
| editorState.mouseDownY = undefined; | ||
| editorState.curMouseX = ev.offsetX + OFFSET_X; | ||
| editorState.curMouseY = ev.offsetY + OFFSET_Y; | ||
| editorState.curMouseScreenX = ev.offsetX; | ||
| editorState.curMouseScreenY = ev.offsetY; | ||
| editorState.curMouseX = toWorldMouseX(ev.offsetX); | ||
| editorState.curMouseY = toWorldMouseY(ev.offsetY); | ||
| editorState.changeFocus(highlightedArea, ev.shiftKey, ev.ctrlKey); | ||
| if (ev.buttons === 1) { | ||
| isInScrubber = false; | ||
| } | ||
| }); | ||
|
|
||
| // Make sure qubit grid and timeline don't deviate from the area of interest. | ||
| function restrictQubitGridAndTimeline() { | ||
| const width = editorState.canvas.width / 2; | ||
| const height = editorState.canvas.height; | ||
| const zoom = editorState.viewportZoom; | ||
| const gridMin = -1 * pitch - OFFSET_X; | ||
| const gridMax = MAX_QUBIT_COORDINATE * pitch - OFFSET_X; | ||
|
|
||
| editorState.viewportX = Math.max( | ||
| width - gridMax * zoom, | ||
| Math.min(-gridMin * zoom, editorState.viewportX) | ||
| ); | ||
| editorState.viewportY = Math.max( | ||
| height - gridMax * zoom, | ||
| Math.min(-gridMin * zoom, editorState.viewportY) | ||
| ); | ||
|
|
||
| editorState.timelineScrollY = Math.max( | ||
| 0, | ||
| editorState.timelineScrollY | ||
| ); | ||
| } | ||
|
|
||
| function handleTimelineVerticalScroll(ev) { | ||
| editorState.timelineScrollY += ev.deltaY; | ||
| restrictQubitGridAndTimeline(); | ||
| editorState.force_redraw(); | ||
| return; | ||
| } | ||
|
|
||
| function handleQubitGridZoomPan(ev) { | ||
| if (ev.ctrlKey || ev.metaKey) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the control scheme should be the following:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, as for (1), using the middle click+drag does makes sense. Regarding whether the timeline X panning should be locked - the implementation is definitely simpler if we keep it locked, but I agree that allowing for X scroll is something that a user might want. I think I'll give it a go and see how it feels. (2) that makes sense for the mouse. However, I think it would make using the touchpad (which is what I tested on) unintuitive. I'd argue that using the modifier key for zoom is a better compromise, but I'll give it some more thought tomorrow. What do you think? (3) if we do decide to go for "scroll is zoom for timeslice" in (2) - then indeed it's not clear whether scrolling for scroll would feel natural here. I will say that with current implementation (scroll is pan) it does feel natural even though it only scrolls the Y axis at the moment.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, I didn't consider using a touchpad. It would also make sense to consider pinch actions etc for users with touch screens on their laptops. I'm fine with timeslice wheeling meaning to scroll rather than to zoom.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently pinch works on my touchpad, so I assume it should work similarly on a touch screen - but I'll check. So to summarize, I think the approach would be:
|
||
| // Handle zoom. | ||
| const zoomMultiplier = ev.deltaY < 0 ? 1.05 : (1 / 1.05); | ||
| const newZoom = Math.max(MIN_ZOOM, Math.min(MAX_ZOOM, editorState.viewportZoom * zoomMultiplier)); | ||
| const ratio = newZoom / editorState.viewportZoom; | ||
| editorState.viewportZoom = newZoom; | ||
|
|
||
| // Center zoom around mouse. | ||
| editorState.viewportX = ev.offsetX - (ev.offsetX - editorState.viewportX) * ratio; | ||
| editorState.viewportY = ev.offsetY - (ev.offsetY - editorState.viewportY) * ratio; | ||
| } else { | ||
| // Handle pan. | ||
| editorState.viewportX -= ev.deltaX; | ||
| editorState.viewportY -= ev.deltaY; | ||
| } | ||
|
|
||
| editorState.curMouseX = toWorldMouseX(ev.offsetX); | ||
| editorState.curMouseY = toWorldMouseY(ev.offsetY); | ||
| restrictQubitGridAndTimeline(); | ||
| editorState.force_redraw(); | ||
| } | ||
|
|
||
| editorState.canvas.addEventListener('wheel', ev => { | ||
| ev.preventDefault(); | ||
| const width = editorState.canvas.width / 2; | ||
|
|
||
| if (ev.offsetX > width) { | ||
| handleTimelineVerticalScroll(ev); | ||
| } else { | ||
| handleQubitGridZoomPan(ev); | ||
| } | ||
| }, { passive: false }); | ||
|
|
||
| /** | ||
| * @return {!Map<!string, !function(preview: !boolean) : void>} | ||
| */ | ||
|
|
@@ -504,7 +581,13 @@ editorState.rev.changes().subscribe(() => { | |
| drawToolbox(editorState.chorder.toEvent(false)); | ||
| }); | ||
| initUrlCircuitSync(editorState.rev); | ||
| editorState.obs_val_draw_state.observable().subscribe(ds => requestAnimationFrame(() => draw(editorState.canvas.getContext('2d'), ds))); | ||
| editorState.obs_val_draw_state.observable().subscribe(ds => requestAnimationFrame(() => { | ||
| const maxTimelineScrollY = draw(editorState.canvas.getContext('2d'), ds); | ||
| // Prevent over-scrolling. | ||
| if (editorState.timelineScrollY > maxTimelineScrollY) { | ||
| editorState.timelineScrollY = maxTimelineScrollY; | ||
| } | ||
| })); | ||
| window.addEventListener('focus', () => { | ||
| editorState.chorder.handleFocusChanged(); | ||
| }); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's super counter-intuitive that the draw command would return
maxTimelineScrollY. Provide an alternate route to get at it, like agetMaxTimelineScrollYmethod, and don't return it here.Alternatively, declare a
DrawSummarystruct with amaxTimelineScrollYand return that. And update the docstring of this method to say it's returning that value.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. added DrawSummary.