fix: improve collapse / uncollapse performance (#6860)

* feat: enable render queueing for collapse-related things

* chore: delay bumping

* chore: fixup tests

core/block_svg.ts

@@ -143,6 +143,12 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg,
 
   private translation = '';
 
+  /**
+   * The ID of the setTimeout callback for bumping neighbours, or 0 if no bump
+   * is currently scheduled.
+   */
+  private bumpNeighboursPid = 0;
+
   /**
    * The location of the top left of this block (in workspace coordinates)
    * relative to either its parent block, or the workspace origin if it has no
@@ -152,6 +158,7 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg,
    */
   relativeCoords = new Coordinate(0, 0);
 
+
   /**
    * @param workspace The block's workspace.
    * @param prototypeName Name of the language object containing type-specific
@@ -508,13 +515,7 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg,
       return;
     }
     super.setCollapsed(collapsed);
-    if (!collapsed) {
-      this.updateCollapsed_();
-    } else if (this.rendered) {
-      this.render();
-      // Don't bump neighbours. Users like to store collapsed functions together
-      // and bumping makes them go out of alignment.
-    }
+    this.updateCollapsed_();
   }
 
   /**
@@ -1255,7 +1256,7 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg,
     const removed = super.removeInput(name, opt_quiet);
 
     if (this.rendered) {
-      this.render();
+      this.queueRender();
       // Removing an input will cause the block to change shape.
       this.bumpNeighbours();
     }
@@ -1291,7 +1292,7 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg,
     const input = super.appendInput_(type, name);
 
     if (this.rendered) {
-      this.render();
+      this.queueRender();
       // Adding an input will cause the block to change shape.
       this.bumpNeighbours();
     }
@@ -1441,7 +1442,16 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg,
    * up on screen, because that creates confusion for end-users.
    */
   override bumpNeighbours() {
-    this.getRootBlock().bumpNeighboursInternal();
+    if (this.bumpNeighboursPid) return;
+    const group = eventUtils.getGroup();
+
+    this.bumpNeighboursPid = setTimeout(() => {
+      const oldGroup = eventUtils.getGroup();
+      eventUtils.setGroup(group);
+      this.getRootBlock().bumpNeighboursInternal();
+      eventUtils.setGroup(oldGroup);
+      this.bumpNeighboursPid = 0;
+    }, config.bumpDelay);
   }
 
   /**
@@ -1496,11 +1506,7 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg,
       eventUtils.setGroup(false);
     }, config.bumpDelay / 2);
 
-    setTimeout(() => {
-      eventUtils.setGroup(group);
-      this.bumpNeighbours();
-      eventUtils.setGroup(false);
-    }, config.bumpDelay);
+    this.bumpNeighbours();
   }
 
   /**
@@ -1642,6 +1648,11 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg,
       // TODO(#4592): Update all markers on the block.
       this.workspace.getMarker(MarkerManager.LOCAL_MARKER)!.draw();
     }
+    for (const input of this.inputList) {
+      for (const field of input.fieldRow) {
+        field.updateMarkers_();
+      }
+    }
   }
 
   /**

core/field.ts

@@ -960,9 +960,8 @@ export abstract class Field<T = any> implements IASTNodeLocationSvg,
   forceRerender() {
     this.isDirty_ = true;
     if (this.sourceBlock_ && this.sourceBlock_.rendered) {
-      (this.sourceBlock_ as BlockSvg).render();
+      (this.sourceBlock_ as BlockSvg).queueRender();
       (this.sourceBlock_ as BlockSvg).bumpNeighbours();
-      this.updateMarkers_();
     }
   }
 
@@ -1286,8 +1285,12 @@ export abstract class Field<T = any> implements IASTNodeLocationSvg,
     this.markerSvg_ = markerSvg;
   }
 
-  /** Redraw any attached marker or cursor svgs if needed. */
-  protected updateMarkers_() {
+  /**
+   * Redraw any attached marker or cursor svgs if needed.
+   *
+   * @internal
+   */
+  updateMarkers_() {
     const block = this.getSourceBlock();
     if (!block) {
       throw new UnattachedFieldError();

core/input.ts

@@ -126,7 +126,7 @@ export class Input {
     }
 
     if (this.sourceBlock.rendered) {
-      (this.sourceBlock as BlockSvg).render();
+      (this.sourceBlock as BlockSvg).queueRender();
       // Adding a field will cause the block to change shape.
       this.sourceBlock.bumpNeighbours();
     }
@@ -148,7 +148,7 @@ export class Input {
         field.dispose();
         this.fieldRow.splice(i, 1);
         if (this.sourceBlock.rendered) {
-          (this.sourceBlock as BlockSvg).render();
+          (this.sourceBlock as BlockSvg).queueRender();
           // Removing a field will cause the block to change shape.
           this.sourceBlock.bumpNeighbours();
         }

tests/mocha/block_test.js

@@ -1112,48 +1112,42 @@ suite('Blocks', function() {
     suite('Add Connections Programmatically', function() {
       test('Output', function() {
         const block = createRenderedBlock(this.workspace, 'empty_block');
-        // this.workspace.newBlock('empty_block');
-        // block.initSvg();
-        // block.render();
 
         block.setOutput(true);
 
+        this.clock.runAll();
         chai.assert.equal(this.getOutputs().length, 1);
       });
       test('Value', function() {
-        const block = this.workspace.newBlock('empty_block');
-        block.initSvg();
-        block.render();
+        const block = createRenderedBlock(this.workspace, 'empty_block');
 
         block.appendValueInput('INPUT');
 
+        this.clock.runAll();
         chai.assert.equal(this.getInputs().length, 1);
       });
       test('Previous', function() {
-        const block = this.workspace.newBlock('empty_block');
-        block.initSvg();
-        block.render();
+        const block = createRenderedBlock(this.workspace, 'empty_block');
 
         block.setPreviousStatement(true);
 
+        this.clock.runAll();
         chai.assert.equal(this.getPrevious().length, 1);
       });
       test('Next', function() {
-        const block = this.workspace.newBlock('empty_block');
-        block.initSvg();
-        block.render();
+        const block = createRenderedBlock(this.workspace, 'empty_block');
 
         block.setNextStatement(true);
 
+        this.clock.runAll();
         chai.assert.equal(this.getNext().length, 1);
       });
       test('Statement', function() {
-        const block = this.workspace.newBlock('empty_block');
-        block.initSvg();
-        block.render();
+        const block = createRenderedBlock(this.workspace, 'empty_block');
 
         block.appendStatementInput('STATEMENT');
 
+        this.clock.runAll();
         chai.assert.equal(this.getNext().length, 1);
       });
     });
@@ -1719,8 +1713,10 @@ suite('Blocks', function() {
       test('Add Input', function() {
         const blockA = createRenderedBlock(this.workspace, 'empty_block');
         blockA.setCollapsed(true);
-        assertCollapsed(blockA);
+
         blockA.appendDummyInput('NAME');
+
+        this.clock.runAll();
         assertCollapsed(blockA);
         chai.assert.isNotNull(blockA.getInput('NAME'));
       });
@@ -1794,20 +1790,20 @@ suite('Blocks', function() {
         const blockA = createRenderedBlock(this.workspace, 'variable_block');
 
         blockA.setCollapsed(true);
-        assertCollapsed(blockA, 'x');
-
         const variable = this.workspace.getVariable('x', '');
         this.workspace.renameVariableById(variable.getId(), 'y');
+
+        this.clock.runAll();
         assertCollapsed(blockA, 'y');
       });
       test('Coalesce, Different Case', function() {
         const blockA = createRenderedBlock(this.workspace, 'variable_block');
 
         blockA.setCollapsed(true);
-        assertCollapsed(blockA, 'x');
-
         const variable = this.workspace.createVariable('y');
         this.workspace.renameVariableById(variable.getId(), 'X');
+
+        this.clock.runAll();
         assertCollapsed(blockA, 'X');
       });
     });

tests/mocha/blocks/procedures_test.js

@@ -541,6 +541,7 @@ suite('Procedures', function() {
           Blockly.Events.setGroup(false);
 
           this.workspace.undo();
+          this.clock.runAll();
 
           chai.assert.isTrue(
             defBlock.getFieldValue('PARAMS').includes('param1'),

tests/mocha/field_checkbox_test.js

@@ -152,7 +152,7 @@ suite('Checkbox Fields', function() {
           workspace: {
             keyboardAccessibilityMode: false,
           },
-          render: function() {field.render_();},
+          queueRender: function() {field.render_();},
           bumpNeighbours: function() {},
         };
         field.constants_ = {

tests/mocha/input_test.js

@@ -23,7 +23,7 @@ suite('Inputs', function() {
         '<block type="empty_block"/>'
     ), this.workspace);
 
-    this.renderStub = sinon.stub(this.block, 'render');
+    this.renderStub = sinon.stub(this.block, 'queueRender');
     this.bumpNeighboursStub = sinon.stub(this.block, 'bumpNeighbours');
 
     this.dummy = this.block.appendDummyInput('DUMMY');