Fix workspace clean up not considering immovables.

At a high-level, this change ensures that cleaning up a workspace
doesn't move blocks in a way that overlaps with immovable blocks. It
also adds missing testing coverage for both Rect (used for bounding box
calculations during workspace cleanup) and WorkspaceSvg (for verifying
the updated clean up functionality).

This also renames the clean up function to be 'tidyUp' since that better
suits what's happening (as opposed to other clean-up routines which are
actually deinitializing objects).

core/contextmenu_items.ts

@@ -91,7 +91,7 @@ export function registerCleanup() {
       return 'hidden';
     },
     callback(scope: Scope) {
-      scope.workspace!.cleanUp();
+      scope.workspace!.tidyUp();
     },
     scopeType: ContextMenuRegistry.ScopeType.WORKSPACE,
     id: 'cleanWorkspace',

core/utils/rect.ts

@@ -13,6 +13,8 @@
  */
 // Former goog.module ID: Blockly.utils.Rect
 
+import {Coordinate} from './coordinate.js';
+
 /**
  * Class for representing rectangular regions.
  */
@@ -30,10 +32,21 @@ export class Rect {
     public right: number,
   ) {}
 
+  /**
+   * Creates a new copy of this rectangle.
+   *
+   * @returns A copy of this Rect.
+   */
+  clone(): Rect {
+    return new Rect(this.top, this.bottom, this.left, this.right);
+  }
+
+  /** Returns the height of this rectangle. */
   getHeight(): number {
     return this.bottom - this.top;
   }
 
+  /** Returns the width of this rectangle. */
   getWidth(): number {
     return this.right - this.left;
   }
@@ -59,11 +72,45 @@ export class Rect {
    * @returns Whether this rectangle intersects the provided rectangle.
    */
   intersects(other: Rect): boolean {
-    return !(
-      this.left > other.right ||
-      this.right < other.left ||
-      this.top > other.bottom ||
-      this.bottom < other.top
-    );
+    // The following logic can be derived and then simplified from a longer form symmetrical check
+    // of verifying each rectangle's borders with the other rectangle by checking if either end of
+    // the border's line segment is contained within the other rectangle. The simplified version
+    // used here can be logically interpreted as ensuring that each line segment of 'this' rectangle
+    // is not outside the bounds of the 'other' rectangle (proving there's an intersection).
+    return (this.left <= other.right)
+      && (this.right >= other.left)
+      && (this.bottom >= other.top)
+      && (this.top <= other.bottom);
+  }
+
+  /**
+   * Compares bounding rectangles for equality.
+   *
+   * @param a A Rect.
+   * @param b A Rect.
+   * @returns True iff the bounding rectangles are equal, or if both are null.
+   */
+  static equals(a?: Rect | null, b?: Rect | null): boolean {
+    if (a === b) {
+      return true;
+    }
+    if (!a || !b) {
+      return false;
+    }
+    return a.top === b.top && a.bottom === b.bottom && a.left === b.left && a.right === b.right;
+  }
+
+  /**
+   * Creates a new Rect using a position and supplied dimensions.
+   *
+   * @param position The upper left coordinate of the new rectangle.
+   * @param width The width of the rectangle, in pixels.
+   * @param height The height of the rectangle, in pixels.
+   * @returns A newly created Rect using the provided Coordinate and dimensions.
+   */
+  static createFromPoint(position: Coordinate, width: number, height: number): Rect {
+    const left = position.x;
+    const top = position.y;
+    return new Rect(top, top + height, left, left + width);
   }
 }

core/workspace_svg.ts

@@ -1644,23 +1644,48 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg {
     return boundary;
   }
 
-  /** Clean up the workspace by ordering all the blocks in a column. */
-  cleanUp() {
+  /** Tidy up the workspace by ordering all the blocks in a column such that none overlap. */
+  tidyUp() {
     this.setResizesEnabled(false);
     eventUtils.setGroup(true);
+
     const topBlocks = this.getTopBlocks(true);
-    let cursorY = 0;
-    for (let i = 0, block; (block = topBlocks[i]); i++) {
-      if (!block.isMovable()) {
-        continue;
+    const movableBlocks = topBlocks.filter((block) => block.isMovable());
+    const immovableBlocks = topBlocks.filter((block) => !block.isMovable());
+
+    const immovableBlockBounds = immovableBlocks.map((block) => block.getBoundingRectangle());
+
+    const getNextIntersectingImmovableBlock = function(rect: Rect): Rect|null {
+      for (const immovableRect of immovableBlockBounds) {
+        if (rect.intersects(immovableRect)) {
+          return immovableRect;
+        }
       }
-      const xy = block.getRelativeToSurfaceXY();
-      block.moveBy(-xy.x, cursorY - xy.y, ['cleanup']);
+      return null;
+    };
+
+    let cursorY = 0;
+    const minBlockHeight = this.renderer.getConstants().MIN_BLOCK_HEIGHT;
+    for (const block of movableBlocks) {
+      // Make the initial movement of shifting the block to its best possible position.
+      let boundingRect = block.getBoundingRectangle();
+      block.moveBy(-boundingRect.left, cursorY - boundingRect.top, ['cleanup']);
       block.snapToGrid();
-      cursorY =
-        block.getRelativeToSurfaceXY().y +
-        block.getHeightWidth().height +
-        this.renderer.getConstants().MIN_BLOCK_HEIGHT;
+
+      boundingRect = block.getBoundingRectangle();
+      let conflictingRect = getNextIntersectingImmovableBlock(boundingRect);
+      while (conflictingRect != null) {
+        // If the block intersects with an immovable block, move it down past that immovable block.
+        cursorY = conflictingRect.top + conflictingRect.getHeight() + minBlockHeight;
+        block.moveBy(0, cursorY - boundingRect.top, ['cleanup']);
+        block.snapToGrid();
+        boundingRect = block.getBoundingRectangle();
+        conflictingRect = getNextIntersectingImmovableBlock(boundingRect);
+      }
+
+      // Ensure all next blocks start past the most recent (which will also put them past all
+      // previously intersecting immovable blocks).
+      cursorY = block.getRelativeToSurfaceXY().y + block.getHeightWidth().height + minBlockHeight;
     }
     eventUtils.setGroup(false);
     this.setResizesEnabled(true);