Skip to content

plotGlPixelRatio has no effect on scattergl #5497

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
archmoj opened this issue Feb 12, 2021 · 4 comments · Fixed by #5500
Closed

plotGlPixelRatio has no effect on scattergl #5497

archmoj opened this issue Feb 12, 2021 · 4 comments · Fixed by #5500
Labels
bug something broken

Comments

@archmoj
Copy link
Contributor

archmoj commented Feb 12, 2021

demo

@jonmmease
Copy link
Contributor

Looking at https://www.khronos.org/webgl/wiki/HandlingHighDPI, I wonder if we need to use a larger canvas size here:

fullLayout._glcanvas
.attr('width', fullLayout.width)
.attr('height', fullLayout.height);

This doesn't work on its own, but something like:

            fullLayout._glcanvas
                .attr('width', fullLayout.width * config.plotGlPixelRatio)
                .attr('height', fullLayout.height * config.plotGlPixelRatio)
                .style('width', fullLayout.width)
                .style('height', fullLayout.height);

In this case, I think we would not want to use a constant marker size (#5093).

But I don't understand this very well. And not sure why it wouldn't be an issue with gl3d plots.

@jonmmease
Copy link
Contributor

Looks like this is what scatter3d is doing:

https://github.com/gl-vis/gl-plot3d/blob/0ae1280e1d366fcf96419b6acd85c194ee1f43a6/scene.js#L253-L266

    var nextWidth  = Math.ceil(width  * scene.pixelRatio)|0
    var nextHeight = Math.ceil(height * scene.pixelRatio)|0
    if(nextWidth !== canvas.width || nextHeight !== canvas.height) {
      canvas.width   = nextWidth
      canvas.height  = nextHeight
      var style = canvas.style
      style.position = style.position || 'absolute'
      style.left     = '0px'
      style.top      = '0px'
      style.width    = width  + 'px'
      style.height   = height + 'px'
      dirty = true
    }

@archmoj
Copy link
Contributor Author

archmoj commented Feb 12, 2021

Looking at https://www.khronos.org/webgl/wiki/HandlingHighDPI, I wonder if we need to use a larger canvas size here:

fullLayout._glcanvas
.attr('width', fullLayout.width)
.attr('height', fullLayout.height);

This doesn't work on its own, but something like:

            fullLayout._glcanvas
                .attr('width', fullLayout.width * config.plotGlPixelRatio)
                .attr('height', fullLayout.height * config.plotGlPixelRatio)
                .style('width', fullLayout.width)
                .style('height', fullLayout.height);

In this case, I think we would not want to use a constant marker size (#5093).

But I don't understand this very well. And not sure why it wouldn't be an issue with gl3d plots.

In that case perhaps we also need to patch reg-line2d and some other modules to take into account pixelRatio.

@archmoj archmoj added the bug something broken label Feb 12, 2021
@jonmmease
Copy link
Contributor

jonmmease commented Feb 12, 2021

I think I have a solution... Will open WIP PR in a bit.

Edit: only needs plotly.js updates

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
2 participants