Maintaining marker size with various pixelRatio values #20

archmoj · 2019-03-18T14:40:28Z

Added new option constPointSize to help maintain marker sizes with different pixelRatio values.

Fix for issue plotly/plotly.js#3246.
Kindly visit PR plotly/plotly.js#3637 for more information.
@dy
cc: @etpinard

dy · 2019-03-18T15:56:45Z

marker-vert.glsl

@@ -38,7 +38,7 @@ void main() {
  float size = size * maxSize / 255.;
  float borderSize = borderSize * maxSize / 255.;

-  gl_PointSize = 2. * size * pixelRatio;
+  gl_PointSize = 2. * size * 2.;


Why not fixing pixel ratio from uniforms side?

Thanks @dy for looking at this.
It seems to be used in one other place in the shader:

regl-scatter2d/marker-vert.glsl

Line 42 in 6a8f357

fragPointSize = size * pixelRatio;

.

But fragPointSize is different from gl_PointSize. Can you possibly add a test-case for the issue?

Why not fixing pixel ratio from uniforms side?

pixelRatio could be set as a constant of 2 here:

regl-scatter2d/scatter.js

Line 84 in 091a1a7

pixelRatio: regl.context('pixelRatio'),

; but I thought it may be more consistent e.g. with other gl-vis/3d modules where other pixelRatio values are possible.

That should be done as a separate option if you want to make some default value. Here https://github.com/gl-vis/regl-scatter2d/blob/master/scatter.js#L199

Not sure I understand. Why not avoiding passing plotGlPixelRatio as pixelRatio at all?
The cases mentioned in the codepen make sure the point size is consisted in devices with various pixelRatio. If you want to pass "fake" pixelRatio - the point size will be increased, that's what expected. Am I getting it wrong?

In Plotly.js plotGlPixelRatio is the same as pixelRatio. Here is another codepen example comparing scatter, scattergl & scatter3d with different pixelRatio values lines and markers. The only module which treat pixelRatio differently seems to be this module. When passing different pixelRatio values we don't expect a change in point sizes but slightly higher render quality.

Have you considered that on the devices with another default pixelRatio? The point size is likely smaller there, other gl- components don’t consider that.

The codepens on different devices appear to be rendered OK with this fix.
@etpinard what's your thoughts here.
Should we consider another fix something like this? https://github.com/plotly/plotly.js/compare/markersize-second-approach

Thanks for your hard work @archmoj - your patch appears to make scattergl consistent with how markers are drawn in scatter3d so 👌

As this repo remains in @dy 's hands, we could perhaps make this new/consistent pixelRatio behaviour an opt-in e.g. by adding a boolean flag to regl-scatter2d that we would turn on upon instantiating in plotly.js

dy · 2019-03-18T16:31:24Z

marker-vert.glsl

@@ -38,7 +38,7 @@ void main() {
  float size = size * maxSize / 255.;
  float borderSize = borderSize * maxSize / 255.;

-  gl_PointSize = 2. * size * pixelRatio;
+  gl_PointSize = 2. * size * 2.;


But fragPointSize is different from gl_PointSize. Can you possibly add a test-case for the issue?

…f pixelRatio 2

archmoj · 2019-04-01T22:35:04Z

@dy @etpinard OK. The tests are tweaked with half sizes and passed noting that we should maintain sizes identical to (the old use of) pixelRatio=2 in plotly.js baselines. If all is good we may publish a minor and include these changes in the next plotly.js minor.

archmoj · 2019-04-05T14:58:36Z

@dy Possibly any updates on this?
If you still think that this is not the right fix, please let me know. Maybe we could find another way to tweak the API on the plotly.js side?
Thank you in advance.

dy · 2019-04-08T19:21:12Z

@archmoj I did not mean to delay, sorry. Yes, I think that now pixelRatio is handled correctly - it scales points up depending on the device or passed pixelRatio. If we remove that support, the regl-scatter2d will render points with 1:1 pixels binding, which makes pixelRatio option useless.
If you want to fix pixelRatio to some value, you can do that externally, that option is exactly for that.

archmoj · 2019-04-08T19:59:15Z

@dy Thank you very much for the follow ups.
In fact we prefer not to pass a constant value there.
Here is another codepen comparing pixelRatio values equal to 1 and 2 that uses current regl-line2d & regl-scatter2d modules. The line widths are identical but the marker sizes are not.

dy · 2019-04-11T01:04:37Z

Yep, the problem is on the regl-line2d side. It just doesn't account for pixelRatio. regl-scatter2d works as intended.

revert sizes in tests

archmoj · 2019-04-11T17:50:15Z

Thanks for your hard work @archmoj - your patch appears to make scattergl consistent with how markers are drawn in scatter3d so

As this repo remains in @dy 's hands, we could perhaps make this new/consistent pixelRatio behaviour an opt-in e.g. by adding a boolean flag to regl-scatter2d that we would turn on upon instantiating in plotly.js

Great idea @etpinard.
Applied in 9c0933a.

@dy this should be good to go as tested on plotly side as well as here without any changes to the tests.
Would you mind reviewing it again?
Thanks in advance.

dy · 2019-04-14T19:15:54Z

Gentlemen, I can't understand - why passing plotly's pixelRatio at all, if you need that constant? Just initialize regl-scatter2d with {pixelRatio: 2} from any trace and we're good to go, it will behave the same way as the other gl components. It is also shorter than {pixelRatio: plotlyPixelRatio, constantPixelRatio: true}.

etpinard · 2019-04-15T15:48:22Z

, I can't understand - why passing plotly's pixelRatio at all, if you need that constant?

We offer that option to users on static exports (example) - which can make a appreciable difference in the quality of the exported images.

dy · 2019-04-16T10:49:33Z

@etpinard that's great option, but no need to pass it to regl-scatter, pass {pixelRatio: constant} instead and that's it. Eg. here
https://github.com/plotly/plotly.js/blob/225521271f2eb7f5803713a28d8a7e0ef16ccc57/src/traces/scattergl/index.js#L116
just add an extra-line

// fix pixel ratio to accommodate other gl-components api
opts.marker.pixelRatio = 2.

archmoj · 2019-04-16T18:55:47Z

@dy Thanks for the suggestion.
Unfortunately it does not seem to help fix the issue : (
Here is a codepen based on the fix you suggested.
Please notice how the markers are getting bigger with greater pixelRatio values.

…o-change-marker-size

archmoj added 2 commits March 14, 2019 18:47

do not scale markers and circles using pixelRatio

0fcb2a5

update bundle

6a8f357

dy reviewed Mar 18, 2019

View reviewed changes

dy requested changes Mar 18, 2019

View reviewed changes

archmoj added 3 commits March 24, 2019 20:11

Merge branch 'master' into fix3246-glPixelRatio-change-marker-size

b5f0cf5

update bundle

1899089

adjust tests to maintain images and perform similar to previous use o…

8b781ac

…f pixelRatio 2

archmoj mentioned this pull request Apr 10, 2019

reproducible orca running image tests plotly/plotly.js#3237

Closed

10 tasks

added constPointSize flag

9c0933a

revert sizes in tests

archmoj requested a review from dy April 12, 2019 14:13

archmoj added 3 commits August 21, 2020 07:25

Merge remote-tracking branch 'origin/master' into fix3246-glPixelRati…

df5b691

…o-change-marker-size

remove package-lock

fd1db24

update bundle

758b06f

archmoj merged commit d9784d9 into master Aug 21, 2020

archmoj deleted the fix3246-glPixelRatio-change-marker-size branch August 21, 2020 11:42

archmoj mentioned this pull request Aug 21, 2020

Keep the size of gl2d markers constant when using different plotGlPixelRatio values plotly/plotly.js#5093

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Maintaining marker size with various pixelRatio values #20

Maintaining marker size with various pixelRatio values #20

archmoj commented Mar 18, 2019 •

edited

Loading

dy Mar 18, 2019

archmoj Mar 18, 2019

dy Mar 18, 2019

archmoj Mar 18, 2019

dy Mar 19, 2019

dy Apr 6, 2019

archmoj Apr 6, 2019

dy Apr 6, 2019

archmoj Apr 8, 2019

etpinard Apr 10, 2019 •

edited

Loading

dy Mar 18, 2019

archmoj commented Apr 1, 2019

archmoj commented Apr 5, 2019

dy commented Apr 8, 2019

archmoj commented Apr 8, 2019 •

edited

Loading

dy commented Apr 11, 2019

archmoj commented Apr 11, 2019

dy commented Apr 14, 2019

etpinard commented Apr 15, 2019

dy commented Apr 16, 2019

archmoj commented Apr 16, 2019

Maintaining marker size with various pixelRatio values #20

Maintaining marker size with various pixelRatio values #20

Conversation

archmoj commented Mar 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard Apr 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

archmoj commented Apr 1, 2019

archmoj commented Apr 5, 2019

dy commented Apr 8, 2019

archmoj commented Apr 8, 2019 • edited Loading

dy commented Apr 11, 2019

archmoj commented Apr 11, 2019

dy commented Apr 14, 2019

etpinard commented Apr 15, 2019

dy commented Apr 16, 2019

archmoj commented Apr 16, 2019

archmoj commented Mar 18, 2019 •

edited

Loading

etpinard Apr 10, 2019 •

edited

Loading

archmoj commented Apr 8, 2019 •

edited

Loading