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

Update DragSurfaceSvg to not repaint on drag #207

Closed
tmickel opened this issue Apr 12, 2016 · 12 comments
Closed

Update DragSurfaceSvg to not repaint on drag #207

tmickel opened this issue Apr 12, 2016 · 12 comments
Assignees
Milestone

Comments

@tmickel
Copy link
Contributor

tmickel commented Apr 12, 2016

@picklesrus
Our blocks currently repaint on drag, but this demo doesn't seem to:
https://jsfiddle.net/zaaex6bx/

What's going on? Can we avoid repaint?

@tmickel tmickel self-assigned this Apr 12, 2016
@tmickel
Copy link
Contributor Author

tmickel commented Apr 12, 2016

Ah! One potential difference. In my drag surface, the block group inside the SVG is being translated, instead of the actual SVG itself. That would make sense why it would repaint the SVG. This is a potential big performance hit for no reason.

@tmickel
Copy link
Contributor Author

tmickel commented Apr 12, 2016

I've got a basic demo where it's not repainting during drag using the idea from above. Exciting

@tmickel tmickel changed the title Why doesn't this repaint? Update DragSurfaceSvg to not repaint on drag Apr 12, 2016
@tmickel tmickel added this to the April 21 milestone Apr 12, 2016
@picklesrus
Copy link
Contributor

Super neat! So did you turn your drag group into its own svg? I would love to see the demo or the code. I've been playing with nesting svgs too, though with a goal of making panning and zooming work as well as the overflow div trick did. So far it isn't winning as much as I hoped, but I might be able to apply your approach to that too.

@tmickel
Copy link
Contributor Author

tmickel commented Apr 13, 2016

The drag group is already its own SVG. Just need to apply the translation on the SVG itself rather than inside it :)

Hopefully should have a really working demo by the end of the day - I had a basic one but all the coordinates are broken...

Curious about what you've been doing also. By nesting SVGs, do you mean using a foreign object?

@picklesrus
Copy link
Contributor

Mine isn't proving to be that interesting yet. I'm playing with the viewbox attribute to see if it helps with panning and zooming.
My demo currently has a structure that looks something like this:


shapes/blocks in here



The idea is that when interacting with the scroll bars, I just change the viewbox and the inner svg shifts or scales appropriately. I haven't gotten the repaints to be as infrequent as I'd like though. Changing the viewbox seems to repaint the whole interior of that blocks svg. I'm not quite ready to give up though. I'll let you know if anything useful comes of it.

@tmickel
Copy link
Contributor Author

tmickel commented Apr 13, 2016

Merged into develop, @picklesrus

That's an interesting idea with the viewbox. I'd guess that it's not as optimized as browser scrolls, so I don't know. It would be cool if you get results!

@tmickel tmickel closed this as completed Apr 13, 2016
@tmickel
Copy link
Contributor Author

tmickel commented Apr 14, 2016

@picklesrus

Have you tried this:
-Move everything out of the Blockly svg except for the block canvas and other things that translate with it.
-instead of applying the translations and scales to the block canvas group, apply the transform on the outer svg tag

My understanding based on this issue and results would be that the browser likely won't repaint the svg if you do a translation on the outer element. Then we should get perf as good as the div trick maaaaybe? Although maybe the browser is doing extra tricks with scrolling...

Just a thought and might save a lot of work.

@picklesrus
Copy link
Contributor

Doh! Yes, I had been trying something like that after mostly giving up on viewBox, but now looking back at my example, I think maybe I've been confusing the svg transform attribute and the css one all day today. Couple that with the fact that you can't apply a transform to an tag (in some browsers anyway) and I took a nice journey down the rabbit hole. But, I just read your drag surface CL and realized my mistake. Thanks! I should have a real answer to whether this works tomorrow.

@tmickel
Copy link
Contributor Author

tmickel commented Apr 14, 2016

Ah! Yeah this has been very tricky, I've done similar things wrong many, many times. I hope it works!!

@picklesrus
Copy link
Contributor

I may have some good news! I may have figured out a way to do this without changing too much.
I can have multiple s sitting directly on top of each other if I set them to position:absolute. Then, I can apply transforms to each of them independently and the block canvas doesn't repaint. The scrollbars can move up while the blocks move down. Amazing stuff here ;) I can't do this if the s are nested (well, except for in browsers that have started supporting svg2.0 features like firefox).

If I do it this way, I don't have to move things in and out of the svgs and I think the code might be relatively simple. I only have this working in a toy example so I'm going to try and apply it to mainline blocky first and see what problems I run into. I believe it will be easier to apply to scratch-blockly if mainline doesn't work.

@picklesrus
Copy link
Contributor

And I threw the toy example up on a webserver. It isn't that exciting since I cheated by adding buttons to move things around rather than adding scroll bars, but if you're curious about the dom structure, this is it: http://blocktests-1261.appspot.com/bt/tests/layeredsvgtest.html

@tmickel
Copy link
Contributor Author

tmickel commented Apr 15, 2016

@picklesrus Awesome yes!!! This looks great and the demo is exciting.

Basically are you thinking, all absolutely positioned:
-One SVG that contains blocklyBlockCanvas and blocklyBubbleCanvas - this is the one you transform on pan/scale
-One SVG that contains the scrollbars, zoom buttons, trash can
-One SVG that contains the flyout somehow?
-One SVG for the drag layer (in scratch-blockly - maybe in mainline eventually?)

In doing the drag layer, I put the two absolute-positioned SVGs inside a container div that is position:relative - just a trick that might help you with the layout stuff.

Based on how good of performance with even very large stacks with the drag layer, I'd buy that this will give us really good workspace panning on mobile.

I'm a little worried that the above might still not be easy. Even splitting out the dragging block to a separate SVG was tricky because there were some assumptions being made in the code about relative coordinates. Hopefully there will be less of an issue with this though because the components seem mostly independent. The main place where it seems to me there may seem to be risk of breaking is flyout vs. workspace coordinates, but Rachel also just worked on that so if you run into problems she might be able to help.

One more related note: as a stretch goal -- I really don't think this is necessary -- it may be cool to apply the same trick to the flyout workspace as well. The scrolling of the flyout is definitely not 60fps (less important since you use it less), but it seems like it could be with this trick. Since the flyout itself is just a workspace, you may get it for free if you structure so all workspaces are added flatly to the container div, not nesting the flyout inside the parent SVG, or something like that.

I'm very excited to see this in action and I'm glad it's going to be simpler than the div solution!

cwillisf pushed a commit that referenced this issue Oct 11, 2024
* feat: add a block inflater that supports recycling

* chore: update JSDoc
github-actions bot pushed a commit that referenced this issue Oct 18, 2024
# [1.2.0-beta.1](v1.1.86...v1.2.0-beta.1) (2024-10-18)

### Bug Fixes

* Add support for flyout labels with status indicators ([#212](#212)) ([665d196](665d196))
* add support for Scratch-style block comments ([#83](#83)) ([8902091](8902091))
* add support for Scratch-style procedures ([#39](#39)) ([13647eb](13647eb))
* add zoom controls config ([#126](#126)) ([a09ae24](a09ae24))
* allow focusing fields in the flyout on mobile ([#184](#184)) ([6c9d3a6](6c9d3a6))
* allow specifying the function to be used for prompting about variable creation/edits ([#106](#106)) ([4cfe66f](4cfe66f))
* clean up data block definitions ([#90](#90)) ([9ea96e2](9ea96e2))
* correctly align extension block icons ([#182](#182)) ([e3dbad1](e3dbad1))
* delete context menu to display the correct number of blocks ([#127](#127)) ([a65d24a](a65d24a))
* **deps:** after installing deps, replace Blockly v11 with v12 RC ([677ff6f](677ff6f))
* **deps:** clone Blockly RC over HTTP instead of SSH ([8a861f0](8a861f0))
* display icons in the toolbox for extension categories ([#47](#47)) ([b53eadd](b53eadd))
* don't hide the drag surface ([#38](#38)) ([7f70f09](7f70f09))
* don't show global/local options when renaming a variable ([#123](#123)) ([22a6b73](22a6b73))
* don't show scope options when renaming a variable from the variable getter context menu ([#139](#139)) ([375e56d](375e56d))
* don't warn about procedure references when moving the definition on the workspace ([#131](#131)) ([cda58cc](cda58cc))
* enable and style workspace comments ([#82](#82)) ([98ccb62](98ccb62))
* enable dragging arguments out of procedure blocks ([#119](#119)) ([0ca0620](0ca0620))
* Export colours. ([7c346fa](7c346fa))
* Export Scratch messages. ([09326a1](09326a1))
* fix a crash when adding a broadcast message ([#150](#150)) ([8e165ce](8e165ce))
* fix alignment of "define" text baseline on custom blocks ([#220](#220)) ([cc4d9f9](cc4d9f9))
* fix bug that caused the number/string input in the custom block editor to have square corners ([#213](#213)) ([c3ee958](c3ee958))
* fix bug that could cause duplicated procedure argument blocks to create more duplicates on drag ([#217](#217)) ([6a1c8a9](6a1c8a9))
* fix bug that prevented modal dialogs from appearing on mobile ([#183](#183)) ([37e0f10](37e0f10))
* fix bug that prevented showing the contextual menu on blocks ([#176](#176)) ([2e98ff1](2e98ff1))
* fix color of block reporter dropdown text ([#205](#205)) ([73d978e](73d978e))
* fix dropdown menu metrics ([#148](#148)) ([40eee91](40eee91))
* fix exception when editing custom blocks ([#105](#105)) ([7478546](7478546))
* fix positioning of categories when scrolling via the toolbox ([#186](#186)) ([6d14530](6d14530))
* fix styling of dropdown menus ([#152](#152)) ([0e80277](0e80277))
* fix the color of procedure argument blocks ([#216](#216)) ([88c700e](88c700e))
* fix the colors of the angle picker dropdown ([#179](#179)) ([59896d2](59896d2))
* fix the flyout width at 250 pixels ([#168](#168)) ([a47aba6](a47aba6))
* fix the styling of contextual menus ([#147](#147)) ([6fbc2e5](6fbc2e5))
* fix toolbox category selection ([#141](#141)) ([d3e1a1b](d3e1a1b))
* fix wrapping of long category labels ([#166](#166)) ([7b39ac1](7b39ac1))
* fixed bug where broadcast messages would appear in the variable dropdown list ([#124](#124)) ([b1e67f6](b1e67f6))
* improve positioning of newly created procedure blocks ([#121](#121)) ([84a9e5b](84a9e5b))
* improve reliability of block value reporting ([#77](#77)) ([cb5b068](cb5b068))
* improve sizing and rendering of comments ([#219](#219)) ([1279c0a](1279c0a))
* load CSS and fix up UI appearance ([#33](#33)) ([1645129](1645129))
* load the continuous toolbox ([#31](#31)) ([ea68b1c](ea68b1c))
* make block images work in all contexts ([#30](#30)) ([920febf](920febf))
* make FieldNumber a subclass of FieldTextInput ([#214](#214)) ([3ae2235](3ae2235))
* make variable names case-sensitive ([#122](#122)) ([46854cd](46854cd))
* match Scratch behaviors around dragging and connection stickiness ([#80](#80)) ([fd1bc58](fd1bc58))
* miscellaneous UI fixits ([#41](#41)) ([300a1ce](300a1ce))
* modernize and reenable the colour slider field ([#42](#42)) ([4f97982](4f97982))
* more closely align flyout layout with Scratch ([#45](#45)) ([49663ed](49663ed))
* prevent deleting procedure definition blocks with references by dragging to the flyout ([#120](#120)) ([fa9367d](fa9367d))
* prevent dragging blocks into the slot occupied by the procedure definition block's example caller block ([#118](#118)) ([453ffa9](453ffa9))
* re-export scratch-blocks utility functions ([#26](#26)) ([685ecfc](685ecfc))
* readd the control blocks ([#22](#22)) ([f69d4ac](f69d4ac))
* readd the data blocks ([#29](#29)) ([fafed65](fafed65))
* readd the event blocks ([#21](#21)) ([4de530f](4de530f))
* readd the looks blocks ([#23](#23)) ([34f07c0](34f07c0))
* readd the motion blocks ([#20](#20)) ([79398c2](79398c2))
* readd the operator blocks and dependencies ([#19](#19)) ([8024e9f](8024e9f))
* readd the sensing blocks ([#27](#27)) ([9f5f135](9f5f135))
* readd the sound blocks ([#24](#24)) ([6837513](6837513))
* reenable reporting block values ([#55](#55)) ([70c8cfd](70c8cfd))
* reenable shadows for blocks being dragged ([#79](#79)) ([94d2a2c](94d2a2c))
* reenable support for checkboxes in the flyout ([#43](#43)) ([e603c67](e603c67))
* reenable support for dragging blocks between sprites ([#130](#130)) ([3d8b998](3d8b998))
* reenable the matrix field ([#49](#49)) ([aa3341b](aa3341b))
* reenable the mobile numpad field ([#54](#54)) ([003afd0](003afd0))
* reenable the note block and picker field ([#48](#48)) ([de62d77](de62d77))
* reenable the vertical separator field ([#46](#46)) ([48e931f](48e931f))
* remove canvas transition ([#129](#129)) ([ff4b115](ff4b115))
* remove underscore from a few createProcedureDefCallback calls ([#40](#40)) ([4e794f6](4e794f6))
* render the procedure definition block like Scratch ([#115](#115)) ([2a543f5](2a543f5))
* resolve error when adding the stop block to the workspace ([#56](#56)) ([f3e059c](f3e059c))
* resolve various UI issues ([#117](#117)) ([4b74d5c](4b74d5c))
* select new variable blocks' monitor checkboxes after creation ([#140](#140)) ([3811d93](3811d93))
* show connection highlights for boolean inputs ([#181](#181)) ([303611a](303611a))
* show the glow only when blocks are running ([#57](#57)) ([33e9e91](33e9e91))
* show the name of the list in the list getter block context menu ([#132](#132)) ([eb839fc](eb839fc))
* update the flyout for compatibility with the new flyout API ([#209](#209)) ([7ce9991](7ce9991))
* use non-deprecated input type constants ([#78](#78)) ([1f1c859](1f1c859))
* use Scratch-style text blocks ([#37](#37)) ([6bbbdf7](6bbbdf7))
* use Scratch's FieldAngle ([#138](#138)) ([ef7911c](ef7911c))

### Features

* add a block inflater that supports recycling ([#207](#207)) ([0701679](0701679))
* add bubbles/icons for block flyout checkboxes ([#208](#208)) ([39b2162](39b2162))
* add custom Scratch variable model and creation event classes ([#86](#86)) ([2598ede](2598ede))
* clean up and export Scratch's variables.js ([#88](#88)) ([5c1acfe](5c1acfe))
* readd support for the custom Data toolbox category ([#87](#87)) ([dcfbf39](dcfbf39))
* reenable Scratch's FieldVariable subclass ([#91](#91)) ([7c891e3](7c891e3))

### Reverts

* Revert "fix: add zoom controls config (#126)" (#128) ([8e1dc14](8e1dc14)), closes [#126](#126) [#128](#128)
github-actions bot pushed a commit that referenced this issue Oct 21, 2024
# [2.0.0-spork.1](v1.1.86...v2.0.0-spork.1) (2024-10-21)

### Bug Fixes

* Add support for flyout labels with status indicators ([#212](#212)) ([665d196](665d196))
* add support for Scratch-style block comments ([#83](#83)) ([8902091](8902091))
* add support for Scratch-style procedures ([#39](#39)) ([13647eb](13647eb))
* add zoom controls config ([#126](#126)) ([a09ae24](a09ae24))
* allow focusing fields in the flyout on mobile ([#184](#184)) ([6c9d3a6](6c9d3a6))
* allow specifying the function to be used for prompting about variable creation/edits ([#106](#106)) ([4cfe66f](4cfe66f))
* clean up data block definitions ([#90](#90)) ([9ea96e2](9ea96e2))
* correctly align extension block icons ([#182](#182)) ([e3dbad1](e3dbad1))
* delete context menu to display the correct number of blocks ([#127](#127)) ([a65d24a](a65d24a))
* **deps:** after installing deps, replace Blockly v11 with v12 RC ([677ff6f](677ff6f))
* **deps:** clone Blockly RC over HTTP instead of SSH ([8a861f0](8a861f0))
* display icons in the toolbox for extension categories ([#47](#47)) ([b53eadd](b53eadd))
* don't hide the drag surface ([#38](#38)) ([7f70f09](7f70f09))
* don't include Blockly RC source in npm package ([21b15d1](21b15d1))
* don't show global/local options when renaming a variable ([#123](#123)) ([22a6b73](22a6b73))
* don't show scope options when renaming a variable from the variable getter context menu ([#139](#139)) ([375e56d](375e56d))
* don't warn about procedure references when moving the definition on the workspace ([#131](#131)) ([cda58cc](cda58cc))
* enable and style workspace comments ([#82](#82)) ([98ccb62](98ccb62))
* enable dragging arguments out of procedure blocks ([#119](#119)) ([0ca0620](0ca0620))
* Export colours. ([7c346fa](7c346fa))
* Export Scratch messages. ([09326a1](09326a1))
* fix a crash when adding a broadcast message ([#150](#150)) ([8e165ce](8e165ce))
* fix alignment of "define" text baseline on custom blocks ([#220](#220)) ([cc4d9f9](cc4d9f9))
* fix bug that caused the number/string input in the custom block editor to have square corners ([#213](#213)) ([c3ee958](c3ee958))
* fix bug that could cause duplicated procedure argument blocks to create more duplicates on drag ([#217](#217)) ([6a1c8a9](6a1c8a9))
* fix bug that prevented modal dialogs from appearing on mobile ([#183](#183)) ([37e0f10](37e0f10))
* fix bug that prevented showing the contextual menu on blocks ([#176](#176)) ([2e98ff1](2e98ff1))
* fix color of block reporter dropdown text ([#205](#205)) ([73d978e](73d978e))
* fix dropdown menu metrics ([#148](#148)) ([40eee91](40eee91))
* fix exception when editing custom blocks ([#105](#105)) ([7478546](7478546))
* fix positioning of categories when scrolling via the toolbox ([#186](#186)) ([6d14530](6d14530))
* fix styling of dropdown menus ([#152](#152)) ([0e80277](0e80277))
* fix the color of procedure argument blocks ([#216](#216)) ([88c700e](88c700e))
* fix the colors of the angle picker dropdown ([#179](#179)) ([59896d2](59896d2))
* fix the flyout width at 250 pixels ([#168](#168)) ([a47aba6](a47aba6))
* fix the styling of contextual menus ([#147](#147)) ([6fbc2e5](6fbc2e5))
* fix toolbox category selection ([#141](#141)) ([d3e1a1b](d3e1a1b))
* fix wrapping of long category labels ([#166](#166)) ([7b39ac1](7b39ac1))
* fixed bug where broadcast messages would appear in the variable dropdown list ([#124](#124)) ([b1e67f6](b1e67f6))
* hide disable and inline inputs contextual menu items ([#35](#35)) ([c548298](c548298))
* improve positioning of newly created procedure blocks ([#121](#121)) ([84a9e5b](84a9e5b))
* improve reliability of block value reporting ([#77](#77)) ([cb5b068](cb5b068))
* improve sizing and rendering of comments ([#219](#219)) ([1279c0a](1279c0a))
* load CSS and fix up UI appearance ([#33](#33)) ([1645129](1645129))
* load the continuous toolbox ([#31](#31)) ([ea68b1c](ea68b1c))
* make block images work in all contexts ([#30](#30)) ([920febf](920febf))
* make FieldNumber a subclass of FieldTextInput ([#214](#214)) ([3ae2235](3ae2235))
* make variable names case-sensitive ([#122](#122)) ([46854cd](46854cd))
* match Scratch behaviors around dragging and connection stickiness ([#80](#80)) ([fd1bc58](fd1bc58))
* miscellaneous UI fixits ([#41](#41)) ([300a1ce](300a1ce))
* modernize and reenable the colour slider field ([#42](#42)) ([4f97982](4f97982))
* more closely align flyout layout with Scratch ([#45](#45)) ([49663ed](49663ed))
* prevent deleting procedure definition blocks with references by dragging to the flyout ([#120](#120)) ([fa9367d](fa9367d))
* prevent dragging blocks into the slot occupied by the procedure definition block's example caller block ([#118](#118)) ([453ffa9](453ffa9))
* re-export scratch-blocks utility functions ([#26](#26)) ([685ecfc](685ecfc))
* readd the control blocks ([#22](#22)) ([f69d4ac](f69d4ac))
* readd the data blocks ([#29](#29)) ([fafed65](fafed65))
* readd the event blocks ([#21](#21)) ([4de530f](4de530f))
* readd the looks blocks ([#23](#23)) ([34f07c0](34f07c0))
* readd the motion blocks ([#20](#20)) ([79398c2](79398c2))
* readd the operator blocks and dependencies ([#19](#19)) ([8024e9f](8024e9f))
* readd the sensing blocks ([#27](#27)) ([9f5f135](9f5f135))
* readd the sound blocks ([#24](#24)) ([6837513](6837513))
* reenable reporting block values ([#55](#55)) ([70c8cfd](70c8cfd))
* reenable shadows for blocks being dragged ([#79](#79)) ([94d2a2c](94d2a2c))
* reenable support for checkboxes in the flyout ([#43](#43)) ([e603c67](e603c67))
* reenable support for dragging blocks between sprites ([#130](#130)) ([3d8b998](3d8b998))
* reenable the matrix field ([#49](#49)) ([aa3341b](aa3341b))
* reenable the mobile numpad field ([#54](#54)) ([003afd0](003afd0))
* reenable the note block and picker field ([#48](#48)) ([de62d77](de62d77))
* reenable the vertical separator field ([#46](#46)) ([48e931f](48e931f))
* remove canvas transition ([#129](#129)) ([ff4b115](ff4b115))
* remove underscore from a few createProcedureDefCallback calls ([#40](#40)) ([4e794f6](4e794f6))
* render the procedure definition block like Scratch ([#115](#115)) ([2a543f5](2a543f5))
* resolve error when adding the stop block to the workspace ([#56](#56)) ([f3e059c](f3e059c))
* resolve various UI issues ([#117](#117)) ([4b74d5c](4b74d5c))
* select new variable blocks' monitor checkboxes after creation ([#140](#140)) ([3811d93](3811d93))
* show connection highlights for boolean inputs ([#181](#181)) ([303611a](303611a))
* show the glow only when blocks are running ([#57](#57)) ([33e9e91](33e9e91))
* show the name of the list in the list getter block context menu ([#132](#132)) ([eb839fc](eb839fc))
* update the flyout for compatibility with the new flyout API ([#209](#209)) ([7ce9991](7ce9991))
* use non-deprecated input type constants ([#78](#78)) ([1f1c859](1f1c859))
* use Scratch-style text blocks ([#37](#37)) ([6bbbdf7](6bbbdf7))
* use Scratch's FieldAngle ([#138](#138)) ([ef7911c](ef7911c))

* fix!: bump to v2.0 to reflect Blockly un-forking ([899a981](899a981))

### Features

* add a block inflater that supports recycling ([#207](#207)) ([0701679](0701679))
* add bubbles/icons for block flyout checkboxes ([#208](#208)) ([39b2162](39b2162))
* add custom Scratch variable model and creation event classes ([#86](#86)) ([2598ede](2598ede))
* clean up and export Scratch's variables.js ([#88](#88)) ([5c1acfe](5c1acfe))
* readd support for the custom Data toolbox category ([#87](#87)) ([dcfbf39](dcfbf39))
* reenable Scratch's FieldVariable subclass ([#91](#91)) ([7c891e3](7c891e3))

### Reverts

* Revert "fix: add zoom controls config (#126)" (#128) ([8e1dc14](8e1dc14)), closes [#126](#126) [#128](#128)

### BREAKING CHANGES

* scratch-blocks is no longer a divergent fork of
Blockly, and instead depends on Blockly as a regular node_modules
dependency.

Thanks, @gonfunko and everyone else at Google who helped with this
effort!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants