Implement draw_markers in the cairo backend. - #2370

Thanks for submitting this. I actually took a crack at something similar in cairo in another project. The problem with this approach is that it creates very long paths which can ultimately be even slower than just drawing one at a time.

The magic recipe I arrived at was to create a "similar surface", draw the stamp to that, and the stamp it multiple times. On a raster backend, this boils down to a fast blit operation. On a vector backend, it turns into an XObject that is "used" multiple times.

This is the gist of what I mean (untested pseudocode):

        surface = self.cr.get_target().create_similar(...)
        subctx = cairo.Context(surface)
        # Draw the stamp to subctx
        surface = subctx.get_target()

        try:
            ctx.set_source_surface(
                surface, x, y)
            ctx.paint()
        finally:
            ctx.restore()

Hm, if really long paths are an issue, then we could split it up eg. every 1000 marks. But seriously, if it is an issue, we should probably just head over to the cairo people with a small testcase and get them to fix it upstream.
I don't think it should ever slow down with a lot of separate paths (with one large one I understand that calculating intersections and stuff is not linear with path length).

Using a similar surface has the issue that subpixel offsets are not correctly handled, which means the image would change in appearance and will probably look fuzzy. If some hinting/pixel snapping is done, then using a subsurface and only blitting that would be my preferred solution.

OK, thought about it some more, and you are right that very long paths will probably slow down at some point.

That said, I just tried with ~28000 points (had to remove that broken assert in draw_path), and things work fine. Not fast, but considering the amount of vertices it seems OK. Batching it up into 1000 points at a time seems slightly faster in a quick trail.

The Agg backend currently rounds to pixel centers when it draws the markers and then blits -- so it's at least a "proven" approach in that context -- but I agree it would change the look, which some may prefer (maybe a config option)? We should also test that large paths don't mess up PDF or SVG files in the standard tools for viewing (poppler, Acrobat Reader for PDF; Chrome, Firefox, Inkscape for SVG) -- though if we're chunking them in 1000 points anyway, it's probably fine.

Also, the fact that filling requires each marker to be drawn individually -- I suspect that's because some markers are not closed paths, is that correct? I wonder if we could detect that the path is closed and not have to do that... but maybe that's overoptimizing what's already a good improvement here.

Yeah, one could do the same as the Agg backend there. But I don't think there is a point; I think the main bottleneck now is the path iteration in python.

About the fill. The only difference it makes is if markers are overlapping. In that case drawing all of them at the same time changes the on screen appearance slightly as the stroke is on top of any fill. So if a lot of filled triangles overlap with only slight offsets, then the fill for the "topmost" marker could simply disappear.
Nothing too bad, but I didn't want to break it without any thought.

Here is the current code:

And drawing the markers at the same time:

It is not a huge difference, but you can see that sometimes the black outline is on top of fills in the second image. In this example it doesn't look too bad actually.

Ok -- I think this is reasonable then. We unfortunately don't have any test coverage for the Cairo backend -- it's definitely something we'd like to do, but we already struggle with how long our test suite takes to run. In the meantime, it would be advantageous to just make sure all of the examples involving markers create the same output as the Agg backend (it won't be identical, but they should be logically the same).

Thanks for the clarification about fill. That makes sense.

I guess I can look into creating some testcases; that may take some time, as it is not high on my todo list.

The marker code could also be directly checked against the old cairo implementation.

@benzea Did you make any progress on adding test?

No, I never got around to write a testcase.

@mdboom I am in favor of just merging this as-is. After 2.5 years I do not think we are going to get tests ;)

'power cycled' to see if this still passes tests against current master.

well, it doesn't seem to break anything...