Skip to content

Fix mouse event on high resolution screens#9

Open
kapythone wants to merge 2 commits intogl-vis:masterfrom
kapythone:closest-point
Open

Fix mouse event on high resolution screens#9
kapythone wants to merge 2 commits intogl-vis:masterfrom
kapythone:closest-point

Conversation

@kapythone
Copy link
Copy Markdown

It doesn't handle cases for high resolution screens such as Retina,

You can test it by running tests in plotly.js:

npm run test-jasmine -- gl_plot_interact

Comment thread mesh.js Outdated
var data = closestPoint(
simplex,
[pickData.coord[0], this._resolution[1]-pickData.coord[1]],
[window.devicePixelRatio * pickData.coord[0], this._resolution[1] - window.devicePixelRatio * pickData.coord[1]],
Copy link
Copy Markdown
Member

@rreusser rreusser Apr 24, 2017

Choose a reason for hiding this comment

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

I don't know the full structure of these libs, but is it possible or desirable to get the pixel ratio from the scene instead of the window? Seems it's possible to set a device pixel ratio that might differ from the window device pixel ratio: https://github.com/gl-vis/gl-plot3d/blob/e573384c1302f8c4358e85faf5293c74447dc25f/scene.js#L150

@etpinard
Copy link
Copy Markdown
Member

@dfcreative what do you think about think?

@kapythone
Copy link
Copy Markdown
Author

I checked the relevant codes again and found it's more complicated than I think.

First, pickBuffer was initiated with ViewShape (whose size is in the unit of physical pixels):
https://github.com/gl-vis/gl-plot3d/blob/master/scene.js#L241

Then it is somehow resized to the CSS pixel (which could represent more than one physical pixel for high DPI screens):
https://github.com/gl-vis/gl-plot3d/blob/master/scene.js#L442
I don't quite understand the logic here, since the buffer is still initiated with previous physical pixels.

In addition, the objects including this mesh seem to be attached an attribute pixelRatio:
https://github.com/gl-vis/gl-plot3d/blob/master/scene.js#L491

I don't think if we can just use that variable in mesh.js, because it is only initiated in:
https://github.com/gl-vis/gl-plot3d/blob/master/scene.js#L491

As far as I know, we should convert the position of x and y, not using another pickShape, the commits related to pickShape such as this one are not necessary:

gl-vis/gl-plot3d@b5c06ad

Let me know if I misunderstand anything, thanks!

FYI, these are some links might be helpful:

w3c/pointerevents#602

https://www.khronos.org/webgl/wiki/HandlingHighDPI

@dy
Copy link
Copy Markdown
Member

dy commented Apr 24, 2017

@etpinard I think we have to address this issue with a test case, as it seems more complicated than just passing pixelRatio from options.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants