Here and below:

This seems like it should be a virtual method on Contents.

It also seems like it would be better to have a method like SetGeometry that takes some kind of virtual object that can be tessellated. For paths this would use the tessellator. For vertices it would just return the vertex/index buffers.

One thing that might make drawVertices a little more complicated is when there are color and/or texture coordinates. That doesn't have to be handled here though.

This seems like it should be a virtual method on Contents.

This would be mostly the same as keeping it as a method on Entity. Often times we don't really need any geometry, and the type of geometry we need is sort of an implementation detail of the Contents.

In general, I think the specific Contents implementation needs to ultimately decide how it's going to source and render its geometry. If we try to have a catch-all parameter, it'll end up getting unused/worked around most of the time (which is the current state of affairs).

Something to note here is that strokes and fills aren't the only cases where we need to perform path-based geometry generation. For example, we will likely end up having a Contents that renders shadows by generating inset/outset geometry. The implementation changes depends on whether the path crosses over itself, and the specifics of the technique we use will depend on whether the Path contours are fully convex or not -- we should probably get this info from the Path object itself, and so it seems like a generic Geometry object would end up being worked around/redundant here.

It also seems like it would be better to have a method like SetGeometry that takes some kind of virtual object that can be tessellated. For paths this would use the tessellator. For vertices it would just return the vertex/index buffers.

In some Contents, we'll probably wanna combine things/handle this branching inline. For that, I was planning to just do SetGeometry(std::variant<Path, Vertices>) in contents where it makes sense.

Having an intermediary object that's responsible for outputting the geometry was something I considered, but I ended up stepping back and going this route because it doesn't make combining Contents any easier. We'd still have to have a branch that handles Vertices output separately from the "tessellated Path" output. The specific way we'll need to normalize the vertex data depends on what what exactly needs to be rendered (which is decided through the previous setColorSource call).
For example: If the source type is a "color" and drawVertices is called, we copy the vertex colors in (or fallback to the paint color if no vertex colors are given); if a drawPath-like is called, we always just use the paint color. drawVetices uses either the SolidColorPipeline or VertexColorPipeline, and drawPath (the more common case), ends up always using SolidColorPipeline.

In more complicated cases, I'd like to retain the flexibility to just implement the different cases as totally separate Contents if need be -- we'd still be able to do that with a generic geometry parameter everywhere, but it wouldn't be possible to know what the Contents actually accepts based on the API this way.

So should these classes have Path in the name, and we'll have a new type of Contents entirely for vertices?

I'm not a huge fan of std::variant. I imagine we'd end up having code that looks something like

  switch (geometry_content_type) {
    case Path: // tessellate a path using a special method that knows how to deal with paths
    case Vertices: // use a different special method that knows how to deal with vertices
  }

Which will bloat up the class and likely require duplicated logic elsewhere.

Yeah, I'm not against having separate minimal Contents for each of the Path and Vertices paint situations just to keep it simple. I think they'll all end up being very straight forward and not have very much shared stuff (apart from parameters and maybe uniform bindings for the gradients). They're even small enough to justify maybe placing them next to their complements in the same TUs.

In total, we need to cover each of these color sources for both Paths and Vertices:

• kColor
• kLinearGradient
• kImage
• kRadialGradient
• kConicalGradient
• kSweepGradient

Ok. In that case, does it make sense to rename this to something like FooPathContents to make that clear? Either way, LGTM.

Argh, sorry, I didn't see this message before I merged. Yes, I think that makes sense -- I will follow up and normalize these names a bit when adding the Vertices case for kColor.

No worries, fine to handle in a follow up or decide against it. Shouldn't hold anything up