Contour lines are rendered incorrectly when closed loops #19568

The C++ code behind contour and tricontour returns a list of lines, each line being a numpy array of shape (?, 2) and is either a closed line loop (if the first and last points are identical) or an open line strip. Line strips are rendered correctly, but line loops are not. Simple example:

Here the line strip is fine, the line loop is not because they are rendered using a LineCollection which is a collection of Line2D that do not understand the closed-ness of line loops. The example is slightly artificial because of the large linewidth; at normal line widths it looks OK but there is a pixel or two which is not quite correct. If you draw the same contour/Line2D but starting at a different index the output isn't quite the same. If you tinker with the contouring algorithm (which I am doing for other reasons) test images are annoyingly slightly different.

I think this has been a problem for many years, just not a particularly important problem. There are a few complaints about it on SO that haven't made it to this issue tracker. I have a solution in mind but rather than just write a PR I'd like to explain the alternatives and see what other devs think in case there is some sort of trap waiting for me here.

If we cheat and put round end caps on the lines, the line loop is correct. But then the line strip also has semicircles on the end and hence stick out too far, i.e. they won't align with a contourf plot of the same data. We could use 2 LineCollections, one for line loops with the cheat and one for line strips without. This would be easy to do but I think it messes up legends as we'd have 2 artists representing one contour set. We could write a CompoundArtist class to wrap the two, but that is expanding the scope of the fix to potentially break lots of things.

What about other Collections? Here are the possibilities:

PolyCollection understands closed-ness, but applies it to all of its polygons. Hence we'd need two of them, so rejected for the same reasons as LineCollection above. The other 3 options (PathCollection and PatchCollection of either Polygons or PathPatches) give us what we want. These all work as they are essentially collections of individual objects (Paths or Polygonss) and you can specify closed=True or not on them individually.

The simplest of the 3 is PathCollection. The fix is to replace the existing code, approximately

coll = mcollections.LineCollection(list_of_lines, kwargs...)
ax.add_collection(coll)

with something like

paths = [mpath.Path(line, closed=is_it_closed_or_not) for line in list_of_lines]
coll = mcoll.PathCollection(paths, kwargs...)
ax.add_collection(coll)

which is surprisingly easy.

Downsides?

1. Churn in test images for contour and tricontour. Although it should never have to happen again (!!!)
2. We are explicitly creating potentially a lot of objects, one Path for each contour line. I don't fully understand the impact of this. Looking at the backend code, which I am not an expert in, it seems that all of these Artists end up calling draw_path or draw_path_collection, so everything ends up being a Path in the end, with arrays of vertices and 'kind' codes so maybe it isn't a great impact.

There is a more complicated alternative. The C++ code could be modified to return two lists, one containing the line strips and one the line loops. Then only 2 Paths would be needed, one with closed=False and one with closed=True. We'd have to explicitly create the 'kind' code arrays rather than them being created automatically in the examples above, but that isn't difficult.