-
Notifications
You must be signed in to change notification settings - Fork 3
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
SDF strategy #5
SDF strategy #5
Conversation
@etpinard thanks for the feedback, fixed everything except for marker displacement. Old gl-scatter2d-fancy aligned the point vertically regardless of font baseline etc, based on glyph shape. Here we have a hard time doing that. One way I see is replacing marker in gl_markers from |
@etpinard fixed everything. Turned out all marker glyphs used in plotly are shifted downwards within font, so we can safely use char offset. The mentioned tests work alright except for point arrangement. How much is it of importance? |
Thanks @dfcreative ! I'll give this PR another look at some point today 🎉 |
@dfcreative fantastic. I confirm that the hover picking issue of #5 (comment) is fixed ✅ In addition, the new marker offset still generate a diff in the image test, but nothing to worry about at first glance 🎉 Unfortunately, the Moreover, when generating the new baseline one after the other, it some case the axes failed to be drawn e.g. for the reminiscing of https://github.com/plotly/streambed/issues/6307 |
@etpinard yes, sorry about colors, I've merged your PR to const Plotly = require('plotly.js');
let list = [
require('plotly.js/test/image/mocks/gl2d_12.json'),
require('plotly.js/test/image/mocks/gl2d_14.json'),
require('plotly.js/test/image/mocks/gl2d_17.json'),
require('plotly.js/test/image/mocks/gl2d_10.json')
]
list.forEach(mock => {
let div = document.body.appendChild(document.createElement('div'))
Plotly.plot(div, mock.data, mock.layout).then((div) => {
return Plotly.toImage(div).then(dataUrl => {
let img = document.body.appendChild(document.createElement('img'))
img.src = dataUrl
})
})
.then(() => {
Plotly.purge(div)
})
}) and it's fine |
No worries! I pulled down the looks like the point ordering is off. But all is good in my browser window. Strange.
Try: let list = [
require('plotly.js/test/image/mocks/gl2d_10.json'),
require('plotly.js/test/image/mocks/gl2d_12.json')
] In the test dashboard, I get: |
@dfcreative before I forgot, can you check that @monfera 's http://codepen.io/monfera/full/VmYeQb/ still works off this branch. I think that |
... and for more info on our high-precision specs, see plotly/plotly.js#1114 |
@etpinard ok, fixed sorting order for small point sets and optimized @monfera's UPD. increasing the delay here up to |
Actually mb it is better to create a separate repo, like gl-scatter2d-sdf? Changes are pretty significant, also loosing vectorizing logic completely is not too good. @etpinard |
I like this idea a lot. This will make comparing the two algorithms easier for future iterations.
For reference, could you elaborate a little more on the drawbacks of SDF? Is there anything else worth mentioning besides what's in limitations section above?
|
@etpinard done.
|
@dfcreative Amazing job. Here's how the baselines look like with your latest commits here and of plotly/plotly.js#1388: https://github.com/plotly/plotly.js/compare/scattergl-sdf Everything looks in place to me. But I wouldn't mind having someone else look at the new baselines to double check 👓 |
@dfcreative I agree, the new rendering does look better for symbols 👍 But for bubbles charts, the size of each circle has an important meaning, so the new rendering in its current form could be considered a breaking change by some. Would it be possible to some logic for |
@etpinard with gl-vis/gl-scatter2d-sdf@707658c I moved as close as I could to the initial fancy-scatter in size/border, but there are still tiny differences (1px-wise) due to method artefacts, it will be extremely difficult to remove them. |
off https://github.com/plotly/plotly.js/compare/gl-scatter2d-sdf is looking good 🎉 Can you publish the latest changes to I'll take care of getting this in plotly.js master. Awesome work! |
Done https://www.npmjs.com/package/gl-scatter2d-sdf. Thank you! |
SDF method to render fancy scatter. I reached
3e6
custom points.Changelog
Limitations
TODO
Some observations
scattergl
still renders up to1e8
points, whereas this patch is only1e6
points. That is due to size/borderWidth/char/color buffers, it is impossible to get rid of them. We can try to patchgl-scatter2d
to take color variations though. Or we can think of an alternate scatter plot with multiple render passes for different data groups. Although for big data we need special data structures, like wavelets, see ref.pixelRatio
was the reason)Possible optimizations
update
calculationsupdate
into texture, draw rendered texture in idle/pick mode