From eb185255a32fcd62bb06572bfd755bf4a4bb4e94 Mon Sep 17 00:00:00 2001 From: doouding Date: Mon, 19 May 2025 16:05:33 +0000 Subject: [PATCH] fix: selection rect should reflect viewport change (#12355) Fixes [BS-3349](https://linear.app/affine-design/issue/BS-3349/) ## Summary by CodeRabbit - **New Features** - Improved edge scrolling during selection dragging for smoother and more responsive viewport navigation. - Dragging area and mouse position tracking now update reactively with viewport changes, ensuring more accurate selection and movement. - **Refactor** - Unified and clarified coordinate handling for dragging and mouse position, with clearer naming and separation between model and browser coordinates. - Simplified selection logic and removed unnecessary accumulated state for cleaner and more maintainable behavior. - Enhanced flexibility in coordinate conversion by allowing viewport transformations relative to arbitrary zoom and center. - Streamlined clipboard paste handling by simplifying mouse position extraction and adjusting attachment options. - **Bug Fixes** - Enhanced overlay and dragging area accuracy by updating position calculations and coordinate transformations. - Fixed paste operations to correctly handle mouse position without unnecessary coordinate conversions. - Corrected drag initiation positions in toolbar and shape dragging to align with viewport-relative coordinates. --- .../root/src/edgeless/clipboard/clipboard.ts | 15 +- .../root/src/edgeless/clipboard/command.ts | 4 +- .../surface/src/renderer/tool-overlay.ts | 2 +- .../blocks/surface/src/tool/default-tool.ts | 96 ++++------ .../src/toolbar/mindmap-tool-button.ts | 2 +- .../shape/src/draggable/shape-draggable.ts | 2 +- blocksuite/affine/gfx/shape/src/shape-tool.ts | 14 +- .../src/edgeless-dragging-area-rect.ts | 2 +- .../std/src/gfx/tool/tool-controller.ts | 170 +++++++++++++----- blocksuite/framework/std/src/gfx/viewport.ts | 16 +- 10 files changed, 188 insertions(+), 135 deletions(-) diff --git a/blocksuite/affine/blocks/root/src/edgeless/clipboard/clipboard.ts b/blocksuite/affine/blocks/root/src/edgeless/clipboard/clipboard.ts index 9a5282cc9a..b5d74b2385 100644 --- a/blocksuite/affine/blocks/root/src/edgeless/clipboard/clipboard.ts +++ b/blocksuite/affine/blocks/root/src/edgeless/clipboard/clipboard.ts @@ -209,9 +209,10 @@ export class EdgelessClipboardController extends PageClipboard { await addImages(this.std, imageFiles, { point, maxWidth: MAX_IMAGE_WIDTH, + shouldTransformPoint: false, }); } else { - await addAttachments(this.std, [...files], point); + await addAttachments(this.std, [...files], point, false); } this.std.getOptional(TelemetryProvider)?.track('CanvasElementAdded', { @@ -227,11 +228,7 @@ export class EdgelessClipboardController extends PageClipboard { if (isUrlInClipboard(data)) { const url = data.getData('text/plain'); - const lastMousePos = this.toolManager.lastMousePos$.peek(); - const [x, y] = this.gfx.viewport.toModelCoord( - lastMousePos.x, - lastMousePos.y - ); + const { x, y } = this.toolManager.lastMousePos$.peek(); // try to interpret url as affine doc url const parseDocUrlService = this.std.getOptional(ParseDocUrlProvider); @@ -562,11 +559,7 @@ export class EdgelessClipboardController extends PageClipboard { } private async _pasteTextContentAsNote(content: BlockSnapshot[] | string) { - const lastMousePos = this.toolManager.lastMousePos$.peek(); - const [x, y] = this.gfx.viewport.toModelCoord( - lastMousePos.x, - lastMousePos.y - ); + const { x, y } = this.toolManager.lastMousePos$.peek(); const noteProps = { xywh: new Bound( diff --git a/blocksuite/affine/blocks/root/src/edgeless/clipboard/command.ts b/blocksuite/affine/blocks/root/src/edgeless/clipboard/command.ts index 22e4db4df8..6b9b97c04f 100644 --- a/blocksuite/affine/blocks/root/src/edgeless/clipboard/command.ts +++ b/blocksuite/affine/blocks/root/src/edgeless/clipboard/command.ts @@ -52,9 +52,7 @@ export const createElementsFromClipboardDataCommand: Command = ( let oldCommonBound, pasteX, pasteY; { const lastMousePos = toolManager.lastMousePos$.peek(); - pasteCenter = - pasteCenter ?? - gfx.viewport.toModelCoord(lastMousePos.x, lastMousePos.y); + pasteCenter = pasteCenter ?? [lastMousePos.x, lastMousePos.y]; const [modelX, modelY] = pasteCenter; oldCommonBound = edgelessElementsBoundFromRawData(elementsRawData); diff --git a/blocksuite/affine/blocks/surface/src/renderer/tool-overlay.ts b/blocksuite/affine/blocks/surface/src/renderer/tool-overlay.ts index 5aaa256bb1..800fdf6219 100644 --- a/blocksuite/affine/blocks/surface/src/renderer/tool-overlay.ts +++ b/blocksuite/affine/blocks/surface/src/renderer/tool-overlay.ts @@ -26,7 +26,7 @@ export class ToolOverlay extends Overlay { this.gfx.viewport.viewportUpdated.pipe(startWith(null)).subscribe(() => { // when viewport is updated, we should keep the overlay in the same position // to get last mouse position and convert it to model coordinates - const pos = this.gfx.tool.lastMousePos$.value; + const pos = this.gfx.tool.lastMouseViewPos$.value; const [x, y] = this.gfx.viewport.toModelCoord(pos.x, pos.y); this.x = x; this.y = y; diff --git a/blocksuite/affine/blocks/surface/src/tool/default-tool.ts b/blocksuite/affine/blocks/surface/src/tool/default-tool.ts index 7bf68572d5..21b57943ca 100644 --- a/blocksuite/affine/blocks/surface/src/tool/default-tool.ts +++ b/blocksuite/affine/blocks/surface/src/tool/default-tool.ts @@ -26,9 +26,7 @@ export enum DefaultModeDragType { export class DefaultTool extends BaseTool { static override toolName: string = 'default'; - private _accumulateDelta: IVec = [0, 0]; - - private _autoPanTimer: number | null = null; + private _edgeScrollingTimer: number | null = null; private readonly _clearDisposable = () => { if (this._disposables) { @@ -38,19 +36,17 @@ export class DefaultTool extends BaseTool { }; private readonly _clearSelectingState = () => { - this._stopAutoPanning(); + this._stopEdgeScrolling(); this._clearDisposable(); }; private _disposables: DisposableGroup | null = null; - private _panViewport(delta: IVec) { - this._accumulateDelta[0] += delta[0]; - this._accumulateDelta[1] += delta[1]; + private _scrollViewport(delta: IVec) { this.gfx.viewport.applyDeltaCenter(delta[0], delta[1]); } - private _selectionRectTransition: null | { + private _spaceTranslationRect: null | { w: number; h: number; startX: number; @@ -59,61 +55,43 @@ export class DefaultTool extends BaseTool { endY: number; } = null; - private readonly _startAutoPanning = (delta: IVec) => { - this._panViewport(delta); - this._updateSelectingState(delta); - this._stopAutoPanning(); + private readonly _enableEdgeScrolling = (delta: IVec) => { + this._stopEdgeScrolling(); + this._scrollViewport(delta); - this._autoPanTimer = window.setInterval(() => { - this._panViewport(delta); - this._updateSelectingState(delta); + this._edgeScrollingTimer = window.setInterval(() => { + this._scrollViewport(delta); }, 30); }; - private readonly _stopAutoPanning = () => { - if (this._autoPanTimer) { - clearTimeout(this._autoPanTimer); - this._autoPanTimer = null; + private readonly _stopEdgeScrolling = () => { + if (this._edgeScrollingTimer) { + clearInterval(this._edgeScrollingTimer); + this._edgeScrollingTimer = null; } }; private _toBeMoved: GfxModel[] = []; - private readonly _updateSelectingState = (delta: IVec = [0, 0]) => { + private readonly _updateSelection = () => { const { gfx } = this; - if (gfx.keyboard.spaceKey$.peek() && this._selectionRectTransition) { - /* Move the selection if space is pressed */ - const curDraggingViewArea = this.controller.draggingViewArea$.peek(); - const { w, h, startX, startY, endX, endY } = - this._selectionRectTransition; - const { endX: lastX, endY: lastY } = curDraggingViewArea; + if (gfx.keyboard.spaceKey$.peek() && this._spaceTranslationRect) { + const { w, h, startX, startY, endX, endY } = this._spaceTranslationRect; + const { endX: lastX, endY: lastY } = this.controller.draggingArea$.peek(); - const dx = lastX + delta[0] - endX + this._accumulateDelta[0]; - const dy = lastY + delta[1] - endY + this._accumulateDelta[1]; + const dx = lastX - endX; + const dy = lastY - endY; - this.controller.draggingViewArea$.value = { - ...curDraggingViewArea, + this.controller.draggingArea$.value = { x: Math.min(startX + dx, lastX), y: Math.min(startY + dy, lastY), w, h, startX: startX + dx, startY: startY + dy, - }; - } else { - const curDraggingArea = this.controller.draggingViewArea$.peek(); - const newStartX = curDraggingArea.startX - delta[0]; - const newStartY = curDraggingArea.startY - delta[1]; - - this.controller.draggingViewArea$.value = { - ...curDraggingArea, - startX: newStartX, - startY: newStartY, - x: Math.min(newStartX, curDraggingArea.endX), - y: Math.min(newStartY, curDraggingArea.endY), - w: Math.abs(curDraggingArea.endX - newStartX), - h: Math.abs(curDraggingArea.endY - newStartY), + endX: endX + dx, + endY: endY + dy, }; } @@ -174,7 +152,7 @@ export class DefaultTool extends BaseTool { } private _determineDragType(evt: PointerEventState): DefaultModeDragType { - const { x, y } = this.controller.lastMouseModelPos$.peek(); + const { x, y } = this.controller.lastMousePos$.peek(); if (this.selection.isInSelectedRect(x, y)) { if (this.selection.selectedElements.length === 1) { const currentHoveredElem = this._getElementInGroup(x, y); @@ -243,10 +221,9 @@ export class DefaultTool extends BaseTool { this.gfx.viewport.viewportUpdated.subscribe(() => { if ( this.dragType === DefaultModeDragType.Selecting && - this.controller.dragging$.peek() && - !this._autoPanTimer + this.controller.dragging$.peek() ) { - this._updateSelectingState(); + this._updateSelection(); } }) ); @@ -280,9 +257,8 @@ export class DefaultTool extends BaseTool { } override deactivate() { - this._stopAutoPanning(); + this._stopEdgeScrolling(); this._clearDisposable(); - this._accumulateDelta = [0, 0]; } override doubleClick(e: PointerEventState) { @@ -323,13 +299,12 @@ export class DefaultTool extends BaseTool { switch (this.dragType) { case DefaultModeDragType.Selecting: { // Record the last drag pointer position for auto panning and view port updating - - this._updateSelectingState(); + this._updateSelection(); const moveDelta = calPanDelta(viewport, e); if (moveDelta) { - this._startAutoPanning(moveDelta); + this._enableEdgeScrolling(moveDelta); } else { - this._stopAutoPanning(); + this._stopEdgeScrolling(); } break; } @@ -385,18 +360,11 @@ export class DefaultTool extends BaseTool { const pressed = this.gfx.keyboard.spaceKey$.value; if (pressed) { - const currentDraggingArea = this.controller.draggingViewArea$.peek(); + const currentDraggingArea = this.controller.draggingArea$.peek(); - this._selectionRectTransition = { - w: currentDraggingArea.w, - h: currentDraggingArea.h, - startX: currentDraggingArea.startX, - startY: currentDraggingArea.startY, - endX: currentDraggingArea.endX, - endY: currentDraggingArea.endY, - }; + this._spaceTranslationRect = currentDraggingArea; } else { - this._selectionRectTransition = null; + this._spaceTranslationRect = null; } }) ); diff --git a/blocksuite/affine/gfx/mindmap/src/toolbar/mindmap-tool-button.ts b/blocksuite/affine/gfx/mindmap/src/toolbar/mindmap-tool-button.ts index 29adf47047..faadc5716f 100644 --- a/blocksuite/affine/gfx/mindmap/src/toolbar/mindmap-tool-button.ts +++ b/blocksuite/affine/gfx/mindmap/src/toolbar/mindmap-tool-button.ts @@ -315,7 +315,7 @@ export class EdgelessMindmapToolButton extends EdgelessToolbarToolMixin( } this.setEdgelessTool(EmptyTool); const icon = this.mindmapElement; - const { x, y } = gfx.tool.lastMousePos$.peek(); + const { x, y } = gfx.tool.lastMouseViewPos$.peek(); const { viewport } = this.edgeless.std.get(ViewportElementProvider); const { left, top } = viewport; const clientPos = { x: x + left, y: y + top }; diff --git a/blocksuite/affine/gfx/shape/src/draggable/shape-draggable.ts b/blocksuite/affine/gfx/shape/src/draggable/shape-draggable.ts index 16ed9c8314..c994cccc19 100644 --- a/blocksuite/affine/gfx/shape/src/draggable/shape-draggable.ts +++ b/blocksuite/affine/gfx/shape/src/draggable/shape-draggable.ts @@ -258,7 +258,7 @@ export class EdgelessToolbarShapeDraggable extends EdgelessToolbarToolMixin( console.error('Edgeless toolbar Shape element not found'); return; } - const { x, y } = this.gfx.tool.lastMousePos$.peek(); + const { x, y } = this.gfx.tool.lastMouseViewPos$.peek(); const { viewport } = this.edgeless.std.get(ViewportElementProvider); const { left, top } = viewport; const clientPos = { x: x + left, y: y + top }; diff --git a/blocksuite/affine/gfx/shape/src/shape-tool.ts b/blocksuite/affine/gfx/shape/src/shape-tool.ts index d02fe57f11..de04980e9c 100644 --- a/blocksuite/affine/gfx/shape/src/shape-tool.ts +++ b/blocksuite/affine/gfx/shape/src/shape-tool.ts @@ -117,24 +117,24 @@ export class ShapeTool extends BaseTool { if (spacePressed && this._spacePressedCtx) { const { - startX, - startY, w, h, + startX, + startY, endX: pressedX, endY: pressedY, } = this._spacePressedCtx.draggingArea; - const curDraggingArea = controller.draggingViewArea$.peek(); - const { endX: lastX, endY: lastY } = curDraggingArea; + const { endX: lastX, endY: lastY } = controller.draggingArea$.peek(); const dx = lastX - pressedX; const dy = lastY - pressedY; - this.controller.draggingViewArea$.value = { - ...curDraggingArea, + this.controller.draggingArea$.value = { x: Math.min(startX + dx, lastX), y: Math.min(startY + dy, lastY), w, h, + endX: endX + dx, + endY: endY + dy, startX: startX + dx, startY: startY + dy, }; @@ -306,7 +306,7 @@ export class ShapeTool extends BaseTool { if (spacePressed && this._draggingElementId) { this._spacePressedCtx = { - draggingArea: this.controller.draggingViewArea$.peek(), + draggingArea: this.controller.draggingArea$.peek(), }; } }) diff --git a/blocksuite/affine/widgets/edgeless-dragging-area/src/edgeless-dragging-area-rect.ts b/blocksuite/affine/widgets/edgeless-dragging-area/src/edgeless-dragging-area-rect.ts index 43848efde3..636e3fd330 100644 --- a/blocksuite/affine/widgets/edgeless-dragging-area/src/edgeless-dragging-area-rect.ts +++ b/blocksuite/affine/widgets/edgeless-dragging-area/src/edgeless-dragging-area-rect.ts @@ -36,7 +36,7 @@ export class EdgelessDraggingAreaRectWidget extends WidgetComponent(false); /** - * The area that is being dragged. - * The coordinates are in browser space. + * The dragging area in browser coordinates space. + * + * This is similar to `draggingViewArea$`, but if the viewport is changed during dragging, + * it will be reflected in this area. + */ + readonly draggingViewportArea$ = computed(() => { + const compute = (modelArea: AreaBound) => { + const [viewStartX, viewStartY] = this.gfx.viewport.toViewCoord( + modelArea.x, + modelArea.y + ); + const [viewEndX, viewEndY] = this.gfx.viewport.toViewCoord( + modelArea.x + modelArea.w, + modelArea.y + modelArea.h + ); + + return { + startX: viewStartX, + startY: viewStartY, + endX: viewEndX, + endY: viewEndY, + x: Math.min(viewStartX, viewEndX), + y: Math.min(viewStartY, viewEndY), + w: Math.abs(viewStartX - viewEndX), + h: Math.abs(viewStartY - viewEndY), + }; + }; + + return compute(this.draggingArea$.value); + }); + + /** + * The dragging area in browser coordinates space. */ readonly draggingViewArea$ = new Signal< IBound & { @@ -113,18 +151,20 @@ export class ToolController extends GfxExtension { }); /** - * The last mouse move position - * The coordinates are in browser space + * The last mouse move position in browser coordinates space. */ - readonly lastMousePos$ = new Signal({ + readonly lastMouseViewPos$ = new Signal({ x: 0, y: 0, }); - readonly lastMouseModelPos$ = computed(() => { + /** + * The last mouse position in model coordinates space. + */ + readonly lastMousePos$ = computed(() => { const [x, y] = this.gfx.viewport.toModelCoord( - this.lastMousePos$.value.x, - this.lastMousePos$.value.y + this.lastMouseViewPos$.value.x, + this.lastMouseViewPos$.value.y ); return { @@ -166,38 +206,23 @@ export class ToolController extends GfxExtension { * The area that is being dragged. * The coordinates are in model space. */ - get draggingArea$() { - const compute = (peek: boolean) => { - const area = peek - ? this.draggingViewArea$.peek() - : this.draggingViewArea$.value; - const [startX, startY] = this.gfx.viewport.toModelCoord( - area.startX, - area.startY - ); - const [endX, endY] = this.gfx.viewport.toModelCoord(area.endX, area.endY); - - return { - x: Math.min(startX, endX), - y: Math.min(startY, endY), - w: Math.abs(endX - startX), - h: Math.abs(endY - startY), - startX, - startY, - endX, - endY, - }; - }; - - return { - value() { - return compute(false); - }, - peek() { - return compute(true); - }, - }; - } + readonly draggingArea$ = new Signal< + IBound & { + startX: number; + startY: number; + endX: number; + endY: number; + } + >({ + startX: 0, + startY: 0, + endX: 0, + endY: 0, + x: 0, + y: 0, + w: 0, + h: 0, + }); static override extendGfx(gfx: GfxController) { Object.defineProperty(gfx, 'tool', { @@ -299,6 +324,7 @@ export class ToolController extends GfxExtension { let dragContext: { tool: BaseTool; } | null = null; + let viewportSub: Subscription | null = null; this._disposableGroup.add( this.std.event.add('dragStart', ctx => { @@ -315,6 +341,8 @@ export class ToolController extends GfxExtension { evt.raw.preventDefault(); } + const [modelX, modelY] = this.gfx.viewport.toModelCoord(evt.x, evt.y); + this.dragging$.value = true; this.draggingViewArea$.value = { startX: evt.x, @@ -326,11 +354,42 @@ export class ToolController extends GfxExtension { w: 0, h: 0, }; - this.lastMousePos$.value = { + this.draggingArea$.value = { + startX: modelX, + startY: modelY, + endX: modelX, + endY: modelY, + x: modelX, + y: modelY, + w: 0, + h: 0, + }; + this.lastMouseViewPos$.value = { x: evt.x, y: evt.y, }; + viewportSub?.unsubscribe(); + viewportSub = this.gfx.viewport.viewportUpdated.subscribe(() => { + const lastPost = this.lastMouseViewPos$.peek(); + const [modelX, modelY] = this.gfx.viewport.toModelCoord( + lastPost.x, + lastPost.y + ); + + const original = this.draggingArea$.peek(); + + this.draggingArea$.value = { + ...this.draggingArea$.peek(), + x: Math.min(modelX, original.startX), + y: Math.min(modelY, original.startY), + w: Math.abs(modelX - original.startX), + h: Math.abs(modelY - original.startY), + endX: modelX, + endY: modelY, + }; + }); + // this means the dragEnd event is not even fired // so we need to manually call the dragEnd method if (dragContext?.tool) { @@ -361,6 +420,7 @@ export class ToolController extends GfxExtension { originX: this.draggingViewArea$.peek().startX, originY: this.draggingViewArea$.peek().startY, }; + const [modelX, modelY] = this.gfx.viewport.toModelCoord(evt.x, evt.y); this.draggingViewArea$.value = { ...this.draggingViewArea$.peek(), @@ -372,7 +432,17 @@ export class ToolController extends GfxExtension { endY: evt.y, }; - this.lastMousePos$.value = { + this.draggingArea$.value = { + ...this.draggingArea$.peek(), + w: Math.abs(modelX - draggingStart.x), + h: Math.abs(modelY - draggingStart.y), + x: Math.min(modelX, draggingStart.x), + y: Math.min(modelY, draggingStart.y), + endX: modelX, + endY: modelY, + }; + + this.lastMouseViewPos$.value = { x: evt.x, y: evt.y, }; @@ -399,6 +469,8 @@ export class ToolController extends GfxExtension { dragContext.tool.dragEnd(evt); } + viewportSub?.unsubscribe(); + viewportSub = null; dragContext = null; this.draggingViewArea$.value = { x: 0, @@ -410,6 +482,16 @@ export class ToolController extends GfxExtension { w: 0, h: 0, }; + this.draggingArea$.value = { + x: 0, + y: 0, + startX: 0, + startY: 0, + endX: 0, + endY: 0, + w: 0, + h: 0, + }; }) ); @@ -417,7 +499,7 @@ export class ToolController extends GfxExtension { this.std.event.add('pointerMove', ctx => { const evt = ctx.get('pointerState'); - this.lastMousePos$.value = { + this.lastMouseViewPos$.value = { x: evt.x, y: evt.y, }; diff --git a/blocksuite/framework/std/src/gfx/viewport.ts b/blocksuite/framework/std/src/gfx/viewport.ts index 725498282c..e4ab5acbd1 100644 --- a/blocksuite/framework/std/src/gfx/viewport.ts +++ b/blocksuite/framework/std/src/gfx/viewport.ts @@ -591,8 +591,20 @@ export class Viewport { return new Bound(x, y, w / this.zoom, h / this.zoom); } - toModelCoord(viewX: number, viewY: number): IVec { - const { viewportX, viewportY, zoom, viewScale } = this; + toModelCoord( + viewX: number, + viewY: number, + zoom = this.zoom, + center?: IPoint + ): IVec { + const { viewScale } = this; + const viewportX = center + ? center.x - this.width / 2 / zoom + : this.viewportX; + const viewportY = center + ? center.y - this.height / 2 / zoom + : this.viewportY; + return [ viewportX + viewX / zoom / viewScale, viewportY + viewY / zoom / viewScale,