Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add basic render queueing #6851

Merged
merged 7 commits into from
Mar 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions core/block_dragger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ export class BlockDragger implements IBlockDragger {
// These are expensive and don't need to be done if we're deleting.
this.draggingBlock_.setDragging(false);
if (delta) { // !preventMove
this.updateBlockAfterMove_(delta);
this.updateBlockAfterMove_();
} else {
// Blocks dragged directly from a flyout may need to be bumped into
// bounds.
Expand Down Expand Up @@ -283,18 +283,14 @@ export class BlockDragger implements IBlockDragger {

/**
* Updates the necessary information to place a block at a certain location.
*
* @param delta The change in location from where the block started the drag
* to where it ended the drag.
*/
protected updateBlockAfterMove_(delta: Coordinate) {
this.draggingBlock_.moveConnections(delta.x, delta.y);
protected updateBlockAfterMove_() {
this.fireMoveEvent_();
if (this.draggedConnectionManager_.wouldConnectBlock()) {
// Applying connections also rerenders the relevant blocks.
this.draggedConnectionManager_.applyConnections();
} else {
this.draggingBlock_.render();
this.draggingBlock_.queueRender();
}
this.draggingBlock_.scheduleSnapAndBump();
}
Expand Down
22 changes: 16 additions & 6 deletions core/block_svg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ import * as svgMath from './utils/svg_math.js';
import {Warning} from './warning.js';
import type {Workspace} from './workspace.js';
import type {WorkspaceSvg} from './workspace_svg.js';
import {queueRender} from './render_management.js';


/**
Expand Down Expand Up @@ -340,9 +341,6 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg,
const oldXY = this.getRelativeToSurfaceXY();
if (newParent) {
(newParent as BlockSvg).getSvgRoot().appendChild(svgRoot);
const newXY = this.getRelativeToSurfaceXY();
// Move the connections to match the child's new position.
this.moveConnections(newXY.x - oldXY.x, newXY.y - oldXY.y);
} else if (oldParent) {
// If we are losing a parent, we want to move our DOM element to the
// root of the workspace.
Expand Down Expand Up @@ -1579,7 +1577,17 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg,
}

/**
* Lays out and reflows a block based on its contents and settings.
* Triggers a rerender after a delay to allow for batching.
*
* @internal
*/
queueRender() {
queueRender(this);
}
Comment on lines +1584 to +1586
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method doesn't have very many callers, and they are all in either block_dragger.ts or rendered_connection.ts. Do you think indirecting via BlockSvg.prototype.queueRender (rather than calling render_management.ts's queueRender directly) is justified on redability or other grounds?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Later changes make several more calls to this, so I think it is justified on grounds of readability. Plus, it's internal, so the maintainance cost to including it is minimal :P If we want to tear it out later we can!


/**
* Immediately lays out and reflows a block based on its contents and
* settings.
*
* @param opt_bubble If false, just render this block.
* If true, also render block's parent, grandparent, etc. Defaults to true.
Expand All @@ -1597,7 +1605,7 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg,
this.updateCollapsed_();
}
this.workspace.getRenderer().render(this);
this.updateConnectionLocations_();
this.updateConnectionLocations();

if (opt_bubble !== false) {
const parentBlock = this.getParent();
Expand Down Expand Up @@ -1631,8 +1639,10 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg,
* Update all of the connections on this block with the new locations
* calculated during rendering. Also move all of the connected blocks based
* on the new connection locations.
*
* @internal
*/
private updateConnectionLocations_() {
updateConnectionLocations() {
const blockTL = this.getRelativeToSurfaceXY();
// Don't tighten previous or output connections because they are inferior
// connections.
Expand Down
4 changes: 3 additions & 1 deletion core/insertion_marker_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,9 @@ export class InsertionMarkerManager {
blockAnimations.connectionUiEffect(inferiorConnection.getSourceBlock());
// Bring the just-edited stack to the front.
const rootBlock = this.topBlock.getRootBlock();
rootBlock.bringToFront();
setTimeout(() => {
rootBlock.bringToFront();
}, 0);
BeksOmega marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
71 changes: 71 additions & 0 deletions core/render_management.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/**
* @license
* Copyright 2023 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

import {BlockSvg} from './block_svg';


const rootBlocks = new Set<BlockSvg>();
const dirtyBlocks = new WeakSet<BlockSvg>();
let pid = 0;

/**
* Registers that the given block and all of its parents need to be rerendered,
* and registers a callback to do so after a delay, to allowf or batching.
*
* @param block The block to rerender.
* @internal
*/
export function queueRender(block: BlockSvg) {
queueBlock(block);
if (!pid) pid = window.requestAnimationFrame(doRenders);
}

/**
* Adds the given block and its parents to the render queue. Adds the root block
* to the list of root blocks.
*
* @param block The block to queue.
*/
function queueBlock(block: BlockSvg) {
dirtyBlocks.add(block);
const parent = block.getParent();
if (parent) {
queueBlock(parent);
} else {
rootBlocks.add(block);
}
}

/**
* Rerenders all of the blocks in the queue.
*/
function doRenders() {
for (const block of rootBlocks) {
if (block.isDisposed()) continue;
renderBlock(block);
}

pid = 0;
}

/**
* Recursively renders all of the children of the given block, and then renders
* the block.
BeksOmega marked this conversation as resolved.
Show resolved Hide resolved
*
* @param block The block to rerender.
*/
function renderBlock(block: BlockSvg) {
for (const child of block.getChildren(false)) {
renderBlock(child);
}
if (dirtyBlocks.has(block)) {
dirtyBlocks.delete(block);
rootBlocks.delete(block);
block.render(false);
} else {
block.updateConnectionLocations();
}
}
24 changes: 14 additions & 10 deletions core/rendered_connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,13 +185,19 @@ export class RenderedConnection extends Connection {
* @param y New absolute y coordinate, in workspace coordinates.
*/
moveTo(x: number, y: number) {
const dx = this.x - x;
const dy = this.y - y;
BeksOmega marked this conversation as resolved.
Show resolved Hide resolved

if (this.trackedState_ === RenderedConnection.TrackedState.WILL_TRACK) {
this.db_.addConnection(this, y);
this.trackedState_ = RenderedConnection.TrackedState.TRACKED;
} else if (this.trackedState_ === RenderedConnection.TrackedState.TRACKED) {
} else if (
this.trackedState_ === RenderedConnection.TrackedState.TRACKED &&
(dx !== 0 || dy !== 0)) {
this.db_.removeConnection(this, this.y);
this.db_.addConnection(this, y);
}

this.x = x;
this.y = y;
}
Expand Down Expand Up @@ -302,14 +308,12 @@ export class RenderedConnection extends Connection {
(shape as unknown as PathLeftShape).pathLeft +
svgPaths.lineOnAxis('h', xLen);
}
const xy = this.sourceBlock_.getRelativeToSurfaceXY();
const x = this.x - xy.x;
const y = this.y - xy.y;
const offset = this.offsetInBlock_;
this.highlightPath = dom.createSvgElement(
Svg.PATH, {
'class': 'blocklyHighlightedConnectionPath',
'd': steps,
'transform': 'translate(' + x + ',' + y + ')' +
'transform': `translate(${offset.x}, ${offset.y})` +
(this.sourceBlock_.RTL ? ' scale(-1 1)' : ''),
},
this.sourceBlock_.getSvgRoot());
Expand Down Expand Up @@ -459,11 +463,11 @@ export class RenderedConnection extends Connection {
const renderedChild = childBlock as BlockSvg;
// Rerender the parent so that it may reflow.
if (renderedParent.rendered) {
renderedParent.render();
renderedParent.queueRender();
}
if (renderedChild.rendered) {
renderedChild.updateDisabled();
renderedChild.render();
renderedChild.queueRender();
// Reset visibility, since the child is now a top block.
renderedChild.getSvgRoot().style.display = 'block';
}
Expand All @@ -484,7 +488,7 @@ export class RenderedConnection extends Connection {

const parentBlock = this.getSourceBlock();
if (parentBlock.rendered) {
parentBlock.render();
parentBlock.queueRender();
}
}

Expand Down Expand Up @@ -528,11 +532,11 @@ export class RenderedConnection extends Connection {
this.type === ConnectionType.PREVIOUS_STATEMENT) {
// Child block may need to square off its corners if it is in a stack.
// Rendering a child will render its parent.
childBlock.render();
childBlock.queueRender();
} else {
// Child block does not change shape. Rendering the parent node will
// move its connected children into position.
parentBlock.render();
parentBlock.queueRender();
}
}

Expand Down
Loading