Fix scatter fill with isolated endpoints

[rreusser]

See: #1912

This PR fixes scatter fill behavior in which single-point segments cause d3 errors and undesirable visual appearance when they're either the first or last line segments. It fixes it by simply drawing them as empty paths instead of cutting them out entirely.

The good: current behavior for isolated points is unaffected unless they're the first or last point

The bad: single points do affect the fill if they're the first or last segment but do not if they're interior points, which is a bit inconsistent.

Question: Should the fill go through isolated interior points? It's easy to accomplish but may change current instead of simply fixing endpoint cases.

Example:

Input:

"data": [ 
  {"x": [1.5], "y": [1.75], "fill": "tozeroy"},
  {"x": [0, 1, 2, 3], "y": [1.5, null, 1.5, 1.5], "fill": "tozeroy"},
  {"x": [0, 1, 1.5, 2, 2.5, 3], "y": [1.25, 1.25, null, 1.35, null, 1.25, 1.25], "fill": "tozeroy"},
  {"x": [0, 1, 2, 3], "y": [1.0, 1.1, null, 1.0], "fill": "tozeroy"},
  {"x": [0, 1.5, 3], "y": [0.75, null, 0.75], "fill": "tozeroy"}
]

Before:

After:

[rreusser]

I've been thinking about this a bit more. There are a few options here, I guess. Perhaps the fill for isolated first/last segments should be drawn as actual isolated points that don't create any filled area. My solution here was that the first and last segments always box out some area, even if they are isolated, while interior isolated points do not. That's the inconsistency that bothers me just a bit. The bottom line though is that the current behavior just leads to errors, so I'm trying to decide whether it's worthwhile to aim for a slightly more nuanced fix or whether this solution solves it reasonably enough. Feedback appreciated.

[alexcjohnson]

Lets just call it a bug in the interior case as well - if we consider this behavior to be as intended:

Plotly.newPlot(gd,[{y: [1, 3, null, 2, 4, null, 3, 5], fill: 'tozeroy'}], {width: 400, height: 400})

Then it makes little sense that dropping the middle segment to 1 point would disconnect it from the fill as it does now:

Plotly.newPlot(gd,[{y: [1, 3, null, 2, null, 3, 5], fill: 'tozeroy'}], {width: 400, height: 400})

It should really look like:

[etpinard]

@rreusser @alexcjohnson not sure if you are aware of #1205, it might be related.

[alexcjohnson]

#1205 is an independent, and trickier, issue - I'll add a comment over there.

[rreusser]

I've pushed a modified version that simply removes lineSegments entirely rather than just using it differently. That is, empty lines segments are drawn as empty line segments instead of getting culled.

This changes my above baseline so that the single points do connect with the fill:

[rreusser]

@alexcjohnson with the most recent pushed code, your example looks like this:

{
  "y": [1, 3, null, 2, null, 3, 5],
  "fill": "tozeroy"
}

[alexcjohnson]

@rreusser looks great! To play devil's advocate, can you include a test with some other line.shape values and fill: 'tonexty' between traces that both include isolated end and mid points? I don't think it will apply the line shape to the connections across null values - and that seems fine to me - but I do want to make sure that the lines and fills all still follow each other.

[rreusser]

Good call, @alexcjohnson. There was a bug. It was considering single points to be a closed spline since the last point is (trivially but not meaningfully) the same as the first. I've modified the scatter trace not to try to draw single points as closed curves. I could make this change to the drawing routine itself, but I don't have strong feelings since I don't know the deep implications here.

Note that it connects spline gaps with straight lines so that for these cases with no more than two points per segment, the appearance is the same:

[rreusser]

Oops. Also adding tonexty test.

[alexcjohnson]

To put my mind at ease, can you also either make some of these segments 3+ points so spline is visibly different from straight, and/or change to one of the stepped shapes (ideally 'hv' or 'vh' so it's asymmetric)?

[rreusser]

@alexcjohnson it now looks like this. Glad to make more, but should probably split it into new mocks if so. (realizes a couple of these are redundant…)

[alexcjohnson]

That mock is thing of beauty 😻

💃