Skip to content
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
8 changes: 0 additions & 8 deletions blocks/procedures.js
Original file line number Diff line number Diff line change
Expand Up @@ -756,9 +756,6 @@ const PROCEDURE_CALL_COMMON = {
this.quarkConnections_ = {};
this.quarkIds_ = [];
}
// Switch off rendering while the block is rebuilt.
const savedRendered = this.rendered;
this.rendered = false;
// Update the quarkConnections_ with existing connections.
for (let i = 0; i < this.arguments_.length; i++) {
const input = this.getInput('ARG' + i);
Expand Down Expand Up @@ -798,11 +795,6 @@ const PROCEDURE_CALL_COMMON = {
}
}
}
// Restore rendering and show the changes.
this.rendered = savedRendered;
if (this.rendered) {
this.render();
}
},
/**
* Modify this block to have the correct number of arguments.
Expand Down
6 changes: 6 additions & 0 deletions core/block_svg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,8 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg,
this.comment = null; // For backwards compatibility.
}
if (this.rendered) {
// Icons must force an immediate render so that bubbles can be opened
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we instead maybe add the comment icon in the afterQueuedRenders or whatever we called it? I don't actually know what I'm talking about here so feel free to ignore me if this is nonsense :p

otherwise: nit: (here and below) immediately typo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wrt your comment about finishQueuedRenders, that doesn't solve for this use case because the icon needs to be appended to the block before the render is triggered so the renderer can position it correctly.

But now that I'm thinking about this again, I'm going to investigate this one more time.

I was trying to solve for the use case of:

myBlock.setCommentText('yay a comment');
myBlock.getCommentIcon().setVisible(true);

I thought that for this to correctly position the bubble, the setCommentText method must:

  1. Append the comment to the block.
  2. Rerender the block to position the icon.
  3. Tell the icon what its new position is.

So that when setVisible is called, the icon has the correct information to position the bubble.

But thinking about it again, if we use the render queueing it should look like:

  1. Append the comment to the block.
  2. Open the bubble at the incorrect position.
  3. Rerender the block to position the icon.
  4. Tell the icon what it new position is.
  5. Update the position of the bubble for the new position of the icon.
  6. Actually paint the frame to the screen.

This should still be correct. But it was causing some unit tests to fail, so I need to remember why that was.

// immedately at the correct position.
this.render();
// Adding or removing a comment icon will cause the block to change shape.
this.bumpNeighbours();
Expand Down Expand Up @@ -1078,6 +1080,8 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg,
}
}
if (changedState && this.rendered) {
// Icons must force an immediate render so that bubbles can be opened
// immedately at the correct position.
this.render();
// Adding or removing a warning icon will cause the block to change shape.
this.bumpNeighbours();
Expand All @@ -1099,6 +1103,8 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg,
mutator.createIcon();
}
if (this.rendered) {
// Icons must force an immediate render so that bubbles can be opened
// immedately at the correct position.
this.render();
// Adding or removing a mutator icon will cause the block to change shape.
this.bumpNeighbours();
Expand Down
1 change: 0 additions & 1 deletion core/flyout_horizontal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,6 @@ export class HorizontalFlyout extends Flyout {
// a block.
child.isInFlyout = true;
}
block!.render();
const root = block!.getSvgRoot();
const blockHW = block!.getHeightWidth();
// Figure out where to place the block.
Expand Down
1 change: 0 additions & 1 deletion core/flyout_vertical.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,6 @@ export class VerticalFlyout extends Flyout {
// a block.
child.isInFlyout = true;
}
block!.render();
const root = block!.getSvgRoot();
const blockHW = block!.getHeightWidth();
const moveX =
Expand Down
2 changes: 1 addition & 1 deletion core/mutator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ export class Mutator extends Icon {
this.rootBlock = block.decompose!(ws)!;
const blocks = this.rootBlock.getDescendants(false);
for (let i = 0, child; child = blocks[i]; i++) {
child.render();
child.queueRender();
}
// The root block should not be draggable or deletable.
this.rootBlock.setMovable(false);
Expand Down
7 changes: 1 addition & 6 deletions core/rendered_connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -514,12 +514,7 @@ export class RenderedConnection extends Connection {
return;
}
blockShadow.initSvg();
blockShadow.render(false);

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

/**
Expand Down
2 changes: 1 addition & 1 deletion core/workspace_svg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1301,7 +1301,7 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg {
const blocks = this.getAllBlocks(false);
// Render each block.
for (let i = blocks.length - 1; i >= 0; i--) {
blocks[i].render(false);
blocks[i].queueRender();
}

if (this.currentGesture_) {
Expand Down