Skip to content

Preserve marker size in scattergl traces with different glPixelRatio values #3637

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

Closed
wants to merge 4 commits into from

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Mar 14, 2019

Fix #3246 for scattergl trace.
Before codepen.
After codepen.

@archmoj archmoj added bug something broken status: reviewable labels Mar 14, 2019
package.json Outdated
@@ -104,7 +104,7 @@
"regl": "^1.3.11",
"regl-error2d": "^2.0.6",
"regl-line2d": "3.0.13",
"regl-scatter2d": "^3.1.3",
"regl-scatter2d": "git://github.com/gl-vis/regl-scatter2d.git#6a8f3576467f41bc2f8d6a4e78c646e933532439",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@antoinerg
Copy link
Contributor

Thanks @archmoj for looking into this issue! This might give a new life to my old Orca PR #3237 😄

Can you explain why we're completely dropping glPixelRatio from the regl-scatter2d module? I understand it wasn't working properly before but isn't it supposed to do something like control the resolution/sharpness of markers? @alexcjohnson was hinting at this in #3246 (comment) . I'm not really familiar with the 3D modules so it'd be nice if you could shed light upon this for me and tell me if glPixelRatio should be added in the future or not!

@archmoj
Copy link
Contributor Author

archmoj commented Mar 15, 2019

We don't completely drop glPixelRatio from regl-scatter2d and one could find the variable all over the place. But it looks that it was applied twice in the sizes. And since the plotGlPixelRatio is generally set to 2.0 on plotly.js it is replaced by 2.0 in the vertex shader programs so that the graphs may remain unchanged.

@antoinerg
Copy link
Contributor

We don't completely drop glPixelRatio from regl-scatter2d and one could find the variable all over the place. But it looks that it was applied twice in the sizes. And since the plotGlPixelRatio is generally set to 2.0 on plotly.js it is replaced by 2.0 in the vertex shader programs so that the graphs may remain unchanged.

I was misled by the bad grep command I was using. Sorry about that! I now see we're still using glPixelRatio 👍 ! I will let @etpinard review this one. Thank you again 😄

@etpinard
Copy link
Contributor

etpinard commented Mar 18, 2019

Cool. Thanks @archmoj

Can you turn https://github.com/gl-vis/regl-scatter2d/compare/fix3246-glPixelRatio-change-marker-size into a PR and ping @dy ?

Tagging this under 1.46.0

@etpinard
Copy link
Contributor

Dropping from 1.46.0

@etpinard etpinard removed this from the v1.46.0 milestone Mar 27, 2019
@archmoj
Copy link
Contributor Author

archmoj commented Aug 21, 2020

It was better to redo the PR in #5093 than fixing these conflicts.
Closing.

@archmoj archmoj closed this Aug 21, 2020
@archmoj archmoj deleted the fix3246-glPixelRatio-change-marker-size branch August 21, 2020 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

config option plotGlPixelRatio changes size of markers
3 participants