Fix rendering of WebGL traces on latest Chrome-128 and Edge-128 browsers
#7131
Conversation
| @@ -24,7 +24,7 @@ | |||
| "gl-pointcloud2d": "^1.0.3", | |||
| "gl-scatter3d": "^1.4.1", | |||
| "gl-select-box": "^1.0.4", | |||
| "gl-shader": "4.3.1", | |||
| "gl-shader": "github:archmoj/gl-shader#0409f68f8005ef3a3f3fccb6121bc7f0a5fda8bb", | |||
There was a problem hiding this comment.
| function ensureFinite(arr) { | ||
| return arr.map(function(v) { | ||
| return ( | ||
| isNaN(v) ? 0 : |
There was a problem hiding this comment.
Why does this one exclude NaN values? Doesn’t that pose a risk that missing data will be filled in spuriously?
There was a problem hiding this comment.
We didn't encounter that case in our tests.
There was a problem hiding this comment.
Do we have that case (missing data, particularly with lines but also points and surfaces) in our image tests? This is stackgl so I guess my question is scoped to the 3D trace types.
But also, this is different from what you did in parcoords, why?
There was a problem hiding this comment.
In stackgl jasmine tests I encountered a case with both infinity and NaN. So I had to guard against both.
There was a problem hiding this comment.
I still want to understand:
- Why are there differences between the stackgl and parcoords fixes? "because only one has a test that cares about NaN" isn’t enough. Have you tried to find a way to push a NaN into the other one? Does it cause problems?
- Is there any chance that setting NaN to zero will make missing data suddenly appear? This I would consider a VERY bad bug, so it’s worth looking into carefully. Is there any way you can find to pass in an array containing some null data mixed in with valid data, or a typed array with NaN mixed in, that results in a point appearing where there should be none? If this code is processing the x/y/z arrays for scatter3D for example, I could imagine this happening due to NaN at the beginning, middle, or end of one of the arrays; NaN in all three at once; or different length arrays; all of these coming from either plain arrays or typed arrays. We could enumerate similar sets of conditions for the other trace types, and ideally we should have these conditions in test images so we have confidence the behavior is stable over time. These wouldn't need to all be different images, the same trace can have many incomplete or totally missing data points. And perhaps we already have some of these conditions represented in our test images - if so, fantastic, just point them out to me and we'll be good to go.
There was a problem hiding this comment.
The case of NaN didn't happen in the parcoords cases.
But it was present in a mesh3d case.
I start thinking perhaps NaNs could/should be avoided in one layer higher in the stackgl case.
Does that make sense?
There was a problem hiding this comment.
@archmoj @alexcjohnson is this a must-have for our next minor release?
There was a problem hiding this comment.
Given that several trace types seem fully broken on latest Chrome (and Edge, per plotly/plotly.py#4729, not surprising as it’s built on Chrome) I certainly think so. I don’t particularly care how the fix is done but I do want us to test thoroughly that the fix can’t generate incorrect data points on a graph
|
@alexcjohnson In the case of stackgl these NaN conditions appear to occur for the 3D model matrix when the trace has empty arrays e.g. Plotly.newPlot(gd, [{
x: [],
y: [],
z: [],
type: 'scatter3d'
}])Or when they are turned off: Plotly.newPlot(gd, [{
visible: 'legendonly',
x: [1],
y: [2],
z: [3],
type: 'scatter3d'
}])Whereas other applications namely ArcGIS also encountered this exact issue: https://community.esri.com/t5/arcgis-online-questions/map-error-in-new-chrome-update-128-0-6613-85/td-p/1525829 I hope this temporary fix is good enough to be included in the upcoming minor to bring back these traces. |
|
Good news: I found this bug as labeled P1 on |
|
@alexcjohnson @gvwilson |
|
15 min ago - some people says things are working. Is there maybe a patch out already? 
My browser just now updated from: |
|
This is now fixed by Chromium I was able to update my Chrome. |
|
👍 |
Fixes #7130.
@plotly/plotly_js
cc: @birkskyum