From 1c9af1f835aab0bc457d0750d86832d403de08dd Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Fri, 12 Jan 2024 19:08:42 +0000 Subject: [PATCH 1/2] fix: triggering flyout show from field render causing infinite loop --- core/flyout_base.ts | 2 +- core/render_management.ts | 21 +++++++++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/core/flyout_base.ts b/core/flyout_base.ts index 52d9a2298bf..4b022c5357d 100644 --- a/core/flyout_base.ts +++ b/core/flyout_base.ts @@ -649,7 +649,7 @@ export abstract class Flyout const parsedContent = toolbox.convertFlyoutDefToJsonArray(flyoutDef); const flyoutInfo = this.createFlyoutInfo(parsedContent); - renderManagement.triggerQueuedRenders(); + renderManagement.triggerQueuedRenders(this.workspace_); this.layout_(flyoutInfo.contents, flyoutInfo.gaps); diff --git a/core/render_management.ts b/core/render_management.ts index 541459860ee..d3f1ed4f156 100644 --- a/core/render_management.ts +++ b/core/render_management.ts @@ -6,6 +6,7 @@ import {BlockSvg} from './block_svg.js'; import * as userAgent from './utils/useragent.js'; +import type {WorkspaceSvg} from './workspace_svg.js'; /** The set of all blocks in need of rendering which don't have parents. */ const rootBlocks = new Set(); @@ -75,11 +76,13 @@ export function finishQueuedRenders(): Promise { * cases where queueing renders breaks functionality + backwards compatibility * (such as rendering icons). * + * @param workspace If provided, only rerender blocks in this workspace. + * * @internal */ -export function triggerQueuedRenders() { +export function triggerQueuedRenders(workspace?: WorkspaceSvg) { window.cancelAnimationFrame(animationRequestId); - doRenders(); + doRenders(workspace); if (afterRendersResolver) afterRendersResolver(); } @@ -110,10 +113,16 @@ function queueBlock(block: BlockSvg) { /** * Rerenders all of the blocks in the queue. + * + * @param workspace If provided, only rerender blocks in this workspace. */ -function doRenders() { - const workspaces = new Set([...rootBlocks].map((block) => block.workspace)); - const blocks = [...rootBlocks].filter(shouldRenderRootBlock); +function doRenders(workspace?: WorkspaceSvg) { + const workspaces = workspace + ? new Set([workspace]) + : new Set([...rootBlocks].map((block) => block.workspace)); + const blocks = [...rootBlocks] + .filter(shouldRenderRootBlock) + .filter((b) => workspaces.has(b.workspace)); for (const block of blocks) { renderBlock(block); } @@ -126,7 +135,7 @@ function doRenders() { } rootBlocks.clear(); - dirtyBlocks = new Set(); + dirtyBlocks = new WeakSet(); afterRendersPromise = null; } From 75f0b8fb5b32fb7426ef070b8c0f8b2af86acf76 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Fri, 12 Jan 2024 19:37:30 +0000 Subject: [PATCH 2/2] chore: add tests for triggering queued renders --- tests/mocha/render_management_test.js | 65 +++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/tests/mocha/render_management_test.js b/tests/mocha/render_management_test.js index d5d957df458..4a69d2bab99 100644 --- a/tests/mocha/render_management_test.js +++ b/tests/mocha/render_management_test.js @@ -57,4 +57,69 @@ suite('Render Management', function () { return promise; }); }); + + suite('triggering queued renders', function () { + function createMockBlock(ws) { + return { + hasRendered: false, + renderEfficiently: function () { + this.hasRendered = true; + }, + + // All of the APIs the render management system needs. + getParent: () => null, + getChildren: () => [], + isDisposed: () => false, + getRelativeToSurfaceXY: () => ({x: 0, y: 0}), + updateComponentLocations: () => {}, + workspace: ws || createMockWorkspace(), + }; + } + + function createMockWorkspace() { + return { + resizeContents: () => {}, + }; + } + + test('triggering queued renders rerenders blocks', function () { + const block = createMockBlock(); + Blockly.renderManagement.queueRender(block); + + Blockly.renderManagement.triggerQueuedRenders(); + + chai.assert.isTrue(block.hasRendered, 'Expected block to be rendered'); + }); + + test('triggering queued renders rerenders blocks in all workspaces', function () { + const workspace1 = createMockWorkspace(); + const workspace2 = createMockWorkspace(); + const block1 = createMockBlock(workspace1); + const block2 = createMockBlock(workspace2); + Blockly.renderManagement.queueRender(block1); + Blockly.renderManagement.queueRender(block2); + + Blockly.renderManagement.triggerQueuedRenders(); + + chai.assert.isTrue(block1.hasRendered, 'Expected block1 to be rendered'); + chai.assert.isTrue(block2.hasRendered, 'Expected block2 to be rendered'); + }); + + test('triggering queued renders in one workspace does not rerender blocks in another workspace', function () { + const workspace1 = createMockWorkspace(); + const workspace2 = createMockWorkspace(); + const block1 = createMockBlock(workspace1); + const block2 = createMockBlock(workspace2); + Blockly.renderManagement.queueRender(block1); + Blockly.renderManagement.queueRender(block2); + + Blockly.renderManagement.triggerQueuedRenders(workspace1); + + chai.assert.isTrue(block1.hasRendered, 'Expected block1 to be rendered'); + chai.assert.isFalse( + block2.hasRendered, + 'Expected block2 to not be rendered', + ); + }); + }); });