-
Notifications
You must be signed in to change notification settings - Fork 19
fix: debounce minimap rendering #97
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
base: main
Are you sure you want to change the base?
Conversation
f8423a2 to
d9982d6
Compare
d9982d6 to
cb7abf8
Compare
|
@Skaiir Let's please have a test case for the "extreme setup" that we're targeting. I'd love to ensure this is properly tested. |
|
@nikku I've set something up quickly but because it's asynchronous I wasn't able to turn it into a proper unit test. Maybe I need to make it a little less intense though because at 3000+ items it looks like diagram-js starts falling apart. Edit: What do you think of this, we have this asynchronous observable extreme extreme case test which we keep as a .skip for dev testing ad-hoc, and we add another slightly less insane synchronous test (no yielding back to the browser, so it will freeze for a while) and expect a maximum execution time. I think if I keep the number of elements drawn around 1500-2000 it's somewhat within 5-10 seconds execution time with the current perf. |
52fb0bd to
9e631b0
Compare
Related to #96
9e631b0 to
414001f
Compare
I'd ensure that the minimap does not actually block things, i.e. continuing to model works.
Yes, that could be feasible. |
Closes #81
Related to #96
Proposed Changes
This adds two configs
debounceDelayanddebounceSkipDelay, which prevent updates from running frequently when many update triggering actions are fired consecutively. In practice this means any updates fired withindebounceDelayms are fired off as a single update at the end UNLESSdebounceSkipDelayis reached, which triggers an update regardless to allow for very long running renders to still display a somewhat current mini-map. Note this does not apply to user interactions with the minimap, as debouncing interferes with the viewport visuals (I haven't figured out a good way to handle this yet).Secondly, while the minimap is closed, we simply do not update it. However we always force a non-debounced update on open (to ensure viewport information is properly initialized).
Checklist
To ensure you provided everything we need to look at your PR:
@bpmn-io/srtoolCloses {LINK_TO_ISSUE}orRelated to {LINK_TO_ISSUE}