- fix issue scattergl and fill to zero and undefined value #2965
Conversation
|
Thanks for the PR (and demonstration of the issue) @ErwanMAS ! Looks like a good solution to me, just two things I'd like to add to it:
|
| } | ||
| var lastdef = srcPos.length - 1; | ||
| while(isNaN(srcPos[lastdef])) { | ||
| lastdef += -2; |
| lastdef += -2; | ||
| } | ||
| pos = [ srcPos[firstdef - 1], 0 ]; | ||
| pos = pos.concat(srcPos.slice(firstdef - 1, lastdef + 1)); |
There was a problem hiding this comment.
🐎 so we don't need to slice unnecessarily, can we keep track of whether there actually were invalid points on either end?
| } | ||
| pos = [ srcPos[firstdef - 1], 0 ]; | ||
| pos = pos.concat(srcPos.slice(firstdef - 1, lastdef + 1)); | ||
| pos = pos.concat([ srcPos[lastdef - 1], 0 ]); |
There was a problem hiding this comment.
I notice the existing code didn't take advantage of this, but push is variadic:
pos.push(srcPos[lastdef - 1], 0);
| pos.push(srcPos[srcPos.length - 2]); | ||
| pos.push(0); | ||
| var firstdef = 1; | ||
| while(isNaN(srcPos[firstdef])) { |
There was a problem hiding this comment.
oh hmm - this checks for invalid y, what about invalid x?
| @@ -0,0 +1,52 @@ | |||
| { | |||
| "data": [ | |||
There was a problem hiding this comment.
Would you mind renaming this mock gl2d_fill_trace.json? We like to prefix gl-based mocks by either gl2d_ or gl3d_.
Would should probably write down in the CONTRIBUTING file at some point.
There was a problem hiding this comment.
Good point for this PR - but rather than describing this in CONTRIBUTING.md how about we make the image test system support subfolders and give them a little more organization? It's getting impossible to find what you want in mocks and baselines...
|
i create a new pull request [#2979] |
Example of the issue => https://codepen.io/erwanmas/pen/aaJXLY
When we add the first point and the last point , we must be sure that the first/last point does not have a NaN value .