From c09699ba3f4e03c5d9c7501aaa17d1c820694dcb Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Fri, 17 Jul 2026 14:54:28 -0700 Subject: [PATCH 1/4] feat: Rewrite insertion markers to improve performance and fix memory leaks --- .../core/dragging/block_drag_strategy.ts | 12 +- packages/blockly/core/insertion_marker.ts | 165 ++++++++++++++++++ .../core/insertion_marker_previewer.ts | 52 +++++- packages/blockly/core/rendered_connection.ts | 38 +++- .../measurables/external_value_input.ts | 8 +- .../renderers/measurables/inline_input.ts | 4 +- .../renderers/measurables/input_connection.ts | 16 +- .../renderers/measurables/next_connection.ts | 5 + .../renderers/measurables/statement_input.ts | 2 +- 9 files changed, 285 insertions(+), 17 deletions(-) create mode 100644 packages/blockly/core/insertion_marker.ts diff --git a/packages/blockly/core/dragging/block_drag_strategy.ts b/packages/blockly/core/dragging/block_drag_strategy.ts index 2e6dd476bc5..66f660a2d65 100644 --- a/packages/blockly/core/dragging/block_drag_strategy.ts +++ b/packages/blockly/core/dragging/block_drag_strategy.ts @@ -648,7 +648,6 @@ export class BlockDragStrategy implements IDragStrategy { const newCandidate = this.getInitialCandidate() ?? this.getConnectionCandidate(delta); - this.connectionPreviewer?.hidePreview(); this.connectionCandidate = null; if (!newCandidate) { // Position above or below the first/last block. @@ -657,13 +656,19 @@ export class BlockDragStrategy implements IDragStrategy { currCandidate?.neighbour.getSourceBlock() ?? null; let root = connectedBlock?.getRootBlock() ?? connectedBlock; if (root === draggingBlock) root = connectedBlock; - if (!root) return; + if (!root) { + this.connectionPreviewer?.hidePreview(); + return; + } const blockRects = draggingBlock.workspace .getTopBlocks() .filter((block) => block !== draggingBlock.getRootBlock()) .map((block) => block.getBoundingRectangle()); - if (!blockRects.length) return; + if (!blockRects.length) { + this.connectionPreviewer?.hidePreview(); + return; + } // If the block was previously connected, position the block near its previous connection. const destinationX = @@ -694,6 +699,7 @@ export class BlockDragStrategy implements IDragStrategy { new Coordinate(destinationX, destinationY), ); } + this.connectionPreviewer?.hidePreview(); return; } const candidate = diff --git a/packages/blockly/core/insertion_marker.ts b/packages/blockly/core/insertion_marker.ts new file mode 100644 index 00000000000..68613d0f124 --- /dev/null +++ b/packages/blockly/core/insertion_marker.ts @@ -0,0 +1,165 @@ +/** + * @license + * Copyright 2026 Raspberry Pi Foundation + * SPDX-License-Identifier: Apache-2.0 + */ + +import type {BlockSvg} from './block_svg.js'; +import * as renderManagement from './render_management.js'; +import {RenderedConnection} from './rendered_connection.js'; +import {Coordinate} from './utils/coordinate.js'; +import * as dom from './utils/dom.js'; +import {Size} from './utils/size.js'; +import {Svg} from './utils/svg.js'; + +/** + * Visual representation of a mid-drag block if it were to be connected. + */ +export class InsertionMarker { + /** The current size of the insertion marker, in workspace units. */ + private size: Size = new Size(0, 0); + + /** The DOM element representing the insertion marker. */ + private marker?: SVGGElement; + + /** The static connection to which the insertion marker is attached. */ + private parentConnection?: RenderedConnection; + + /** + * Returns the current size of the insertion marker in workspace units. + */ + getHeightWidth() { + return this.size; + } + + /** + * Displays an insertion marker representing the block structure if + * `draggingConnection` were to be connected to `staticConnection`. + * + * @param staticConnection The proposed connection on the stationary block. + * @param draggingConnection The proposed connection on a block being dragged. + */ + show( + staticConnection: RenderedConnection, + draggingConnection: RenderedConnection, + ) { + this.parentConnection = staticConnection; + this.parentConnection.attachInsertionMarker(this); + + const connectingBlock = draggingConnection.getSourceBlock(); + + // Save all the existing connections pairs on the dragged block. + const oldConnections = new Map< + RenderedConnection, + RenderedConnection | null + >(); + for (const connection of connectingBlock.getConnections_(false)) { + oldConnections.set(connection, connection.targetConnection); + connection.targetConnection = null; + } + + // Temporarily connect the dragging and static connections and render the + // dragging block to steal its size and path for use by the insertion + // marker. Importantly, this does *not* call connect(), which causes DOM + // manipulation, fires events, etc – it just swaps the target connection. + draggingConnection.targetConnection = staticConnection; + connectingBlock.workspace.getRenderer().render(connectingBlock); + this.partiallyTighten(connectingBlock, draggingConnection); + + this.marker = this.makeMarker(connectingBlock); + + const bounds = connectingBlock.getBoundingRectangleWithoutChildren(); + this.size = new Size(bounds.getWidth(), bounds.getHeight()); + + const blockOffset = staticConnection + .getSourceBlock() + .getRelativeToSurfaceXY(); + + const connectionOffset = Coordinate.difference( + staticConnection.getOffsetInBlock(), + draggingConnection.getOffsetInBlock(), + ); + + // Restore the state of the dragging block and rerender it. + for (const [source, target] of oldConnections.entries()) { + source.targetConnection = target; + } + connectingBlock.workspace.getRenderer().render(connectingBlock); + this.partiallyTighten(connectingBlock, draggingConnection); + + // Move the insertion marker to abut the static connection and render the + // static block in case it needs to grow to accommodate the insertion + // marker. + this.marker.setAttribute( + 'transform', + `translate(${blockOffset.x + connectionOffset.x}, ${blockOffset.y + connectionOffset.y})`, + ); + staticConnection.getSourceBlock().queueRender(); + renderManagement.triggerQueuedRenders(); + } + + /** + * Hides the insertion marker. + */ + hide() { + if (!this.parentConnection) return; + + this.marker?.remove(); + this.marker = undefined; + this.parentConnection.detachInsertionMarker(); + + this.parentConnection?.getSourceBlock().queueRender(); + renderManagement.triggerQueuedRenders(); + this.parentConnection = undefined; + } + + /** + * Creates a new insertion marker corresponding to the given block. + * + * @param block The block whose path will be used for the insertion marker. + * @returns An SVG group representing an insertion marker. + */ + private makeMarker(block: BlockSvg) { + const newMarker = dom.createSvgElement(Svg.G, { + 'transform': `translate(${block.relativeCoords.x}, ${block.relativeCoords.y})`, + 'class': 'blocklyInsertionMarker', + }); + + const path = block.pathObject.svgPath; + dom.createSvgElement( + Svg.PATH, + { + 'd': path.getAttribute('d') ?? '', + 'class': 'blocklyPath', + }, + newMarker, + ); + + block.workspace.getCanvas().appendChild(newMarker); + + return newMarker; + } + + /** + * Moves all connections on the given block adjacent to one another, excepting + * a specified connection. Otherwise identical to + * `BlockSvg.tightenChildrenEfficiently()`. + * + * @param block The block whose connections should be tightened. + * @param skip A connection to avoid tightening; generally that to which the + * insertion marker is being connected to, which may need to have a gap + * between it and its partner connection which the insertion marker will + * be spliced into. + */ + private partiallyTighten(block: BlockSvg, skip: RenderedConnection) { + for (const input of block.inputList) { + if (input === skip.getParentInput()) { + continue; + } + const conn = input.connection as RenderedConnection; + if (conn) conn.tightenEfficiently(); + } + if (block.nextConnection && block.nextConnection !== skip) + block.nextConnection.tightenEfficiently(); + } +} diff --git a/packages/blockly/core/insertion_marker_previewer.ts b/packages/blockly/core/insertion_marker_previewer.ts index 2a31327ce6b..61ad7a67af9 100644 --- a/packages/blockly/core/insertion_marker_previewer.ts +++ b/packages/blockly/core/insertion_marker_previewer.ts @@ -7,10 +7,13 @@ import {BlockSvg} from './block_svg.js'; import {ConnectionType} from './connection_type.js'; import * as eventUtils from './events/utils.js'; +import {InsertionMarker} from './insertion_marker.js'; import {IConnectionPreviewer} from './interfaces/i_connection_previewer.js'; import * as registry from './registry.js'; import * as renderManagement from './render_management.js'; import {RenderedConnection} from './rendered_connection.js'; +import {Renderer as GerasRenderer} from './renderers/geras/renderer.js'; +import {Renderer as ThrasosRenderer} from './renderers/thrasos/renderer.js'; import {Renderer as ZelosRenderer} from './renderers/zelos/renderer.js'; import * as blocks from './serialization/blocks.js'; import {WorkspaceSvg} from './workspace_svg.js'; @@ -26,8 +29,21 @@ export class InsertionMarkerPreviewer implements IConnectionPreviewer { private staticConn: RenderedConnection | null = null; + private insertionMarker: InsertionMarker; + + /** + * If set to true, uses a faster method for rendering insertion markers which + * will become the default in v14. This rendering method is enabled for the + * built-in Thrasos, Geras and Zelos renderers regardless of the state of this + * flag. Custom renderers will use the old rendering behavior unless this is + * set to true. This field will be removed in v14. + */ + static useFastInsertionMarkers = false; + constructor(draggedBlock: BlockSvg) { this.workspace = draggedBlock.workspace; + + this.insertionMarker = new InsertionMarker(); } /** @@ -62,7 +78,7 @@ export class InsertionMarkerPreviewer implements IConnectionPreviewer { /** * Display a connection preview where the draggedCon connects to the - * staticCon, and no block is being relaced. + * staticCon, and no block is being replaced. * * @param draggedConn The connection on the block stack being dragged. * @param staticConn The connection not being dragged that we are @@ -85,7 +101,11 @@ export class InsertionMarkerPreviewer implements IConnectionPreviewer { // static connection, so that it doesn't disconnect unless that // (+ a bit) has been exceeded. if (this.shouldUseMarkerPreview(draggedConn, staticConn)) { - this.markerConn = this.previewMarker(draggedConn, staticConn); + if (this.shouldUseFastInsertionMarkers()) { + this.insertionMarker.show(staticConn, draggedConn); + } else { + this.markerConn = this.previewMarker(draggedConn, staticConn); + } } if (this.workspace.getRenderer().shouldHighlightConnection(staticConn)) { @@ -236,10 +256,14 @@ export class InsertionMarkerPreviewer implements IConnectionPreviewer { this.fadedBlock.fadeForReplacement(false); this.fadedBlock = null; } - if (this.markerConn) { - this.hideInsertionMarker(this.markerConn); - this.markerConn = null; - this.draggedConn = null; + if (this.shouldUseFastInsertionMarkers()) { + this.insertionMarker.hide(); + } else { + if (this.markerConn) { + this.hideInsertionMarker(this.markerConn); + this.markerConn = null; + this.draggedConn = null; + } } } finally { eventUtils.enable(); @@ -267,6 +291,22 @@ export class InsertionMarkerPreviewer implements IConnectionPreviewer { dispose() { this.hidePreview(); } + + /** + * Returns whether or not new fast insertion marker rendering should be used. + * Defaults on for built-in renderers and off for custom renderers. Can be + * enabled for custom renderers by setting + * `InsertionMarkerPreviewer.useFastInsertionMarkers = true`. + */ + private shouldUseFastInsertionMarkers() { + const renderer = this.workspace.getRenderer(); + return ( + renderer.constructor === ThrasosRenderer || + renderer.constructor === GerasRenderer || + renderer.constructor === ZelosRenderer || + InsertionMarkerPreviewer.useFastInsertionMarkers + ); + } } registry.register( diff --git a/packages/blockly/core/rendered_connection.ts b/packages/blockly/core/rendered_connection.ts index bb10aae0ab3..a1ece215c32 100644 --- a/packages/blockly/core/rendered_connection.ts +++ b/packages/blockly/core/rendered_connection.ts @@ -20,6 +20,7 @@ import {ConnectionType} from './connection_type.js'; import * as ContextMenu from './contextmenu.js'; import {ContextMenuRegistry} from './contextmenu_registry.js'; import * as eventUtils from './events/utils.js'; +import type {InsertionMarker} from './insertion_marker.js'; import {IContextMenu} from './interfaces/i_contextmenu.js'; import type {IFocusableNode} from './interfaces/i_focusable_node.js'; import type {IFocusableTree} from './interfaces/i_focusable_tree.js'; @@ -49,6 +50,7 @@ export class RenderedConnection private readonly offsetInBlock: Coordinate; private trackedState: TrackedState; private highlighted: boolean = false; + private insertionMarker?: InsertionMarker; /** Connection this connection connects to. Null if not connected. */ override targetConnection: RenderedConnection | null = null; @@ -302,6 +304,15 @@ export class RenderedConnection this.offsetInBlock, target.offsetInBlock, ); + + if (this.insertionMarker) { + if (this.type === ConnectionType.INPUT_VALUE) { + offset.x += this.insertionMarker.getHeightWidth().width; + } else { + offset.y += this.insertionMarker.getHeightWidth().height; + } + } + block.translate(offset.x, offset.y); } @@ -741,9 +752,34 @@ export class RenderedConnection // This cast is valid as TypeScript's definition is wrong. See: // https://github.com/microsoft/TypeScript/issues/60996. const root = this.getSourceBlock().getSvgRoot().getRootNode() as - ShadowRoot | HTMLDocument; + | ShadowRoot + | HTMLDocument; return root.getElementById(this.id) as SVGPathElement | null; } + + /** + * Associates the given insertion marker with this connection. + * @internal + */ + attachInsertionMarker(marker: InsertionMarker) { + this.insertionMarker = marker; + } + + /** + * Removes the insertion marker associated with this connection, if any. + * @internal + */ + detachInsertionMarker() { + this.insertionMarker = undefined; + } + + /** + * Returns the insertion marker associated with this connection, if any. + * @internal + */ + getInsertionMarker() { + return this.insertionMarker; + } } export namespace RenderedConnection { diff --git a/packages/blockly/core/renderers/measurables/external_value_input.ts b/packages/blockly/core/renderers/measurables/external_value_input.ts index 4dea7541336..2f8e629ece5 100644 --- a/packages/blockly/core/renderers/measurables/external_value_input.ts +++ b/packages/blockly/core/renderers/measurables/external_value_input.ts @@ -29,9 +29,15 @@ export class ExternalValueInput extends InputConnection { constructor(constants: ConstantProvider, input: Input) { super(constants, input); this.type |= Types.EXTERNAL_VALUE_INPUT; - if (!this.connectedBlock) { + + if (!this.connectedBlockHeight) { this.height = this.shape.height as number; } else { + const insertionMarker = this.connectionModel.getInsertionMarker(); + if (insertionMarker) { + this.connectedBlockHeight = insertionMarker.getHeightWidth().height; + } + this.height = this.connectedBlockHeight - this.constants_.TAB_OFFSET_FROM_TOP - diff --git a/packages/blockly/core/renderers/measurables/inline_input.ts b/packages/blockly/core/renderers/measurables/inline_input.ts index 307daef8d56..48e3ea22e8a 100644 --- a/packages/blockly/core/renderers/measurables/inline_input.ts +++ b/packages/blockly/core/renderers/measurables/inline_input.ts @@ -27,7 +27,7 @@ export class InlineInput extends InputConnection { super(constants, input); this.type |= Types.INLINE_INPUT; - if (!this.connectedBlock) { + if (!this.connectedBlockWidth && !this.connectedBlockHeight) { this.height = this.constants_.EMPTY_INLINE_INPUT_HEIGHT; this.width = this.constants_.EMPTY_INLINE_INPUT_PADDING; } else { @@ -44,7 +44,7 @@ export class InlineInput extends InputConnection { this.connectionWidth = !this.isDynamicShape ? (this.shape.width as number) : (this.shape.width as (p1: number) => number)(this.height); - if (!this.connectedBlock) { + if (!this.connectedBlockHeight) { this.width += this.connectionWidth * (this.isDynamicShape ? 2 : 1); } diff --git a/packages/blockly/core/renderers/measurables/input_connection.ts b/packages/blockly/core/renderers/measurables/input_connection.ts index e4e265b4edd..e47c263a37d 100644 --- a/packages/blockly/core/renderers/measurables/input_connection.ts +++ b/packages/blockly/core/renderers/measurables/input_connection.ts @@ -44,13 +44,23 @@ export class InputConnection extends Connection { ? (input.connection.targetBlock() as BlockSvg) : null; + const insertionMarker = ( + input.connection as RenderedConnection + ).getInsertionMarker(); + + this.connectedBlockWidth = 0; + this.connectedBlockHeight = 0; + if (this.connectedBlock) { const bBox = this.connectedBlock.getHeightWidth(); this.connectedBlockWidth = bBox.width; this.connectedBlockHeight = bBox.height; - } else { - this.connectedBlockWidth = 0; - this.connectedBlockHeight = 0; + } + + if (insertionMarker) { + const bBox = insertionMarker.getHeightWidth(); + this.connectedBlockWidth += bBox.width; + this.connectedBlockHeight += bBox.height; } } } diff --git a/packages/blockly/core/renderers/measurables/next_connection.ts b/packages/blockly/core/renderers/measurables/next_connection.ts index c10a26904bc..439cb917a91 100644 --- a/packages/blockly/core/renderers/measurables/next_connection.ts +++ b/packages/blockly/core/renderers/measurables/next_connection.ts @@ -37,5 +37,10 @@ export class NextConnection extends Connection { this.type |= Types.NEXT_CONNECTION; this.height = this.shape.height as number; this.width = this.shape.width as number; + + const insertionMarker = connectionModel.getInsertionMarker(); + if (insertionMarker) { + this.height += insertionMarker.getHeightWidth().height; + } } } diff --git a/packages/blockly/core/renderers/measurables/statement_input.ts b/packages/blockly/core/renderers/measurables/statement_input.ts index b0b527d36dd..dfadcd2a324 100644 --- a/packages/blockly/core/renderers/measurables/statement_input.ts +++ b/packages/blockly/core/renderers/measurables/statement_input.ts @@ -32,7 +32,7 @@ export class StatementInput extends InputConnection { super(constants, input); this.type |= Types.STATEMENT_INPUT; - if (!this.connectedBlock) { + if (!this.connectedBlockHeight) { this.height = this.constants_.EMPTY_STATEMENT_INPUT_HEIGHT; } else { // We allow the dark path to show on the parent block so that the child From 3f19243490f704a06f89e2eb35b624814689521a Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Fri, 17 Jul 2026 14:54:48 -0700 Subject: [PATCH 2/4] chore: Fix tests --- packages/blockly/tests/mocha/keyboard_movement_test.js | 5 +++++ packages/blockly/tests/mocha/shortcut_items_test.js | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/blockly/tests/mocha/keyboard_movement_test.js b/packages/blockly/tests/mocha/keyboard_movement_test.js index 43022ed6c56..0b52d663cf8 100644 --- a/packages/blockly/tests/mocha/keyboard_movement_test.js +++ b/packages/blockly/tests/mocha/keyboard_movement_test.js @@ -1226,6 +1226,7 @@ suite('Keyboard-driven movement', function () { this.workspace.clear(); this.liveRegion = document.getElementById('blocklyAriaAnnounce'); this.moveAndAssert = (moveFn, incPhrases, exclPhrases = []) => { + this.clock.tick(11); moveFn(this.workspace); this.clock.tick(11); let text = this.liveRegion.textContent; @@ -1247,6 +1248,10 @@ suite('Keyboard-driven movement', function () { this.block1.render(); }); + teardown(function () { + cancelMove(this.workspace); + }); + test('announces simple block moving on workspace', function () { Blockly.getFocusManager().focusNode(this.block1); this.moveAndAssert( diff --git a/packages/blockly/tests/mocha/shortcut_items_test.js b/packages/blockly/tests/mocha/shortcut_items_test.js index f59476e0245..d75141acb9e 100644 --- a/packages/blockly/tests/mocha/shortcut_items_test.js +++ b/packages/blockly/tests/mocha/shortcut_items_test.js @@ -1539,8 +1539,8 @@ suite('Keyboard Shortcut Items', function () { const hasInsertionMarker = this.workspace .getTopBlocks() - .flatMap((b) => b.getChildren()) - .some((b) => b.isInsertionMarker()); + .flatMap((b) => b.getConnections_()) + .some((c) => !!c.getInsertionMarker()); assert.isTrue(hasInsertionMarker); Blockly.KeyboardMover.mover.abortMove(); From ca197b877c153d36eaf377d9fca1f8d4d17be8b5 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Fri, 17 Jul 2026 15:20:45 -0700 Subject: [PATCH 3/4] chore: Run formatter --- packages/blockly/core/rendered_connection.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/blockly/core/rendered_connection.ts b/packages/blockly/core/rendered_connection.ts index a1ece215c32..0c9a89caf1a 100644 --- a/packages/blockly/core/rendered_connection.ts +++ b/packages/blockly/core/rendered_connection.ts @@ -752,8 +752,7 @@ export class RenderedConnection // This cast is valid as TypeScript's definition is wrong. See: // https://github.com/microsoft/TypeScript/issues/60996. const root = this.getSourceBlock().getSvgRoot().getRootNode() as - | ShadowRoot - | HTMLDocument; + ShadowRoot | HTMLDocument; return root.getElementById(this.id) as SVGPathElement | null; } From ca6549d2059210d6853cfe1ec0e28707ad3e5abb Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Fri, 17 Jul 2026 15:23:54 -0700 Subject: [PATCH 4/4] chore: Make linter happy --- packages/blockly/core/rendered_connection.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/blockly/core/rendered_connection.ts b/packages/blockly/core/rendered_connection.ts index 0c9a89caf1a..a696b2f35fc 100644 --- a/packages/blockly/core/rendered_connection.ts +++ b/packages/blockly/core/rendered_connection.ts @@ -758,6 +758,7 @@ export class RenderedConnection /** * Associates the given insertion marker with this connection. + * * @internal */ attachInsertionMarker(marker: InsertionMarker) { @@ -766,6 +767,7 @@ export class RenderedConnection /** * Removes the insertion marker associated with this connection, if any. + * * @internal */ detachInsertionMarker() { @@ -774,6 +776,7 @@ export class RenderedConnection /** * Returns the insertion marker associated with this connection, if any. + * * @internal */ getInsertionMarker() {