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

Remove workspace drag surface and block drag surface #6160

Closed
BeksOmega opened this issue May 9, 2022 · 4 comments · Fixed by #7070
Closed

Remove workspace drag surface and block drag surface #6160

BeksOmega opened this issue May 9, 2022 · 4 comments · Fixed by #7070

Comments

@BeksOmega
Copy link
Collaborator

Describe the bug

With the drag surface (~2-5 FPS) on b92cbd3:
image

Without the drag surface (~10 FPS) b92cbd3:

image

To Reproduce

Steps to reproduce the behavior:

  1. Go to the playground.
  2. Hit the 'Airstrike' button
  3. Go to the performance tab of the chrome devtools.
  4. Enable recording screenshots.
  5. Set CPU Throttling to 6x.
  6. Start a recording.
  7. Drag the workspace left... right... and left again.
  8. Stop the recording.

Expected behavior

Ideally we wouldn't drop frames while dragging. Unfortunately most of the costs of dragging occur in the SVG engine, which we don't have a ton of control over.

Additional context

I have been able to make this a lot faster by changing things from using setAttribute to using <element>.style. This lets the browser use its CSS optimizations.

On https://github.com/BeksOmega/blockly/tree/perf/ws-drag (~30 FPS):

image

But that branch needs a lot of cleanup. Eg this does not work in IE, because IE does not support CSS transforms on SVG elements. So we need some utils that will handle setting SVG locations in a generalized way.

@BeksOmega
Copy link
Collaborator Author

BeksOmega commented Mar 14, 2023

I'm going to reopen this because the removal was reverted due to it being a breaking change, but we still want to remove it in the future.

Note: we're not going to add deprecation warnings b/c we use the surfaces in core, and spamming developers with warnings they can't turn off is not nice.

@BeksOmega BeksOmega reopened this Mar 14, 2023
@rachel-fenichel
Copy link
Collaborator

We decided not to include this change in the March 2023 release because it's a breaking change. Instead we're holding it until we release v10.

When reapplying the fix in #6758, also apply #6874 to resolve #6852.

@BeksOmega BeksOmega assigned BeksOmega and unassigned gonfunko Mar 31, 2023
@BeksOmega BeksOmega added the issue: triage Issues awaiting triage by a Blockly team member label Apr 5, 2023
@maribethb
Copy link
Contributor

Note: We need to fix the scroll-options plugin when we release this as well.

@maribethb maribethb removed the issue: triage Issues awaiting triage by a Blockly team member label Apr 12, 2023
@BeksOmega
Copy link
Collaborator Author

Related to: google/blockly-samples#1664

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants