Support multiple marker types in WebGL scatter

[ianthomas23]

Previously if you used the WebGL backend and called scatter with more than one marker type, e.g.

p = figure(output_backend="webgl", ...)
p.scatter(marker=["circle", "square"], ...)

then rendering would drop back to using canvas. This PR addresses this, fixing item 4 of discussion #11369.

Changes:

1. MarkerGL used to accept the marker_type in its constructor. This is no longer required.
2. MarkerGL identifies the unique marker types it needs to render, and calls the ReGL rendering function once for each type. All but one array/buffer is common in these calls so the data transfer to GPU is minimal, only the show array/buffer is different each time.
3. ScatterView no longer checks for the type of marker to change the underlying glglyph, it keeps the same glglyph.
4. ScatterView assumes that MarkerGL can render all marker types, so I have removed the is_supported function. If a new marker type is added for canvas it must also be added for WebGL. This may need discussion.
5. I have added a new visual test containing multiple marker types in a single scatter call. This has always worked of course, but previously the WebGL rendering dropped back to canvas. Image before this PR:
   
   and after
   
   The WebGL-trained eyeball will note that in the before image the webgl output is the same as canvas, but it is different in the after image as the antialiasing is different for webgl: the cross is darker and the dots are lighter for example. Any future regression of webgl back to canvas will be caught by this test.

I have kept the handling of both circle and scatter markers in MarkerGL even though it is a bit clunky (it always was, and dates back to "before my time" here). I intend to refactor this when I combine circle, scatter and rect into a single united implementation, presumably through a more sensible class hierarchy in the webgl code.

[ianthomas23]

Argh, I forgot to set the labels yet again 😲

[bryevdv]

@ianthomas23 is this ready ?

[mattpap]

@ianthomas23, does this approach preserve z-order of consecutive data points?

[bryevdv]

@ianthomas23, does this approach preserve z-order of consecutive data points?

I don't see how it could, webgl naturally wants to draw the same things in batches. Assuming that is so, I am definitely in favor of simply documenting that trade-off and letting users make the decision that fits their needs accordingly.

[ianthomas23]

@ianthomas23, does this approach preserve z-order of consecutive data points?

Not always.

I don't see how it could, webgl naturally wants to draw the same things in batches.

Correct.

The marker WebGL shaders can only render a single marker type at a time. The rendering of a scatter containing multiple marker types will call the WebGL renderer once for each unique marker type, and the order of these is the same as the order that the marker types are first mentioned in the marker property. So if each marker type is contiguous in the marker property then the rendering will have the same order as in canvas. But if a marker type is not contiguous, e.g. marker=["star", "circle", "star"] then the order will be different as all stars will be rendered before all circles.

It doesn't have to be this way. It would not be difficult to rewrite the code to render each group of contiguous marker types in order. But it is easy to invent some pathological cases for which the performance would be atrocious, e.g. a single scatter call of 100,000 markers which alternate between "star" and "circle" would require 100,000 ReGL rendering calls with this approach rather than the 2 rendering calls of this PR.

[bryevdv]

@ianthomas23 can you add a note to the scatter and Scatter docs explaining that the draw order may vary? I don't think we need to go into details of why, just enough to let users know this is an explicit trade-off that exists for the better performance of webgl. cc @tcmetzger for any docs input

[ianthomas23]

@ianthomas23 can you add a note to the scatter and Scatter docs explaining that the draw order may vary? I don't think we need to go into details of why, just enough to let users know this is an explicit trade-off that exists for the better performance of webgl. cc @tcmetzger for any docs input

Yes, I will do that tomorrow.

[mattpap]

I would prefer all backends to have principally the same semantics, so if we allow grouped renderer of scatter markers in webgl, then we should allow that in canvas and svg as well. Though this is a topic for a broader discussion; for the time being, this PR is fine.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.