-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Scattergl selection / clustering fix #2593
Conversation
Sweet! @dy can you explain what changes you needed to make to fix the problem? |
Amazing. I'll run a few performance tests tomorrow morning to see these changes have a side-effect. |
In the meantime @dy, can you confirm that this branch works in IE11? |
Moreover @dy, can you test out the other regl components (regl-line2d, regl-error2d, and sploms) in Safari? |
@dy I'm noticing differences in behavior using 1.36.1 and this branch off https://codepen.io/anon/pen/zjoWpO?editors=0010 in 1.36.1: this branch: where it looks like 1.36.1 clustered things a little more aggressively making dimmed points appears when selection on a 1e5 pt graph. Is this expected? |
Yes, that is the result of additional levels of clustering rendered due to #2586 mentioned by Alex |
In terms of performance, comparing 1.36.1 and this branch:
1.36.1: this branch: I'd even say panning off this branch perform a little worse than in 1.35.2: https://codepen.io/etpinard/pen/MGpRdW?editors=0010
So, @dy can you explain why panning is affected by your patches here? Does it have to? |
Oh and can you add a mock that lock down the fix for #2334? Thanks! |
I imagine there might be a way of adaptive opacity based on the density of a cluster, but that better be addressed as a separate issue. |
Ok, but why does a selection fix affect panning? |
Slow panning is the direct result of increased number of points caused by #2334 fix. Although it should not be significantly slower than snap-points-2d in 1.35, but possibly can. I did not notice fps drop though. That is the result of z-ordering a point-cluster tree instead of sorting by x, which saves ~100ms of sorting routine on init by displaying extra 10-15% points in runtime. |
Although there might be another reason - non-square clustering. It needs a bit of experiment though. |
Thanks for the info @dy ! Can you describe what will happen in turns of performance improvement once (if 😉 ) regl-project/regl#474 is fixed? |
@etpinard 1k 1-point scattergl traces will be rendered ~6 times faster. If number of points per trace more than 1, it is not that noticeable. |
Ok. Is there anything we can do to improve panning performance at 1e6 pts? |
Very strong panning perf off dbd6049: the bug from #2593 (comment) appears to be fixed too 🎉 |
@@ -549,6 +549,21 @@ describe('@gl Test splom interactions:', function() { | |||
.catch(failTest) | |||
.then(done); | |||
}); | |||
|
|||
it('should work with typed arrays', function(done) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
locking a fix for #2585 from the new regl-splom
version
All right. Tests are ✅ Let's merge this thing! 💃 🔈 Big shoutout to @dy for closing 5 issues in one PR. Huge! 🔈 1.37.0 will be out in the next few hours. |
Fixes #2586 (both issues), fixes #2334, fixes #2543, fixes #2457
Changes:
regl._refresh()
stub to workaround Constant attribute does not update regl-project/regl#474regl.buffer
data