Discrete heatmapgl

[archmoj]

Initial PR to add zsmooth options to heatmapgl and resolve #4924.

@ordiology let's submit potential changes (as a separate PR) to this branch.

@plotly/plotly_js

[archmoj]

@ordiology nice work!
The only issue I noticed is that (when using low resolution input grid) the offsets at the corners are not identical and different from heatmap version.
Here is a codepen illstrating the problem.
When possible could you please fixing that on top of commits of this branch?

[ordiology]

Thanks for taking a look, @archmoij. I'll look into what's going on and get back to you. Looking at your codepen example and ignoring the strange offset for a moment, do you know why in the heatmapgl plot the tooltip lines up with the axis grid, but in the straight heatmap plot it doesn't?

[alexcjohnson]

There is some subtlety about tooltip positioning: if you don't use zsmooth, each data point is drawn as a brick of constant color, and the tooltip should be drawn at the edge of the brick - normally the right edge, but when hovering on a point on the far right, when the tooltip moves to the left, its anchor jumps to the left edge of the brick. But with smoothing enabled the tooltip should be drawn at the exact data point. I haven't tested heatmapgl but heatmap does this correctly, so we should aim for heatmapgl to match this behavior.

[ordiology]

That's interesting as I would expect it to work the other way. With heatmapgl, the interpolated heatmap has the data located on the edges of each brick with the interpolation occurring in between, whereas for the discretised heatmap, the whole brick contains the mapping for the data so the central location is where I intended the tooltip to point -- my version of heatmapgl offsets x and y by half a brick and draw an extra brick at the maximum location... This may be related to the offset issue we're observing, but I didn't have it when I tested my code inside plotly so I need to look into it further.

[ordiology]

Sorry for the delay, @archmoj -- had something I had to work on last week for Friday deadline. The issue should be fixed... please let me know if it doesn't behave as expected.

[archmoj]

Sorry for the delay, @archmoj -- had something I had to work on last week for Friday deadline. The issue should be fixed... please let me know if it doesn't behave as expected.

@ordiology no worries about this and thanks very much for the fix.
We still have few weeks to work on this feature before the next minor release.
The next step would be to open a PR from your fork to gl-vis.
gl-vis/gl-heatmap2d@master...ordiology:fix4924-discrete-heatmap2d
Thanks in advance.

[archmoj]

Also here is the updated codepen including latest fix.
@ordiology could you please investigate why the hover is not showing up on the very first row & column?

[ordiology]

Thanks @archmoj, the hover is not showing up due to the bounds that are set up for the chart in the plotly.js convert.js trace for gl-heatmap2d. The discrete webgl heatmap requires an extra patch that the interpolated heatmap doesn't need. This patch amounts to half a patch before the first data location and half a patch after the last data location. In my implementation, I increased the ppad for both x and y in fullTrace._extremes to adjust for this.

I have also made a PR for my fork to gl-vis.

Thank you so much for your support with getting this feature into release.

[archmoj]

@ordiology I merged your gl-vis PR 🥇 i.e. gl-vis/gl-heatmap2d#6 and published a gl-heatmap2d minor.
Could you please investigate hover positioning issue mentioned in #4953 (comment)?

[ordiology]

That's great! Thanks @archmoj.

As I tried to explain in the comment just above... The size of the discrete heatmap is one patch larger than the smoothed heatmap. The bounds now extend half a patch beyond the minima and maxima of the x and y data ranges. I resolved this in my implementation by increasing the padding (ppad) of fullTrace._extremes in the plotly trace convert.js for gl-heatmap2d. Please let me know if this doesn't make sense.

[archmoj]

@ordiology thanks for your work!
This option would be available in the next plotly.js minor.
💃

[ordiology]

Thank you so, so much, @archmoj!