fix: cancelling all renders on triggering queued renders

[BeksOmega]

The basics

• I validated my changes

The details

Resolves

Fixes N/A

Proposed Changes + Reasoning

Follow up to #7784 We were canceling all queued renders when triggering a render in one workspace. We shouldn't do that because there may still be queued renders in other workspaces

Test Coverage

All tests pass.

1. Defined the following block in the playground in develop.

Blockly.Blocks['test'] = {
        init: function() {
          this.setColour('#ccc');
          this.appendDummyInput().appendField(
              new Blockly.FieldImage('media/arrow.png', 50, 50, '', this.onClick.bind(this)));
        },

        onClick: function() {
          this.setNextStatement(true);
          this.workspace.getFlyout().show([{'kind': 'block', 'type': 'controls_if'}])
        }
      }

2. Observed that when clicked, the flyout was shown but the next statement was not rendered.
3. Did the same thing on this branch.
4. Observed that the flyout was shown and the next statement was rendered.

Documentation

N/A

Additional Information

N/A

To Review

Please double check that my logic for dequeuing blocks seems correct. I don't want to accidentally drop blocks that we still need to render, or let blocks accumulate in memory. To check this you might also want to read through the logic on doRenders

[BeksOmega]

@maribethb could you actually take a look at this for me? I think it needs to go in today for me to get a point release to Mike by the time they do bug bash.

[maribethb]

The logic looks correct to me. The only thing I'm wondering is if it makes sense to have the animationRequestId stored per-workspace so that you can still cancel it. but adding a map and lookup is also adding work so it may not be worth it. If we don't cancel it, then assume we have a request for delayed rendering (for the same workspace) already waiting for a frame, then we request an immediate rendering, so the immediate render happens, then by the time we get to the animation frame, there's nothing left to render because dirtyBlocks is already cleared out. So I think that's probably fine to leave as is, if I'm interpreting what's happening correctly.

[BeksOmega]

The logic looks correct to me. The only thing I'm wondering is if it makes sense to have the animationRequestId stored per-workspace so that you can still cancel it. but adding a map and lookup is also adding work so it may not be worth it. If we don't cancel it, then assume we have a request for delayed rendering (for the same workspace) already waiting for a frame, then we request an immediate rendering, so the immediate render happens, then by the time we get to the animation frame, there's nothing left to render because dirtyBlocks is already cleared out. So I think that's probably fine to leave as is, if I'm interpreting what's happening correctly.

Yeah I think it might be worth coming back and adding more per-workspace functionality to the rendering system. E.g. the animation frame request like you mentioned, and finishQueuedRenders but since this is a patch I don't want to add a bunch of new functionality.