Contour bug #1309
Contour bug #1309
Conversation
this would only happen if its line rounded away to nothing, because it only crossed the boundary an infinitesimal amount
this avoids an edge case where the colorbar and the colorscale got out of sync or did not end at the same place
| @@ -52,7 +53,7 @@ module.exports = function colorbar(gd, cd) { | |||
| }) | |||
| .levels({ | |||
| start: contours.start, | |||
| end: contours.end, | |||
| end: endPlus(contours), | |||
There was a problem hiding this comment.
colorbar actually adds some more to end for the same purpose - but it turns out not to matter in this case, additional expansion doesn't have any ill effects that I could find, as long as it always ends up at or beyond the level we used to make the colorscale. Sometime though we may want to fold ALL the places we extend a range a little for rounding purposes into the same endPlus mold.
|
Gah, good thing our test cases include yet another edge case... more work to do here. |
these edge cases literally involve strange things at the edges
|
@rreusser the test image I added here shows some contour crossings - well beyond the scope here to clean those up, this is just to fix the overall topology, but when we get around to making a better smoothing algorithm, these will be good ones to watch and fix. There are also cases where a contour crosses itself, which are not included here but I can pull some up when we get to it. |
| (marchStep[1] && (loc[1] < 0 || loc[1] > m - 2))))) { | ||
| break; | ||
| } | ||
| if((closedLoop) || (edgeflag && atEdge)) break; |
There was a problem hiding this comment.
I could revert this change actually, as I don't need atEdge separately anymore. It's way more readable now, but a bit faster before as it could short-circuit some of the logic...
There was a problem hiding this comment.
Your call here. I'm fine with it either way.
There was a problem hiding this comment.
Actually, if we're worried about perf, I should get rid of all those str = pair.join(',') statements - they make the comparison code shorter, but it's way faster to just compare integers twice. Since marchStep.join(',') is in the first clause it would save almost nothing to revert.
#1311
|
💃 |
@etpinard @chriddyp fixes https://github.com/plotly/streambed/issues/4236 and another edge case I found while investigating it.