plotly / plotly.js

Open-source JavaScript charting library behind Plotly and Dash
https://plotly.com/javascript/
MIT License
17.14k stars 1.87k forks source link

plotGlPixelRatio has no effect on scattergl #5497

Closed archmoj closed 3 years ago

archmoj commented 3 years ago

demo

jonmmease commented 3 years ago

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

https://github.com/plotly/plotly.js/blob/7347bedc7f90b1fee91f45cb41fd9bde246033db/src/plot_api/plot_api.js#L225-L227

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 (https://github.com/plotly/plotly.js/pull/5093).

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

jonmmease commented 3 years ago

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 commented 3 years ago

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

https://github.com/plotly/plotly.js/blob/7347bedc7f90b1fee91f45cb41fd9bde246033db/src/plot_api/plot_api.js#L225-L227

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.

jonmmease commented 3 years ago

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

Edit: only needs plotly.js updates